Skip to content

fix(argo): clear stale child overrides + flip --commit to opt-in#243

Merged
gilesknap merged 2 commits into
mainfrom
switch-to-overrides
May 14, 2026
Merged

fix(argo): clear stale child overrides + flip --commit to opt-in#243
gilesknap merged 2 commits into
mainfrom
switch-to-overrides

Conversation

@gilesknap
Copy link
Copy Markdown
Member

Fixes #242. Pairs with epics-containers/argocd-monitor#43 (Monitor's Stop/Start UI).

Summary

  • Bug fix: push_value now also unsets parameter overrides on child keys (e.g. services.X.enabled) — previously, an override set by ec stop --no-commit (or by Monitor) would survive a subsequent ec deploy X v2 and the redeployed service would stay stopped. Lifts the existing child-walk loop from push_remove_key into a shared helper.
  • CLI: ec start / ec stop now default to the transient parameter-override path (same as --no-commit did before). Pass --commit to also write to the deployment repo for an audit trail. This mirrors what Monitor will do, so all Stop/Start traffic — CLI and UI — converges on one mechanism.
  • No chart changes. The services.X.enabledglobal.enabledreplicas chain in argocd-apps and ioc-instance already does the right thing.

Test plan

  • uv run pytest — 35/35 pass
  • New regression test test_deploy_clears_enabled_override reproduces the bug case (lingering services.X.enabled=false parameter override → deploy unsets it before refresh)
  • Manual: ec stop <svc> --no-commit, then ec deploy <svc> <ver>, confirm the service starts (was previously stuck stopped)
  • Manual: ec start <svc> (no flag) issues argocd app set only (no git push); ec start <svc> --commit writes to git

gilesknap added 2 commits May 14, 2026 07:30
push_value only unset the exact key it just wrote, so a parameter
override on a child key survived. Concrete failure: Monitor (or
`ec stop --no-commit`) sets `services.X.enabled=false`, then
`ec deploy X v2` writes `services.X` to values.yaml but the
`services.X.enabled=false` override remains and the redeployed
service stays stopped.

Lift the child-walk pattern from push_remove_key into a shared
helper and call it from both functions.
Flip --commit from on-by-default to opt-in. The default behaviour
now matches `ec stop --no-commit` (transient `argocd app set` on
the parent app's helm.parameters), which is the same mechanism
argocd-monitor's Stop/Start button will use. Pass --commit when
you want the change written to the deployment repo for an audit
trail.

Also aligns ArgoCommands.start/stop kwarg defaults with the
abstract base, k8s, and demo backends, which were already
commit=False.
@gilesknap gilesknap merged commit 400995f into main May 14, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

push_value leaves stale child parameter overrides; align start/stop with Monitor's override path

1 participant