Uh oh!
There was an error while loading. Please reload this page.
✨ Add checks for helm deployment status - #1349
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@## main #1349 +/- ##
==========================================
- Coverage 76.42% 74.55% -1.88%
==========================================
Files 41 42 +1 Lines 2431 2515 +84 ==========================================
+ Hits 1858 1875 +17 - Misses 400 453 +53 - Partials 173 187 +14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Uh oh!
There was an error while loading. Please reload this page.
| case rel.Info.Status == release.StatusDeployed: | ||
| case rel.Info.Status == release.StatusUninstalled: | ||
| case rel.Info.Status == release.StatusSuperseded: | ||
| case rel.Info.Status == release.StatusFailed: |
There was a problem hiding this comment.
If the release is failed, do we want to allow it to be overridden by default?
There was a problem hiding this comment.
Based on my understanding of #1296 I would assume we would not want to do this.
There was a problem hiding this comment.
It's in a terminal state; and if an upgrade is the only possible way to fix it, then we need to allow it to happen. Otherwise, the installation will remain in a failed state and be unable to be replaced without manual intervention (i.e. removal).
| switch { | ||
| case rel.Info.Status == release.StatusUnknown: | ||
| return nil, fmt.Errorf("installation status is unknown") | ||
| case rel.Info.Status == release.StatusDeployed: | ||
| case rel.Info.Status == release.StatusUninstalled: | ||
| case rel.Info.Status == release.StatusSuperseded: | ||
| case rel.Info.Status == release.StatusFailed: | ||
| case rel.Info.Status == release.StatusUninstalling: | ||
| return nil, fmt.Errorf("installation is still pending: %s", rel.Info.Status) | ||
| case rel.Info.Status.IsPending(): | ||
| return nil, fmt.Errorf("installation is still pending: %s", rel.Info.Status) | ||
| default: | ||
| return nil, fmt.Errorf("unknown installation status: %s", rel.Info.Status) | ||
| } | ||
There was a problem hiding this comment.
If the status of the release is "Terminal", then we ought to be able to upgrade etc. If it is unknown (in the multiple ways that unknown is defined, ha!) then it is considered an error, The default case catches the situation when the helm API changes. Only if something is pending (and for some reason, StatusUninstalling is not considered pending), then should we wait.
Uh oh!
There was an error while loading. Please reload this page.
Don't allow an update to occur when the deployment is still installing. Signed-off-by: Todd Short <tshort@redhat.com>
tmshort
commented
Oct 8, 2024
ping @thetechnick |
Fix#1296
Don't allow an update to occur when the deployment is still installing.
Description
Reviewer Checklist