diff --git a/docs/release-control/v6/internal/subsystems/deployment-installability.md b/docs/release-control/v6/internal/subsystems/deployment-installability.md index f56f236218..fba4dd38f2 100644 --- a/docs/release-control/v6/internal/subsystems/deployment-installability.md +++ b/docs/release-control/v6/internal/subsystems/deployment-installability.md @@ -16,6 +16,29 @@ ## Purpose +### Completion requires confirmed server startup + +All five server installer entry paths require successful `start_pulse` before +writing the completion marker or reporting installation success, including +conditional/OR-list callers that disable Bash `errexit`. A failed or timed-out +start, or unavailable/inactive readback, cannot be softened into a promise that +the service will start on the next container boot. Boot-time enablement remains +a separate observation: its failure warns without claiming a known cause. + +Fresh and previously stopped installations wait for an active-state readback +within the existing bounded polling policy. Only updates that stopped a running +Pulse keep the existing single explicit recovery-start retry. A confirmed active +service can satisfy startup even when the start command returned an error; the +error remains visible. An unconfirmed result fails with service-specific status +and journal guidance, retaining installed files, configuration and recovery data. +It does not claim rollback, HTTP health, sustained liveness or native recovery. + +`TestRootInstallStartupRequiresActiveReadback` registers confined controls for +command/query failures and timeouts, delayed activation, prior-active retry, +missing units, all five actual main consumers and preservation of synthetic +configuration/key/identity bytes. No real service or original stopped workload +is operated by those controls. + ### Directory setup failure cannot become installation success The server installer's five source, pinned, menu-update, reinstall and fresh diff --git a/install.sh b/install.sh index 3ff8fb3075..f5cbf72cb5 100644 --- a/install.sh +++ b/install.sh @@ -5227,34 +5227,34 @@ start_pulse() { return 1 fi - # Try to enable/start service (may fail in unprivileged containers) - if ! safe_systemctl enable $SERVICE_NAME; then - print_info "Note: systemctl enable failed (common in unprivileged containers)" - fi - - if ! safe_systemctl start $SERVICE_NAME; then - print_info "Note: systemctl start failed (common in unprivileged containers)" - if [[ "$PULSE_WAS_ACTIVE" != "true" ]]; then - print_info "The service will start automatically when the container starts" - return 0 - fi - # An update stopped a running Pulse; do not accept a silent start failure - # here — fall through to the active-state verification below (#1323). - fi - - # Wait for service to start - sleep 3 - - if timeout 5 systemctl is-active --quiet $SERVICE_NAME 2>/dev/null; then - print_success "Pulse started successfully" - else - print_error "Failed to start Pulse" - journalctl -u $SERVICE_NAME -n 20 2>/dev/null || true - # Don't exit, just warn - print_info "Service may not be running. You might need to start it manually." + # Enabling boot-time startup and observing a running service are separate. + # A command error does not establish its cause or a future successful boot. + if ! safe_systemctl enable "$SERVICE_NAME"; then + print_warn "Could not enable Pulse at boot; check the service's enabled state" fi - ensure_pulse_running_after_update + if ! safe_systemctl start "$SERVICE_NAME"; then + print_warn "Pulse start did not complete successfully; checking actual service state" + fi + + # Wait for service to start + sleep 3 + + if [[ "$PULSE_WAS_ACTIVE" == "true" ]]; then + # Preserve the existing bounded retry only for an update that stopped + # a running Pulse. Fresh/prior-stopped installs still require readback. + ensure_pulse_running_after_update || return 1 + elif ! wait_for_service_active "$SERVICE_NAME" 20; then + print_error "Could not confirm Pulse is active; installation is incomplete" + print_info "Installed files and configuration have been retained; check the service before retrying:" + print_info " systemctl status $SERVICE_NAME" + print_info " journalctl -u $SERVICE_NAME -n 50" + timeout 5 journalctl -u "$SERVICE_NAME" -n 20 2>/dev/null || true + return 1 + fi + + print_success "Pulse started successfully" + return 0 } create_marker_file() { @@ -5418,7 +5418,7 @@ main() { refresh_auto_updates fi - start_pulse + start_pulse || return 1 create_marker_file print_completion exit 0 @@ -5466,7 +5466,7 @@ main() { refresh_auto_updates fi - start_pulse + start_pulse || return 1 create_marker_file print_completion return 0 @@ -5673,7 +5673,7 @@ main() { refresh_auto_updates fi - start_pulse + start_pulse || return 1 create_marker_file print_completion exit 0 @@ -5701,7 +5701,7 @@ main() { refresh_auto_updates fi - start_pulse + start_pulse || return 1 create_marker_file print_completion exit 0 @@ -5811,7 +5811,7 @@ main() { refresh_auto_updates fi - start_pulse + start_pulse || return 1 create_marker_file print_completion fi diff --git a/scripts/installtests/root_install_sh_test.go b/scripts/installtests/root_install_sh_test.go index 23e28b2b50..41e3a67043 100644 --- a/scripts/installtests/root_install_sh_test.go +++ b/scripts/installtests/root_install_sh_test.go @@ -2568,6 +2568,7 @@ func TestRootInstallStartPulseFailsWhenUnitMissing(t *testing.T) { set -euo pipefail PULSE_WAS_ACTIVE="false" print_info() { echo "INFO: $*"; } +print_warn() { echo "WARN: $*"; } print_error() { echo "ERROR: $*"; } print_success() { echo "SUCCESS: $*"; } safe_systemctl() { return 0; } @@ -2575,7 +2576,8 @@ timeout() { shift; "$@"; } sleep() { :; } journalctl() { return 0; } ensure_pulse_running_after_update() { return 0; } -` + extractRootInstallShellFunction(t, "start_pulse") + "\n" +` + extractRootInstallShellFunction(t, "wait_for_service_active") + "\n" + + extractRootInstallShellFunction(t, "start_pulse") + "\n" // Unit genuinely absent: no unit file and `systemctl cat` cannot find it. // start_pulse must fail with a clear error instead of reporting success @@ -3033,6 +3035,20 @@ func TestRootInstallDirectorySetupStopsOnFailures(t *testing.T) { }, 30*time.Second, "PULSE_INSTALLER_UNDER_TEST="+repoFile("install.sh")) } +// Run the real startup/recovery producer through each main consumer without +// touching a host service, binary or saved configuration. Conditional callers +// must not mask failure or issue completion/marker writes over unknown liveness. +func TestRootInstallStartupRequiresActiveReadback(t *testing.T) { + runPythonFixtureControls(t, repoFile("scripts", "tests", "test_server_start_admission.py"), "ServerStartAdmission", []string{ + "test_unconfirmed_start_fails_for_fresh_and_prior_active_installations", + "test_confirmed_active_and_delayed_start_consume_liveness_flag", + "test_retry_is_bounded_and_only_restores_prior_active_service", + "test_missing_unit_stops_before_enable_or_start", + "test_all_five_main_paths_stop_before_marker_or_completion_in_or_list", + "test_all_five_main_paths_complete_only_after_active_readback", + }, 30*time.Second, "PULSE_INSTALLER_UNDER_TEST="+repoFile("install.sh")) +} + // Run the emitted helper and real SSH verifier. Curl exits are modelled, so // these controls prove deadline arguments and safe failure, not a live outage. func TestRootManualUpdateDownloadDeadlines(t *testing.T) { diff --git a/scripts/tests/test_server_start_admission.py b/scripts/tests/test_server_start_admission.py new file mode 100644 index 0000000000..d327647911 --- /dev/null +++ b/scripts/tests/test_server_start_admission.py @@ -0,0 +1,179 @@ +#!/usr/bin/env python3 +"""Exercise actual installer startup admission without controlling any service. + +The real start/wait/recovery functions and five main paths consume confined +systemctl responses. Persistent text fixtures never leave this process. These +are failure-propagation controls, not native startup or installed recovery proof. +""" +import os +from pathlib import Path +import subprocess +import tempfile +import unittest + +ROOT = Path(__file__).resolve().parents[2] +INSTALLER = Path(os.environ.get("PULSE_INSTALLER_UNDER_TEST", ROOT / "install.sh")).resolve() + +PRELUDE = r''' +set -euo pipefail +source "$INSTALLER" +print_info() { printf 'INFO: %s\n' "$*"; } +print_warn() { printf 'WARN: %s\n' "$*"; } +print_error() { printf 'ERROR: %s\n' "$*" >&2; } +print_success() { printf 'SUCCESS: %s\n' "$*"; } +PULSE_WAS_ACTIVE="$PRIOR_ACTIVE" +STARTS=0 +QUERIES=0 +sleep() { :; } +timeout() { + local duration="$1"; shift + printf '%s %s\n' "$duration" "$*" >> "$FIXTURE/deadlines" + [[ "$FAULT:$*" != "start-timeout:systemctl start "* ]] || return 124 + [[ "$FAULT:$*" != "query-timeout:systemctl is-active "* ]] || return 124 + "$@" +} +systemctl() { + printf '%s\n' "$*" >> "$FIXTURE/calls" + case "$1" in + cat) [[ "$FAULT" != missing-unit ]] ;; + enable) [[ "$FAULT" != enable-failure ]] ;; + start) STARTS=$((STARTS + 1)); [[ "$FAULT" != start-failure && "$FAULT" != start-error-active ]] ;; + is-active) + QUERIES=$((QUERIES + 1)) + case "$FAULT" in + active|enable-failure|start-error-active) return 0 ;; + delayed) (( QUERIES >= 3 )) ;; + retry-recovers) (( STARTS >= 2 )) ;; + query-error) return 5 ;; + *) return 3 ;; + esac + ;; + *) echo 'Unexpected service operation' >&2; return 98 ;; + esac +} +journalctl() { printf '%s\n' "$*" >> "$FIXTURE/journal"; } +''' + +MAIN = r''' +CURRENT_VERSION=v6.5.0 +FORCE_VERSION="" +FORCE_CHANNEL=stable +BUILD_FROM_SOURCE=false +IN_CONTAINER=true +IN_DOCKER=false +[[ "$FLOW" != pinned ]] || FORCE_VERSION=v6.6.0 +[[ "$FLOW" != source ]] || BUILD_FROM_SOURCE=true +for fn in print_header check_root detect_os check_docker_environment offer_existing_auto_updates run_upgrade_readiness_preflight backup_existing install_dependencies create_user download_pulse build_from_source setup_directories setup_update_command install_systemd_service ensure_systemd_service_installed setup_auto_updates refresh_auto_updates; do + eval "$fn() { :; }" +done +check_existing_installation() { [[ "$FLOW" != fresh ]]; } +check_proxmox_host() { return 1; } +resolve_latest_release_tag_for_channel() { printf 'v6.6.0\n'; } +detect_service_name() { printf '%s\n' "$SERVICE_NAME"; } +safe_read() { local selection=1; [[ "$FLOW" != menu-reinstall ]] || selection=2; printf -v "$2" '%s' "$selection"; } +update_timer_exists() { return 1; } +create_marker_file() { printf 'marker\n' >> "$FIXTURE/completion"; } +print_completion() { printf 'complete\n' >> "$FIXTURE/completion"; } +main || exit "$?" +''' + + +class ServerStartAdmission(unittest.TestCase): + def exercise(self, fault, prior=False, body=None, flow=""): + with tempfile.TemporaryDirectory(prefix="pulse-server-start-") as temporary: + fixture = Path(temporary) + install = fixture / "install with spaces" + config = fixture / "config with spaces" + install.mkdir() + config.mkdir() + originals = { + install / "VERSION": b"v6.5.0\n", + install / "pulse": b"synthetic old executable\n", + config / ".encryption.key": b"synthetic saved key\n", + config / "system.json": b'{"preserve":"settings"}\n', + config / "agent-id": b"saved identity\n", + } + for path, content in originals.items(): + path.write_bytes(content) + path.chmod(0o600) + env = {k: v for k, v in os.environ.items() if not k.startswith("PULSE_")} + env.update(INSTALLER=str(INSTALLER), FIXTURE=str(fixture), FAULT=fault, + FLOW=flow, PRIOR_ACTIVE=str(prior).lower(), + PULSE_SERVICE_NAME="pulse-start-fixture", PULSE_INSTALL_DIR=str(install), + PULSE_CONFIG_DIR=str(config)) + if body is None: + body = 'start_pulse || exit "$?"\nprintf "FLAG=%s\\n" "$PULSE_WAS_ACTIVE"\n' + result = subprocess.run(["bash", "-c", PRELUDE + body], cwd=fixture, + env=env, capture_output=True, text=True, timeout=10) + self.assertEqual({p: p.read_bytes() for p in originals}, originals) + self.assertTrue(all(p.stat().st_mode & 0o777 == 0o600 for p in originals)) + return dict(result=result, calls=(fixture / "calls").read_text().splitlines() + if (fixture / "calls").exists() else [], + deadlines=(fixture / "deadlines").read_text().splitlines(), + completion=(fixture / "completion").read_text().splitlines() + if (fixture / "completion").exists() else [], + journal=(fixture / "journal").read_text().splitlines() + if (fixture / "journal").exists() else []) + + def test_unconfirmed_start_fails_for_fresh_and_prior_active_installations(self): + for prior in (False, True): + for fault in ("start-failure", "start-timeout", "inactive", "query-error", "query-timeout"): + with self.subTest(prior=prior, fault=fault): + e = self.exercise(fault, prior) + self.assertEqual(e["result"].returncode, 1, e["result"]) + self.assertNotIn("SUCCESS:", e["result"].stdout) + self.assertNotIn("will start automatically", e["result"].stdout) + self.assertIn("systemctl status pulse-start-fixture", e["result"].stdout) + if not prior: + # The fixed timeout double returns before systemctl for this case. + self.assertEqual(e["calls"].count("start pulse-start-fixture"), + 0 if fault == "start-timeout" else 1) + + def test_confirmed_active_and_delayed_start_consume_liveness_flag(self): + for prior in (False, True): + for fault in ("active", "delayed", "start-error-active", "enable-failure"): + with self.subTest(prior=prior, fault=fault): + e = self.exercise(fault, prior) + self.assertEqual(e["result"].returncode, 0, e["result"]) + self.assertIn("SUCCESS:", e["result"].stdout) + self.assertIn("FLAG=false\n", e["result"].stdout) + self.assertNotIn("will start automatically", e["result"].stdout) + self.assertEqual(e["calls"].count("start pulse-start-fixture"), 1) + self.assertIn("is-active --quiet pulse-start-fixture", e["calls"]) + + def test_retry_is_bounded_and_only_restores_prior_active_service(self): + for prior in (False, True): + with self.subTest(prior=prior): + e = self.exercise("retry-recovers", prior) + self.assertEqual(e["result"].returncode, 0 if prior else 1, e["result"]) + self.assertEqual(e["calls"].count("start pulse-start-fixture"), 2 if prior else 1) + self.assertEqual(e["calls"].count("is-active --quiet pulse-start-fixture"), + 21 if prior else 20) + + def test_missing_unit_stops_before_enable_or_start(self): + e = self.exercise("missing-unit") + self.assertEqual(e["result"].returncode, 1, e["result"]) + self.assertIn("does not exist", e["result"].stderr) + self.assertEqual(e["calls"], ["cat pulse-start-fixture.service"]) + + def test_all_five_main_paths_stop_before_marker_or_completion_in_or_list(self): + for flow in ("source", "pinned", "menu-update", "menu-reinstall", "fresh"): + for prior in (False, True): + with self.subTest(flow=flow, prior=prior): + e = self.exercise("inactive", prior, body=MAIN, flow=flow) + self.assertEqual(e["result"].returncode, 1, e["result"]) + self.assertEqual(e["completion"], [], e["result"]) + self.assertNotIn("SUCCESS:", e["result"].stdout) + self.assertIn("is-active --quiet pulse-start-fixture", e["calls"]) + + def test_all_five_main_paths_complete_only_after_active_readback(self): + for flow in ("source", "pinned", "menu-update", "menu-reinstall", "fresh"): + with self.subTest(flow=flow): + e = self.exercise("active", body=MAIN, flow=flow) + self.assertEqual(e["result"].returncode, 0, e["result"]) + self.assertEqual(e["completion"], ["marker", "complete"]) + self.assertIn("is-active --quiet pulse-start-fixture", e["calls"]) + + +if __name__ == "__main__": + unittest.main(verbosity=2)