fix(oci): let AdGuard Home pass its update check on the web port once it is set up

oci/remote/install_oci.sh

@@ -358,13 +358,13 @@ 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() {
+healthcheck_probe_url() {
-  local code status=0
+  local url=$1 code status=0
   if [[ ${HC_ANY_STATUS:-false} == true ]]; then
-    code=$(oci_curl "$VMID" "${CURL_ARGS[@]}" "$HC_URL" 2>/dev/null) || status=$?
+    code=$(oci_curl "$VMID" "${CURL_ARGS[@]}" "$url" 2>/dev/null) || status=$?
     (( status == 0 )) && [[ $code =~ ^[1-4][0-9][0-9]$ ]] && return 0
   else
-    oci_curl "$VMID" "${CURL_ARGS[@]}" "$HC_URL" >/dev/null 2>&1 || status=$?
+    oci_curl "$VMID" "${CURL_ARGS[@]}" "$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
@@ -373,6 +373,19 @@ healthcheck_probe() {
     && (( status == 1 || status == 8 || status == 52 || status == 56 ))
 }
 
+# Any of the addresses answering means the application is up; HC_URL keeps the
+# one that answered.
+healthcheck_probe() {
+  local url
+  for url in "${HC_URLS[@]}"; do
+    if healthcheck_probe_url "$url"; then
+      HC_URL=$url
+      return 0
+    fi
+  done
+  return 1
+}
+
 cleanup_failed_install() {
   local status=$?
   stop_spinner
@@ -2333,6 +2346,12 @@ if [[ $START_AFTER == 1 && $HAS_STARTUP_HEALTHCHECK == 1 ]]; then
     || die "$(translate "The stability period must be shorter than the healthcheck timeout")"
   [[ -n $IP ]] || die "$(translate "The healthcheck cannot run without an IP address")"
   HC_URL="${HC_SCHEME}://${IP}:${HC_PORT}${HC_PATH}"
+  HC_URLS=("$HC_URL")
+  while IFS= read -r HC_ALT_PORT; do
+    [[ $HC_ALT_PORT =~ ^[0-9]+$ && $HC_ALT_PORT -ge 1 && $HC_ALT_PORT -le 65535 ]] \
+      || die "$(translate "Invalid healthcheck port")"
+    HC_URLS+=("${HC_SCHEME}://${IP}:${HC_ALT_PORT}${HC_PATH}")
+  done < <(jq -r '.alternative_ports[]? | tostring' <<<"$STARTUP_HEALTHCHECK")
   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

oci/remote/oci_instance_transaction.py

@@ -435,13 +435,22 @@ def probe_http(args):
     return result.stdout, result.returncode
 
 
+def setup_then_web_port(template, check):
+    """AdGuard Home answers its setup wizard on 3000 and, once set up, its web UI
+    on 80. A check on 3000, derived or saved with an installation, takes 80 too."""
+    if (template.get('id') == 'image-adguard-home' and check.get('port') == 3000
+            and not check.get('alternative_ports')):
+        return dict(check, alternative_ports=[80])
+    return check
+
+
 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."""
     profile = template.get('proxmox', {}).get('installer_profile', {})
     check = profile.get('startup_healthcheck')
     if check:
-        return check
+        return setup_then_web_port(template, check)
     if profile.get('haos_healthcheck'):
         # Home Assistant OS has a check of its own: Supervisor, Core and the
         # services it runs inside. The installer runs it on the new container.
@@ -453,6 +462,7 @@ def effective_healthcheck(template):
             check = {'scheme': endpoint['scheme'], 'port': int(endpoint['port']), 'path': path,
                      'timeout_seconds': 300, 'request_timeout_seconds': 10, 'stability_seconds': 4,
                      'verify_tls': False, 'accept_any_status': True}
+            check = setup_then_web_port(template, check)
             # 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:
@@ -923,10 +933,12 @@ def healthcheck(vmid, template):
             time.sleep(2)
         log(f'container {vmid} kept running for {stability}s')
         return None
-    scheme, port, path = check['scheme'], int(check['port']), check['path']
+    scheme, path = check['scheme'], check['path']
+    # Any of the ports answering means the application is up.
+    ports = [int(check['port']), *(int(p) for p in check.get('alternative_ports') or [])]
     timeout = int(check.get('timeout_seconds', 120))
     stability = int(check.get('stability_seconds', 0))
-    if (scheme not in ('http', 'https') or not 1 <= port <= 65535
+    if (scheme not in ('http', 'https') or not all(1 <= port <= 65535 for port in ports)
             or not path.startswith('/') or any(c.isspace() for c in path)
             or not 0 <= stability < timeout <= 3600):
         raise ValueError(translate('Invalid health check'))
@@ -947,10 +959,13 @@ def healthcheck(vmid, template):
                 args += ['-w', '%{http_code}']
             if not check.get('verify_tls', False):
                 args.append('-k')
-            answer, status = probe_http([*args, f'{scheme}://{ip}:{port}{path}'])
+            for port in ports:
-            ok = status == 0 and (not any_status or re.fullmatch(rb'[1-4][0-9][0-9]', answer.strip()) is not None)
+                answer, status = probe_http([*args, f'{scheme}://{ip}:{port}{path}'])
-            if not ok and check.get('accept_non_http') and status in NON_HTTP_ANSWER:
+                ok = status == 0 and (not any_status or re.fullmatch(rb'[1-4][0-9][0-9]', answer.strip()) is not None)
-                ok = True
+                if not ok and check.get('accept_non_http') and status in NON_HTTP_ANSWER:
+                    ok = True
+                if ok:
+                    break
         if ok:
             stable_since = stable_since or time.monotonic()
             if time.monotonic() - stable_since >= stability:

oci/tests/test_guessed_endpoint_healthcheck.py

@@ -23,8 +23,8 @@ DECLARED = {'first_run': {'endpoints': [{'label': 'Web UI', 'scheme': 'http', 'p
 
 def probe_function():
     source = INSTALLER.read_text()
-    start = source.index('healthcheck_probe() {')
+    start = source.index('healthcheck_probe_url() {')
-    end = source.index('\n}\n', start) + 3
+    end = source.index('\n}\n', source.index('healthcheck_probe() {')) + 3
     return source[start:end]
 
 
@@ -38,7 +38,7 @@ class GuessedEndpointCheckTests(unittest.TestCase):
             fake = Path(directory) / 'curl'
             fake.write_text(f'#!/bin/sh\nprintf %s {code}\nexit {exit_code}\n')
             fake.chmod(0o755)
-            script = (probe_function() + '\noci_curl() { shift; curl "$@"; }\nCURL_ARGS=()\nHC_URL=http://192.0.2.1:3306/\n'
+            script = (probe_function() + '\noci_curl() { shift; curl "$@"; }\nCURL_ARGS=()\nHC_URLS=(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
@@ -83,5 +83,62 @@ class GuessedEndpointCheckTests(unittest.TestCase):
                 transaction.healthcheck(113, template)
 
 
+ADGUARD = {'id': 'image-adguard-home',
+           'first_run': {'endpoints': [{'label': 'Web UI', 'scheme': 'http', 'port': 3000, 'path': '/'}]}}
+
+
+class SetupThenWebPortTests(unittest.TestCase):
+    """AdGuard Home answers on 3000 until it is set up, then on 80: an update of
+    a configured installation must not wait for the setup port."""
+
+    def test_adguard_also_accepts_its_web_port(self):
+        check = transaction.effective_healthcheck(ADGUARD)
+        self.assertEqual((check['port'], check['alternative_ports']), (3000, [80]))
+        self.assertNotIn('alternative_ports', transaction.effective_healthcheck(DECLARED))
+
+    def test_a_check_saved_with_an_installation_takes_the_web_port_too(self):
+        # An installation stores the check it was given: still on 3000 only.
+        saved = {'scheme': 'http', 'port': 3000, 'path': '/', 'timeout_seconds': 300, 'accept_any_status': True}
+        template = dict(ADGUARD, proxmox={'installer_profile': {'startup_healthcheck': saved}})
+        self.assertEqual(transaction.effective_healthcheck(template)['alternative_ports'], [80])
+        self.assertNotIn('alternative_ports', saved)
+        other = dict(template, id='image-other')
+        self.assertNotIn('alternative_ports', transaction.effective_healthcheck(other))
+
+    def test_recovery_check_passes_on_the_web_port_when_setup_is_closed(self):
+        check = transaction.effective_healthcheck(ADGUARD)
+        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)
+
+        def probe(args):
+            return (b'200', 0) if args[-1].endswith(':80/') else (b'000', 7)
+        with patch.object(transaction, 'run', side_effect=run), \
+                patch.object(transaction, 'probe_http', side_effect=probe), \
+                patch.object(transaction, 'log'):
+            self.assertEqual(transaction.healthcheck(101, template), 'http://192.0.2.10:80/')
+
+    def test_installer_probe_tries_every_address_and_keeps_the_one_that_answered(self):
+        with tempfile.TemporaryDirectory() as directory:
+            fake = Path(directory) / 'curl'
+            fake.write_text('#!/bin/sh\nfor last; do :; done\ncase "$last" in *:80/) printf 200; exit 0;; esac\n'
+                            'printf 000\nexit 7\n')
+            fake.chmod(0o755)
+            script = (probe_function() + '\noci_curl() { shift; curl "$@"; }\nCURL_ARGS=()\n'
+                      'HC_URLS=(http://192.0.2.1:3000/ http://192.0.2.1:80/)\nHC_ANY_STATUS=true\n'
+                      'healthcheck_probe && echo "$HC_URL"\n')
+            env = {**os.environ, 'PATH': f'{directory}:{os.environ["PATH"]}'}
+            done = subprocess.run(['bash', '-c', script], env=env, capture_output=True, text=True)
+        self.assertEqual((done.returncode, done.stdout.strip()), (0, 'http://192.0.2.1:80/'))
+
+
 if __name__ == '__main__':
     unittest.main()