gsadmin/pulse
1
0
Fork 0
mirror of https://github.com/rcourtman/Pulse.git synced 2026-10-27 05:57:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

Treat no matching unit files as an empty service inventory (#2859)

Browse Source 
The first beta (v6.6.0-beta.1) now passes its preparation checks and
both source pairs, and its release workflow failed twice at the
install.sh smoke test: on a clean Debian 12 container install.sh stops
with "Cannot identify the Pulse systemd service: unit inventory failed".
That is a real regression on main, not a flake. v6.5.0 does not have it,
so no release carries it.

Cause: the 2026-10-10 service discovery change reads `systemctl
list-unit-files -- pulse-backend.service pulse.service` and refuses on
any non-zero exit. systemd exits 1, printing nothing on stdout or
stderr, when none of the named unit files exist, which is every fresh
host (checked on systemd 255 here and by the smoke container's systemd
252). The change's comment says a successful empty read permits a new
installation, but an empty read is not exit 0.

Why the tests missed it: their systemctl double reports no units as exit
0, and `test_partial_failed_inventory_is_not_absence` asserted that an
empty inventory with exit 1 is refused.

Fix: one shared inventory read, used by the installer, the unattended
updater and the auto-update mask check (which had the same hazard). It
accepts exactly the silent exit 1 as an empty inventory, after a second
read confirms there is still no output and no message. A timeout, a
signal, any other status, or an exit 1 that carries output or a message
stays a failed inventory, so real failures still refuse.

Tests: the doubles now model systemd (a failure prints its message, a
silent exit 1 is absence), the two tests that encoded the defect model a
real failure, and new tests cover a fresh host for both scripts,
including real `systemctl --root` over an empty unit root. Contract doc
updated. The registered runtime proofs for install.sh and the unattended
updater (the Go tests that drive this suite) now require the fresh-host
controls to have run and passed.

Checked on pulse-dev: against the unfixed scripts the four new
fresh-host tests fail and the rest pass; against this change all 18
discovery tests, the 2 repair tests and the 16 timer tests pass, and the
Go installer tests pass (184 s).

This replaces #2855, whose first commit lacked the registered proof
files the completion guard requires per commit.

Refs #2785
This commit is contained in:
rcourtman
2026-10-12 10:07:19 +01:00
committed by GitHub
parent e2f4bdff03 3e3ad9a5bd
commit bed5052eb5
7 changed files with 144 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/release-control/v6/internal/subsystems/deployment-installability.md
+12
View File
@@ -50,6 +50,18 @@ partial, malformed, duplicate, impostor or unknown-state inventory does not.
This avoids an early pipe close hiding an installed legacy service under
`pipefail`. Discovery itself never stops, enables or unmasks a unit.
Successful absence is what systemd reports on a fresh host: `systemctl
list-unit-files` exits 1 and prints nothing, on stdout or stderr, when none of
the named unit files exist. The installer, the unattended updater and the
auto-update mask check share one inventory read that accepts exactly that
silent exit 1 as an empty inventory, after a second read confirms no output and
no message. A timeout, a signal, any other status, or an exit 1 that carries
output or a message remains a failed inventory. The fixture doubles must model
that status: a silent exit 1 is absence, an exit 1 with a message is failure,
and `systemctl --root` over an empty unit root exercises the real status. A
double that reports absence as exit 0 hides the defect that made every fresh
install refuse.
The standalone unattended updater uses the same bounded name/state admission
before passing its explicit service identity to the installer. It checks the
command-substitution result separately from local variable declaration, and
install.sh
+23 -2
View File
@@ -750,6 +750,27 @@ recover_pulse_after_failed_replacement() {
fi
}
# Read the unit-file inventory for exact unit names. systemd exits 1 and prints
# nothing at all, on stdout or stderr, when none of the named unit files exist,
# which is every fresh host. That is an empty inventory. A timeout, a signal, or
# any status that comes with output or a message is a failed inventory, so a real
# failure still cannot pass as absence. A second read separates the two cases.
unit_file_inventory() {
local out="" again="" rc=0 rc2=0
out=$(LC_ALL=C timeout -k 1 5 systemctl list-unit-files --no-legend --no-pager --full -- "$@" 2>/dev/null) || rc=$?
if (( rc == 1 )) && [[ -z "$out" ]]; then
again=$(LC_ALL=C timeout -k 1 5 systemctl list-unit-files --no-legend --no-pager --full -- "$@" 2>&1) || rc2=$?
if (( rc2 == 0 )); then
out="$again"
rc=0
elif (( rc2 == 1 )) && [[ -z "$again" ]]; then
rc=0
fi
fi
printf '%s' "$out"
return "$rc"
}
# Detect existing service name (pulse or pulse-backend)
detect_service_name() {
if [[ "$SERVICE_NAME_EXPLICIT" == "true" ]]; then
@@ -767,7 +788,7 @@ detect_service_name() {
# grep -q closes a full inventory early: systemctl can report Broken pipe,
# and pipefail then hides a real pulse-backend unit behind the default.
# Consume one bounded inventory of the two exact historical names instead.
if ! unit_files=$(LC_ALL=C timeout -k 1 5 systemctl list-unit-files --no-legend --no-pager --full -- pulse-backend.service pulse.service 2>/dev/null); then
if ! unit_files=$(unit_file_inventory pulse-backend.service pulse.service); then
print_error "Cannot identify the Pulse systemd service: unit inventory failed. Reconcile service state before retrying." >&2
return 1
fi
@@ -4521,7 +4542,7 @@ auto_update_units_refreshable() {
timer_unit="$(basename "$timer_path")"
service_unit="$(basename "$service_path")"
if ! command -v systemctl >/dev/null 2>&1 ||
! unit_files=$(LC_ALL=C timeout -k 1 5 systemctl list-unit-files --no-legend --no-pager --full -- "$timer_unit" "$service_unit" 2>/dev/null); then
! unit_files=$(unit_file_inventory "$timer_unit" "$service_unit"); then
print_warn "Cannot confirm auto-update unit masks; preserving the existing helper and units."
return 1
fi
scripts/installtests/install_sh_test.go
+22
View File
@@ -4569,6 +4569,28 @@ func TestRootInstallServiceDiscoveryFailsClosedWithoutShortReads(t *testing.T) {
if err != nil {
t.Fatalf("installer service discovery/caller controls: %v\n%s", err, out)
}
// systemd exits 1 and prints nothing when no named unit file exists, which is every
// fresh host. These controls must have actually run and passed, with both the
// confined double and real systemctl over an empty unit root, for the installer and
// the unattended updater: a skipped or deleted control would hide a refusing installer.
for _, name := range []string{
"test_fresh_host_no_matching_unit_files_is_an_empty_inventory",
"test_partial_failed_inventory_is_not_absence",
} {
if !regexp.MustCompile(regexp.QuoteMeta(name) + ` \(.*\) \.\.\. ok`).Match(out) {
t.Fatalf("service discovery control %s did not run and pass:\n%s", name, out)
}
}
// The real-systemctl control needs systemctl: it must pass where systemctl exists
// and be skipped, not silently absent, where it does not (macOS).
realControl := "test_real_systemctl_on_a_host_without_pulse_units_selects_the_default"
outcome := `\.\.\. ok`
if _, lookErr := exec.LookPath("systemctl"); lookErr != nil {
outcome = `\.\.\. skipped`
}
if !regexp.MustCompile(regexp.QuoteMeta(realControl) + ` \(.*\) ` + outcome).Match(out) {
t.Fatalf("service discovery control %s did not report %q:\n%s", realControl, outcome, out)
}
}
func TestRootInstallStableReleaseTagRejectsPrereleaseShapes(t *testing.T) {
scripts/installtests/pulse_auto_update_test.go
+5 -1
View File
@@ -116,7 +116,11 @@ func TestAutoUpdateServiceDiscoveryAndInstallerHandoff(t *testing.T) {
"ServiceDiscovery.test_automatic_helper_preserves_exact_names_and_explicit_instances",
"ServiceDiscovery.test_automatic_helper_cannot_short_read_the_installer_handoff",
"ServiceDiscovery.test_automatic_helper_refuses_before_backup_or_installer_even_in_or_list",
"ServiceDiscovery.test_automatic_helper_timeout_stops_before_service_observation")
"ServiceDiscovery.test_automatic_helper_timeout_stops_before_service_observation",
// A fresh host has no Pulse unit files and systemd then exits 1 with no output; the
// updater's own discovery must read that as absence, not as a failed inventory.
"ServiceDiscovery.test_fresh_host_no_matching_unit_files_is_an_empty_inventory",
"ServiceDiscovery.test_real_systemctl_on_a_host_without_pulse_units_selects_the_default")
cmd.Env = append(os.Environ(),
"PULSE_INSTALLER_UNDER_TEST="+repoFile("install.sh"),
"PULSE_AUTO_UPDATER_UNDER_TEST="+repoFile("scripts", "pulse-auto-update.sh"))
scripts/pulse-auto-update.sh
+22 -1
View File
@@ -312,6 +312,27 @@ version_greater_than() {
return 1 # v1 <= v2
}
# Read the unit-file inventory for exact unit names. systemd exits 1 and prints
# nothing at all, on stdout or stderr, when none of the named unit files exist.
# That is an empty inventory. A timeout, a signal, or any status that comes with
# output or a message is a failed inventory, so a real failure still cannot pass
# as absence. A second read separates the two cases.
unit_file_inventory() {
local out="" again="" rc=0 rc2=0
out=$(LC_ALL=C timeout -k 1 5 systemctl list-unit-files --no-legend --no-pager --full -- "$@" 2>/dev/null) || rc=$?
if (( rc == 1 )) && [[ -z "$out" ]]; then
again=$(LC_ALL=C timeout -k 1 5 systemctl list-unit-files --no-legend --no-pager --full -- "$@" 2>&1) || rc2=$?
if (( rc2 == 0 )); then
out="$again"
rc=0
elif (( rc2 == 1 )) && [[ -z "$again" ]]; then
rc=0
fi
fi
printf '%s' "$out"
return "$rc"
}
# Detect service name (could be pulse or pulse-backend)
detect_service_name() {
if [[ -n "${PULSE_SERVICE_NAME:-}" ]]; then
@@ -328,7 +349,7 @@ detect_service_name() {
# Keep the standalone helper aligned with the server installer's bounded
# discovery. Its explicit installer handoff must not carry a short-read
# default past the installer's own discovery gate.
if ! unit_files=$(LC_ALL=C timeout -k 1 5 systemctl list-unit-files --no-legend --no-pager --full -- pulse-backend.service pulse.service 2>/dev/null); then
if ! unit_files=$(unit_file_inventory pulse-backend.service pulse.service); then
log error "Cannot identify the Pulse systemd service: unit inventory failed. Reconcile service state before retrying." >&2
return 1
fi
scripts/tests/test_server_service_discovery.py
+41 -10
View File
@@ -24,6 +24,8 @@ UPDATER = Path(os.environ.get("PULSE_AUTO_UPDATER_UNDER_TEST", ROOT / "scripts/p
REAL_SYSTEMCTL = shutil.which("systemctl")
QUERY = ["list-unit-files", "--no-legend", "--no-pager", "--full", "--",
"pulse-backend.service", "pulse.service"]
# What systemctl prints when the listing itself fails, as opposed to matching nothing.
FAILED_LISTING = "Failed to get unit file list: No such file or directory\n"
SYSTEMCTL = r'''#!/usr/bin/env python3
import json, os, sys, time
@@ -53,6 +55,8 @@ if args[0] == "list-unit-files":
os._exit(1)
sys.exit(0)
os.write(1, os.environ["ROWS"].encode())
if os.environ.get("ERR"):
os.write(2, os.environ["ERR"].encode())
sys.exit(int(os.environ["QUERY_EXIT"]))
if args[0] == "show":
state = "inactive" if (f / "stopped").exists() else "active"
@@ -183,7 +187,7 @@ exit "$status"
class ServiceDiscovery(unittest.TestCase):
def run_fixture(self, *, rows="", mode="rows", query_exit=0, flow="query", explicit=None,
producer="installer"):
producer="installer", err="", units_present=True):
with tempfile.TemporaryDirectory(prefix="pulse-service-discovery-") as directory:
f = Path(directory)
for name in ["bin", "install/bin", "config", "units", "root/etc/systemd/system"]:
@@ -212,12 +216,14 @@ class ServiceDiscovery(unittest.TestCase):
body = source[start:end].replace("/etc/systemd/system/", str(f / "units") + "/")
(f / "unit-writer.sh").write_text(body)
unit = "[Unit]\nDescription=Fixture only\n[Service]\nExecStart=/bin/true\n"
(f / "root/etc/systemd/system/pulse-backend.service").write_text(unit)
(f / "root/etc/systemd/system/pulse.service").write_text(unit)
if units_present:
(f / "root/etc/systemd/system/pulse-backend.service").write_text(unit)
(f / "root/etc/systemd/system/pulse.service").write_text(unit)
env = {k: v for k, v in os.environ.items() if not k.startswith("PULSE_")}
env.update(FIXTURE=str(f), INSTALLER=str(INSTALLER), UPDATER=str(UPDATER),
PRODUCER=producer, FLOW=flow, MODE=mode,
ROWS=rows, QUERY_EXIT=str(query_exit), REAL_SYSTEMCTL=REAL_SYSTEMCTL or "",
ROWS=rows, QUERY_EXIT=str(query_exit), ERR=err,
REAL_SYSTEMCTL=REAL_SYSTEMCTL or "",
PATH=str(f / "bin") + os.pathsep + os.environ["PATH"],
PULSE_INSTALL_DIR=str(f / "install"), PULSE_CONFIG_DIR=str(f / "config"),
PULSE_BINARY_LINK_PATH=str(f / "pulse-link"))
@@ -279,9 +285,23 @@ class ServiceDiscovery(unittest.TestCase):
self.assertEqual(e["calls"], [])
def test_partial_failed_inventory_is_not_absence(self):
for rows in ["", "pulse-backend.service enabled enabled\n", "pulse.service disabled enabled\n"]:
with self.subTest(rows=rows):
self.assert_refused(self.run_fixture(rows=rows, query_exit=1))
failure = "Failed to get unit file list: No such file or directory\n"
for rows, err in [("", failure), ("pulse-backend.service enabled enabled\n", ""),
("pulse.service disabled enabled\n", ""),
("pulse-backend.service enabled enabled\n", failure)]:
with self.subTest(rows=rows, err=err):
self.assert_refused(self.run_fixture(rows=rows, err=err, query_exit=1))
def test_fresh_host_no_matching_unit_files_is_an_empty_inventory(self):
# systemd exits 1 and prints nothing when none of the named unit files exist, which is
# every fresh host. The installer and the update helper must both select the default.
for producer in ("installer", "updater"):
with self.subTest(producer=producer):
e = self.run_fixture(producer=producer, query_exit=1)
self.assertEqual(e["result"].returncode, 0, e["result"].stderr)
self.assertEqual(e["result"].stdout, "SERVICE=pulse\n")
# The second read is what tells a silent absence from a failure with a message.
self.assertEqual(e["calls"], [QUERY, QUERY])
def test_malformed_duplicate_impostor_and_unknown_inventory_stop(self):
for rows in ["pulse-backendXservice enabled\n", "pulse-backend.service.extra enabled\n",
@@ -304,14 +324,15 @@ class ServiceDiscovery(unittest.TestCase):
self.assertEqual(e["result"].stdout, "SERVICE=pulse-backend\n")
def test_failed_discovery_is_distinct_from_no_installation(self):
e = self.run_fixture(flow="existing", query_exit=1)
e = self.run_fixture(flow="existing", query_exit=1,
err="Failed to get unit file list: No such file or directory\n")
self.assert_refused(e)
self.assertEqual(e["result"].returncode, 2)
def test_all_mutating_callers_refuse_when_errexit_is_disabled(self):
for flow in ["main", "late-main", "archive", "build", "uninstall", "reset", "unit", "ensure-unit"]:
with self.subTest(flow=flow):
self.assert_refused(self.run_fixture(flow=flow, query_exit=1))
self.assert_refused(self.run_fixture(flow=flow, query_exit=1, err=FAILED_LISTING))
def test_archive_and_source_paths_stop_only_the_selected_legacy_unit(self):
for flow in ["archive", "build"]:
@@ -323,6 +344,15 @@ class ServiceDiscovery(unittest.TestCase):
self.assertNotIn(["stop", "pulse"], e["calls"])
self.assertFalse(e["broken_pipe"])
@unittest.skipUnless(REAL_SYSTEMCTL, "systemctl unavailable")
def test_real_systemctl_on_a_host_without_pulse_units_selects_the_default(self):
# Real systemctl, empty unit root: the genuine exit status for "nothing matched".
for producer in ("installer", "updater"):
with self.subTest(producer=producer):
e = self.run_fixture(mode="real", producer=producer, units_present=False)
self.assertEqual(e["result"].returncode, 0, e["result"].stderr)
self.assertEqual(e["result"].stdout, "SERVICE=pulse\n")
@unittest.skipUnless(REAL_SYSTEMCTL, "systemctl unavailable")
def test_real_systemctl_fixture_inventory_has_compatible_rows(self):
e = self.run_fixture(mode="real")
@@ -356,7 +386,8 @@ class ServiceDiscovery(unittest.TestCase):
("pulse-backendXservice enabled\n", 0),
("pulse.service enabled enabled extra\n", 0)]:
with self.subTest(rows=rows, status=status):
e = self.run_fixture(producer="updater", rows=rows, query_exit=status, flow="automatic")
e = self.run_fixture(producer="updater", rows=rows, query_exit=status, flow="automatic",
err=FAILED_LISTING if (rows, status) == ("", 1) else "")
self.assert_refused(e)
self.assertIsNone(e["installer_service"])
scripts/tests/test_server_update_timer.py
+19 -13
View File
@@ -32,6 +32,7 @@ case "$1" in
fi
if [[ "$GUARD_INVENTORY" == hang ]]; then exec sleep 30; fi
printf '%s' "$GUARD_ROWS"
printf '%s' "${GUARD_ERR:-}" >&2
exit "$GUARD_EXIT"
fi
if [[ "$INVENTORY" == real ]]; then
@@ -125,7 +126,7 @@ class ServerUpdateTimerTest(unittest.TestCase):
def exercise(self, *, flow="query", state="enabled", service="pulse",
rows=None, query_exit=0, inventory="double", mask="",
config_enabled=True, mask_location="configured", guard_rows=None,
guard_exit=0, guard_inventory=None):
guard_exit=0, guard_inventory=None, guard_err=""):
with tempfile.TemporaryDirectory() as temporary:
fixture = Path(temporary)
config = fixture / "config"
@@ -188,7 +189,7 @@ class ServerUpdateTimerTest(unittest.TestCase):
guard_rows = f"{timer_name} {timer_state} enabled\n{service_name} {service_state} enabled\n"
env = {**os.environ, "INSTALLER": str(INSTALLER), "FIXTURE": str(fixture),
"FLOW": flow, "ROWS": rows, "QUERY_EXIT": str(query_exit),
"GUARD_ROWS": guard_rows, "GUARD_EXIT": str(guard_exit),
"GUARD_ROWS": guard_rows, "GUARD_EXIT": str(guard_exit), "GUARD_ERR": guard_err,
"GUARD_INVENTORY": guard_inventory or inventory,
"INVENTORY": inventory, "REAL_SYSTEMCTL": REAL_SYSTEMCTL or "",
"PATH": f"{tools}:{os.environ['PATH']}", "PULSE_SERVICE_NAME": service,
@@ -356,14 +357,16 @@ class ServerUpdateTimerTest(unittest.TestCase):
self.assertIn("masked-runtime", e["result"].stdout)
def test_failed_unknown_and_duplicate_effective_inventory_cannot_admit_writes(self):
for rows, code in (("pulse-update.timer disabled enabled\n", 1),
("pulse-update.timer not-understood enabled\n", 0),
("pulse-update.timer\n", 0),
("pulse-updateXtimer disabled enabled\n", 0),
("pulse-update.timer disabled enabled\npulse-update.timer disabled enabled\n", 0),
("", 1)):
with self.subTest(rows=rows, code=code):
self.assert_preserved(self.exercise(flow="version", guard_rows=rows, guard_exit=code))
failure = "Failed to get unit file list: No such file or directory\n"
for rows, code, err in (("pulse-update.timer disabled enabled\n", 1, ""),
("pulse-update.timer not-understood enabled\n", 0, ""),
("pulse-update.timer\n", 0, ""),
("pulse-updateXtimer disabled enabled\n", 0, ""),
("pulse-update.timer disabled enabled\npulse-update.timer disabled enabled\n", 0, ""),
("", 1, failure)):
with self.subTest(rows=rows, code=code, err=err):
self.assert_preserved(self.exercise(flow="version", guard_rows=rows, guard_exit=code,
guard_err=err))
def test_effective_inventory_is_bounded_and_missing_systemctl_preserves_assets(self):
self.assert_preserved(self.exercise(flow="version", guard_inventory="hang"))
@@ -372,9 +375,12 @@ class ServerUpdateTimerTest(unittest.TestCase):
self.assertEqual(e["calls"], [])
def test_successful_absent_or_unmasked_inventory_keeps_asset_repair_working(self):
for rows in ("", "pulse-update.timer disabled enabled\npulse-update.service disabled enabled\n"):
with self.subTest(rows=rows):
self.assert_refreshed(self.exercise(flow="assets", state="disabled", guard_rows=rows), False)
# systemd exits 1 and prints nothing when neither unit file exists: absence, not failure.
for rows, code in (("", 0), ("", 1),
("pulse-update.timer disabled enabled\npulse-update.service disabled enabled\n", 0)):
with self.subTest(rows=rows, code=code):
self.assert_refreshed(self.exercise(flow="assets", state="disabled", guard_rows=rows,
guard_exit=code), False)
def test_effective_inventory_names_are_exact_full_custom_instance_names(self):
service = "pulse-" + "department-" * 12