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.
brandtkeller
left a comment
brandtkeller
left a comment
There was a problem hiding this comment.
leaving one thought as I'm reading through
jasonwashburn
left a comment
jasonwashburn
left a comment
There was a problem hiding this comment.
For SDK applications like the Tofu provider, I think we should consider persisting the actual successful deploy configuration itself in addition to its digest. ConfigDigest can tell us whether two configurations differ, but it cannot tell us what deployment-time configuration and values are currently represented by cluster state. Without the config itself, the tofu provider cant accurately import an existing package and hydrate its resource state or detect drift/update tofu state to show an accurate plan without making some assumptions.
I suggest adding a single top-level DeployConfig and matching ConfigDigest to DeployedPackage which represents the latest successful deployment. Failed or cancelled deploy attempts should remain visible through PackageEvent.ConfigDigest, but probably shouldn't replace the config representing the package's current installed state.
Optionally we could store each event's full config alongside PackageEvent.ConfigDigest, but I can't currently think of any immediately useful need for consuming that history...so it's probably not worth the additional Secret growth.
Signed-off-by: Wayne Starr <me@racer159.com>
Signed-off-by: Wayne Starr <me@racer159.com>
Signed-off-by: Wayne Starr <me@racer159.com>
Signed-off-by: Wayne Starr <me@racer159.com>
Signed-off-by: Wayne Starr <me@racer159.com>
Signed-off-by: Wayne Starr <me@racer159.com>
| - Redeploy (when the source is online - e.g. https/oci) | ||
| - Report deploy state / progress | ||
| - Report remove state / progress | ||
| - Resume multi-package deployment (identified by package digest + config digest) |
There was a problem hiding this comment.
I think the requirements laid out can be used to accomplish this goal but I have concerns about including it as a goal explicitly. Primarily on the basis of the Zarf project focusing on package primitives and the scope of single package operations - and enabling other tooling to build more complex handling/orchestration.
There was a problem hiding this comment.
changed it to just focus on identifying if this is the same package deployment instead of having the multi package wording.
Signed-off-by: Wayne Starr <me@racer159.com>
brandtkeller
left a comment
brandtkeller
left a comment
There was a problem hiding this comment.
lgtm - added a thought after giving this another pass with RBAC considerations but otherwise believe it to be well scoped.
|
|
||
| ##### Image reconciliation | ||
|
|
||
| When `images` is selected, Zarf diffs each changed or removed component's previously deployed `Images` against the new component's `Images` (both plain `[]string` image references on `v1alpha1.ZarfComponent`). An image present in the old set but not the new one is a pruning candidate only if no other deployed package still needs it: Zarf calls `Cluster.GetDeployedZarfPackages` to list every `DeployedPackage` secret in the cluster and checks whether the candidate image appears in any other package's component images. If nothing else references it, both tags Zarf pushes for that image are removed from the internal registry - the plain tag and the CRC-32-suffixed tag the Zarf agent uses for transparent redirection (see `images/push.go`). If another package still references it, both tags are left alone. |
There was a problem hiding this comment.
This idea is still sitting in a draft locally - but I want to at least callout the change in potential RBAC scope here. charts and components are scoped to a single package - where pruning images is going to require privileges to read all state secrets.
I haven't amended the namespace ZEP with this context yet so I wouldn't block on it - but I am trying to look for patterns we may want to start to separate from expectations.
|
|
||
| Today, if a `zarf package deploy` or `zarf package remove` is interrupted, the latest package `PackageEvent`/component `LastEvent` (once this proposal exists) can be left at `InProgress` indefinitely, with nothing to correct it afterward. | ||
|
|
||
| On a controlled stop - Zarf catching `SIGINT`/`SIGTERM` via a cancellable `context.Context` (e.g. `signal.NotifyContext`) - Zarf will attempt to append a `Cancelled` package event and set the in-progress component's `LastEvent` to `Cancelled` before exiting. This is best-effort: an ungraceful termination (`SIGKILL`, node crash, power loss) gives Zarf no opportunity to run any code, so the latest event can still be left at `InProgress` - see [Risks and Mitigations](#risks-and-mitigations). |
There was a problem hiding this comment.
in support of this where possible - I believe we started down this path with chart installations. (no action)
4 participants
