feat: update strategy for reinstall (#2019)

* feat: Add updateStrategy option in the state file with 'reinstall'/'reinstallIfForbidden' choices to uninstall and apply the specific release(s) (if forbidden to update) Signed-off-by: Simon Bouchard <sbouchard@rbbn.com> * Fix unit tests related to the new updateStrategy feature Signed-off-by: Simon Bouchard <sbouchard@rbbn.com> * Fix unit tests related to the new updateStrategy feature Signed-off-by: Simon Bouchard <sbouchard@rbbn.com> * Resolve linter issue due to cognitive complexity Signed-off-by: Simon Bouchard <sbouchard@rbbn.com> * Updated index.md to describe the possible values of updateStrategy Signed-off-by: Simon Bouchard <sbouchard@rbbn.com> * Add validation of updateStrategy parameter and unit test Signed-off-by: Simon Bouchard <sbouchard@rbbn.com> * Updated unit test Signed-off-by: Simon Bouchard <sbouchard@rbbn.com> * Removed 'reinstall' update strategy option to only have reinstallIfForbidden, cleanup of pre-sync changes, adapted unit tests Signed-off-by: Simon Bouchard <sbouchard@rbbn.com> * Display affected releases that were reinstalled Signed-off-by: Simon Bouchard <sbouchard@rbbn.com> * Make sure to add --wait when deleting a release to be reinstalled due to reinstallIfForbidden Signed-off-by: Simon Bouchard <sbouchard@rbbn.com> * Apply suggestions from Copilot code review Signed-off-by: Simon Bouchard <sbouchard@rbbn.com> --------- Signed-off-by: Simon Bouchard <sbouchard@rbbn.com>
2026-10-18 19:01:50 +02:00 · 2025-10-29 08:47:46 +08:00
parent a45d681a08
commit a6fab4dc75
12 changed files with 563 additions and 23 deletions
@@ -26,6 +26,8 @@ const (
 	DefaultHCLFileExtension = ".hcl"
 )

+var ValidUpdateStrategyValues = []string{UpdateStrategyReinstallIfForbidden}
+
 type StateLoadError struct {
 	Msg   string
 	Cause error
@@ -43,6 +45,14 @@ func (e *UndefinedEnvError) Error() string {
 	return fmt.Sprintf("environment \"%s\" is not defined", e.Env)
 }

+type InvalidUpdateStrategyError struct {
+	UpdateStrategy string
+}
+
+func (e *InvalidUpdateStrategyError) Error() string {
+	return fmt.Sprintf("updateStrategy %q is invalid, valid values are: %s or not set", e.UpdateStrategy, strings.Join(ValidUpdateStrategyValues, ", "))
+}
+
 type StateCreator struct {
 	logger *zap.SugaredLogger

@@ -43,6 +43,9 @@ const (
 	// This is used by an interim solution to make the urfave/cli command report to the helmfile internal about that the
 	// --timeout flag is missingl
 	EmptyTimeout = -1
+
+	// Valid enum for updateStrategy values
+	UpdateStrategyReinstallIfForbidden = "reinstallIfForbidden"
 )

 // ReleaseSetSpec is release set spec
@@ -277,6 +280,8 @@ type ReleaseSpec struct {
 	Force *bool `yaml:"force,omitempty"`
 	// Installed, when set to true, `delete --purge` the release
 	Installed *bool `yaml:"installed,omitempty"`
+	// UpdateStrategy, when set, indicate the strategy to use to update the release
+	UpdateStrategy string `yaml:"updateStrategy,omitempty"`
 	// Atomic, when set to true, restore previous state in case of a failed install/upgrade attempt
 	Atomic *bool `yaml:"atomic,omitempty"`
 	// CleanupOnFail, when set to true, the --cleanup-on-fail helm flag is passed to the upgrade command
@@ -467,6 +472,7 @@ type SetValue struct {
 // AffectedReleases hold the list of released that where updated, deleted, or in error
 type AffectedReleases struct {
 	Upgraded     []*ReleaseSpec
+	Reinstalled  []*ReleaseSpec
 	Deleted      []*ReleaseSpec
 	Failed       []*ReleaseSpec
 	DeleteFailed []*ReleaseSpec
@@ -1037,20 +1043,24 @@ func (st *HelmState) SyncReleases(affectedReleases *AffectedReleases, helm helme
 						}
 						m.Unlock()
 					}
-				} else if err := helm.SyncRelease(context, release.Name, chart, release.Namespace, flags...); err != nil {
-					m.Lock()
-					affectedReleases.Failed = append(affectedReleases.Failed, release)
-					m.Unlock()
-					relErr = newReleaseFailedError(release, err)
+				} else if release.UpdateStrategy == UpdateStrategyReinstallIfForbidden {
+					relErr = st.performSyncOrReinstallOfRelease(affectedReleases, helm, context, release, chart, m, flags...)
 				} else {
-					m.Lock()
-					affectedReleases.Upgraded = append(affectedReleases.Upgraded, release)
-					m.Unlock()
-					installedVersion, err := st.getDeployedVersion(context, helm, release)
-					if err != nil { // err is not really impacting so just log it
-						st.logger.Debugf("getting deployed release version failed: %v", err)
+					if err := helm.SyncRelease(context, release.Name, chart, release.Namespace, flags...); err != nil {
+						m.Lock()
+						affectedReleases.Failed = append(affectedReleases.Failed, release)
+						m.Unlock()
+						relErr = newReleaseFailedError(release, err)
 					} else {
-						release.installedVersion = installedVersion
+						m.Lock()
+						affectedReleases.Upgraded = append(affectedReleases.Upgraded, release)
+						m.Unlock()
+						installedVersion, err := st.getDeployedVersion(context, helm, release)
+						if err != nil { // err is not really impacting so just log it
+							st.logger.Debugf("getting deployed release version failed: %v", err)
+						} else {
+							release.installedVersion = installedVersion
+						}
 					}
 				}

@@ -1096,6 +1106,77 @@ func (st *HelmState) SyncReleases(affectedReleases *AffectedReleases, helm helme
 	return nil
 }

+func (st *HelmState) performSyncOrReinstallOfRelease(affectedReleases *AffectedReleases, helm helmexec.Interface, context helmexec.HelmContext, release *ReleaseSpec, chart string, m *sync.Mutex, flags ...string) *ReleaseError {
+	if err := helm.SyncRelease(context, release.Name, chart, release.Namespace, flags...); err != nil {
+		st.logger.Debugf("update strategy - sync failed: %s", err.Error())
+		// Only fail if a different error than forbidden updates
+		if !strings.Contains(err.Error(), "Forbidden: updates") {
+			st.logger.Debugf("update strategy - sync failed not due to Forbidden updates")
+			m.Lock()
+			affectedReleases.Failed = append(affectedReleases.Failed, release)
+			m.Unlock()
+			return newReleaseFailedError(release, err)
+		}
+	} else {
+		st.logger.Debugf("update strategy - sync success")
+		m.Lock()
+		affectedReleases.Upgraded = append(affectedReleases.Upgraded, release)
+		m.Unlock()
+		installedVersion, err := st.getDeployedVersion(context, helm, release)
+		if err != nil { // err is not really impacting so just log it
+			st.logger.Debugf("update strategy - getting deployed release version failed: %v", err)
+		} else {
+			release.installedVersion = installedVersion
+		}
+		return nil
+	}
+
+	st.logger.Infof("Failed to sync due to forbidden updates, attempting to reinstall %q allowed by update strategy", release.Name)
+	installed, err := st.isReleaseInstalled(context, helm, *release)
+	if err != nil {
+		return newReleaseFailedError(release, err)
+	}
+	if installed {
+		var args []string
+		if release.Namespace != "" {
+			args = append(args, "--namespace", release.Namespace)
+		}
+		deleteWaitFlag := true
+		release.DeleteWait = &deleteWaitFlag
+		args = st.appendDeleteWaitFlags(args, release)
+		deletionFlags := st.appendConnectionFlags(args, release)
+		m.Lock()
+		if _, err := st.triggerReleaseEvent("preuninstall", nil, release, "sync"); err != nil {
+			affectedReleases.Failed = append(affectedReleases.Failed, release)
+			return newReleaseFailedError(release, err)
+		} else if err := helm.DeleteRelease(context, release.Name, deletionFlags...); err != nil {
+			affectedReleases.Failed = append(affectedReleases.Failed, release)
+			return newReleaseFailedError(release, err)
+		} else if _, err := st.triggerReleaseEvent("postuninstall", nil, release, "sync"); err != nil {
+			affectedReleases.Failed = append(affectedReleases.Failed, release)
+			return newReleaseFailedError(release, err)
+		}
+		m.Unlock()
+	}
+	if err := helm.SyncRelease(context, release.Name, chart, release.Namespace, flags...); err != nil {
+		m.Lock()
+		affectedReleases.Failed = append(affectedReleases.Failed, release)
+		m.Unlock()
+		return newReleaseFailedError(release, err)
+	} else {
+		m.Lock()
+		affectedReleases.Reinstalled = append(affectedReleases.Reinstalled, release)
+		m.Unlock()
+		installedVersion, err := st.getDeployedVersion(context, helm, release)
+		if err != nil { // err is not really impacting so just log it
+			st.logger.Debugf("update strategy - getting deployed release version failed: %v", err)
+		} else {
+			release.installedVersion = installedVersion
+		}
+	}
+	return nil
+}
+
 func (st *HelmState) listReleases(context helmexec.HelmContext, helm helmexec.Interface, release *ReleaseSpec) (string, error) {
 	flags := st.kubeConnectionFlags(release)
 	if release.Namespace != "" {
@@ -3735,6 +3816,28 @@ func (ar *AffectedReleases) DisplayAffectedReleases(logger *zap.SugaredLogger) {
 		}
 		logger.Info(tbl.String())
 	}
+	if len(ar.Reinstalled) > 0 {
+		logger.Info("\nREINSTALLED RELEASES:")
+		tbl, _ := prettytable.NewTable(prettytable.Column{Header: "NAME"},
+			prettytable.Column{Header: "NAMESPACE", MinWidth: 6},
+			prettytable.Column{Header: "CHART", MinWidth: 6},
+			prettytable.Column{Header: "VERSION", MinWidth: 6},
+			prettytable.Column{Header: "DURATION", AlignRight: true},
+		)
+		tbl.Separator = "   "
+		for _, release := range ar.Reinstalled {
+			modifiedChart, modErr := hideChartCredentials(release.Chart)
+			if modErr != nil {
+				logger.Warn("Could not modify chart credentials, %v", modErr)
+				continue
+			}
+			err := tbl.AddRow(release.Name, release.Namespace, modifiedChart, release.installedVersion, release.duration.Round(time.Second))
+			if err != nil {
+				logger.Warn("Could not add row, %v", err)
+			}
+		}
+		logger.Info(tbl.String())
+	}
 	if len(ar.Deleted) > 0 {
 		logger.Info("\nDELETED RELEASES:")
 		tbl, _ := prettytable.NewTable(prettytable.Column{Header: "NAME"},
@@ -1640,6 +1640,112 @@ func TestHelmState_SyncReleasesAffectedRealeases(t *testing.T) {
 	}
 }

+func TestHelmState_SyncReleasesAffectedReleasesWithReinstallIfForbidden(t *testing.T) {
+	no := false
+	tests := []struct {
+		name         string
+		releases     []ReleaseSpec
+		installed    []bool
+		wantAffected exectest.Affected
+	}{
+		{
+			name: "2 new",
+			releases: []ReleaseSpec{
+				{
+					Name:           "releaseNameFoo-forbidden",
+					Chart:          "foo",
+					UpdateStrategy: "reinstallIfForbidden",
+				},
+				{
+					Name:           "releaseNameBar-forbidden",
+					Chart:          "foo",
+					UpdateStrategy: "reinstallIfForbidden",
+				},
+			},
+			wantAffected: exectest.Affected{
+				Upgraded: []*exectest.Release{
+					{Name: "releaseNameFoo-forbidden", Flags: []string{}},
+					{Name: "releaseNameBar-forbidden", Flags: []string{}},
+				},
+				Reinstalled: nil,
+				Deleted:     nil,
+				Failed:      nil,
+			},
+		},
+		{
+			name: "1 removed, 1 new, 1 reinstalled first new",
+			releases: []ReleaseSpec{
+				{
+					Name:           "releaseNameFoo-forbidden",
+					Chart:          "foo",
+					UpdateStrategy: "reinstallIfForbidden",
+				},
+				{
+					Name:           "releaseNameBar",
+					Chart:          "foo",
+					UpdateStrategy: "reinstallIfForbidden",
+					Installed:      &no,
+				},
+				{
+					Name:           "releaseNameFoo-forbidden",
+					Chart:          "foo",
+					UpdateStrategy: "reinstallIfForbidden",
+				},
+			},
+			installed: []bool{true, true, true},
+			wantAffected: exectest.Affected{
+				Upgraded: []*exectest.Release{
+					{Name: "releaseNameFoo-forbidden", Flags: []string{}},
+				},
+				Reinstalled: []*exectest.Release{
+					{Name: "releaseNameFoo-forbidden", Flags: []string{}},
+				},
+				Deleted: []*exectest.Release{
+					{Name: "releaseNameBar", Flags: []string{}},
+				},
+				Failed: nil,
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			state := &HelmState{
+				ReleaseSetSpec: ReleaseSetSpec{
+					Releases: tt.releases,
+				},
+				logger:         logger,
+				valsRuntime:    valsRuntime,
+				RenderedValues: map[string]any{},
+			}
+			helm := &exectest.Helm{
+				Lists: map[exectest.ListKey]string{},
+			}
+			//simulate the release is already installed
+			for i, release := range tt.releases {
+				if tt.installed != nil && tt.installed[i] {
+					helm.Lists[exectest.ListKey{Filter: "^" + release.Name + "$", Flags: "--uninstalling --deployed --failed --pending"}] = release.Name
+				}
+			}
+
+			affectedReleases := AffectedReleases{}
+			if err := state.SyncReleases(&affectedReleases, helm, []string{}, 1); err != nil {
+				if !testEq(affectedReleases.Failed, tt.wantAffected.Failed) {
+					t.Errorf("HelmState.SyncReleases() error failed for [%s] = %v, want %v", tt.name, affectedReleases.Failed, tt.wantAffected.Failed)
+				} //else expected error
+			}
+			if !testEq(affectedReleases.Upgraded, tt.wantAffected.Upgraded) {
+				t.Errorf("HelmState.SyncReleases() upgrade failed for [%s] = %v, want %v", tt.name, affectedReleases.Upgraded, tt.wantAffected.Upgraded)
+			}
+			if !testEq(affectedReleases.Reinstalled, tt.wantAffected.Reinstalled) {
+				t.Errorf("HelmState.SyncReleases() reinstalled failed for [%s] = %v, want %v", tt.name, affectedReleases.Reinstalled, tt.wantAffected.Reinstalled)
+			}
+			if !testEq(affectedReleases.Deleted, tt.wantAffected.Deleted) {
+				t.Errorf("HelmState.SyncReleases() deleted failed for [%s] = %v, want %v", tt.name, affectedReleases.Deleted, tt.wantAffected.Deleted)
+			}
+		})
+	}
+}
+
 func testEq(a []*ReleaseSpec, b []*exectest.Release) bool {
 	// If one is nil, the other must also be nil.
 	if (a == nil) != (b == nil) {
@@ -38,39 +38,39 @@ func TestGenerateID(t *testing.T) {
 	run(testcase{
 		subject: "baseline",
 		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"},
-		want:    "foo-values-7d454b9558",
+		want:    "foo-values-67dc97cbcb",
 	})

 	run(testcase{
 		subject: "different bytes content",
 		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"},
 		data:    []byte(`{"k":"v"}`),
-		want:    "foo-values-59c86d55bf",
+		want:    "foo-values-75d7c4758c",
 	})

 	run(testcase{
 		subject: "different map content",
 		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"},
 		data:    map[string]any{"k": "v"},
-		want:    "foo-values-6f87c5cd79",
+		want:    "foo-values-685f8cf685",
 	})

 	run(testcase{
 		subject: "different chart",
 		release: ReleaseSpec{Name: "foo", Chart: "stable/envoy"},
-		want:    "foo-values-5dfd748475",
+		want:    "foo-values-75597d9c57",
 	})

 	run(testcase{
 		subject: "different name",
 		release: ReleaseSpec{Name: "bar", Chart: "incubator/raw"},
-		want:    "bar-values-858b9c55cc",
+		want:    "bar-values-7b77df65ff",
 	})

 	run(testcase{
 		subject: "specific ns",
 		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw", Namespace: "myns"},
-		want:    "myns-foo-values-58dc9c6667",
+		want:    "myns-foo-values-85f979545c",
 	})

 	for id, n := range ids {