Fix setting default value updating

Avoid updating empty value as support-bundle-image setting default value Signed-off-by: Jian Wang <[email protected]> (cherry picked from commit 5481e8a)
tserong · Nov 28, 2024 · b6814fe · b6814fe
1 parent 97068c3
commit b6814fe
Show file tree

Hide file tree

Showing 6 changed files with 182 additions and 4 deletions.
diff --git a/deploy/charts/harvester/templates/_helpers.tpl b/deploy/charts/harvester/templates/_helpers.tpl
@@ -105,3 +105,21 @@ NB(thxCode): Use this value to unify the control tag and condition of KubeVirt.
 {{- .Values.tags.kubevirt | toString -}}
 {{- end -}}
 {{- end }}
+
+{{/*
+Get Support-bundle-kit image environment for updating the default values per current release.
+*/}}
+{{- define "harvester.supportBundleImageEnv" -}}
+{{- $result := dict -}}
+{{- range $k, $v := .Values -}}
+{{- if eq (toString $k) "support-bundle-kit" -}}
+{{- $result = $v -}}
+{{- end -}}
+{{- end -}}
+{{- with $result -}}
+{{- with .image -}}
+- name: HARVESTER_SUPPORT_BUNDLE_IMAGE_DEFAULT_VALUE
+  value: {{ printf "{\"repository\":\"%s\",\"tag\":\"%s\",\"imagePullPolicy\":\"%s\"}" .repository .tag .imagePullPolicy | squote }}
+{{- end -}}
+{{- end -}}
+{{- end }}
diff --git a/deploy/charts/harvester/templates/deployment.yaml b/deploy/charts/harvester/templates/deployment.yaml
@@ -76,6 +76,7 @@ spec:
             - name: GOCOVERDIR
               value: /go-cover-dir
 {{- end }}
+{{ include "harvester.supportBundleImageEnv" . | indent 12 }}
             - name: NAMESPACE
               valueFrom:
                 fieldRef:

