diff --git a/oci/remote/install_oci.sh b/oci/remote/install_oci.sh index dd93e862..3a1b59d0 100755 --- a/oci/remote/install_oci.sh +++ b/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() { - local code status=0 +healthcheck_probe_url() { + 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 diff --git a/oci/remote/oci_instance_transaction.py b/oci/remote/oci_instance_transaction.py index fa01b2dc..5639d97d 100644 --- a/oci/remote/oci_instance_transaction.py +++ b/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}']) - ok = status == 0 and (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: - ok = True + for port in ports: + answer, status = probe_http([*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) + 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: diff --git a/oci/tests/test_guessed_endpoint_healthcheck.py b/oci/tests/test_guessed_endpoint_healthcheck.py index b5393948..afbfa9aa 100644 --- a/oci/tests/test_guessed_endpoint_healthcheck.py +++ b/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() {') - end = source.index('\n}\n', start) + 3 + start = source.index('healthcheck_probe_url() {') + 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()