From 4c16bf743c29db8008d03b290d04d1c22784a3b1 Mon Sep 17 00:00:00 2001 From: Jian Qiu Date: Thu, 16 May 2024 17:36:32 +0800 Subject: [PATCH] Interate config reversely Signed-off-by: Jian Qiu --- examples/helloworld_helm/helloworld_helm.go | 8 +- pkg/addonfactory/addondeploymentconfig.go | 22 ++--- .../addondeploymentconfig_test.go | 84 ++++++++++++------- pkg/utils/addon_config.go | 9 +- 4 files changed, 77 insertions(+), 46 deletions(-) diff --git a/examples/helloworld_helm/helloworld_helm.go b/examples/helloworld_helm/helloworld_helm.go index 0b1f921f9..b10af7729 100644 --- a/examples/helloworld_helm/helloworld_helm.go +++ b/examples/helloworld_helm/helloworld_helm.go @@ -73,7 +73,13 @@ func GetImageValues(kubeClient kubernetes.Interface) addonfactory.GetValuesFunc continue } - configMap, err := kubeClient.CoreV1().ConfigMaps(config.Namespace).Get(context.Background(), config.Name, metav1.GetOptions{}) + if config.DesiredConfig == nil { + continue + } + + configMap, err := kubeClient.CoreV1(). + ConfigMaps(config.DesiredConfig.Namespace). + Get(context.Background(), config.DesiredConfig.Name, metav1.GetOptions{}) if err != nil { return nil, err } diff --git a/pkg/addonfactory/addondeploymentconfig.go b/pkg/addonfactory/addondeploymentconfig.go index ab5059aa7..f297c4870 100644 --- a/pkg/addonfactory/addondeploymentconfig.go +++ b/pkg/addonfactory/addondeploymentconfig.go @@ -1,7 +1,6 @@ package addonfactory import ( - "context" "encoding/json" "fmt" "strings" @@ -151,6 +150,10 @@ func GetAddOnDeploymentConfigValues( return lastValues, err } + if addOnDeploymentConfig == nil { + return lastValues, nil + } + for _, toValuesFunc := range toValuesFuncs { values, err := toValuesFunc(*addOnDeploymentConfig) if err != nil { @@ -274,19 +277,12 @@ func getRegistriesFromClusterAnnotation( // - Image registries configured in the addonDeploymentConfig will take precedence over the managed cluster annotation func GetAgentImageValues(getter utils.AddOnDeploymentConfigGetter, imageKey, image string) GetValuesFunc { return func(cluster *clusterv1.ManagedCluster, addon *addonapiv1alpha1.ManagedClusterAddOn) (Values, error) { - + addOnDeploymentConfig, err := utils.GetDesiredAddOnDeploymentConfig(addon, getter) + if err != nil { + return nil, err + } // Get image from AddOnDeploymentConfig - for _, config := range addon.Status.ConfigReferences { - if config.ConfigGroupResource.Group != utils.AddOnDeploymentConfigGVR.Group || - config.ConfigGroupResource.Resource != utils.AddOnDeploymentConfigGVR.Resource { - continue - } - - addOnDeploymentConfig, err := getter.Get(context.Background(), config.Namespace, config.Name) - if err != nil { - return nil, err - } - + if addOnDeploymentConfig != nil { values, overrode, err := overrideImageWithKeyValue(imageKey, image, getRegistriesFromAddonDeploymentConfig(*addOnDeploymentConfig)) if err != nil { diff --git a/pkg/addonfactory/addondeploymentconfig_test.go b/pkg/addonfactory/addondeploymentconfig_test.go index 91d2560e0..4d0b8f9fe 100644 --- a/pkg/addonfactory/addondeploymentconfig_test.go +++ b/pkg/addonfactory/addondeploymentconfig_test.go @@ -11,6 +11,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "open-cluster-management.io/addon-framework/pkg/addonmanager/addontesting" + "open-cluster-management.io/addon-framework/pkg/utils" addonapiv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" fakeaddon "open-cluster-management.io/api/client/addon/clientset/versioned/fake" clusterv1 "open-cluster-management.io/api/cluster/v1" @@ -39,9 +40,12 @@ func TestGetAddOnDeploymentConfigValues(t *testing.T) { Group: "config.test", Resource: "testconfigs", }, - ConfigReferent: addonapiv1alpha1.ConfigReferent{ - Namespace: "cluster1", - Name: "testConfig", + DesiredConfig: &addonapiv1alpha1.ConfigSpecHash{ + ConfigReferent: addonapiv1alpha1.ConfigReferent{ + Namespace: "cluster1", + Name: "testConfig", + }, + SpecHash: "dummy", }, }, } @@ -62,9 +66,12 @@ func TestGetAddOnDeploymentConfigValues(t *testing.T) { Group: "addon.open-cluster-management.io", Resource: "addondeploymentconfigs", }, - ConfigReferent: addonapiv1alpha1.ConfigReferent{ - Namespace: "cluster1", - Name: "config1", + DesiredConfig: &addonapiv1alpha1.ConfigSpecHash{ + ConfigReferent: addonapiv1alpha1.ConfigReferent{ + Namespace: "cluster1", + Name: "config1", + }, + SpecHash: "dummy", }, }, { @@ -72,9 +79,12 @@ func TestGetAddOnDeploymentConfigValues(t *testing.T) { Group: "addon.open-cluster-management.io", Resource: "addondeploymentconfigs", }, - ConfigReferent: addonapiv1alpha1.ConfigReferent{ - Namespace: "cluster1", - Name: "config2", + DesiredConfig: &addonapiv1alpha1.ConfigSpecHash{ + ConfigReferent: addonapiv1alpha1.ConfigReferent{ + Namespace: "cluster1", + Name: "config2", + }, + SpecHash: "dummy", }, }, } @@ -124,9 +134,12 @@ func TestGetAddOnDeploymentConfigValues(t *testing.T) { Group: "addon.open-cluster-management.io", Resource: "addondeploymentconfigs", }, - ConfigReferent: addonapiv1alpha1.ConfigReferent{ - Namespace: "cluster1", - Name: "config", + DesiredConfig: &addonapiv1alpha1.ConfigSpecHash{ + ConfigReferent: addonapiv1alpha1.ConfigReferent{ + Namespace: "cluster1", + Name: "config", + }, + SpecHash: "dummy", }, }, } @@ -164,9 +177,12 @@ func TestGetAddOnDeploymentConfigValues(t *testing.T) { Group: "addon.open-cluster-management.io", Resource: "addondeploymentconfigs", }, - ConfigReferent: addonapiv1alpha1.ConfigReferent{ - Namespace: "cluster1", - Name: "config", + DesiredConfig: &addonapiv1alpha1.ConfigSpecHash{ + ConfigReferent: addonapiv1alpha1.ConfigReferent{ + Namespace: "cluster1", + Name: "config", + }, + SpecHash: "dummy", }, }, } @@ -211,9 +227,12 @@ func TestGetAddOnDeploymentConfigValues(t *testing.T) { Group: "addon.open-cluster-management.io", Resource: "addondeploymentconfigs", }, - ConfigReferent: addonapiv1alpha1.ConfigReferent{ - Namespace: "cluster1", - Name: "config", + DesiredConfig: &addonapiv1alpha1.ConfigSpecHash{ + ConfigReferent: addonapiv1alpha1.ConfigReferent{ + Namespace: "cluster1", + Name: "config", + }, + SpecHash: "dummy", }, }, } @@ -253,9 +272,12 @@ func TestGetAddOnDeploymentConfigValues(t *testing.T) { Group: "addon.open-cluster-management.io", Resource: "addondeploymentconfigs", }, - ConfigReferent: addonapiv1alpha1.ConfigReferent{ - Namespace: "cluster1", - Name: "config", + DesiredConfig: &addonapiv1alpha1.ConfigSpecHash{ + ConfigReferent: addonapiv1alpha1.ConfigReferent{ + Namespace: "cluster1", + Name: "config", + }, + SpecHash: "dummy", }, }, } @@ -293,7 +315,7 @@ func TestGetAddOnDeploymentConfigValues(t *testing.T) { t.Run(c.name, func(t *testing.T) { fakeAddonClient := fakeaddon.NewSimpleClientset(c.addOnObjs...) - getter := NewAddOnDeploymentConfigGetter(fakeAddonClient) + getter := utils.NewAddOnDeploymentConfigGetter(fakeAddonClient) addOn, ok := c.addOnObjs[0].(*addonapiv1alpha1.ManagedClusterAddOn) if !ok { @@ -606,9 +628,12 @@ func TestGetAgentImageValues(t *testing.T) { Group: "addon.open-cluster-management.io", Resource: "addondeploymentconfigs", }, - ConfigReferent: addonapiv1alpha1.ConfigReferent{ - Namespace: "cluster1", - Name: "config1", + DesiredConfig: &addonapiv1alpha1.ConfigSpecHash{ + ConfigReferent: addonapiv1alpha1.ConfigReferent{ + Namespace: "cluster1", + Name: "config1", + }, + SpecHash: "dummy", }, }, { @@ -616,9 +641,12 @@ func TestGetAgentImageValues(t *testing.T) { Group: "addon.open-cluster-management.io", Resource: "addondeploymentconfigs", }, - ConfigReferent: addonapiv1alpha1.ConfigReferent{ - Namespace: "cluster1", - Name: "config2", + DesiredConfig: &addonapiv1alpha1.ConfigSpecHash{ + ConfigReferent: addonapiv1alpha1.ConfigReferent{ + Namespace: "cluster1", + Name: "config2", + }, + SpecHash: "dummy", }, }, } diff --git a/pkg/utils/addon_config.go b/pkg/utils/addon_config.go index eaa91a9f0..b407792a7 100644 --- a/pkg/utils/addon_config.go +++ b/pkg/utils/addon_config.go @@ -120,14 +120,15 @@ func GetAddOnDeploymentConfigSpecHash(config *addonapiv1alpha1.AddOnDeploymentCo }) } -// GetAddOnConfigRef returns the first addon config ref for the given config type +// GetAddOnConfigRef returns the last addon config ref for the given config type func GetAddOnConfigRef( configReferences []addonapiv1alpha1.ConfigReference, group, resource string) (bool, addonapiv1alpha1.ConfigReference) { - for _, config := range configReferences { - if config.Group == group && config.Resource == resource { - return true, config + // iterate the config reversely, the one with larger index is picked at first + for i := len(configReferences) - 1; i >= 0; i-- { + if configReferences[i].Group == group && configReferences[i].Resource == resource { + return true, configReferences[i] } } return false, addonapiv1alpha1.ConfigReference{}