fix(host-agent): a large update records its package count (#6794)
The root helper counted updated packages from the last 2,000 characters of the package manager's output. Every manager prints its count (apt's "N upgraded, N newly installed", dnf's "Upgrade N Packages", zypper's "N packages to upgrade", pacman's "Packages (N)") before the long log of what it then does, so a large update pushed the line out of the tail and the job recorded no count. Docker03's 2026-11-01 job applied 17 updates and reported none; Docker02's single package was counted. The update steps now read their whole output (run(keep=None)), and only the failure text and the job's detail are cut short. Agent 1.26.1, so hosts show the helper as an update to take. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -22,7 +22,7 @@ from collections import deque
|
|||||||
from concurrent.futures import ThreadPoolExecutor, wait as wait_futures
|
from concurrent.futures import ThreadPoolExecutor, wait as wait_futures
|
||||||
from datetime import datetime, timezone
|
from datetime import datetime, timezone
|
||||||
|
|
||||||
AGENT_VERSION = "1.26.0"
|
AGENT_VERSION = "1.26.1"
|
||||||
|
|
||||||
# Default path to the local Docker Engine socket. Overridable via the
|
# Default path to the local Docker Engine socket. Overridable via the
|
||||||
# `docker_socket` config key; collection is silently skipped if it's absent or
|
# `docker_socket` config key; collection is silently skipped if it's absent or
|
||||||
|
|||||||
@@ -101,10 +101,10 @@ def valid(req) -> bool:
|
|||||||
|
|
||||||
|
|
||||||
def run(cmd: list, deadline: int = COMMAND_DEADLINE_SECONDS, env: dict | None = None,
|
def run(cmd: list, deadline: int = COMMAND_DEADLINE_SECONDS, env: dict | None = None,
|
||||||
ok_codes: tuple = (0,), keep: int = OUTPUT_KEPT_CHARS):
|
ok_codes: tuple = (0,), keep: int | None = OUTPUT_KEPT_CHARS):
|
||||||
"""(ok, output tail) of a command, bounded by `deadline`. `ok_codes` are
|
"""(ok, output tail) of a command, bounded by `deadline`. `ok_codes` are
|
||||||
the exit codes that mean it worked (zypper says more than 0); `keep` is
|
the exit codes that mean it worked (zypper says more than 0); `keep` is
|
||||||
how much of the output's end is kept."""
|
how much of the output's end is kept, None for all of it."""
|
||||||
try:
|
try:
|
||||||
done = subprocess.run(cmd, capture_output=True, text=True, timeout=deadline,
|
done = subprocess.run(cmd, capture_output=True, text=True, timeout=deadline,
|
||||||
env=dict(os.environ, LANG="C", LC_ALL="C", **(env or {})))
|
env=dict(os.environ, LANG="C", LC_ALL="C", **(env or {})))
|
||||||
@@ -113,7 +113,7 @@ def run(cmd: list, deadline: int = COMMAND_DEADLINE_SECONDS, env: dict | None =
|
|||||||
except OSError as exc:
|
except OSError as exc:
|
||||||
return False, str(exc)
|
return False, str(exc)
|
||||||
out = (done.stdout or "") + (done.stderr or "")
|
out = (done.stdout or "") + (done.stderr or "")
|
||||||
return done.returncode in ok_codes, out[-keep:]
|
return done.returncode in ok_codes, out if keep is None else out[-keep:]
|
||||||
|
|
||||||
|
|
||||||
REBOOT_MARKER = "/var/run/reboot-required" # Debian and Ubuntu
|
REBOOT_MARKER = "/var/run/reboot-required" # Debian and Ubuntu
|
||||||
@@ -226,9 +226,11 @@ def do_update() -> dict:
|
|||||||
"(apt, dnf, yum, zypper, pacman) on this host"}
|
"(apt, dnf, yum, zypper, pacman) on this host"}
|
||||||
out = ""
|
out = ""
|
||||||
for label, cmd, deadline, env, ok_codes in update_steps(manager):
|
for label, cmd, deadline, env, ok_codes in update_steps(manager):
|
||||||
ok, out = run(cmd, deadline=deadline, env=env, ok_codes=ok_codes)
|
# The whole output: every manager prints its count before the long
|
||||||
|
# log of what it then does, so a tail loses it on a large update.
|
||||||
|
ok, out = run(cmd, deadline=deadline, env=env, ok_codes=ok_codes, keep=None)
|
||||||
if not ok:
|
if not ok:
|
||||||
return {"ok": False, "detail": f"{label} failed: " + out}
|
return {"ok": False, "detail": f"{label} failed: " + out[-OUTPUT_KEPT_CHARS:]}
|
||||||
changed = upgraded_count(out)
|
changed = upgraded_count(out)
|
||||||
if manager == "apt":
|
if manager == "apt":
|
||||||
run(["apt-get", "-y", "autoremove"], env={"DEBIAN_FRONTEND": "noninteractive"})
|
run(["apt-get", "-y", "autoremove"], env={"DEBIAN_FRONTEND": "noninteractive"})
|
||||||
|
|||||||
@@ -270,7 +270,7 @@ def test_a_zypper_update_that_asks_for_a_reboot_still_worked(maint, monkeypatch)
|
|||||||
monkeypatch.setattr(maint, "_os_release_id", lambda path="/etc/os-release": "sles")
|
monkeypatch.setattr(maint, "_os_release_id", lambda path="/etc/os-release": "sles")
|
||||||
ran = []
|
ran = []
|
||||||
|
|
||||||
def fake_run(cmd, deadline=0, env=None, ok_codes=(0,)):
|
def fake_run(cmd, deadline=0, env=None, ok_codes=(0,), keep=0):
|
||||||
ran.append(cmd[2])
|
ran.append(cmd[2])
|
||||||
code = 102 if cmd[2] == "update" else 0
|
code = 102 if cmd[2] == "update" else 0
|
||||||
return code in ok_codes, "3 packages to upgrade.\n"
|
return code in ok_codes, "3 packages to upgrade.\n"
|
||||||
@@ -282,6 +282,20 @@ def test_a_zypper_update_that_asks_for_a_reboot_still_worked(maint, monkeypatch)
|
|||||||
assert ran == ["refresh", "update"]
|
assert ran == ["refresh", "update"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_large_update_still_counts_its_packages(maint, monkeypatch):
|
||||||
|
"""apt prints its count before the log of unpacking and setting up each
|
||||||
|
package, which runs far past what a detail keeps."""
|
||||||
|
_manager(monkeypatch, maint, ("apt-get",))
|
||||||
|
log = "17 upgraded, 0 newly installed, 0 to remove and 0 not upgraded.\n" + \
|
||||||
|
"Setting up libfoo (1.2-3) ...\n" * 200
|
||||||
|
monkeypatch.setattr(maint, "run", lambda cmd, **kw: (True, log if kw.get("keep", 0) is None
|
||||||
|
else log[-maint.OUTPUT_KEPT_CHARS:]))
|
||||||
|
monkeypatch.setattr(maint, "reboot_required", lambda: False)
|
||||||
|
got = maint.do_update()
|
||||||
|
assert got["ok"] and got["packages"] == 17
|
||||||
|
assert len(got["detail"]) <= 500
|
||||||
|
|
||||||
|
|
||||||
def test_a_failed_step_says_which(maint, monkeypatch):
|
def test_a_failed_step_says_which(maint, monkeypatch):
|
||||||
_manager(monkeypatch, maint, ("pacman",))
|
_manager(monkeypatch, maint, ("pacman",))
|
||||||
monkeypatch.setattr(maint, "run", lambda cmd, **kw: (False, "error: failed to synchronize all databases"))
|
monkeypatch.setattr(maint, "run", lambda cmd, **kw: (False, "error: failed to synchronize all databases"))
|
||||||
|
|||||||
Reference in New Issue
Block a user