feat: explain required install with no planned resource changes#645
Open
Sharvash wants to merge 4 commits into
Open
feat: explain required install with no planned resource changes#645Sharvash wants to merge 4 commits into
Sharvash wants to merge 4 commits into
Conversation
The `release plan install` command now explains why a release must still be installed even when no Kubernetes resource changes are planned. Previously, the plan command only printed a yellow warning saying it must still install the release, without explaining why. Users had no way to know whether the cause was a failed previous release, changed values, updated manifests, or something else. The `IsReleaseUpToDate` function now returns a typed `ReleaseOutdatedReason` alongside the existing boolean result. The reasons surfaced are: - there is no previously deployed release - the previously deployed release was not successful - the release notes changed - the release values changed - the release hooks changed - the release manifests changed The single function that already decides whether a release is up to date now also returns the reason, so there is no duplicated logic. Only the `release plan install` command surfaces the reason; the "skipped, already as desired" messages of the install and rollback commands are unchanged. Table-driven unit tests cover every branch of `IsReleaseUpToDate` and the message-builder guard (no dangling " because" when the reason is empty). Build, lint, and test pipeline is green. Closes werf#512 Signed-off-by: Alexey Gorovenko <sharvashinho@gmail.com>
Sharvash
force-pushed
the
feat/release-plan-outdated-reason
branch
from
4566520 to
f0c844c
Compare
ilya-lesikov
requested changes
Jun 23, 2026
| // Check if the new Release is up-to-date compared to the old Release. It doesn't check any | ||
| // resources of the release in the cluster, just compares Release objects. | ||
| func IsReleaseUpToDate(oldRel, newRel *helmrelease.Release) (bool, error) { | ||
| func IsReleaseUpToDate(oldRel, newRel *helmrelease.Release) (bool, ReleaseOutdatedReason, error) { |
Member
There was a problem hiding this comment.
I like the idea generally, but I wonder if we can make it cleaner. Later I'd like to be able to also show (at least in some cases) diffs on what changed, e.g. which values changed exactly or in what way manifests changed.
Maybe we shouldn't return isUpToDate + reason, but should return IsReleaseUpToDateResult instead? It can encapsulate both, can also contain additional info for displaying stuff later.
Contributor
Author
There was a problem hiding this comment.
Done, IsReleaseUpToDate now returns an IsReleaseUpToDateResult that carries UpToDate and Reason, instead of the separate bool and reason. That gives us a place to add the diff details later (which values changed, how the manifests differ) without touching the signature again.
IsReleaseUpToDate now returns (IsReleaseUpToDateResult, error) instead of (bool, ReleaseOutdatedReason, error). The struct carries the UpToDate flag and the Reason a release is outdated, replacing the positional (bool, reason) pair that every caller had to destructure by hand. All callers in pkg/action are updated accordingly. Behavior is unchanged: the same checks yield the same verdict and reason as before. Signed-off-by: Alexey Gorovenko <sharvashinho@gmail.com>
Sharvash
force-pushed
the
feat/release-plan-outdated-reason
branch
from
32b4716 to
feba3bd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
nelm's
release plan installcommand now explains why a release must still be installed even when no Kubernetes resource changes are planned.Previously, when the plan found no resource changes but the release still had to be installed, it printed only a generic yellow warning:
There was no way to tell what triggered the install — a failed previous release, changed values, updated manifests, or something else.
IsReleaseUpToDateis the single place that decides whether a release is up to date. It now returns a typedReleaseOutdatedReasonnext to its boolean result instead of just(bool, error), and the check is split into separate guard clauses that each return their specific reason:release plan installappends the reason to the existing warning through a smallreleaseMustInstallMessagehelper, so it now reads:The helper leaves the message untouched when the reason is empty, so there is never a dangling " because". The
release installandrelease rollbackcommands ignore the reason and keep their current "already as desired" messages unchanged.Table-driven unit tests cover every branch of
IsReleaseUpToDateand the message-builder guard. Build, lint and unit tests are green.Closes #512