Merge pull request #4866 from emosbaugh/issue-4864-helm-extensions-di…

…sable-upgrade-force

feat(helm): add option to disable helm upgrade force flag

docs/helm-charts.md

@@ -35,15 +35,16 @@ It is possible to customize the timeout by using the `timeout' field.
 
 ### Chart configuration
 
-| Field     | Default value | Description                                                          |
-|-----------|---------------|----------------------------------------------------------------------|
-| name      | -             | Release name                                                         |
-| chartname | -             | chartname in form "repository/chartname" or path to tgz file         |
-| version   | -             | version to install                                                   |
-| timeout   | -             | timeout to wait for release install                                  |
-| values    | -             | yaml as a string, custom chart values                                |
-| namespace | -             | namespace to install chart into                                      |
-| order     | 0             | order to apply manifest. For equal values, alphanum ordering is used |
+| Field        | Default value | Description                                                                            |
+|--------------|---------------|----------------------------------------------------------------------------------------|
+| name         | -             | Release name                                                                           |
+| chartname    | -             | chartname in form "repository/chartname" or path to tgz file                           |
+| version      | -             | version to install                                                                     |
+| timeout      | -             | timeout to wait for release install                                                    |
+| values       | -             | yaml as a string, custom chart values                                                  |
+| namespace    | -             | namespace to install chart into                                                        |
+| forceUpgrade | true          | when set to false, disables the use of the "--force" flag when upgrading the the chart |
+| order        | 0             | order to apply manifest. For equal values, alphanum ordering is used                   |
 
 ## Example
 

inttest/addons/addons_test.go

@@ -37,6 +37,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
 	k8s "k8s.io/client-go/kubernetes"
+	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	crlog "sigs.k8s.io/controller-runtime/pkg/log"
@@ -369,17 +370,19 @@ func (as *AddonsSuite) doTestAddonUpdate(addonName string, values map[string]int
 		Name:     "testChartUpdate",
 		Template: chartCrdTemplate,
 		Data: struct {
-			Name      string
-			ChartName string
-			Values    string
-			Version   string
-			TargetNS  string
+			Name         string
+			ChartName    string
+			Values       string
+			Version      string
+			TargetNS     string
+			ForceUpgrade *bool
 		}{
-			Name:      "test-addon",
-			ChartName: "ealenn/echo-server",
-			Values:    string(valuesBytes),
-			Version:   "0.5.0",
-			TargetNS:  "default",
+			Name:         "test-addon",
+			ChartName:    "ealenn/echo-server",
+			Values:       string(valuesBytes),
+			Version:      "0.5.0",
+			TargetNS:     "default",
+			ForceUpgrade: ptr.To(false),
 		},
 	}
 	buf := bytes.NewBuffer([]byte{})
@@ -424,6 +427,7 @@ spec:
             version: "0.0.1"
             values: ""
             namespace: kube-system
+            forceUpgrade: false
 `
 
 // TODO: this actually duplicates logic from the controller code
@@ -443,4 +447,7 @@ spec:
 {{ .Values | nindent 4 }}
   version: {{ .Version }}
   namespace: {{ .TargetNS }}
