accept non-HTTP answers from guessed service ports

.github/scripts/build_translation_cache.py

@@ -38,6 +38,18 @@ DEFAULT_CONTEXT = "Context: Technical message for Proxmox and IT. Translate:"
 # provider, then restore the exact source spelling afterwards. Keep longer
 # terms first so ``gasket`` cannot consume part of ``gasket-dkms``.
 PROTECTED_TECHNICAL_TERMS = (
+    # Product names, kept whole. Left to a translator, "Proxmox Backup Server"
+    # became "Proxmox Sicherungsserver" and "Serveur de sauvegarde Proxmox",
+    # and when a provider rightly handed it back unchanged the string was
+    # retried and then dropped as untranslated.
+    "Proxmox Backup Server",
+    "Proxmox Datacenter Manager",
+    "Proxmox Mail Gateway",
+    "Proxmox VE",
+    "ProxMenux Monitor",
+    "ProxMenux Optimizations",
+    "Secure Gateway",
+    "Tailscale",
     "google/gasket-driver",
     "feranick/gasket-driver",
     "libedgetpu1-std",

.github/workflows/test-oci-registries.yml

@@ -45,3 +45,4 @@ jobs:
         run: |
           python3 -m unittest discover -s oci/tests -p 'test_external_tags.py' -v
           python3 -m unittest discover -s oci/tests -p 'test_instance_reconcile.py' -v
+          python3 -m unittest discover -s oci/tests -p 'test_guessed_endpoint_healthcheck.py' -v

oci/remote/install_oci.sh

@@ -359,13 +359,18 @@ cleanup_canonical_inputs() {
 # A derived check accepts any HTTP answer below 500: the application is up,
 # even when its first page is a redirect or asks for a login.
 healthcheck_probe() {
+  local code status=0
   if [[ ${HC_ANY_STATUS:-false} == true ]]; then
-    local code
+    code=$(curl "${CURL_ARGS[@]}" "$HC_URL" 2>/dev/null) || status=$?
-    code=$(curl "${CURL_ARGS[@]}" "$HC_URL" 2>/dev/null) || return 1
+    (( status == 0 )) && [[ $code =~ ^[1-4][0-9][0-9]$ ]] && return 0
-    [[ $code =~ ^[1-4][0-9][0-9]$ ]]
   else
-    curl "${CURL_ARGS[@]}" "$HC_URL" 2>/dev/null
+    curl "${CURL_ARGS[@]}" "$HC_URL" >/dev/null 2>&1 || status=$?
+    (( status == 0 )) && return 0
   fi
+  # A port taken from the compose file may serve a database or SSH: an answer
+  # in another protocol (HTTP/0.9, weird or empty reply, reset) means it is up.
+  [[ ${HC_ACCEPT_NON_HTTP:-false} == true ]] \
+    && (( status == 1 || status == 8 || status == 52 || status == 56 ))
 }
 
 cleanup_failed_install() {
@@ -2267,6 +2272,7 @@ if [[ $START_AFTER == 1 && $HAS_STARTUP_HEALTHCHECK == 1 ]]; then
   [[ -n $IP ]] || die "$(translate "The healthcheck cannot run without an IP address")"
   HC_URL="${HC_SCHEME}://${IP}:${HC_PORT}${HC_PATH}"
   HC_ANY_STATUS=$(jq -r '.accept_any_status // false' <<<"$STARTUP_HEALTHCHECK")
+  HC_ACCEPT_NON_HTTP=$(jq -r '.accept_non_http // false' <<<"$STARTUP_HEALTHCHECK")
   if [[ $HC_ANY_STATUS == true ]]; then
     CURL_ARGS=(-sS --noproxy '*' -o /dev/null -w '%{http_code}' --max-time "$HC_REQUEST_TIMEOUT")
   else

oci/remote/oci_instance_transaction.py

@@ -352,6 +352,22 @@ def profile_keys(deployment):
     return keys
 
 
+GUESSED_ENDPOINT = 'compose-first-tcp-port-fallback'
+# curl exit codes for a port that accepted the connection and answered in a
+# protocol other than HTTP: HTTP/0.9 refused, weird reply, empty reply, reset.
+NON_HTTP_ANSWER = (1, 8, 52, 56)
+
+
+def probe_http(args):
+    """Run one curl probe and return its output and exit code."""
+    log('$ ' + shlex.join(str(arg) for arg in args))
+    try:
+        result = subprocess.run(args, capture_output=True, timeout=60, close_fds=True)
+    except subprocess.TimeoutExpired:
+        return b'', -1
+    return result.stdout, result.returncode
+
+
 def effective_healthcheck(template):
     """The template's own health check or, when it has none, one derived from
     its web address: any HTTP answer below 500 means the application is up."""
@@ -367,9 +383,14 @@ def effective_healthcheck(template):
         path = str(endpoint.get('path') or '/')
         if (endpoint.get('scheme') in ('http', 'https') and str(endpoint.get('port', '')).isdigit()
                 and path.startswith('/') and not any(c.isspace() for c in path)):
-            return {'scheme': endpoint['scheme'], 'port': int(endpoint['port']), 'path': path,
+            check = {'scheme': endpoint['scheme'], 'port': int(endpoint['port']), 'path': path,
-                    'timeout_seconds': 300, 'request_timeout_seconds': 10, 'stability_seconds': 4,
+                     'timeout_seconds': 300, 'request_timeout_seconds': 10, 'stability_seconds': 4,
-                    'verify_tls': False, 'accept_any_status': True}
+                     'verify_tls': False, 'accept_any_status': True}
+            # A port taken from the compose file because the recipe names no
+            # web address may serve a database or SSH: any answer counts.
+            if endpoint.get('source') == GUESSED_ENDPOINT:
+                check['accept_non_http'] = True
+            return check
     # Without a web address, the new image must at least keep the container running.
     return {'type': 'running', 'timeout_seconds': 90, 'stability_seconds': 20}
 
@@ -733,11 +754,10 @@ def healthcheck(vmid, template):
                 args += ['-w', '%{http_code}']
             if not check.get('verify_tls', False):
                 args.append('-k')
-            try:
+            answer, status = probe_http([*args, f'{scheme}://{ip}:{port}{path}'])
-                answer = run(*args, f'{scheme}://{ip}:{port}{path}')
+            ok = status == 0 and (not any_status or re.fullmatch(rb'[1-4][0-9][0-9]', answer.strip()) is not None)
-                ok = not any_status or re.fullmatch(rb'[1-4][0-9][0-9]', answer.strip()) is not None
+            if not ok and check.get('accept_non_http') and status in NON_HTTP_ANSWER:
-            except RuntimeError:
+                ok = True
-                pass
         if ok:
             stable_since = stable_since or time.monotonic()
             if time.monotonic() - stable_since >= stability:

oci/tests/test_guessed_endpoint_healthcheck.py

@@ -0,0 +1,87 @@
+"""A web address guessed from the compose file may belong to a service that does
+not speak HTTP. Updates, Modify and recovery must accept its answer instead of
+waiting for an HTTP reply that never comes."""
+
+import os
+from pathlib import Path
+import subprocess
+import sys
+import tempfile
+import unittest
+from unittest.mock import patch
+
+ROOT = Path(__file__).resolve().parents[1]
+sys.path.insert(0, str(ROOT / 'remote'))
+
+import oci_instance_transaction as transaction
+
+INSTALLER = ROOT / 'remote/install_oci.sh'
+GUESSED = {'first_run': {'endpoints': [{'label': 'Web UI', 'scheme': 'http', 'port': 3306, 'path': '/',
+                                        'source': 'compose-first-tcp-port-fallback'}]}}
+DECLARED = {'first_run': {'endpoints': [{'label': 'Web UI', 'scheme': 'http', 'port': 8080, 'path': '/'}]}}
+
+
+def probe_function():
+    source = INSTALLER.read_text()
+    start = source.index('healthcheck_probe() {')
+    end = source.index('\n}\n', start) + 3
+    return source[start:end]
+
+
+class GuessedEndpointCheckTests(unittest.TestCase):
+    def test_only_a_guessed_endpoint_accepts_other_protocols(self):
+        self.assertTrue(transaction.effective_healthcheck(GUESSED)['accept_non_http'])
+        self.assertNotIn('accept_non_http', transaction.effective_healthcheck(DECLARED))
+
+    def run_probe(self, exit_code, code='000', accept_non_http='true', any_status='true'):
+        with tempfile.TemporaryDirectory() as directory:
+            fake = Path(directory) / 'curl'
+            fake.write_text(f'#!/bin/sh\nprintf %s {code}\nexit {exit_code}\n')
+            fake.chmod(0o755)
+            script = (probe_function() + '\nCURL_ARGS=()\nHC_URL=http://192.0.2.1:3306/\n'
+                      f'HC_ANY_STATUS={any_status}\nHC_ACCEPT_NON_HTTP={accept_non_http}\nhealthcheck_probe\n')
+            env = {**os.environ, 'PATH': f'{directory}:{os.environ["PATH"]}'}
+            return subprocess.run(['bash', '-c', script], env=env, capture_output=True).returncode
+
+    def test_installer_probe_accepts_a_non_http_answer_only_when_guessed(self):
+        for status in transaction.NON_HTTP_ANSWER:
+            with self.subTest(status=status):
+                self.assertEqual(self.run_probe(status), 0)
+                self.assertNotEqual(self.run_probe(status, accept_non_http='false'), 0)
+        # A closed port, a timeout or a server error still fail.
+        for status, code in ((7, '000'), (28, '000'), (0, '503')):
+            with self.subTest(status=status, code=code):
+                self.assertNotEqual(self.run_probe(status, code), 0)
+        self.assertEqual(self.run_probe(0, '200'), 0)
+        self.assertEqual(self.run_probe(0, any_status='false', accept_non_http='false'), 0)
+
+    def test_recovery_check_accepts_a_database_answer(self):
+        check = transaction.effective_healthcheck(GUESSED)
+        check.update(timeout_seconds=6, stability_seconds=0)
+        template = {'proxmox': {'installer_profile': {'startup_healthcheck': check}}}
+
+        def run(*args):
+            if args[:2] == ('pct', 'status'):
+                return b'status: running\n'
+            if args[:2] == ('pct', 'config'):
+                return b''
+            if args[0] == 'lxc-info':
+                return b'192.0.2.10\n'
+            raise AssertionError(args)
+
+        with patch.object(transaction, 'run', side_effect=run), \
+                patch.object(transaction, 'probe_http', return_value=(b'000', 1)), \
+                patch.object(transaction, 'log'):
+            self.assertEqual(transaction.healthcheck(113, template), 'http://192.0.2.10:3306/')
+        declared = transaction.effective_healthcheck(DECLARED)
+        declared.update(timeout_seconds=4, stability_seconds=0)
+        template = {'proxmox': {'installer_profile': {'startup_healthcheck': declared}}}
+        with patch.object(transaction, 'run', side_effect=run), \
+                patch.object(transaction, 'probe_http', return_value=(b'000', 1)), \
+                patch.object(transaction, 'log'), patch.object(transaction.time, 'sleep'):
+            with self.assertRaises(ValueError):
+                transaction.healthcheck(113, template)
+
+
+if __name__ == '__main__':
+    unittest.main()