diff --git a/api/v1beta1/spec.go b/api/v1beta1/spec.go index fd319c88..bdb7406c 100644 --- a/api/v1beta1/spec.go +++ b/api/v1beta1/spec.go @@ -298,6 +298,19 @@ type HelmInstallOptions struct { // +kubebuilder:default:=false // +optional TakeOwnership bool `json:"takeOwnership,omitempty"` + + // RecoverAfterConsecutiveFailures is the number of consecutive install failures for this + // chart after which Sveltos uninstalls any existing release under this name before + // retrying, to clear potentially stale Helm release history that would otherwise keep + // blocking every subsequent install attempt. This only ever runs when there is no + // currently deployed release to protect: a release that is deployed, or mid-upgrade, or + // failed while already existing, is always retried through helm upgrade instead, never + // through this. It only applies to a release that was never successfully installed, or + // was already cleanly uninstalled. + // Default to 5 + // +kubebuilder:default:=5 + // +optional + RecoverAfterConsecutiveFailures int `json:"recoverAfterConsecutiveFailures,omitempty"` } type HelmUpgradeOptions struct { diff --git a/config/crd/bases/config.projectsveltos.io_clusterprofiles.yaml b/config/crd/bases/config.projectsveltos.io_clusterprofiles.yaml index ee597a98..10e4e827 100644 --- a/config/crd/bases/config.projectsveltos.io_clusterprofiles.yaml +++ b/config/crd/bases/config.projectsveltos.io_clusterprofiles.yaml @@ -318,6 +318,19 @@ spec: disable hooks on install Default to false type: boolean + recoverAfterConsecutiveFailures: + default: 5 + description: |- + RecoverAfterConsecutiveFailures is the number of consecutive install failures for this + chart after which Sveltos uninstalls any existing release under this name before + retrying, to clear potentially stale Helm release history that would otherwise keep + blocking every subsequent install attempt. This only ever runs when there is no + currently deployed release to protect: a release that is deployed, or mid-upgrade, or + failed while already existing, is always retried through helm upgrade instead, never + through this. It only applies to a release that was never successfully installed, or + was already cleanly uninstalled. + Default to 5 + type: integer replace: default: true description: Replaces if set indicates to replace an diff --git a/config/crd/bases/config.projectsveltos.io_clusterpromotions.yaml b/config/crd/bases/config.projectsveltos.io_clusterpromotions.yaml index 7a8836c7..ab3d6eb0 100644 --- a/config/crd/bases/config.projectsveltos.io_clusterpromotions.yaml +++ b/config/crd/bases/config.projectsveltos.io_clusterpromotions.yaml @@ -218,6 +218,19 @@ spec: disable hooks on install Default to false type: boolean + recoverAfterConsecutiveFailures: + default: 5 + description: |- + RecoverAfterConsecutiveFailures is the number of consecutive install failures for this + chart after which Sveltos uninstalls any existing release under this name before + retrying, to clear potentially stale Helm release history that would otherwise keep + blocking every subsequent install attempt. This only ever runs when there is no + currently deployed release to protect: a release that is deployed, or mid-upgrade, or + failed while already existing, is always retried through helm upgrade instead, never + through this. It only applies to a release that was never successfully installed, or + was already cleanly uninstalled. + Default to 5 + type: integer replace: default: true description: Replaces if set indicates to replace diff --git a/config/crd/bases/config.projectsveltos.io_clustersummaries.yaml b/config/crd/bases/config.projectsveltos.io_clustersummaries.yaml index 175e9265..9a3de849 100644 --- a/config/crd/bases/config.projectsveltos.io_clustersummaries.yaml +++ b/config/crd/bases/config.projectsveltos.io_clustersummaries.yaml @@ -355,6 +355,19 @@ spec: disable hooks on install Default to false type: boolean + recoverAfterConsecutiveFailures: + default: 5 + description: |- + RecoverAfterConsecutiveFailures is the number of consecutive install failures for this + chart after which Sveltos uninstalls any existing release under this name before + retrying, to clear potentially stale Helm release history that would otherwise keep + blocking every subsequent install attempt. This only ever runs when there is no + currently deployed release to protect: a release that is deployed, or mid-upgrade, or + failed while already existing, is always retried through helm upgrade instead, never + through this. It only applies to a release that was never successfully installed, or + was already cleanly uninstalled. + Default to 5 + type: integer replace: default: true description: Replaces if set indicates to replace diff --git a/config/crd/bases/config.projectsveltos.io_profiles.yaml b/config/crd/bases/config.projectsveltos.io_profiles.yaml index 34f26eb4..c4d7a97c 100644 --- a/config/crd/bases/config.projectsveltos.io_profiles.yaml +++ b/config/crd/bases/config.projectsveltos.io_profiles.yaml @@ -318,6 +318,19 @@ spec: disable hooks on install Default to false type: boolean + recoverAfterConsecutiveFailures: + default: 5 + description: |- + RecoverAfterConsecutiveFailures is the number of consecutive install failures for this + chart after which Sveltos uninstalls any existing release under this name before + retrying, to clear potentially stale Helm release history that would otherwise keep + blocking every subsequent install attempt. This only ever runs when there is no + currently deployed release to protect: a release that is deployed, or mid-upgrade, or + failed while already existing, is always retried through helm upgrade instead, never + through this. It only applies to a release that was never successfully installed, or + was already cleanly uninstalled. + Default to 5 + type: integer replace: default: true description: Replaces if set indicates to replace an diff --git a/controllers/handlers_helm.go b/controllers/handlers_helm.go index 77de9f87..4384ae29 100644 --- a/controllers/handlers_helm.go +++ b/controllers/handlers_helm.go @@ -100,11 +100,12 @@ var ( ) const ( - notInstalledMessage = "Not installed yet and action is uninstall" - defaultMaxHistory = 2 - defaultDeletionPropagation = "background" - defaultChartVersion = "0.1.0" - conditionStatusTrue = "True" + notInstalledMessage = "Not installed yet and action is uninstall" + defaultMaxHistory = 2 + defaultRecoverAfterConsecutiveFailures = 5 + defaultDeletionPropagation = "background" + defaultChartVersion = "0.1.0" + conditionStatusTrue = "True" ) type registryClientOptions struct { @@ -1590,11 +1591,11 @@ func handleInstall(ctx context.Context, dCtx *deploymentContext, logger.V(logs.LogDebug).Info("install helm release") - maxHistory := uint(getMaxHistoryValue(currentChart.Options)) + recoverAfter := getRecoverAfterConsecutiveFailuresValue(currentChart.Options) if !isPullMode { if fs := getFeatureSummaryForFeatureID(dCtx.clusterSummary, libsveltosv1beta1.FeatureHelm); fs != nil { - if fs.ConsecutiveFailures%maxHistory == 0 && fs.FailureMessage != nil { + if fs.ConsecutiveFailures%recoverAfter == 0 && fs.FailureMessage != nil { err := doUninstallRelease(ctx, dCtx.clusterSummary, currentChart, kubeconfig, registryOptions, logger) if err != nil { // Ignore release not found error @@ -2801,33 +2802,25 @@ func recoverRelease(ctx context.Context, clusterSummary *configv1beta1.ClusterSu requestedChart.ReleaseNamespace, kubeconfig, registryOptions, requestedChart, logger) } -// shouldInstall returns true if action is not uninstall and either there -// is no installed or version, or version is same requested by customer but status is -// not yet deployed +// shouldInstall returns true if action is not uninstall and there is no release to manage +// the lifecycle of: either none was ever created, or the last one was cleanly uninstalled. +// Any other state (deployed at a different version, failed, pending-*, superseded, ...) is +// an existing release that shouldUpgrade must handle instead. In particular, a release whose +// last upgrade/install attempt failed still has a release record (Helm keeps history for +// failed attempts too, stamped with the version that attempt tried to reach) — that must not +// be mistaken for "nothing to do here but install", since it goes through handleInstall's +// own uninstall-on-repeated-failure recovery, which is destructive and must stay reserved for +// the case where there is genuinely nothing deployed to lose. func shouldInstall(currentRelease *releaseInfo, requestedChart *configv1beta1.HelmChart) bool { if requestedChart.HelmChartAction == configv1beta1.HelmChartActionUninstall { return false } - if currentRelease != nil && - currentRelease.Status == releasecommon.StatusUninstalled.String() { - + if currentRelease == nil { return true } - if currentRelease != nil && - currentRelease.ChartVersion != requestedChart.ChartVersion { - - return false - } - - if currentRelease != nil && - currentRelease.Status == releasecommon.StatusDeployed.String() { - - return false - } - - return true + return currentRelease.Status == releasecommon.StatusUninstalled.String() } // shouldUpgrade returns true if action is not uninstall and current installed chart is different @@ -4291,6 +4284,17 @@ func getMaxHistoryValue(options *configv1beta1.HelmOptions) int { return defaultMaxHistory } +// getRecoverAfterConsecutiveFailuresValue returns the number of consecutive install failures +// handleInstall tolerates, for a release it is not currently protecting (see shouldInstall), +// before uninstalling any stale release record under that name and retrying. +func getRecoverAfterConsecutiveFailuresValue(options *configv1beta1.HelmOptions) uint { + if options != nil && options.InstallOptions.RecoverAfterConsecutiveFailures > 0 { + return uint(options.InstallOptions.RecoverAfterConsecutiveFailures) + } + + return defaultRecoverAfterConsecutiveFailures +} + func getCleanupOnFailValue(options *configv1beta1.HelmOptions) bool { if options != nil { return options.UpgradeOptions.CleanupOnFail diff --git a/controllers/handlers_helm_test.go b/controllers/handlers_helm_test.go index f60f0810..8db5f942 100644 --- a/controllers/handlers_helm_test.go +++ b/controllers/handlers_helm_test.go @@ -135,6 +135,35 @@ var _ = Describe("HandlersHelm", func() { Expect(controllers.ShouldInstall(nil, requestChart)).To(BeFalse()) }) + It("shouldInstall returns false for a failed release even when its recorded version matches the request", func() { + // A failed upgrade/install attempt still leaves a release record behind, stamped + // with the version that attempt tried (and failed) to reach. That must not be + // mistaken for "nothing to do here but install" - it has to be retried as an + // upgrade instead, never routed through handleInstall's uninstall-on-repeated- + // failure recovery. + currentRelease := &controllers.ReleaseInfo{ + Status: releasecommon.StatusFailed.String(), + ChartVersion: testChartVersion253, + } + requestChart := &configv1beta1.HelmChart{ + ChartVersion: testChartVersion253, + HelmChartAction: configv1beta1.HelmChartActionInstall, + } + Expect(controllers.ShouldInstall(currentRelease, requestChart)).To(BeFalse()) + }) + + It("shouldInstall returns true when the current release was cleanly uninstalled", func() { + currentRelease := &controllers.ReleaseInfo{ + Status: releasecommon.StatusUninstalled.String(), + ChartVersion: testChartVersion253, + } + requestChart := &configv1beta1.HelmChart{ + ChartVersion: testChartVersion253, + HelmChartAction: configv1beta1.HelmChartActionInstall, + } + Expect(controllers.ShouldInstall(currentRelease, requestChart)).To(BeTrue()) + }) + It("shouldUninstall returns false when there is no current release installed", func() { requestChart := &configv1beta1.HelmChart{ ChartVersion: testChartVersion253, diff --git a/lib/crd/clusterprofiles.go b/lib/crd/clusterprofiles.go index 9e585baa..9ae48d9e 100644 --- a/lib/crd/clusterprofiles.go +++ b/lib/crd/clusterprofiles.go @@ -337,6 +337,19 @@ spec: disable hooks on install Default to false type: boolean + recoverAfterConsecutiveFailures: + default: 5 + description: |- + RecoverAfterConsecutiveFailures is the number of consecutive install failures for this + chart after which Sveltos uninstalls any existing release under this name before + retrying, to clear potentially stale Helm release history that would otherwise keep + blocking every subsequent install attempt. This only ever runs when there is no + currently deployed release to protect: a release that is deployed, or mid-upgrade, or + failed while already existing, is always retried through helm upgrade instead, never + through this. It only applies to a release that was never successfully installed, or + was already cleanly uninstalled. + Default to 5 + type: integer replace: default: true description: Replaces if set indicates to replace an diff --git a/lib/crd/clusterpromotions.go b/lib/crd/clusterpromotions.go index dff1a41a..d05226c7 100644 --- a/lib/crd/clusterpromotions.go +++ b/lib/crd/clusterpromotions.go @@ -237,6 +237,19 @@ spec: disable hooks on install Default to false type: boolean + recoverAfterConsecutiveFailures: + default: 5 + description: |- + RecoverAfterConsecutiveFailures is the number of consecutive install failures for this + chart after which Sveltos uninstalls any existing release under this name before + retrying, to clear potentially stale Helm release history that would otherwise keep + blocking every subsequent install attempt. This only ever runs when there is no + currently deployed release to protect: a release that is deployed, or mid-upgrade, or + failed while already existing, is always retried through helm upgrade instead, never + through this. It only applies to a release that was never successfully installed, or + was already cleanly uninstalled. + Default to 5 + type: integer replace: default: true description: Replaces if set indicates to replace diff --git a/lib/crd/clustersummaries.go b/lib/crd/clustersummaries.go index e8ffb670..f72bf692 100644 --- a/lib/crd/clustersummaries.go +++ b/lib/crd/clustersummaries.go @@ -374,6 +374,19 @@ spec: disable hooks on install Default to false type: boolean + recoverAfterConsecutiveFailures: + default: 5 + description: |- + RecoverAfterConsecutiveFailures is the number of consecutive install failures for this + chart after which Sveltos uninstalls any existing release under this name before + retrying, to clear potentially stale Helm release history that would otherwise keep + blocking every subsequent install attempt. This only ever runs when there is no + currently deployed release to protect: a release that is deployed, or mid-upgrade, or + failed while already existing, is always retried through helm upgrade instead, never + through this. It only applies to a release that was never successfully installed, or + was already cleanly uninstalled. + Default to 5 + type: integer replace: default: true description: Replaces if set indicates to replace diff --git a/lib/crd/profiles.go b/lib/crd/profiles.go index e9226bfc..975ad111 100644 --- a/lib/crd/profiles.go +++ b/lib/crd/profiles.go @@ -337,6 +337,19 @@ spec: disable hooks on install Default to false type: boolean + recoverAfterConsecutiveFailures: + default: 5 + description: |- + RecoverAfterConsecutiveFailures is the number of consecutive install failures for this + chart after which Sveltos uninstalls any existing release under this name before + retrying, to clear potentially stale Helm release history that would otherwise keep + blocking every subsequent install attempt. This only ever runs when there is no + currently deployed release to protect: a release that is deployed, or mid-upgrade, or + failed while already existing, is always retried through helm upgrade instead, never + through this. It only applies to a release that was never successfully installed, or + was already cleanly uninstalled. + Default to 5 + type: integer replace: default: true description: Replaces if set indicates to replace an diff --git a/manifest/manifest.yaml b/manifest/manifest.yaml index 55eaa04d..027fc315 100644 --- a/manifest/manifest.yaml +++ b/manifest/manifest.yaml @@ -627,6 +627,19 @@ spec: disable hooks on install Default to false type: boolean + recoverAfterConsecutiveFailures: + default: 5 + description: |- + RecoverAfterConsecutiveFailures is the number of consecutive install failures for this + chart after which Sveltos uninstalls any existing release under this name before + retrying, to clear potentially stale Helm release history that would otherwise keep + blocking every subsequent install attempt. This only ever runs when there is no + currently deployed release to protect: a release that is deployed, or mid-upgrade, or + failed while already existing, is always retried through helm upgrade instead, never + through this. It only applies to a release that was never successfully installed, or + was already cleanly uninstalled. + Default to 5 + type: integer replace: default: true description: Replaces if set indicates to replace an @@ -3068,6 +3081,19 @@ spec: disable hooks on install Default to false type: boolean + recoverAfterConsecutiveFailures: + default: 5 + description: |- + RecoverAfterConsecutiveFailures is the number of consecutive install failures for this + chart after which Sveltos uninstalls any existing release under this name before + retrying, to clear potentially stale Helm release history that would otherwise keep + blocking every subsequent install attempt. This only ever runs when there is no + currently deployed release to protect: a release that is deployed, or mid-upgrade, or + failed while already existing, is always retried through helm upgrade instead, never + through this. It only applies to a release that was never successfully installed, or + was already cleanly uninstalled. + Default to 5 + type: integer replace: default: true description: Replaces if set indicates to replace @@ -6654,6 +6680,19 @@ spec: disable hooks on install Default to false type: boolean + recoverAfterConsecutiveFailures: + default: 5 + description: |- + RecoverAfterConsecutiveFailures is the number of consecutive install failures for this + chart after which Sveltos uninstalls any existing release under this name before + retrying, to clear potentially stale Helm release history that would otherwise keep + blocking every subsequent install attempt. This only ever runs when there is no + currently deployed release to protect: a release that is deployed, or mid-upgrade, or + failed while already existing, is always retried through helm upgrade instead, never + through this. It only applies to a release that was never successfully installed, or + was already cleanly uninstalled. + Default to 5 + type: integer replace: default: true description: Replaces if set indicates to replace @@ -9175,6 +9214,19 @@ spec: disable hooks on install Default to false type: boolean + recoverAfterConsecutiveFailures: + default: 5 + description: |- + RecoverAfterConsecutiveFailures is the number of consecutive install failures for this + chart after which Sveltos uninstalls any existing release under this name before + retrying, to clear potentially stale Helm release history that would otherwise keep + blocking every subsequent install attempt. This only ever runs when there is no + currently deployed release to protect: a release that is deployed, or mid-upgrade, or + failed while already existing, is always retried through helm upgrade instead, never + through this. It only applies to a release that was never successfully installed, or + was already cleanly uninstalled. + Default to 5 + type: integer replace: default: true description: Replaces if set indicates to replace an