+{{- if ne .ForceUpgrade nil }}
+  forceUpgrade: {{ .ForceUpgrade }}
+{{- end }}
 `

pkg/apis/helm/v1beta1/chart_types.go

@@ -33,7 +33,11 @@ type ChartSpec struct {
 	Version     string `json:"version,omitempty"`
 	Namespace   string `json:"namespace,omitempty"`
 	Timeout     string `json:"timeout,omitempty"`
-	Order       int    `json:"order,omitempty"`
+	// ForceUpgrade when set to false, disables the use of the "--force" flag when upgrading the the chart (default: true).
+	// +kubebuilder:default=true
+	// +optional
+	ForceUpgrade *bool `json:"forceUpgrade,omitempty"`
+	Order        int   `json:"order,omitempty"`
 }
 
 // YamlValues returns values as map
@@ -54,6 +58,13 @@ func (cs ChartSpec) HashValues() string {
 	return fmt.Sprintf("%x", h.Sum(nil))
 }
 
+// ShouldForceUpgrade returns true if the chart should be force upgraded
+func (cs ChartSpec) ShouldForceUpgrade() bool {
+	// This defaults to true when not explicitly set to false.
+	// Better have this the other way round in the next API version.
+	return cs.ForceUpgrade == nil || *cs.ForceUpgrade
+}
+
 // ChartStatus defines the observed state of Chart
 type ChartStatus struct {
 	ReleaseName string `json:"releaseName,omitempty"`

pkg/apis/k0s/v1beta1/extensions.go

@@ -114,7 +114,11 @@ type Chart struct {
 	// Timeout specifies the timeout for how long to wait for the chart installation to finish.
 	// A duration string is a sequence of decimal numbers, each with optional fraction and a unit suffix, such as "300ms" or "2h45m". Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h".
 	Timeout metav1.Duration `json:"timeout,omitempty"`
-	Order   int             `json:"order,omitempty"`
+	// ForceUpgrade when set to false, disables the use of the "--force" flag when upgrading the the chart (default: true).
+	// +kubebuilder:default=true
+	// +optional
+	ForceUpgrade *bool `json:"forceUpgrade,omitempty"`
+	Order        int   `json:"order,omitempty"`
 }
 
 // Validate performs validation

pkg/component/controller/extensions_controller.go

@@ -116,24 +116,13 @@ func (ec *ExtensionsController) reconcileHelmExtensions(helmSpec *k0sv1beta1.Hel
 		fileName := chartManifestFileName(&chart)
 		fileNamesToKeep = append(fileNamesToKeep, fileName)
 
-		tw := templatewriter.TemplateWriter{
-			Path:     filepath.Join(ec.manifestsDir, fileName),
-			Name:     "addon_crd_manifest",
-			Template: chartCrdTemplate,
-			Data: struct {
-				k0sv1beta1.Chart
-				Finalizer string
-			}{
-				Chart:     chart,
-				Finalizer: finalizerName,
-			},
-		}
-		if err := tw.Write(); err != nil {
+		path, err := ec.writeChartManifestFile(chart, fileName)
+		if err != nil {
 			errs = append(errs, fmt.Errorf("can't write file for Helm chart manifest %q: %w", chart.ChartName, err))
 			continue
 		}
 
-		ec.L.Infof("Wrote Helm chart manifest file %q", tw.Path)
+		ec.L.Infof("Wrote Helm chart manifest file %q", path)
 	}
 
 	if err := filepath.WalkDir(ec.manifestsDir, func(path string, entry fs.DirEntry, err error) error {
@@ -162,6 +151,25 @@ func (ec *ExtensionsController) reconcileHelmExtensions(helmSpec *k0sv1beta1.Hel
 	return errors.Join(errs...)
 }
 
+func (ec *ExtensionsController) writeChartManifestFile(chart k0sv1beta1.Chart, fileName string) (string, error) {
+	tw := templatewriter.TemplateWriter{
+		Path:     filepath.Join(ec.manifestsDir, fileName),
+		Name:     "addon_crd_manifest",
+		Template: chartCrdTemplate,
+		Data: struct {
+			k0sv1beta1.Chart
+			Finalizer string
+		}{
+			Chart:     chart,
+			Finalizer: finalizerName,
+		},
+	}
+	if err := tw.Write(); err != nil {
+		return "", err
+	}
+	return tw.Path, nil
+}
+
 // Determines the file name to use when storing a chart as a manifest on disk.
 func chartManifestFileName(c *k0sv1beta1.Chart) string {
 	return fmt.Sprintf("%d_helm_extension_%s.yaml", c.Order, c.Name)
@@ -300,6 +308,7 @@ func (cr *ChartReconciler) updateOrInstallChart(ctx context.Context, chart helmv
 			chart.Status.Namespace,
 			chart.Spec.YamlValues(),
 			timeout,
+			chart.Spec.ShouldForceUpgrade(),
 		)
 		if err != nil {
 			return fmt.Errorf("can't reconcile upgrade for %q: %w", chart.GetName(), err)
@@ -372,6 +381,9 @@ spec:
 {{ .Values | nindent 4 }}
   version: {{ .Version }}
   namespace: {{ .TargetNS }}
+{{- if ne .ForceUpgrade nil }}
+  forceUpgrade: {{ .ForceUpgrade }}
+{{- end }}
 `
 
 const finalizerName = "helm.k0sproject.io/uninstall-helm-release"

pkg/component/controller/extensions_controller_test.go

