diff --git a/.github/scripts/build_translation_cache.py b/.github/scripts/build_translation_cache.py index 95825a29..b5797010 100644 --- a/.github/scripts/build_translation_cache.py +++ b/.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", diff --git a/.github/workflows/test-oci-registries.yml b/.github/workflows/test-oci-registries.yml index ff0032cb..16ff314f 100644 --- a/.github/workflows/test-oci-registries.yml +++ b/.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 diff --git a/oci/remote/install_oci.sh b/oci/remote/install_oci.sh index 7d6da264..5f1802ba 100755 --- a/oci/remote/install_oci.sh +++ b/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) || return 1 - [[ $code =~ ^[1-4][0-9][0-9]$ ]] + code=$(curl "${CURL_ARGS[@]}" "$HC_URL" 2>/dev/null) || status=$? + (( status == 0 )) && [[ $code =~ ^[1-4][0-9][0-9]$ ]] && return 0 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 diff --git a/oci/remote/oci_instance_transaction.py b/oci/remote/oci_instance_transaction.py index cdde917b..b5cef528 100644 --- a/oci/remote/oci_instance_transaction.py +++ b/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, - 'timeout_seconds': 300, 'request_timeout_seconds': 10, 'stability_seconds': 4, - 'verify_tls': False, 'accept_any_status': True} + 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} + # 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 = run(*args, f'{scheme}://{ip}:{port}{path}') - ok = not any_status or re.fullmatch(rb'[1-4][0-9][0-9]', answer.strip()) is not None - except RuntimeError: - pass + 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: 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 new file mode 100644 index 00000000..8ce9003f --- /dev/null +++ b/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()