From 2a5e7dba620f79032e04a0e5a5fb2ae685320c36 Mon Sep 17 00:00:00 2001 From: Tarun Pothulapati Date: Wed, 14 Oct 2020 01:42:49 +0530 Subject: [PATCH] Handle grafana add-on config repair (#5059) * Handle grafana add-on config repair Fixes #5014 In Grafana Add-On, Default fields i.e `grafana.image.name`, `grafana.name` have been removed from `linkerd-config-addons` after `2.8.1`. Only overriden values are stored in `linkerd-config-addons` as of now. Hence, `grafana.image.name` has to be removed from `linkerd-config-addons` unless they are overriden so that updates to it can take place especially the move from `gcr` to `ghcr`. This also removes `grafana.name` field if they are set to default, as its removed. This problem will not occur again even if we update default values, as default values are not stored in `linekrd-config-addons` anymore for all add-ons. Signed-off-by: Tarun Pothulapati --- cli/cmd/upgrade_legacy.go | 51 +++++++++++++++++++++++++++ pkg/healthcheck/healthcheck_addons.go | 22 ++++++------ pkg/healthcheck/healthcheck_test.go | 2 +- 3 files changed, 64 insertions(+), 11 deletions(-) diff --git a/cli/cmd/upgrade_legacy.go b/cli/cmd/upgrade_legacy.go index 02927d5a7..d43a56434 100644 --- a/cli/cmd/upgrade_legacy.go +++ b/cli/cmd/upgrade_legacy.go @@ -13,6 +13,7 @@ import ( "github.com/linkerd/linkerd2/pkg/issuercerts" "github.com/linkerd/linkerd2/pkg/k8s" "github.com/linkerd/linkerd2/pkg/version" + log "github.com/sirupsen/logrus" "github.com/spf13/pflag" corev1 "k8s.io/api/core/v1" "k8s.io/client-go/kubernetes" @@ -75,6 +76,15 @@ func loadStoredValuesLegacy(ctx context.Context, k *k8s.KubernetesAPI) (*charts. return nil, fmt.Errorf("values subpath not found in %s configmap", k8s.AddOnsConfigMapName) } + // repair Add-On configs + repairedCm, err := repairAddOnConfig([]byte(cmData)) + if err == nil { + // Update only if there is no error + cmData = string(repairedCm) + } else { + log.Warnf("add-on config repair failed: %s", err) + } + if err = yaml.Unmarshal([]byte(cmData), &values); err != nil { return nil, err } @@ -84,6 +94,47 @@ func loadStoredValuesLegacy(ctx context.Context, k *k8s.KubernetesAPI) (*charts. return values, nil } +func repairAddOnConfig(rawValues []byte) ([]byte, error) { + + var values map[string]interface{} + err := yaml.Unmarshal(rawValues, &values) + if err != nil { + return nil, err + } + + // Grafana Depreciation Fix + // Convert into Map instead of Values, as the latter returns with empty values + if grafana, err := healthcheck.GetMap(values, "grafana"); err == nil { + image, err := healthcheck.GetMap(grafana, "image") + if err == nil { + // Remove image.name tag if only name is present and set to the older image tag + if val, err := healthcheck.GetString(image, "name"); err == nil && val == "gcr.io/linkerd-io/grafana" { + delete(image, "name") + } + + // Remove image tag if its a empty map + if len(image) == 0 { + delete(grafana, "image") + } + } + + // Handle removal of grafana.name field + name, err := healthcheck.GetString(grafana, "name") + if err == nil { + // If default, remove it as its no longer needed + if name == "linkerd-grafana" { + delete(grafana, "name") + } + } + + } + rawValues, err = yaml.Marshal(values) + if err != nil { + return nil, err + } + return rawValues, nil +} + func setFlagsFromInstall(flags *pflag.FlagSet, installFlags []*pb.Install_Flag) { for _, i := range installFlags { if f := flags.Lookup(i.GetName()); f != nil && !f.Changed { diff --git a/pkg/healthcheck/healthcheck_addons.go b/pkg/healthcheck/healthcheck_addons.go index ea7c0b3ea..1fe027ef3 100644 --- a/pkg/healthcheck/healthcheck_addons.go +++ b/pkg/healthcheck/healthcheck_addons.go @@ -103,7 +103,7 @@ func (hc *HealthChecker) addOnCategories() []category { warning: true, check: func(ctx context.Context) error { if grafana, ok := hc.addOns[l5dcharts.GrafanaAddOn]; ok { - name, err := getString(grafana, "name") + name, err := GetString(grafana, "name") if err != nil && !errors.Is(err, errorKeyNotFound) { return err } @@ -123,7 +123,7 @@ func (hc *HealthChecker) addOnCategories() []category { warning: true, check: func(ctx context.Context) error { if grafana, ok := hc.addOns[l5dcharts.GrafanaAddOn]; ok { - name, err := getString(grafana, "name") + name, err := GetString(grafana, "name") if err != nil && !errors.Is(err, errorKeyNotFound) { return err } @@ -172,9 +172,9 @@ func (hc *HealthChecker) addOnCategories() []category { check: func(ctx context.Context) error { if tracing, ok := hc.addOns[l5dcharts.TracingAddOn]; ok { - collector, mapError := getMap(tracing, "collector") + collector, mapError := GetMap(tracing, "collector") - collectorName, keyError := getString(collector, "name") + collectorName, keyError := GetString(collector, "name") if errors.Is(mapError, errorKeyNotFound) || errors.Is(keyError, errorKeyNotFound) { // default name of collector instance @@ -198,9 +198,9 @@ func (hc *HealthChecker) addOnCategories() []category { warning: true, check: func(ctx context.Context) error { if tracing, ok := hc.addOns[l5dcharts.TracingAddOn]; ok { - jaeger, mapError := getMap(tracing, "jaeger") + jaeger, mapError := GetMap(tracing, "jaeger") - jaegerName, keyError := getString(jaeger, "name") + jaegerName, keyError := GetString(jaeger, "name") if errors.Is(mapError, errorKeyNotFound) || errors.Is(keyError, errorKeyNotFound) { // default name of jaeger instance @@ -224,9 +224,9 @@ func (hc *HealthChecker) addOnCategories() []category { warning: true, check: func(ctx context.Context) error { if tracing, ok := hc.addOns[l5dcharts.TracingAddOn]; ok { - collector, mapError := getMap(tracing, "collector") + collector, mapError := GetMap(tracing, "collector") - collectorName, keyError := getString(collector, "name") + collectorName, keyError := GetString(collector, "name") if errors.Is(mapError, errorKeyNotFound) || errors.Is(keyError, errorKeyNotFound) { // default name of collector instance @@ -342,7 +342,8 @@ func (hc *HealthChecker) checkForAddOnCM(ctx context.Context) (string, error) { return values, nil } -func getString(i interface{}, k string) (string, error) { +// GetString returns a String with the given key if present +func GetString(i interface{}, k string) (string, error) { m, ok := i.(map[string]interface{}) if !ok { return "", errors.New("config value is not a map") @@ -361,7 +362,8 @@ func getString(i interface{}, k string) (string, error) { return res, nil } -func getMap(i interface{}, k string) (map[string]interface{}, error) { +// GetMap returns a Map with the given Key if Present +func GetMap(i interface{}, k string) (map[string]interface{}, error) { m, ok := i.(map[string]interface{}) if !ok { return nil, errors.New("config value is not a map") diff --git a/pkg/healthcheck/healthcheck_test.go b/pkg/healthcheck/healthcheck_test.go index 3c6cee08b..85b6f4b35 100644 --- a/pkg/healthcheck/healthcheck_test.go +++ b/pkg/healthcheck/healthcheck_test.go @@ -3567,7 +3567,7 @@ func TestGetString(t *testing.T) { for i, tc := range testCases { tc := tc //pin t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { - ans, err := getString(tc.i, tc.k) + ans, err := GetString(tc.i, tc.k) if ans != tc.expected { t.Logf("Expected value: %s\n", tc.expected)