@@ -17,11 +17,17 @@ limitations under the License.
 package controller
 
 import (
+	"os"
+	"strings"
 	"testing"
+	"time"
 
 	"github.com/k0sproject/k0s/pkg/apis/helm/v1beta1"
 	k0sv1beta1 "github.com/k0sproject/k0s/pkg/apis/k0s/v1beta1"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/ptr"
 )
 
 func TestChartNeedsUpgrade(t *testing.T) {
@@ -162,3 +168,92 @@ func TestChartManifestFileName(t *testing.T) {
 	assert.Equal(t, chartManifestFileName(&chart2), "2_helm_extension_release.yaml")
 	assert.True(t, isChartManifestFileName("0_helm_extension_release.yaml"))
 }
+
+func TestExtensionsController_writeChartManifestFile(t *testing.T) {
+	type args struct {
+		chart    k0sv1beta1.Chart
+		fileName string
+	}
+	tests := []struct {
+		name string
+		args args
+		want string
+	}{
+		{
+			name: "forceUpgrade is nil should omit from manifest",
+			args: args{
+				chart: k0sv1beta1.Chart{
+					Name:      "release",
+					ChartName: "k0s/chart",
+					Version:   "0.0.1",
+					Values:    "values",
+					TargetNS:  "default",
+					Timeout:   metav1.Duration{Duration: 5 * time.Minute},
+				},
+				fileName: "0_helm_extension_release.yaml",
+			},
+			want: `apiVersion: helm.k0sproject.io/v1beta1
+kind: Chart
+metadata:
+  name: k0s-addon-chart-release
+  namespace: "kube-system"
+  finalizers:
+    - helm.k0sproject.io/uninstall-helm-release
+spec:
+  chartName: k0s/chart
+  releaseName: release
+  timeout: 5m0s
+  values: |
+
+    values
+  version: 0.0.1
+  namespace: default
+`,
+		},
+		{
+			name: "forceUpgrade is false should be included in manifest",
+			args: args{
+				chart: k0sv1beta1.Chart{
+					Name:         "release",
+					ChartName:    "k0s/chart",
+					Version:      "0.0.1",
+					Values:       "values",
+					TargetNS:     "default",
+					Timeout:      metav1.Duration{Duration: 5 * time.Minute},
+					ForceUpgrade: ptr.To(false),
+				},
+				fileName: "0_helm_extension_release.yaml",
+			},
+			want: `apiVersion: helm.k0sproject.io/v1beta1
+kind: Chart
+metadata:
+  name: k0s-addon-chart-release
+  namespace: "kube-system"
+  finalizers:
+    - helm.k0sproject.io/uninstall-helm-release
+spec:
+  chartName: k0s/chart
+  releaseName: release
+  timeout: 5m0s
+  values: |
+
+    values
+  version: 0.0.1
+  namespace: default
+  forceUpgrade: false
+`,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ec := &ExtensionsController{
+				manifestsDir: t.TempDir(),
+			}
+			path, err := ec.writeChartManifestFile(tt.args.chart, tt.args.fileName)
+			require.NoError(t, err)
+			contents, err := os.ReadFile(path)
+			require.NoError(t, err)
+			assert.Equal(t, strings.TrimSpace(tt.want), strings.TrimSpace(string(contents)))
+		})
+	}
+}

pkg/helm/helm.go

@@ -264,7 +264,7 @@ func (hc *Commands) InstallChart(ctx context.Context, chartName string, version
 
 // UpgradeChart upgrades a helm chart.
 // InstallChart, UpgradeChart and UninstallRelease(releaseName are *NOT* thread-safe
-func (hc *Commands) UpgradeChart(ctx context.Context, chartName string, version string, releaseName string, namespace string, values map[string]interface{}, timeout time.Duration) (*release.Release, error) {
+func (hc *Commands) UpgradeChart(ctx context.Context, chartName string, version string, releaseName string, namespace string, values map[string]interface{}, timeout time.Duration, force bool) (*release.Release, error) {
 	cfg, err := hc.getActionCfg(namespace)
 	if err != nil {
 		return nil, fmt.Errorf("can't create action configuration: %w", err)
@@ -274,7 +274,7 @@ func (hc *Commands) UpgradeChart(ctx context.Context, chartName string, version
 	upgrade.Wait = true
 	upgrade.WaitForJobs = true
 	upgrade.Install = true
-	upgrade.Force = true
+	upgrade.Force = force
 	upgrade.Atomic = true
 	upgrade.Timeout = timeout
 	chartDir, err := hc.locateChart(chartName, version)

static/_crds/helm/helm.k0sproject.io_charts.yaml

@@ -41,6 +41,11 @@ spec:
             properties:
               chartName:
                 type: string
+              forceUpgrade:
+                default: true
+                description: 'ForceUpgrade when set to false, disables the use of
+                  the "--force" flag when upgrading the the chart (default: true).'
+                type: boolean
               namespace:
                 type: string
               order: