diff --git a/examples/helloworld/helloworld.go b/examples/helloworld/helloworld.go index 36455e195..f5035da05 100644 --- a/examples/helloworld/helloworld.go +++ b/examples/helloworld/helloworld.go @@ -12,7 +12,6 @@ import ( "open-cluster-management.io/addon-framework/pkg/utils" addonapiv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" clusterv1 "open-cluster-management.io/api/cluster/v1" - workapiv1 "open-cluster-management.io/api/work/v1" ) const ( @@ -56,40 +55,6 @@ func GetDefaultValues(cluster *clusterv1.ManagedCluster, func AgentHealthProber() *agent.HealthProber { return &agent.HealthProber{ - Type: agent.HealthProberTypeWork, - WorkProber: &agent.WorkHealthProber{ - ProbeFields: []agent.ProbeField{ - { - ResourceIdentifier: workapiv1.ResourceIdentifier{ - Group: "apps", - Resource: "deployments", - Name: "helloworld-agent", - Namespace: InstallationNamespace, - }, - ProbeRules: []workapiv1.FeedbackRule{ - { - Type: workapiv1.WellKnownStatusType, - }, - }, - }, - }, - HealthCheck: func(identifier workapiv1.ResourceIdentifier, result workapiv1.StatusFeedbackResult) error { - if len(result.Values) == 0 { - return fmt.Errorf("no values are probed for deployment %s/%s", identifier.Namespace, identifier.Name) - } - for _, value := range result.Values { - if value.Name != "ReadyReplicas" { - continue - } - - if *value.Value.Integer >= 1 { - return nil - } - - return fmt.Errorf("readyReplica is %d for deployement %s/%s", *value.Value.Integer, identifier.Namespace, identifier.Name) - } - return fmt.Errorf("readyReplica is not probed") - }, - }, + Type: agent.HealthProberTypeDeploymentAvailability, } } 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 d436a977b..f297c4870 100644 --- a/pkg/addonfactory/addondeploymentconfig.go +++ b/pkg/addonfactory/addondeploymentconfig.go @@ -1,7 +1,6 @@ package addonfactory import ( - "context" "encoding/json" "fmt" "strings" @@ -23,74 +22,6 @@ var AddOnDeploymentConfigGVR = schema.GroupVersionResource{ Resource: "addondeploymentconfigs", } -// AddOnDeloymentConfigToValuesFunc transform the AddOnDeploymentConfig object into Values object -// The transformation logic depends on the definition of the addon template -// Deprecated: use AddOnDeploymentConfigToValuesFunc instead. -type AddOnDeloymentConfigToValuesFunc func(config addonapiv1alpha1.AddOnDeploymentConfig) (Values, error) - -// NewAddOnDeloymentConfigGetter returns a AddOnDeloymentConfigGetter with addon client -// Deprecated: use NewAddOnDeploymentConfigGetter in pkg/utils package instead. -func NewAddOnDeloymentConfigGetter(addonClient addonv1alpha1client.Interface) utils.AddOnDeploymentConfigGetter { - return utils.NewAddOnDeploymentConfigGetter(addonClient) -} - -// GetAddOnDeloymentConfigValues uses AddOnDeloymentConfigGetter to get the AddOnDeploymentConfig object, then -// uses AddOnDeloymentConfigToValuesFunc to transform the AddOnDeploymentConfig object to Values object -// If there are multiple AddOnDeploymentConfig objects in the AddOn ConfigReferences, the big index object will -// override the one from small index -// Deprecated: use GetAddOnDeploymentConfigValues instead. -func GetAddOnDeloymentConfigValues( - getter utils.AddOnDeploymentConfigGetter, toValuesFuncs ...AddOnDeloymentConfigToValuesFunc) GetValuesFunc { - return func(cluster *clusterv1.ManagedCluster, addon *addonapiv1alpha1.ManagedClusterAddOn) (Values, error) { - var lastValues = Values{} - 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 - } - - for _, toValuesFunc := range toValuesFuncs { - values, err := toValuesFunc(*addOnDeploymentConfig) - if err != nil { - return nil, err - } - lastValues = MergeValues(lastValues, values) - } - } - - return lastValues, nil - } -} - -// ToAddOnDeloymentConfigValues transform the AddOnDeploymentConfig object into Values object that is a plain value map -// for example: the spec of one AddOnDeploymentConfig is: -// -// { -// customizedVariables: [{name: "Image", value: "img"}, {name: "ImagePullPolicy", value: "Always"}], -// nodePlacement: {nodeSelector: {"host": "ssd"}, tolerations: {"key": "test"}}, -// } -// -// after transformed, the key set of Values object will be: {"Image", "ImagePullPolicy", "NodeSelector", "Tolerations"} -// Deprecated: use ToAddOnDeploymentConfigValues instead. -func ToAddOnDeloymentConfigValues(config addonapiv1alpha1.AddOnDeploymentConfig) (Values, error) { - values, err := ToAddOnCustomizedVariableValues(config) - if err != nil { - return nil, err - } - - if config.Spec.NodePlacement != nil { - values["NodeSelector"] = config.Spec.NodePlacement.NodeSelector - values["Tolerations"] = config.Spec.NodePlacement.Tolerations - } - - return values, nil -} - // ToAddOnNodePlacementValues only transform the AddOnDeploymentConfig NodePlacement part into Values object that has // a specific for helm chart values // for example: the spec of one AddOnDeploymentConfig is: @@ -214,24 +145,21 @@ func GetAddOnDeploymentConfigValues( getter utils.AddOnDeploymentConfigGetter, toValuesFuncs ...AddOnDeploymentConfigToValuesFunc) GetValuesFunc { return func(cluster *clusterv1.ManagedCluster, addon *addonapiv1alpha1.ManagedClusterAddOn) (Values, error) { var lastValues = Values{} - for _, config := range addon.Status.ConfigReferences { - if config.ConfigGroupResource.Group != utils.AddOnDeploymentConfigGVR.Group || - config.ConfigGroupResource.Resource != utils.AddOnDeploymentConfigGVR.Resource { - continue - } + addOnDeploymentConfig, err := utils.GetDesiredAddOnDeploymentConfig(addon, getter) + if err != nil { + return lastValues, err + } + + if addOnDeploymentConfig == nil { + return lastValues, nil + } - addOnDeploymentConfig, err := getter.Get(context.Background(), config.Namespace, config.Name) + for _, toValuesFunc := range toValuesFuncs { + values, err := toValuesFunc(*addOnDeploymentConfig) if err != nil { return nil, err } - - for _, toValuesFunc := range toValuesFuncs { - values, err := toValuesFunc(*addOnDeploymentConfig) - if err != nil { - return nil, err - } - lastValues = MergeValues(lastValues, values) - } + lastValues = MergeValues(lastValues, values) } return lastValues, nil @@ -349,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..38f999b43 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", }, }, } @@ -50,68 +54,6 @@ func TestGetAddOnDeploymentConfigValues(t *testing.T) { }, expectedValues: Values{}, }, - { - name: "mutiple addon deployment configs", - toValuesFuncs: []AddOnDeploymentConfigToValuesFunc{ToAddOnDeploymentConfigValues}, - addOnObjs: []runtime.Object{ - func() *addonapiv1alpha1.ManagedClusterAddOn { - addon := addontesting.NewAddon("test", "cluster1") - addon.Status.ConfigReferences = []addonapiv1alpha1.ConfigReference{ - { - ConfigGroupResource: addonapiv1alpha1.ConfigGroupResource{ - Group: "addon.open-cluster-management.io", - Resource: "addondeploymentconfigs", - }, - ConfigReferent: addonapiv1alpha1.ConfigReferent{ - Namespace: "cluster1", - Name: "config1", - }, - }, - { - ConfigGroupResource: addonapiv1alpha1.ConfigGroupResource{ - Group: "addon.open-cluster-management.io", - Resource: "addondeploymentconfigs", - }, - ConfigReferent: addonapiv1alpha1.ConfigReferent{ - Namespace: "cluster1", - Name: "config2", - }, - }, - } - return addon - }(), - &addonapiv1alpha1.AddOnDeploymentConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: "config1", - Namespace: "cluster1", - }, - Spec: addonapiv1alpha1.AddOnDeploymentConfigSpec{ - CustomizedVariables: []addonapiv1alpha1.CustomizedVariable{ - {Name: "Test", Value: "test1"}, - }, - }, - }, - &addonapiv1alpha1.AddOnDeploymentConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: "config2", - Namespace: "cluster1", - }, - Spec: addonapiv1alpha1.AddOnDeploymentConfigSpec{ - CustomizedVariables: []addonapiv1alpha1.CustomizedVariable{ - {Name: "Test", Value: "test2"}, - }, - NodePlacement: &addonapiv1alpha1.NodePlacement{ - NodeSelector: map[string]string{"test": "test"}, - }, - }, - }, - }, - expectedValues: Values{ - "Test": "test2", - "NodeSelector": map[string]string{"test": "test"}, - "Tolerations": []corev1.Toleration{}, - }, - }, { name: "to addon node placement", toValuesFuncs: []AddOnDeploymentConfigToValuesFunc{ToAddOnNodePlacementValues}, @@ -124,9 +66,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 +109,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 +159,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 +204,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 +247,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,36 +560,18 @@ func TestGetAgentImageValues(t *testing.T) { Group: "addon.open-cluster-management.io", Resource: "addondeploymentconfigs", }, - ConfigReferent: addonapiv1alpha1.ConfigReferent{ - Namespace: "cluster1", - Name: "config1", - }, - }, - { - 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", }, }, } return addon }(), addonDeploymentConfig: []runtime.Object{ - &addonapiv1alpha1.AddOnDeploymentConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: "config1", - Namespace: "cluster1", - }, - Spec: addonapiv1alpha1.AddOnDeploymentConfigSpec{ - CustomizedVariables: []addonapiv1alpha1.CustomizedVariable{ - {Name: "Test", Value: "test1"}, - }, - }, - }, &addonapiv1alpha1.AddOnDeploymentConfig{ ObjectMeta: metav1.ObjectMeta{ Name: "config2", diff --git a/pkg/utils/addon_config.go b/pkg/utils/addon_config.go index eaa91a9f0..e43f8ea42 100644 --- a/pkg/utils/addon_config.go +++ b/pkg/utils/addon_config.go @@ -120,7 +120,9 @@ func GetAddOnDeploymentConfigSpecHash(config *addonapiv1alpha1.AddOnDeploymentCo }) } -// GetAddOnConfigRef returns the first addon config ref for the given config type +// GetAddOnConfigRef returns the first addon config ref for the given config type. It is fine since the status will only +// have one config for each GK. +// (TODO) this needs to be reconcidered if we support multiple same GK in the config referencese. func GetAddOnConfigRef( configReferences []addonapiv1alpha1.ConfigReference, group, resource string) (bool, addonapiv1alpha1.ConfigReference) { @@ -130,5 +132,6 @@ func GetAddOnConfigRef( return true, config } } + return false, addonapiv1alpha1.ConfigReference{} }