feat(k8s): show Helm events and logs #6626
Merged
Conversation
This file contains 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
eysi09
force-pushed
the
helm-show-events-and-logs
branch
12 times, most recently
from
53e6d26 to
ac842ca
Compare
Previously we would use Garden's own resource monitoring to monitor the health of a Helm install/upgrade and fail fast if one of the resources was unhealthy as well as show K8s events and Pod logs. In commit 7a68373 we replaced that with Helm's own `--wait` flag. The reason for that change was because a user reported issues with Garden returning early from the Helm command in the success case (see #6078 and #6053 for more details). The problem with that change is that since we weren't using our own resource monitoring we stopped showing events and logs when Helm installs/upgrades fail. Another problem is that Garden would now wait for the Helm command to complete which in the case of unhealthy resources means Helm will timeout, with a default of 300 seconds. This commit fixes that and we try to go for the best of both worlds: - We always use the `--wait` flag but also monitor resources at the same time - If the resources are healthy we wait for the Helm command to complete (this was the intent with 7a68373) - If we detect unhealthy resources we fail fast (as we did before on the same major version) - We add a flag to overwrite the fail fast behaviour in case a user might prefer that
eysi09
force-pushed
the
helm-show-events-and-logs
branch
from
ac842ca to
73836bb
Compare
twelvemo
approved these changes
Nov 13, 2024
| Waiting for the timeout can take awhile so using the default value here is recommended unless you'd like to completely mimic Helm's behaviour and not rely on Garden's resource monitoring. | ||
|
|
||
| Note that setting \`atomic\` to \`true\` implies \`waitForUnhealthyResources\`. | ||
| `), |
| log.warn(errorMsg) | ||
| log.debug({ error: toGardenError(error) }) | ||
| } | ||
| helmArgs = ["upgrade", releaseName, ...reference, "--install", ...commonArgs] |
There was a problem hiding this comment.
+1 on always using upgrade --install
There was a problem hiding this comment.
Yeah we also did this before, so know change here.
vvagaytsev
changed the title
vvagaytsev
changed the title
vvagaytsev
changed the title
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What this PR does / why we need it:
Previously we would use Garden's own resource monitoring to monitor the
health of a Helm install/upgrade and fail fast if one of the resources
was unhealthy as well as show K8s events and Pod logs.
In commit 7a68373 we replaced that with Helm's own
--waitflag.The reason for that change was because a user reported issues with
Garden returning early from the Helm command in the success case
(see #6078 and #6053 for more details).
The problem with that change is that since we weren't using our own
resource monitoring we stopped showing events and logs when Helm
installs/upgrades fail. Another problem is that Garden would now wait
for the Helm command to complete which in the case of unhealthy
resources means Helm will timeout, with a default of 300 seconds.
This commit fixes that and we try to go for the best of both worlds:
--waitflag but also monitor resources at the same timecomplete (this was the intent with 7a68373)
same major version)
might prefer that
Which issue(s) this PR fixes:
Fixes #6563
Special notes for your reviewer: