ZEP-0026: Enhanced State Management#27
Conversation
Racer159
commented
Apr 29, 2025
Racer159
commented
- One-line PR description: Enhance Zarf's State management and tracking of resources/packages/components.
- Issue link: Zarf Enhanced State Management #26
- Other comments:
Signed-off-by: Wayne Starr <me@racer159.com>
Signed-off-by: Wayne Starr <me@racer159.com>
Signed-off-by: Wayne Starr <me@racer159.com>
|
Updated this proposal to account for where Zarf is today and looking to address concerns from zarf-dev/zarf#2992 (zarf-dev/zarf#4211) / zarf-dev/zarf#4969 / zarf-dev/zarf#4182 (zarf-dev/zarf#5007) |
Signed-off-by: Wayne Starr <me@racer159.com>
AustinAbro321
left a comment
AustinAbro321
left a comment
There was a problem hiding this comment.
Excited to see this develop, I think there are a lot of value adds. Some comments around UX and potential drawbacks.
| - `DeployedComponent` gains a `LastEvent` field recording only the most recent deploy/remove attempt against that specific component, since components mostly mirror the package's own timeline and rarely need a history of their own. | ||
| - Package status is read via `LatestEvent()`, component status by reading `LastEvent` directly - neither is a separately stored field, so neither can drift from what actually happened. See [Reading status from `Events`](#reading-status-from-events). | ||
| - On a graceful stop, Zarf appends a `Cancelled` event for whatever was in progress at the package level, and sets `Cancelled` on the in-progress component's `LastEvent`, rather than leaving either stuck at `InProgress`. See [Graceful Cancellation Handling](#graceful-cancellation-handling). | ||
| - `zarf package deploy` and `zarf package remove` gain an opt-in `--prune` flag, matching Helm / `kubectl` conventions, that removes any charts, components, or no-longer-referenced images orphaned by the new deployment. |
There was a problem hiding this comment.
I feel that --prune may be a bit overloaded to users, especially if they don't know how state works. For instance, rather than assuming it only gets rid of images, someone may expect that zarf package remove --prune removes all previous components stored in state for that package in the same way that the --prune flag does for zarf package deploy.
What would you think of something like --prune-registry for remove? Or alternatively something like --remove-orphans for deploy?
There was a problem hiding this comment.
I could see changing the flag name on remove but I do like the strong semantics --prune has as an "all-in" flag on deploy - IMO that helps keep clusters a bit more clean when you do want to use the flag - also this image prune should be less destructive than the other image prune because it doesn't do a full catalog it is only looking at the images this package put there (added a note on that).
There was a problem hiding this comment.
Would --prune benefit in any way by being a enum?
|
|
||
| ##### Chart reconciliation | ||
|
|
||
| For a component present in both the old and new package, the deploy path already builds the full list of installed charts for that component into a single `[]state.InstalledChart` - both actual Helm charts (`installCharts`) and charts synthesized from `manifests` (`installManifests`), see `deploy.go:531-546`. Each entry is uniquely identified by `namespace/chartName`, the same key `state.MergeInstalledChartsForComponent` already uses to merge chart state across deployments. `--prune` diffs the component's previously recorded `InstalledCharts` against this new list by that key: any chart present in the old list but absent from the new one is uninstalled with the same `helm.RemoveChart(ctx, chart.Namespace, chart.ChartName, opts.Timeout)` call `zarf package remove` already uses, and dropped from the stored `InstalledCharts` for that component. |
There was a problem hiding this comment.
Assuming you can afford the downtime, am I correct in that this could help solve issues such as zarf-dev/zarf#4031? Where if you run zarf package deploy --prune it would uninstall the existing component because the name no longer matches, then re-install during the deploy.
There was a problem hiding this comment.
yep it could help there
|
|
||
| For a component present in both the old and new package, the deploy path already builds the full list of installed charts for that component into a single `[]state.InstalledChart` - both actual Helm charts (`installCharts`) and charts synthesized from `manifests` (`installManifests`), see `deploy.go:531-546`. Each entry is uniquely identified by `namespace/chartName`, the same key `state.MergeInstalledChartsForComponent` already uses to merge chart state across deployments. `--prune` diffs the component's previously recorded `InstalledCharts` against this new list by that key: any chart present in the old list but absent from the new one is uninstalled with the same `helm.RemoveChart(ctx, chart.Namespace, chart.ChartName, opts.Timeout)` call `zarf package remove` already uses, and dropped from the stored `InstalledCharts` for that component. | ||
|
|
||
| ##### Component reconciliation |
There was a problem hiding this comment.
Probably worth noting as a drawback is that zarf-dev/zarf#3629 if --prune is used on a package that accidentally has the same name, then all components of the previous package will be unintentionally uninstalled.
Might be worth looking to solve here, or having an idea of how it might be solved in the future. One idea would be an optional --release-name flag similar to Helms approach, but I'm not sure many would opt-into that UX before they run into a problem.
There was a problem hiding this comment.
I could see a --release-name flag being added as well to this and that makes sense but curious what others' thoughts may be - it would be ideal if we could warn the user that they are likely installing the same named package over another but I don't know of a clean way to do that.
brandtkeller
left a comment
brandtkeller
left a comment
There was a problem hiding this comment.
leaving one thought as I'm reading through
| - Report deploy state / progress | ||
| - Report remove state / progress | ||
| - Resume multi-package deployment (based on uniqueness of package + config) | ||
| - Match Kubernetes/Helm conventions where we can |
There was a problem hiding this comment.
We also have zarf-dev/zarf#4768 which I'd be curious could be solved as a result or modification to this proposal.
If what we are solving is overall provenance - I wonder if we get closer to solving the gap between static package and deployed package.
Entirely fair is this is a non-goal for this proposal though.
3 participants