diff --git a/pkg/controller/global/settings/register.go b/pkg/controller/global/settings/register.go
@@ -5,6 +5,8 @@ import (
 	"fmt"
 	"os"
 
+	"github.com/sirupsen/logrus"
+
 	"github.com/rancher/steve/pkg/server"
 	"k8s.io/apimachinery/pkg/api/errors"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -92,6 +94,16 @@ func (s *settingsProvider) SetAll(settingsMap map[string]settings.Setting) error
 	for name, setting := range settingsMap {
 		key := settings.GetEnvKey(name)
 		value := os.Getenv(key)
+		defaultvaluekey := settings.GetEnvDefaultValueKey(name)
+		defaultvalue := os.Getenv(defaultvaluekey)
+
+		// override settings from ENV first
+		if defaultvalue != "" && defaultvalue != setting.Default {
+			logrus.WithFields(logrus.Fields{
+				"name": name,
+			}).Debugf("setting default %s is replaced with %s", setting.Default, defaultvalue)
+			setting.Default = defaultvalue
+		}
 
 		obj, err := s.settings.Get(setting.Name, v1.GetOptions{})
 		if errors.IsNotFound(err) {
@@ -109,6 +121,7 @@ func (s *settingsProvider) SetAll(settingsMap map[string]settings.Setting) error
 			} else {
 				fallback[newSetting.Name] = newSetting.Value
 			}
+
 			_, err := s.settings.Create(newSetting)
 			if err != nil {
 				return err

diff --git a/pkg/controller/master/setting/support_bundle_image.go b/pkg/controller/master/setting/support_bundle_image.go
@@ -19,6 +19,13 @@ const (
 )
 
 func UpdateSupportBundleImage(settingClient v1beta1.SettingClient, settingCache v1beta1.SettingCache, app *catalogv1api.App) error {
+	if app.Spec.Chart == nil {
+		logrus.WithFields(logrus.Fields{
+			"namespace": app.Namespace,
+			"name":      app.Name,
+		}).Warning("app has empty chart, skip to update the support-bundle-image setting")
+		return nil
+	}
 	// merge template and chart
 	values, err := chartutil.CoalesceValues(
 		&chart.Chart{
@@ -36,11 +43,24 @@ func UpdateSupportBundleImage(settingClient v1beta1.SettingClient, settingCache
 	var supportBundleYaml map[string]interface{}
 	v, ok := values[SupportBundleRepository]
 	if !ok {
-		logrus.Warningf("cant't find %s in apps %s/%s", SupportBundleRepository, app.Namespace, app.Name)
+		logrus.WithFields(logrus.Fields{
+			"namespace": app.Namespace,
+			"name":      app.Name,
+		}).Warningf("app chart values cant't find %s, skip to update the support-bundle-image setting", SupportBundleRepository)
 		return nil
 	}
 	if supportBundleYaml, ok = v.(map[string]interface{}); !ok {
-		logrus.Warningf("unknown %s yaml struct %+v in apps %s/%s", SupportBundleRepository, v, app.Namespace, app.Name)
+		logrus.WithFields(logrus.Fields{
+			"namespace": app.Namespace,
+			"name":      app.Name,
+		}).Warningf("unknown %s yaml struct %+v, skip to update the support-bundle-image setting", SupportBundleRepository, v)
+		return nil
+	}
+	if len(supportBundleYaml) == 0 {
+		logrus.WithFields(logrus.Fields{
+			"namespace": app.Namespace,
+			"name":      app.Name,
+		}).Warning("supportBundleYaml map is empty, skip to convert to support-bundle-image setting")
 		return nil
 	}
 
@@ -50,16 +70,22 @@ func UpdateSupportBundleImage(settingClient v1beta1.SettingClient, settingCache
 	if err := mapstructure.Decode(supportBundleYaml, &supportBundle); err != nil {
 		return err
 	}
-
+	if supportBundle.Image.ImageName() == "" {
+		logrus.WithFields(logrus.Fields{
+			"namespace": app.Namespace,
+			"name":      app.Name,
+		}).Warning("the converted imagename is empty, skip to update the support-bundle-image setting")
+		return nil
+	}
 	imageBytes, err := json.Marshal(&supportBundle.Image)
 	if err != nil {
 		return err
 	}
-
 	supportBundleImage, err := settingCache.Get(settings.SupportBundleImageName)
 	if err != nil {
 		return err
 	}
+
 	supportBundleImageCpy := supportBundleImage.DeepCopy()
 	supportBundleImageCpy.Default = string(imageBytes)
 

diff --git a/pkg/controller/master/setting/support_bundle_image_test.go b/pkg/controller/master/setting/support_bundle_image_test.go
@@ -63,4 +63,120 @@ func Test_UpdateSupportBundleImage(t *testing.T) {
 
 	assert.Nil(t, err, "failed to get setting")
 	assert.Equal(t, "{\"repository\":\"test-repository\",\"tag\":\"v3.3\",\"imagePullPolicy\":\"IfNotPresent\"}", s.Default)
+
+	// image tag is empty, do not update
+	err = UpdateSupportBundleImage(
+		fakeclients.HarvesterSettingClient(clientset.HarvesterhciV1beta1().Settings),
+		fakeclients.HarvesterSettingCache(clientset.HarvesterhciV1beta1().Settings),
+		&catalogv1.App{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "test.name",
+				Namespace: namespace,
+			},
+			Spec: catalogv1.ReleaseSpec{
+				Chart: &catalogv1.Chart{
+					Values: map[string]interface{}{
+						"support-bundle-kit": map[string]interface{}{
+							"image": map[string]interface{}{
+								"repository":      "",
+								"tag":             "",
+								"imagePullPolicy": "IfNotPresent",
+							},
+						},
+					},
+				},
+			},
+		},
+	)
+	assert.Nil(t, err, "failed to update setting")
+	s, err = clientset.HarvesterhciV1beta1().Settings().Get(
+		context.TODO(),
+		settings.SupportBundleImageName,
+		metav1.GetOptions{})
+
+	assert.Nil(t, err, "failed to get setting")
+	// keeps unchanged
+	assert.Equal(t, "{\"repository\":\"test-repository\",\"tag\":\"v3.3\",\"imagePullPolicy\":\"IfNotPresent\"}", s.Default)
+
+	// image map is empty, do not update
+	err = UpdateSupportBundleImage(
+		fakeclients.HarvesterSettingClient(clientset.HarvesterhciV1beta1().Settings),
+		fakeclients.HarvesterSettingCache(clientset.HarvesterhciV1beta1().Settings),
+		&catalogv1.App{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "test.name",
+				Namespace: namespace,
+			},
+			Spec: catalogv1.ReleaseSpec{
+				Chart: &catalogv1.Chart{
+					Values: map[string]interface{}{
+						"support-bundle-kit": map[string]interface{}{
+							"image": map[string]interface{}{},
+						},
+					},
+				},
+			},
+		},
+	)
+	assert.Nil(t, err, "failed to update setting")
+	s, err = clientset.HarvesterhciV1beta1().Settings().Get(
+		context.TODO(),
+		settings.SupportBundleImageName,
+		metav1.GetOptions{})
+
+	assert.Nil(t, err, "failed to get setting")
+	// keeps unchanged
+	assert.Equal(t, "{\"repository\":\"test-repository\",\"tag\":\"v3.3\",\"imagePullPolicy\":\"IfNotPresent\"}", s.Default)
+
+	// invalid key from app
+	err = UpdateSupportBundleImage(
+		fakeclients.HarvesterSettingClient(clientset.HarvesterhciV1beta1().Settings),
+		fakeclients.HarvesterSettingCache(clientset.HarvesterhciV1beta1().Settings),
+		&catalogv1.App{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "test.name",
+				Namespace: namespace,
+			},
+			Spec: catalogv1.ReleaseSpec{
+				Chart: &catalogv1.Chart{
+					Values: map[string]interface{}{
+						"support-bundle-kit-error-name": map[string]interface{}{
+							"image": map[string]interface{}{},
+						},
+					},
+				},
+			},
+		},
+	)
+	assert.Nil(t, err, "failed to update setting")
+	s, err = clientset.HarvesterhciV1beta1().Settings().Get(
+		context.TODO(),
+		settings.SupportBundleImageName,
+		metav1.GetOptions{})
+
+	assert.Nil(t, err, "failed to get setting")
+	// keeps unchanged
+	assert.Equal(t, "{\"repository\":\"test-repository\",\"tag\":\"v3.3\",\"imagePullPolicy\":\"IfNotPresent\"}", s.Default)
+
+	// empty chart from app
+	err = UpdateSupportBundleImage(
+		fakeclients.HarvesterSettingClient(clientset.HarvesterhciV1beta1().Settings),
+		fakeclients.HarvesterSettingCache(clientset.HarvesterhciV1beta1().Settings),
+		&catalogv1.App{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "test.name",
+				Namespace: namespace,
+			},
+			Spec: catalogv1.ReleaseSpec{},
+		},
+	)
+	assert.Nil(t, err, "failed to update setting")
+	s, err = clientset.HarvesterhciV1beta1().Settings().Get(
+		context.TODO(),
+		settings.SupportBundleImageName,
+		metav1.GetOptions{})
+
+	assert.Nil(t, err, "failed to get setting")
+	// keeps unchanged
+	assert.Equal(t, "{\"repository\":\"test-repository\",\"tag\":\"v3.3\",\"imagePullPolicy\":\"IfNotPresent\"}", s.Default)
 }
diff --git a/pkg/settings/settings.go b/pkg/settings/settings.go
@@ -191,6 +191,10 @@ func GetEnvKey(key string) string {
 	return "HARVESTER_" + strings.ToUpper(strings.Replace(key, "-", "_", -1))
 }
 
+func GetEnvDefaultValueKey(key string) string {
+	return "HARVESTER_" + strings.ToUpper(strings.Replace(key, "-", "_", -1)) + "_DEFAULT_VALUE"
+}
+
 func IsRelease() bool {
 	return !strings.Contains(ServerVersion.Get(), "head") && releasePattern.MatchString(ServerVersion.Get())
 }