[PR #21] [MERGED] fix: address code-review findings across driver, flows, window, and helpers #21

New issue

Closed

opened 2026-06-10 14:01:35 +08:00 by ivenator1 · 0 comments

ivenator1 commented

2026-06-10 14:01:35 +08:00

Owner

📋 Pull Request Information

Original PR: https://github.com/ivenator1/proxmox-management/pull/21
Author: @ivenator1
Created: 6/10/2026
Status: ✅ Merged
Merged: 6/10/2026
Merged by: @ivenator1

Base: main ← Head: claude/code-review-8copts

📝 Commits (1)

facbd77 fix: address code-review findings across driver, flows, window, and helpers

📊 Changes

20 files changed (+422 additions, -88 deletions)

View changed files

📝 fleet-update.py (+3 -23)
📝 proxmox_fleet/cli.py (+25 -9)
📝 proxmox_fleet/driver.py (+14 -5)
📝 proxmox_fleet/executor.py (+1 -1)
📝 proxmox_fleet/flows/lxc.py (+31 -12)
📝 proxmox_fleet/flows/remote.py (+3 -1)
📝 proxmox_fleet/flows/vm.py (+3 -1)
📝 proxmox_fleet/models/config.py (+6 -2)
📝 proxmox_fleet/orchestration.py (+9 -2)
📝 proxmox_fleet/status.py (+53 -17)
📝 proxmox_fleet/window.py (+8 -3)
📝 tests/unit/test_config_model.py (+18 -0)
📝 tests/unit/test_driver.py (+49 -0)
📝 tests/unit/test_flow_lxc.py (+59 -6)
📝 tests/unit/test_flow_remote.py (+20 -0)
📝 tests/unit/test_flow_vm.py (+21 -0)
📝 tests/unit/test_orchestration.py (+25 -0)
📝 tests/unit/test_status_lxc.py (+60 -3)
📝 tests/unit/test_window.py (+10 -1)
📝 tests/unit/test_wrapper.py (+4 -2)

📄 Description

driver: coerce -e extravar strings with _truthy() — bool('false') was True,
so '-e force_window=false' bypassed maintenance windows and
'-e fleet_dry_run=false' forced dry-run in the custom phase; also include
check= in the custom phase's dry-run flag (it ran real update steps under
--check since run_shell primitives have check_mode: false)
window: day matching is now case-insensitive (parity with the retired
check-window.yml) and uses a fixed weekday list instead of locale-sensitive
strftime('%a')
vm/remote flows: never poll Kuma in dry-run — 'changed' only means updates
are available, and an unhealthy monitor failed dry-runs after the full
retry window
lxc flow: reboot-required check runs unconditionally again (os_only_lxc_list
containers were never auto-rebooted after kernel updates); apt commands pin
DEBIAN_FRONTEND=noninteractive so debconf prompts can't stall pct exec
status: new lxc_app_did_update() shared by lxc_app_status and the flow's
health-check gate (no more re-parsing rendered status strings);
lxc_should_report takes script_expected so an unexpectedly missing update
script surfaces as an anomaly while os_only containers stay suppressed
models: MaintenanceWindow is extra='forbid' — a typo'd key silently widened
the window to every day
orchestration: run_concurrent shuts the pool down with wait=False so a hung
worker can't block the phase after its timeout fired
executor: snapshot_with_retry catches Exception (not just RuntimeError),
honoring its no-raise contract when the snapshot primitive throws
cli/wrapper: extravar parsing and settings propagation shared via
apply_extravar_overrides(); drop the wrapper's fleet_dry_run setdefault hack

https://claude.ai/code/session_01DF9GxY796E3QwssS7MMGSC

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/ivenator1/proxmox-management/pull/21 **Author:** [@ivenator1](https://github.com/ivenator1) **Created:** 6/10/2026 **Status:** ✅ Merged **Merged:** 6/10/2026 **Merged by:** [@ivenator1](https://github.com/ivenator1) **Base:** `main` ← **Head:** `claude/code-review-8copts` --- ### 📝 Commits (1) - [`facbd77`](https://github.com/ivenator1/proxmox-management/commit/facbd77675f8cfcb3f1cc0c593814a827366b2c0) fix: address code-review findings across driver, flows, window, and helpers ### 📊 Changes **20 files changed** (+422 additions, -88 deletions) <details> <summary>View changed files</summary> 📝 `fleet-update.py` (+3 -23) 📝 `proxmox_fleet/cli.py` (+25 -9) 📝 `proxmox_fleet/driver.py` (+14 -5) 📝 `proxmox_fleet/executor.py` (+1 -1) 📝 `proxmox_fleet/flows/lxc.py` (+31 -12) 📝 `proxmox_fleet/flows/remote.py` (+3 -1) 📝 `proxmox_fleet/flows/vm.py` (+3 -1) 📝 `proxmox_fleet/models/config.py` (+6 -2) 📝 `proxmox_fleet/orchestration.py` (+9 -2) 📝 `proxmox_fleet/status.py` (+53 -17) 📝 `proxmox_fleet/window.py` (+8 -3) 📝 `tests/unit/test_config_model.py` (+18 -0) 📝 `tests/unit/test_driver.py` (+49 -0) 📝 `tests/unit/test_flow_lxc.py` (+59 -6) 📝 `tests/unit/test_flow_remote.py` (+20 -0) 📝 `tests/unit/test_flow_vm.py` (+21 -0) 📝 `tests/unit/test_orchestration.py` (+25 -0) 📝 `tests/unit/test_status_lxc.py` (+60 -3) 📝 `tests/unit/test_window.py` (+10 -1) 📝 `tests/unit/test_wrapper.py` (+4 -2) </details> ### 📄 Description - driver: coerce -e extravar strings with _truthy() — bool('false') was True, so '-e force_window=false' bypassed maintenance windows and '-e fleet_dry_run=false' forced dry-run in the custom phase; also include check= in the custom phase's dry-run flag (it ran real update steps under --check since run_shell primitives have check_mode: false) - window: day matching is now case-insensitive (parity with the retired check-window.yml) and uses a fixed weekday list instead of locale-sensitive strftime('%a') - vm/remote flows: never poll Kuma in dry-run — 'changed' only means updates are available, and an unhealthy monitor failed dry-runs after the full retry window - lxc flow: reboot-required check runs unconditionally again (os_only_lxc_list containers were never auto-rebooted after kernel updates); apt commands pin DEBIAN_FRONTEND=noninteractive so debconf prompts can't stall pct exec - status: new lxc_app_did_update() shared by lxc_app_status and the flow's health-check gate (no more re-parsing rendered status strings); lxc_should_report takes script_expected so an unexpectedly missing update script surfaces as an anomaly while os_only containers stay suppressed - models: MaintenanceWindow is extra='forbid' — a typo'd key silently widened the window to every day - orchestration: run_concurrent shuts the pool down with wait=False so a hung worker can't block the phase after its timeout fired - executor: snapshot_with_retry catches Exception (not just RuntimeError), honoring its no-raise contract when the snapshot primitive throws - cli/wrapper: extravar parsing and settings propagation shared via apply_extravar_overrides(); drop the wrapper's fleet_dry_run setdefault hack https://claude.ai/code/session_01DF9GxY796E3QwssS7MMGSC --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

ivenator1

2026-06-10 14:01:35 +08:00