Still attempt upgrades on non-deployed releases #958
Conversation
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
Secretions
requested review from
itscaro,
mumoshu,
xiaomudk and
yxxhero
as code owners
Secretions
force-pushed
the
apply-failed-releases
branch
from
76fd201 to
b05b1e4
Compare
Secretions
force-pushed
the
apply-failed-releases
branch
5 times, most recently
from
2d921a8 to
3a0d762
Compare
yxxhero
force-pushed
the
apply-failed-releases
branch
from
40276b6 to
3666133
Compare
Secretions
force-pushed
the
apply-failed-releases
branch
from
3666133 to
b039878
Compare
|
@yxxhero Sorry, I didn't see the change you pushed, and accidentally orphaned it pushing a change of my own. If you still have that commit, would you mind pushing it again? Thanks! |
|
@Secretions don't worry. I just rebased the code. do what you want to do. |
|
@Secretions you should fix the conflicts. |
Currently, if _any_ helm release is extant in the cluster at all, "helmfile apply" will attempt to run a "helm diff" between the release in the cluster and the proposed release, and only run helm upgrade if the diff shows a change. While this is a sensible check for "deployed" releases, for failed (and possibly any other) release states, this results in the worst possible outcome: helmfile *appears* to finish successfully, while completely ignoring the failed release. This skips the diff for non-deployed releases, treating it in the same fashion as non-installed releases, always running the upgrade for these particular releases. This way, if re-running with the same chart would succeed, we have the opportunity to fix things on rerun; if it wouldn't, by rerunning the upgrade, we will again see the error rather than it going under the radar. Signed-off-by: Joaquin Lopez <[email protected]>
Signed-off-by: Joaquin Lopez <[email protected]>
Secretions
force-pushed
the
apply-failed-releases
branch
from
b039878 to
dffcf2f
Compare
* Diff output changed slightly
* Location of temp files not under /tmp running tests on mac
* Update `helm plugin install` reference to use `.git` suffix
* For some reason I haven't sussed out, I get "Unable to retrieve
local repo information" without it. Clearly this works in some
other environment, but doesn't seem like it should matter.
I can revert this part if it's a problem.
Signed-off-by: Joaquin Lopez <[email protected]>
Secretions
force-pushed
the
apply-failed-releases
branch
from
dffcf2f to
d0897bc
Compare
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
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.
Currently, if any helm release is extant in the cluster at all, "helmfile apply" will attempt to run a "helm diff" between the release in the cluster and the proposed release, and only run helm upgrade if the diff shows a change.
While this is a sensible check for "deployed" releases, for failed (and possibly any other) release states, if the diff hasn't changed (even though the release never succeded), it results in the worst possible outcome: helmfile appears to finish successfully, while completely ignoring the failed release.
This ensures we always trigger an upgrade on releases that are in a non-deployed state. This way, if re-running with the same chart would succeed, we have the opportunity to fix things on rerun (let's say a helm hook pre or post-upgrade job will succeed now due to fixing a service external to the chart); if it wouldn't, by rerunning the upgrade, we will again see the error rather than it going under the radar and giving the false impression that everything is hunky dory.