fix: decide uninstall success by exit status, not by keywords #2

2026-09-25T12:41:37Z

christianmanivong commented

2026-09-25 12:41:37 +00:00

Summary

uninstall_package decided success by searching the package manager's output for failure words, because every command ran as ... || true and the exit status was lost. It now reads the real exit status.

Changes

New LinuxDriver._sudo_status(command) returns (output, rc): it appends ; echo __NETORK_RC=$? after the sudo pipeline (so $? is sudo's status, which passes the command's through; a wrong password is non-zero too), strips the marker from the output, and returns rc=None if the marker never arrives. The marker is only accepted on its own line followed by digits, so an echoed command line (literal $?) cannot be mistaken for it. _sudo and its other callers are unchanged.
uninstall_package (apt/dnf/yum/apk/pacman and the dpkg --purge --force-all fallback) uses it. _uninstall_failed(output, rc) decides by rc != 0 when rc is known and uses the old keyword check only when it is not.
OpenMediaVault and QNAP QTS inherit from LinuxDriver and get the same behaviour.

Behaviour change

Removing a package that is not installed now reports success on apt/dnf: they exit 0 and the package is absent afterwards, so netOrk correctly drops it from user_installed_packages. pacman still exits 1 there and stays a failure.

Testing

New TestSudoStatus (5) and TestUninstallExitStatus (8), written first; 12 failed before the fix.
pytest tests/test_linux.py → 88 passed; qnap-qts (36 passed, 1 xfailed) and openmediavault (3 passed) suites pass against this branch.

Follow-ups

install_package uses the same keyword-over-|| true pattern; candidate for _sudo_status too.
linux.py has ~45 pre-existing ruff/mypy errors (undefined Dict/List/Optional in annotations, F841, F541) — untouched here.

Refs christianmanivong/netork#267. NetOrk bumps the pin in a separate PR.

🤖 Generated with Claude Code

## Summary `uninstall_package` decided success by searching the package manager's output for failure words, because every command ran as `... || true` and the exit status was lost. It now reads the real exit status. ## Changes - New `LinuxDriver._sudo_status(command)` returns `(output, rc)`: it appends `; echo __NETORK_RC=$?` after the sudo pipeline (so `$?` is sudo's status, which passes the command's through; a wrong password is non-zero too), strips the marker from the output, and returns `rc=None` if the marker never arrives. The marker is only accepted on its own line followed by digits, so an echoed command line (literal `$?`) cannot be mistaken for it. `_sudo` and its other callers are unchanged. - `uninstall_package` (apt/dnf/yum/apk/pacman and the `dpkg --purge --force-all` fallback) uses it. `_uninstall_failed(output, rc)` decides by `rc != 0` when rc is known and uses the old keyword check only when it is not. - OpenMediaVault and QNAP QTS inherit from `LinuxDriver` and get the same behaviour. ## Behaviour change Removing a package that is not installed now reports success on apt/dnf: they exit 0 and the package is absent afterwards, so netOrk correctly drops it from `user_installed_packages`. pacman still exits 1 there and stays a failure. ## Testing - New `TestSudoStatus` (5) and `TestUninstallExitStatus` (8), written first; 12 failed before the fix. - `pytest tests/test_linux.py` → 88 passed; qnap-qts (36 passed, 1 xfailed) and openmediavault (3 passed) suites pass against this branch. ## Follow-ups - `install_package` uses the same keyword-over-`|| true` pattern; candidate for `_sudo_status` too. - `linux.py` has ~45 pre-existing ruff/mypy errors (undefined `Dict`/`List`/`Optional` in annotations, F841, F541) — untouched here. Refs christianmanivong/netork#267. NetOrk bumps the pin in a separate PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

christianmanivong added 1 commit 2026-09-25 12:41:37 +00:00

fix: decide uninstall success by exit status, not by keywords ac288823a7

uninstall_package judged success by searching apt/dnf/apk/pacman output
for failure words. That is guesswork in both directions: apt's commonest
failure ("E: Sub-process /usr/bin/dpkg returned an error code (1)") read
as success until the previous change, and a prerm that prints "Failed to
stop ..." while the removal completes still reads as failure. The exit
status is the answer the package manager actually gives, but every
command went through `_sudo(... || true)`, which throws it away.

Add `_sudo_status()`, which runs the command via `_sudo` followed by
`; echo __NETORK_RC=$?` and returns `(output, exit_status)` with the
marker stripped. The `|| true` of other `_sudo` callers is untouched:
they still want output rather than a status. The marker is matched only
on a line of its own with digits, so an echoed command line (literal
`$?`) is never mistaken for it. If the marker never arrives the status
is None -- unknown, not success.

uninstall_package and its dpkg fallback now use it, and
`_uninstall_failed(output, rc)` lets rc decide whenever it is known,
falling back to the keyword check only when it is not.

Behaviour change worth knowing: removing a package that is not installed
exits 0 on apt (and dnf), so it now reports success where the keyword
"is not installed" used to report failure. The package is absent
afterwards, which is what the caller asked for, and netOrk dropping it
from the installed record is then correct.

Refs christianmanivong/netork#267

christianmanivong referenced this pull request

2026-09-25 12:43:56 +00:00

install_package still reads success from keywords; ruff/mypy debt; uninstall contract mismatch #4

christianmanivong merged commit b6b1827f96 into master

2026-09-25 20:53:07 +00:00

christianmanivong deleted branch fix/uninstall-exit-status

2026-09-25 20:53:07 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: NAPALM/napalm-linux#2