Skip to content

Commit

Permalink
Interate config reversely
Browse files Browse the repository at this point in the history
Signed-off-by: Jian Qiu <[email protected]>
  • Loading branch information
qiujian16 committed May 16, 2024
1 parent 7cfad20 commit 4c16bf7
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 46 deletions.
8 changes: 7 additions & 1 deletion examples/helloworld_helm/helloworld_helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
22 changes: 9 additions & 13 deletions pkg/addonfactory/addondeploymentconfig.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package addonfactory

import (
"context"
"encoding/json"
"fmt"
"strings"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
84 changes: 56 additions & 28 deletions pkg/addonfactory/addondeploymentconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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",
},
},
}
Expand All @@ -62,19 +66,25 @@ 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",
},
},
{
ConfigGroupResource: addonapiv1alpha1.ConfigGroupResource{
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",
},
},
}
Expand Down Expand Up @@ -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",
},
},
}
Expand Down Expand Up @@ -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",
},
},
}
Expand Down Expand Up @@ -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",
},
},
}
Expand Down Expand Up @@ -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",
},
},
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -606,19 +628,25 @@ 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",
},
},
{
ConfigGroupResource: addonapiv1alpha1.ConfigGroupResource{
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",
},
},
}
Expand Down
9 changes: 5 additions & 4 deletions pkg/utils/addon_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down

0 comments on commit 4c16bf7

Please sign in to comment.