Stage server update archives on the install filesystem, not /tmp (#3006)

## Summary

Unattended updates on small Proxmox LXCs were being OOM-killed. The
server installer staged the release archive (~280 MB for v6.5.0) and its
extracted tree (~750 MB) under `/tmp`. On Debian 13 and many LXC
templates `/tmp` is tmpfs, and tmpfs pages count against the container's
memory cgroup. The reporter's host log shows the `/lxc/118` cgroup at
its 1 GiB limit with ~820 MB of shmem, the kernel OOM-killing the
container's `systemd` (PID 1), and Proxmox then stopping the container.

`install.sh` now downloads and extracts into owner-only hidden
directories under the install directory:
- The install directory is always writable from the hardened
`pulse-update.service` sandbox.
- The staged binary already lives there.
- The disk headroom preflight now measures the filesystem the update
actually stages on.

The Go updater already prefers its data directory over `/tmp` for the
same reason.

Two follow-ups that come with staging on disk:
- **Stale staging sweep.** An interrupted run's staging (~1 GB) now
survives reboots. Staging setup removes stale `.pulse-extract-XXXXXX`
and `.pulse-<release>-linux-<arch>-XXXXXX` directories under the install
directory. It only removes real directories with those exact names,
never symlinks or files, and only when their ctime is more than 6 hours
old. `install.sh` has no update lock, so age is what proves no live run
still owns them.
- **Ownership.** Every recursive `chown -R pulse:pulse "$INSTALL_DIR"`
(archive install, source build, `setup_directories`) now goes through
`chown_install_dir_for_pulse`, which skips staging. The payload that
root still reads is never handed to the service account.

The `Failed to print table: Broken pipe` line in the report came from
`systemctl list-unit-files | grep -q` in v6.5.0's service discovery.
`main` already reads that inventory in full, and it was not the cause of
the failure.

## Tests

- New Go tests `TestRootInstallScriptSweepsOnlyStaleServerStaging`
(fresh staging kept, stale staging removed; look-alike names, files,
symlinks and their targets kept) and
`TestRootInstallScriptPulseOwnershipExcludesServerStaging` (staging
skipped, no raw recursive install-dir chown left).
- New `test_archive_extraction_stages_on_the_install_filesystem` (server
installer stop harness) and
`test_download_stages_archive_on_the_install_filesystem` (temporary
inputs harness) fail against the current installer and pass with this
change.
- `scripts/tests/test_server_installer_stop.py`,
`test_server_installer_temporary_inputs.py`,
`test_server_service_discovery.py`, and the `TestRootInstall*` tests in
`scripts/installtests` pass (run on Linux).

Refs #2785
This commit is contained in:
rcourtman
2026-10-17 02:10:35 +01:00
committed by GitHub
4 changed files with 234 additions and 12 deletions
Executable → Regular
+71 -7
View File
@@ -107,6 +107,7 @@ BUILD_FROM_SOURCE_MARKER="$INSTALL_DIR/BUILD_FROM_SOURCE"
DETECTED_CTID=""
UPDATE_MIN_TEMP_FREE_BYTES=$((900 * 1024 * 1024))
UPDATE_MIN_INSTALL_FREE_BYTES=$((256 * 1024 * 1024))
SERVER_STAGING_STALE_MINUTES=360
CONFIG_BACKUP_MIN_EXTRA_BYTES=$((64 * 1024 * 1024))
release_signature_key_available() {
@@ -3291,8 +3292,62 @@ create_temp_archive_path() {
# Reserving then unlinking a mktemp file does not reserve the .tar.gz or
# sidecar name. Keep both downloads inside one owner-only directory.
# A hidden staging prefix still yields the canonical archive file name.
local archive_name="${prefix##*/}"
archive_name="${archive_name#.}"
temp_dir=$(mktemp -d "${prefix}-XXXXXX") || return 1
printf '%s/%s.tar.gz\n' "$temp_dir" "${prefix##*/}"
printf '%s/%s.tar.gz\n' "$temp_dir" "$archive_name"
}
# Server archives are staged and extracted on the install filesystem. /tmp is
# tmpfs on Debian 13 and many LXC templates, where staged bytes count against
# the container's memory limit rather than its disk (#2785).
prepare_server_staging_parent() {
if ! mkdir -p -- "$INSTALL_DIR"; then
print_error "Could not prepare $INSTALL_DIR for staging the Pulse update; Pulse has not been stopped"
return 1
fi
sweep_stale_server_staging
printf '%s\n' "$INSTALL_DIR"
}
# Owner-only staging names created under $INSTALL_DIR by create_temp_archive_path
# and install_pulse_archive (mktemp's six-character suffix).
is_server_staging_name() {
case "$1" in
.pulse-extract-??????|.pulse-v*-linux-*-??????) return 0 ;;
esac
return 1
}
# Unlike /tmp, the install filesystem keeps an interrupted run's staging
# (~1 GB) across reboots. No installer lock exists, so remove only real
# directories (never symlinks or files) unchanged for longer than any live run
# takes. ctime, unlike mtime, cannot be carried back by extraction.
sweep_stale_server_staging() {
local entry=""
for entry in "$INSTALL_DIR"/.pulse-extract-* "$INSTALL_DIR"/.pulse-v*-linux-*; do
[[ -d "$entry" && ! -L "$entry" ]] || continue
is_server_staging_name "${entry##*/}" || continue
if [[ -n "$(find "$entry" -maxdepth 0 -cmin "+${SERVER_STAGING_STALE_MINUTES}" -print 2>/dev/null)" ]]; then
rm -rf -- "$entry" 2>/dev/null || print_warn "Could not remove stale update staging at $entry"
fi
done
return 0
}
# Recursive pulse ownership for the install directory, excluding root-consumed
# update staging so the extracted payload and archive are never handed to the
# service account while the installer still reads them.
chown_install_dir_for_pulse() {
local entry="" rc=0
chown pulse:pulse "$INSTALL_DIR" || rc=1
for entry in "$INSTALL_DIR"/* "$INSTALL_DIR"/.[!.]* "$INSTALL_DIR"/..?*; do
[[ -e "$entry" || -L "$entry" ]] || continue
is_server_staging_name "${entry##*/}" && continue
chown -R pulse:pulse "$entry" || rc=1
done
return "$rc"
}
cleanup_temp_archive_path() {
@@ -3536,7 +3591,9 @@ install_pulse_archive() {
expected_release=$(infer_release_from_archive_name "$archive_path" 2>/dev/null || true)
fi
if ! temp_extract=$(mktemp -d /tmp/pulse-extract-XXXXXX); then
local staging_parent=""
if ! staging_parent=$(prepare_server_staging_parent) ||
! temp_extract=$(mktemp -d "$staging_parent/.pulse-extract-XXXXXX"); then
print_error "Could not prepare archive extraction; Pulse has not been stopped"
return 1
fi
@@ -3614,7 +3671,7 @@ install_pulse_archive() {
install_additional_agent_binaries "$expected_release" "$temp_extract"
deploy_agent_scripts "$temp_extract"
chown -R pulse:pulse "$INSTALL_DIR"
chown_install_dir_for_pulse
print_success "Pulse binary installed to $INSTALL_DIR/bin/pulse"
install_binary_symlink "$INSTALL_DIR/bin/pulse" "$BINARY_LINK_PATH"
@@ -3653,8 +3710,15 @@ download_pulse() {
local pulse_arch=""
local archive_from_temp=false
local inferred_release=""
local staging_parent=""
if ! ensure_update_disk_headroom "/tmp" "$INSTALL_DIR"; then
# The archive download and its extraction live on the install
# filesystem, never /tmp. Debian 13 and many LXC templates mount /tmp
# as tmpfs, which the kernel charges to the container's memory limit:
# a ~280 MB archive plus its ~750 MB extracted tree OOM-killed a 1 GiB
# LXC's init mid-update and Proxmox stopped the container (#2785).
if ! staging_parent=$(prepare_server_staging_parent) ||
! ensure_update_disk_headroom "$staging_parent" "$INSTALL_DIR"; then
# The configuration snapshot taken earlier is not needed: the update
# never reached staging. Remove it so a retry loop on a low-space
# filesystem does not accumulate backups and worsen the shortage
@@ -3696,7 +3760,7 @@ download_pulse() {
print_info "Downloading Pulse..."
resolve_target_release
archive_path=$(create_temp_archive_path "/tmp/pulse-${LATEST_RELEASE}-linux-${pulse_arch}") || {
archive_path=$(create_temp_archive_path "$staging_parent/.pulse-${LATEST_RELEASE}-linux-${pulse_arch}") || {
print_error "Failed to create temporary archive path"
exit 1
}
@@ -4083,7 +4147,7 @@ build_from_source() {
echo "$branch-$source_revision" > "$INSTALL_DIR/VERSION"
echo "$branch" > "$BUILD_FROM_SOURCE_MARKER"
chown -R pulse:pulse "$INSTALL_DIR" 2>/dev/null || true
chown_install_dir_for_pulse 2>/dev/null || true
chown pulse:pulse "$BUILD_FROM_SOURCE_MARKER" 2>/dev/null || true
cd "$original_dir" >/dev/null 2>&1 || true
@@ -4104,7 +4168,7 @@ setup_directories() {
# Set permissions (preserve existing files)
# Use chown without -R on CONFIG_DIR to avoid changing existing file permissions
chown pulse:pulse "$CONFIG_DIR"
chown -R pulse:pulse "$INSTALL_DIR"
chown_install_dir_for_pulse
chmod 700 "$CONFIG_DIR"
# Ensure critical config files retain proper permissions if they exist
+132 -3
View File
@@ -6,6 +6,7 @@ import (
"os/exec"
"path/filepath"
"regexp"
"sort"
"strings"
"testing"
"time"
@@ -78,7 +79,7 @@ func TestRootInstallScriptArchiveSupportContract(t *testing.T) {
`ensure_update_disk_headroom()`,
`UPDATE_MIN_TEMP_FREE_BYTES=$((900 * 1024 * 1024))`,
`UPDATE_MIN_INSTALL_FREE_BYTES=$((256 * 1024 * 1024))`,
`ensure_update_disk_headroom "/tmp" "$INSTALL_DIR"`,
`ensure_update_disk_headroom "$staging_parent" "$INSTALL_DIR"`,
`prefetch_pulse_archive_for_container()`,
`download_release_archive()`,
`install_pulse_archive()`,
@@ -111,7 +112,7 @@ func TestRootInstallSystemdServiceExposesOnlyPulseOwnedSubscriptionCLIHome(t *te
func TestRootInstallScriptStagesUpdateBeforeStoppingService(t *testing.T) {
downloadPulse := extractRootInstallShellFunction(t, "download_pulse")
orderedSteps := []string{
`ensure_update_disk_headroom "/tmp" "$INSTALL_DIR"`,
`ensure_update_disk_headroom "$staging_parent" "$INSTALL_DIR"`,
`download_release_archive "$LATEST_RELEASE" "$pulse_arch" "$archive_path"`,
`run_upgrade_readiness_preflight "$CURRENT_VERSION" "$expected_release"`,
`install_pulse_archive "$archive_path" "$expected_release"`,
@@ -536,7 +537,7 @@ func TestRootInstallScriptDiscardsUnneededConfigBackup(t *testing.T) {
// backups while the filesystem is short of space (#2127).
func TestRootInstallScriptDiscardsBackupWhenStagingHeadroomFails(t *testing.T) {
downloadPulse := extractRootInstallShellFunction(t, "download_pulse")
headroom := strings.Index(downloadPulse, `ensure_update_disk_headroom "/tmp" "$INSTALL_DIR"`)
headroom := strings.Index(downloadPulse, `ensure_update_disk_headroom "$staging_parent" "$INSTALL_DIR"`)
discard := strings.Index(downloadPulse, `discard_last_config_backup`)
if headroom < 0 || discard < 0 {
t.Fatalf("download_pulse must check staging headroom and discard the unneeded config backup (headroom=%d discard=%d)", headroom, discard)
@@ -2591,3 +2592,131 @@ func TestRootInstallResetBoundsEverySystemdObservationAndMutation(t *testing.T)
}
}
}
// Server update staging lives under the install directory (#2785), which keeps
// an interrupted run's ~1 GB extract across reboots. The sweep removes only
// owner-only staging directories older than any live run, never symlinks,
// files, look-alike names or anything else in the install directory.
func TestRootInstallScriptSweepsOnlyStaleServerStaging(t *testing.T) {
installDir := filepath.Join(t.TempDir(), "install")
outside := filepath.Join(t.TempDir(), "outside")
for _, dir := range []string{
filepath.Join(installDir, "bin"),
filepath.Join(installDir, ".pulse-extract-AbC123", "bin"),
filepath.Join(installDir, ".pulse-v6.6.0-linux-amd64-XyZ789"),
filepath.Join(installDir, ".pulse-extract-toolong1"),
outside,
} {
if err := os.MkdirAll(dir, 0o700); err != nil {
t.Fatal(err)
}
}
files := []string{
filepath.Join(installDir, ".pulse-extract-AbC123", "bin", "pulse"),
filepath.Join(installDir, ".pulse-v6.6.0-linux-amd64-XyZ789", "pulse-v6.6.0-linux-amd64.tar.gz"),
filepath.Join(installDir, ".pulse-extract-file01"),
filepath.Join(installDir, "bin", "pulse"),
filepath.Join(outside, "keep"),
}
for _, file := range files {
if err := os.WriteFile(file, []byte("fixture"), 0o600); err != nil {
t.Fatal(err)
}
}
if err := os.Symlink(outside, filepath.Join(installDir, ".pulse-extract-link01")); err != nil {
t.Fatal(err)
}
sweep := func(minutes string) {
t.Helper()
script := `
set -euo pipefail
print_warn() { echo "$*" >&2; }
INSTALL_DIR="$1"
SERVER_STAGING_STALE_MINUTES="$2"
` + extractRootInstallShellFunction(t, "is_server_staging_name") + `
` + extractRootInstallShellFunction(t, "sweep_stale_server_staging") + `
sweep_stale_server_staging
`
if out, err := exec.Command("bash", "-c", script, "bash", installDir, minutes).CombinedOutput(); err != nil {
t.Fatalf("sweep: %v\n%s", err, out)
}
}
// A live run's staging is fresh: the production age keeps it.
sweep("360")
for _, file := range files {
if _, err := os.Stat(file); err != nil {
t.Fatalf("fresh staging or unrelated file removed: %s", file)
}
}
time.Sleep(1100 * time.Millisecond)
sweep("0")
for _, removed := range []string{".pulse-extract-AbC123", ".pulse-v6.6.0-linux-amd64-XyZ789"} {
if _, err := os.Lstat(filepath.Join(installDir, removed)); !os.IsNotExist(err) {
t.Fatalf("stale staging %s was not removed (err=%v)", removed, err)
}
}
for _, kept := range []string{".pulse-extract-toolong1", ".pulse-extract-file01", ".pulse-extract-link01", "bin/pulse"} {
if _, err := os.Lstat(filepath.Join(installDir, kept)); err != nil {
t.Fatalf("sweep removed non-staging entry %s: %v", kept, err)
}
}
if _, err := os.Stat(filepath.Join(outside, "keep")); err != nil {
t.Fatalf("sweep followed a staging-named symlink: %v", err)
}
}
// The recursive pulse ownership pass must not hand root-consumed update
// staging (verified archive, extracted payload) to the service account, and
// every recursive install-dir chown goes through that one helper.
func TestRootInstallScriptPulseOwnershipExcludesServerStaging(t *testing.T) {
content, err := os.ReadFile(filepath.Join("..", "..", "install.sh"))
if err != nil {
t.Fatal(err)
}
if regexp.MustCompile(`chown -R [^\n]*"\$INSTALL_DIR"`).Match(content) {
t.Fatal(`install.sh must use chown_install_dir_for_pulse, not chown -R on "$INSTALL_DIR"`)
}
installDir := filepath.Join(t.TempDir(), "install")
for _, dir := range []string{"bin", "scripts", ".pulse-extract-AbC123", ".pulse-v6.6.0-linux-amd64-XyZ789", ".hidden"} {
if err := os.MkdirAll(filepath.Join(installDir, dir), 0o700); err != nil {
t.Fatal(err)
}
}
if err := os.WriteFile(filepath.Join(installDir, "VERSION"), []byte("v6.6.0\n"), 0o600); err != nil {
t.Fatal(err)
}
calls := filepath.Join(t.TempDir(), "calls")
script := `
set -euo pipefail
INSTALL_DIR="$1"
CALLS="$2"
chown() { printf '%s\n' "$*" >> "$CALLS"; }
` + extractRootInstallShellFunction(t, "is_server_staging_name") + `
` + extractRootInstallShellFunction(t, "chown_install_dir_for_pulse") + `
chown_install_dir_for_pulse
`
if out, err := exec.Command("bash", "-c", script, "bash", installDir, calls).CombinedOutput(); err != nil {
t.Fatalf("chown_install_dir_for_pulse: %v\n%s", err, out)
}
recorded, err := os.ReadFile(calls)
if err != nil {
t.Fatal(err)
}
got := strings.Split(strings.TrimSpace(string(recorded)), "\n")
want := []string{
"pulse:pulse " + installDir,
"-R pulse:pulse " + filepath.Join(installDir, "VERSION"),
"-R pulse:pulse " + filepath.Join(installDir, "bin"),
"-R pulse:pulse " + filepath.Join(installDir, "scripts"),
"-R pulse:pulse " + filepath.Join(installDir, ".hidden"),
}
sort.Strings(got)
sort.Strings(want)
if strings.Join(got, "\n") != strings.Join(want, "\n") {
t.Fatalf("ownership calls:\n%s\nwant:\n%s", strings.Join(got, "\n"), strings.Join(want, "\n"))
}
}
@@ -231,6 +231,8 @@ class ServerInstallerStopTest(unittest.TestCase):
"originals": {str(p.relative_to(fixture)): b for p, b in originals.items()},
"calls": (fixture / "calls").read_text() if (fixture / "calls").exists() else "",
"archive_retained": (fixture / "archive.tgz").exists(),
"temporary_paths": (fixture / "temporary-paths").read_text().splitlines() if (fixture / "temporary-paths").exists() else [],
"install_dir": installed,
}
def test_refused_replacements_preserve_installed_bytes_and_modes(self):
@@ -353,6 +355,19 @@ class ServerInstallerStopTest(unittest.TestCase):
self.assertEqual(observation["temporary_paths_remaining"], [])
self.assertNotIn("start pulse-custom", calls)
def test_archive_extraction_stages_on_the_install_filesystem(self):
# /tmp is tmpfs on Debian 13 and many LXC templates: an extracted
# release there is charged to the container's memory limit and
# OOM-killed a 1 GiB LXC mid-update (#2785).
for flow in ("archive", "local_archive"):
with self.subTest(flow=flow):
observation = self.exercise(REPLACEMENT, flow, "active")
self.assertEqual(observation["result"].returncode, 0, observation["result"].stderr)
extracts = [Path(p) for p in observation["temporary_paths"] if "pulse-extract-" in p]
self.assertEqual(len(extracts), 1, observation["temporary_paths"])
self.assertEqual(extracts[0].parent, observation["install_dir"])
self.assertEqual(observation["temporary_paths_remaining"], [])
def test_all_manual_update_flows_leave_service_running_when_staging_fails(self):
for flow in ("version", "update", "reinstall"):
with self.subTest(flow=flow):
@@ -26,6 +26,7 @@ mktemp() {
[[ "$FAULT" != allocation ]] || return 1
local template="${@: -1}" created
local -a args=("$@")
printf '%s\n' "$template" >> "$FIXTURE/templates"
args[${#args[@]}-1]="$FIXTURE/shared/${template##*/}"
created=$(command mktemp "${args[@]}") || return 1
printf '%s\n' "$created" >> "$FIXTURE/created"
@@ -66,7 +67,7 @@ BUILD_FROM_SOURCE_MARKER="$INSTALL_DIR/BUILD_FROM_SOURCE"
CURRENT_VERSION=v6.5.0
LATEST_RELEASE=v6.6.0
detect_pulse_architecture() { echo amd64; }
ensure_update_disk_headroom() { return 0; }
ensure_update_disk_headroom() { printf '%s\n' "$1" > "$FIXTURE/headroom-path"; }
resolve_target_release() { :; }
download_release_archive() {
printf '%s\n' "$3" > "$FIXTURE/archive-path"
@@ -204,7 +205,8 @@ class ServerInstallerTemporaryInputsTest(unittest.TestCase):
result = subprocess.run(command, env=env, text=True,
capture_output=True, timeout=15)
observed = {}
for name in ("archive-path", "returned", "inferred", "calls", "transport", "directory-modes"):
for name in ("archive-path", "returned", "inferred", "calls", "transport", "directory-modes",
"templates", "headroom-path"):
p = fixture / name
observed[name] = p.read_text().splitlines() if p.exists() else []
archive_path = Path(observed["archive-path"][0]) if observed["archive-path"] else None
@@ -258,6 +260,18 @@ class ServerInstallerTemporaryInputsTest(unittest.TestCase):
self.assertEqual(o["shared_entries"], [], o)
self.assertEqual(o["calls"], ["INSTALL"] if fault in ("", "archive_admission") else [])
def test_download_stages_archive_on_the_install_filesystem(self):
# /tmp is tmpfs on Debian 13 and many LXC templates, where a staged
# release is charged to the container's memory limit (#2785).
r, o = self.exercise("download")
self.assertEqual(r.returncode, 0, r.stderr)
self.assertEqual(len(o["templates"]), 1, o)
template = Path(o["templates"][0])
self.assertEqual(template.parent.name, "install", o)
self.assertEqual(template.name, ".pulse-v6.6.0-linux-amd64-XXXXXX")
self.assertEqual(Path(o["headroom-path"][0]), template.parent)
self.assertEqual(Path(o["archive-path"][0]).name, "pulse-v6.6.0-linux-amd64.tar.gz")
def test_explicit_archive_and_sidecar_are_never_owned_or_deleted(self):
for fault in ("", "readiness", "archive_admission"):
with self.subTest(fault=fault):