diff --git a/pkg/addon/controllers/addontemplate/controller.go b/pkg/addon/controllers/addontemplate/controller.go index 9d9968c28..8c057b5d9 100644 --- a/pkg/addon/controllers/addontemplate/controller.go +++ b/pkg/addon/controllers/addontemplate/controller.go @@ -18,6 +18,7 @@ import ( "open-cluster-management.io/addon-framework/pkg/addonfactory" "open-cluster-management.io/addon-framework/pkg/addonmanager" + "open-cluster-management.io/addon-framework/pkg/utils" addonapiv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" addonv1alpha1client "open-cluster-management.io/api/client/addon/clientset/versioned" @@ -194,10 +195,11 @@ func (c *addonTemplateController) runController( // image overrides from cluster annotation has lower priority than from the addonDeploymentConfig getValuesClosure, addonfactory.GetAddOnDeploymentConfigValues( - addonfactory.NewAddOnDeploymentConfigGetter(c.addonClient), + utils.NewAddOnDeploymentConfigGetter(c.addonClient), addonfactory.ToAddOnCustomizedVariableValues, templateagent.ToAddOnNodePlacementPrivateValues, templateagent.ToAddOnRegistriesPrivateValues, + templateagent.ToAddOnInstallNamespacePrivateValues, ), ) err = mgr.AddAgent(agentAddon) diff --git a/pkg/addon/templateagent/decorator.go b/pkg/addon/templateagent/decorator.go index b23f8b38c..9d0570758 100644 --- a/pkg/addon/templateagent/decorator.go +++ b/pkg/addon/templateagent/decorator.go @@ -4,28 +4,148 @@ import ( "fmt" "strings" - appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" "open-cluster-management.io/addon-framework/pkg/addonfactory" + "open-cluster-management.io/addon-framework/pkg/utils" addonapiv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" ) -type deploymentDecorator interface { +// decorator mutate the unstructured and returns an unstructured. +type decorator interface { + decorate(obj *unstructured.Unstructured) (*unstructured.Unstructured, error) +} + +type namespaceDecorator struct { + installNamespace string + // paths is the paths of a resource kind that the decorator needs to set namespace field in it. + // if the returned object is a list, decorator will set namespace for each item. + paths map[string]string +} + +func newNamespaceDecorator(configValues addonfactory.Values) *namespaceDecorator { + decorator := &namespaceDecorator{ + paths: map[string]string{ + "ClusterRoleBinding": "subjects", + "RoleBinding": "subjects", + }, + } + namespace, ok := configValues["INSTALL_NAMESPACE"] + if ok { + decorator.installNamespace = namespace.(string) + } + + return decorator +} + +func (d *namespaceDecorator) decorate(obj *unstructured.Unstructured) (*unstructured.Unstructured, error) { + if len(d.installNamespace) == 0 { + return obj, nil + } + + // If obj has no namespace set, we do not mutate namespace assuming it is cluster scoped. + if len(obj.GetNamespace()) > 0 { + obj.SetNamespace(d.installNamespace) + } + + path, ok := d.paths[obj.GetKind()] + if !ok { + return obj, nil + } + + field, found, err := unstructured.NestedFieldNoCopy(obj.Object, path) + if err != nil { + return obj, err + } + if !found { + return obj, fmt.Errorf("failed to find the path %s for kind %s", path, obj.GetKind()) + } + + // it cannot supported nested structure, only list or map. + switch f := field.(type) { + case []interface{}: + for _, item := range f { + if err := setNamespaceForObject(item, d.installNamespace); err != nil { + return obj, err + } + } + case interface{}: + if err := setNamespaceForObject(obj, d.installNamespace); err != nil { + return obj, err + } + } + return obj, nil +} + +func setNamespaceForObject(obj interface{}, namespace string) error { + mapVal, ok := obj.(map[string]interface{}) + if !ok { + return fmt.Errorf("obj %v is not a map, cannot set", obj) + } + mapVal["namespace"] = namespace + return nil +} + +type deploymentDecorator struct { + decorators []podTemplateSpecDecorator +} + +func newDeploymentDecorator( + addonName string, + template *addonapiv1alpha1.AddOnTemplate, + orderedValues orderedValues, + privateValues addonfactory.Values, +) decorator { + return &deploymentDecorator{ + decorators: []podTemplateSpecDecorator{ + newEnvironmentDecorator(orderedValues), + newVolumeDecorator(addonName, template), + newNodePlacementDecorator(privateValues), + newImageDecorator(privateValues), + }, + } +} + +func (d *deploymentDecorator) decorate(obj *unstructured.Unstructured) (*unstructured.Unstructured, error) { + deployment, err := utils.ConvertToDeployment(obj) + // not a deployment, directly return + if err != nil { + return obj, nil + } + + for _, decorator := range d.decorators { + err = decorator.decorate(&deployment.Spec.Template) + if err != nil { + return obj, err + } + } + + result, err := runtime.DefaultUnstructuredConverter.ToUnstructured(deployment) + if err != nil { + return obj, err + } + + return &unstructured.Unstructured{Object: result}, nil +} + +type podTemplateSpecDecorator interface { // decorate modifies the deployment in place - decorate(deployment *appsv1.Deployment) error + decorate(pod *corev1.PodTemplateSpec) error } type environmentDecorator struct { orderedValues orderedValues } -func newEnvironmentDecorator(orderedValues orderedValues) deploymentDecorator { +func newEnvironmentDecorator(orderedValues orderedValues) podTemplateSpecDecorator { return &environmentDecorator{ orderedValues: orderedValues, } } -func (d *environmentDecorator) decorate(deployment *appsv1.Deployment) error { +func (d *environmentDecorator) decorate(pod *corev1.PodTemplateSpec) error { + fmt.Printf("ordered value is %v\n", d.orderedValues) envVars := make([]corev1.EnvVar, len(d.orderedValues)) for index, value := range d.orderedValues { envVars[index] = corev1.EnvVar{ @@ -34,9 +154,9 @@ func (d *environmentDecorator) decorate(deployment *appsv1.Deployment) error { } } - for j := range deployment.Spec.Template.Spec.Containers { - deployment.Spec.Template.Spec.Containers[j].Env = append( - deployment.Spec.Template.Spec.Containers[j].Env, + for j := range pod.Spec.Containers { + pod.Spec.Containers[j].Env = append( + pod.Spec.Containers[j].Env, envVars...) } @@ -48,14 +168,14 @@ type volumeDecorator struct { addonName string } -func newVolumeDecorator(addonName string, template *addonapiv1alpha1.AddOnTemplate) deploymentDecorator { +func newVolumeDecorator(addonName string, template *addonapiv1alpha1.AddOnTemplate) podTemplateSpecDecorator { return &volumeDecorator{ addonName: addonName, template: template, } } -func (d *volumeDecorator) decorate(deployment *appsv1.Deployment) error { +func (d *volumeDecorator) decorate(pod *corev1.PodTemplateSpec) error { volumeMounts := []corev1.VolumeMount{} volumes := []corev1.Volume{} @@ -102,12 +222,12 @@ func (d *volumeDecorator) decorate(deployment *appsv1.Deployment) error { return nil } - for j := range deployment.Spec.Template.Spec.Containers { - deployment.Spec.Template.Spec.Containers[j].VolumeMounts = append( - deployment.Spec.Template.Spec.Containers[j].VolumeMounts, volumeMounts...) + for j := range pod.Spec.Containers { + pod.Spec.Containers[j].VolumeMounts = append( + pod.Spec.Containers[j].VolumeMounts, volumeMounts...) } - deployment.Spec.Template.Spec.Volumes = append(deployment.Spec.Template.Spec.Volumes, volumes...) + pod.Spec.Volumes = append(pod.Spec.Volumes, volumes...) return nil } @@ -116,13 +236,13 @@ type nodePlacementDecorator struct { privateValues addonfactory.Values } -func newNodePlacementDecorator(privateValues addonfactory.Values) deploymentDecorator { +func newNodePlacementDecorator(privateValues addonfactory.Values) podTemplateSpecDecorator { return &nodePlacementDecorator{ privateValues: privateValues, } } -func (d *nodePlacementDecorator) decorate(deployment *appsv1.Deployment) error { +func (d *nodePlacementDecorator) decorate(pod *corev1.PodTemplateSpec) error { nodePlacement, ok := d.privateValues[NodePlacementPrivateValueKey] if !ok { return nil @@ -134,11 +254,11 @@ func (d *nodePlacementDecorator) decorate(deployment *appsv1.Deployment) error { } if np.NodeSelector != nil { - deployment.Spec.Template.Spec.NodeSelector = np.NodeSelector + pod.Spec.NodeSelector = np.NodeSelector } if np.NodeSelector != nil { - deployment.Spec.Template.Spec.Tolerations = np.Tolerations + pod.Spec.Tolerations = np.Tolerations } return nil @@ -148,13 +268,13 @@ type imageDecorator struct { privateValues addonfactory.Values } -func newImageDecorator(privateValues addonfactory.Values) deploymentDecorator { +func newImageDecorator(privateValues addonfactory.Values) podTemplateSpecDecorator { return &imageDecorator{ privateValues: privateValues, } } -func (d *imageDecorator) decorate(deployment *appsv1.Deployment) error { +func (d *imageDecorator) decorate(pod *corev1.PodTemplateSpec) error { registries, ok := d.privateValues[RegistriesPrivateValueKey] if !ok { return nil @@ -165,9 +285,9 @@ func (d *imageDecorator) decorate(deployment *appsv1.Deployment) error { return fmt.Errorf("registries value is invalid") } - for i := range deployment.Spec.Template.Spec.Containers { - deployment.Spec.Template.Spec.Containers[i].Image = addonfactory.OverrideImage( - ims, deployment.Spec.Template.Spec.Containers[i].Image) + for i := range pod.Spec.Containers { + pod.Spec.Containers[i].Image = addonfactory.OverrideImage( + ims, pod.Spec.Containers[i].Image) } return nil diff --git a/pkg/addon/templateagent/decorator_test.go b/pkg/addon/templateagent/decorator_test.go new file mode 100644 index 000000000..79f3063a2 --- /dev/null +++ b/pkg/addon/templateagent/decorator_test.go @@ -0,0 +1,142 @@ +package templateagent + +import ( + "testing" + + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + + "open-cluster-management.io/addon-framework/pkg/addonfactory" + + testingcommon "open-cluster-management.io/ocm/pkg/common/testing" +) + +func TestNamespaceDecorator(t *testing.T) { + tests := []struct { + name string + namespace string + object *unstructured.Unstructured + validateObject func(t *testing.T, obj *unstructured.Unstructured) + }{ + { + name: "no namespace set", + object: testingcommon.NewUnstructured("v1", "Pod", "default", "test"), + validateObject: func(t *testing.T, obj *unstructured.Unstructured) { + testingcommon.AssertEqualNameNamespace(t, obj.GetName(), obj.GetNamespace(), "test", "default") + }, + }, + { + name: "cluster scoped", + object: testingcommon.NewUnstructured("v1", "ClusterRole", "", "test"), + namespace: "newns", + validateObject: func(t *testing.T, obj *unstructured.Unstructured) { + testingcommon.AssertEqualNameNamespace(t, obj.GetName(), obj.GetNamespace(), "test", "") + }, + }, + { + name: "namespace set", + object: testingcommon.NewUnstructured("v1", "Pod", "default", "test"), + namespace: "newns", + validateObject: func(t *testing.T, obj *unstructured.Unstructured) { + testingcommon.AssertEqualNameNamespace(t, obj.GetName(), obj.GetNamespace(), "test", "newns") + }, + }, + { + name: "clusterRoleBinding", + object: func() *unstructured.Unstructured { + clusterRoleBinding := &rbacv1.ClusterRoleBinding{ + TypeMeta: metav1.TypeMeta{ + Kind: "ClusterRoleBinding", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Subjects: []rbacv1.Subject{ + { + Name: "user1", + Namespace: "default", + }, + { + Name: "user2", + Namespace: "default", + }, + }, + } + data, _ := runtime.DefaultUnstructuredConverter.ToUnstructured(clusterRoleBinding) + return &unstructured.Unstructured{Object: data} + }(), + namespace: "newns", + validateObject: func(t *testing.T, obj *unstructured.Unstructured) { + binding := &rbacv1.ClusterRoleBinding{} + err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, binding) + if err != nil { + t.Fatal(err) + } + for _, s := range binding.Subjects { + if s.Namespace != "newns" { + t.Errorf("namespace of subject is not correct, got %v", s) + } + } + }, + }, + { + name: "roleBinding", + object: func() *unstructured.Unstructured { + roleBinding := &rbacv1.RoleBinding{ + TypeMeta: metav1.TypeMeta{ + Kind: "RoleBinding", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + }, + Subjects: []rbacv1.Subject{ + { + Name: "user1", + Namespace: "default", + }, + { + Name: "user2", + Namespace: "default", + }, + }, + } + data, _ := runtime.DefaultUnstructuredConverter.ToUnstructured(roleBinding) + return &unstructured.Unstructured{Object: data} + }(), + namespace: "newns", + validateObject: func(t *testing.T, obj *unstructured.Unstructured) { + testingcommon.AssertEqualNameNamespace(t, obj.GetName(), obj.GetNamespace(), "test", "newns") + binding := &rbacv1.RoleBinding{} + err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, binding) + if err != nil { + t.Fatal(err) + } + for _, s := range binding.Subjects { + if s.Namespace != "newns" { + t.Errorf("namespace of subject is not correct, got %v", s) + } + } + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + values := addonfactory.Values{} + if len(tc.namespace) > 0 { + values["INSTALL_NAMESPACE"] = tc.namespace + } + + d := newNamespaceDecorator(values) + result, err := d.decorate(tc.object) + if err != nil { + t.Fatal(err) + } + tc.validateObject(t, result) + }) + } + +} diff --git a/pkg/addon/templateagent/template_agent.go b/pkg/addon/templateagent/template_agent.go index 6f19a0a44..8ba10cf39 100644 --- a/pkg/addon/templateagent/template_agent.go +++ b/pkg/addon/templateagent/template_agent.go @@ -33,8 +33,7 @@ const ( // to convert it to Values by JsonStructToValues. // the built-in values can not be overridden by getValuesFuncs type templateCRDBuiltinValues struct { - ClusterName string `json:"CLUSTER_NAME,omitempty"` - AddonInstallNamespace string `json:"INSTALL_NAMESPACE,omitempty"` + ClusterName string `json:"CLUSTER_NAME,omitempty"` } // templateDefaultValues includes the default values for crd template agentAddon. @@ -170,43 +169,40 @@ func (a *CRDTemplateAgentAddon) renderObjects( if err := object.UnmarshalJSON([]byte(manifestStr)); err != nil { return objects, err } - objects = append(objects, object) - } - objects, err = a.decorateObjects(template, objects, presetValues, configValues, privateValues) - if err != nil { - return objects, err + object, err = a.decorateObject(template, object, presetValues, configValues, privateValues) + if err != nil { + return objects, err + } + objects = append(objects, object) } return objects, nil } -func (a *CRDTemplateAgentAddon) decorateObjects( +func (a *CRDTemplateAgentAddon) decorateObject( template *addonapiv1alpha1.AddOnTemplate, - objects []runtime.Object, + obj *unstructured.Unstructured, orderedValues orderedValues, - configValues, privateValues addonfactory.Values) ([]runtime.Object, error) { - decorators := []deploymentDecorator{ - newEnvironmentDecorator(orderedValues), - newVolumeDecorator(a.addonName, template), - newNodePlacementDecorator(privateValues), - newImageDecorator(privateValues), + configValues, privateValues addonfactory.Values) (*unstructured.Unstructured, error) { + decorators := []decorator{ + newDeploymentDecorator(a.addonName, template, orderedValues, privateValues), + newNamespaceDecorator(configValues), } - for index, obj := range objects { - deployment, err := utils.ConvertToDeployment(obj) - if err != nil { - continue - } - for _, decorator := range decorators { - err = decorator.decorate(deployment) - if err != nil { - return objects, err - } + a.logger.V(4).Info("decorator", + "orderedValues", orderedValues, + "configValues", configValues, + "privateValues", privateValues) + + var err error + for _, decorator := range decorators { + obj, err = decorator.decorate(obj) + if err != nil { + return obj, err } - objects[index] = deployment } - return objects, nil + return obj, nil } // getDesiredAddOnTemplateInner returns the desired template of the addon diff --git a/pkg/addon/templateagent/template_agent_test.go b/pkg/addon/templateagent/template_agent_test.go index a9e93742f..dca566220 100644 --- a/pkg/addon/templateagent/template_agent_test.go +++ b/pkg/addon/templateagent/template_agent_test.go @@ -7,10 +7,11 @@ import ( "time" "github.com/stretchr/testify/assert" - appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer" kubeinformers "k8s.io/client-go/informers" @@ -145,15 +146,23 @@ func TestAddonTemplateAgentManifests(t *testing.T) { t.Errorf("expected 4 objects, but got %v", len(objects)) } - object, ok := objects[0].(*appsv1.Deployment) + unstructObject, ok := objects[0].(*unstructured.Unstructured) if !ok { t.Errorf("expected object to be *appsv1.Deployment, but got %T", objects[0]) } + object, err := utils.ConvertToDeployment(unstructObject) + if err != nil { + t.Fatal(err) + } image := object.Spec.Template.Spec.Containers[0].Image if image != "quay.io/ocm/addon-examples:v1" { t.Errorf("unexpected image %v", image) } + if object.Namespace != "test-install-namespace" { + t.Errorf("unexpected namespace %s", object.Namespace) + } + nodeSelector := object.Spec.Template.Spec.NodeSelector expectedNodeSelector := map[string]string{"host": "ssd"} if !equality.Semantic.DeepEqual(nodeSelector, expectedNodeSelector) { @@ -171,7 +180,6 @@ func TestAddonTemplateAgentManifests(t *testing.T) { {Name: "LOG_LEVEL", Value: "4"}, {Name: "HUB_KUBECONFIG", Value: "/managed/hub-kubeconfig/kubeconfig"}, {Name: "CLUSTER_NAME", Value: clusterName}, - {Name: "INSTALL_NAMESPACE", Value: "open-cluster-management-agent-addon"}, } if !equality.Semantic.DeepEqual(envs, expectedEnvs) { t.Errorf("unexpected envs %v", envs) @@ -215,64 +223,81 @@ func TestAddonTemplateAgentManifests(t *testing.T) { if !equality.Semantic.DeepEqual(volumeMounts, expectedVolumeMounts) { t.Errorf("expected volumeMounts %v, but got: %v", expectedVolumeMounts, volumeMounts) } + + // check clusterrole + unstructObject, ok = objects[2].(*unstructured.Unstructured) + if !ok { + t.Errorf("expected object to be unstructured, but got %T", objects[0]) + } + clusterRoleBinding := &rbacv1.ClusterRoleBinding{} + err = runtime.DefaultUnstructuredConverter.FromUnstructured(unstructObject.Object, clusterRoleBinding) + if err != nil { + t.Fatal(err) + } + if clusterRoleBinding.Subjects[0].Namespace != "test-install-namespace" { + t.Errorf("clusterRolebinding namespace does not match, got %s", clusterRoleBinding.Subjects[0].Namespace) + } }, }, } for _, tc := range cases { - var managedClusterAddon *addonapiv1alpha1.ManagedClusterAddOn - var objs []runtime.Object - if tc.managedClusterAddonBuilder != nil { - if tc.addonTemplate != nil { - tc.managedClusterAddonBuilder.withAddonTemplate(tc.addonTemplate) - objs = append(objs, tc.addonTemplate) - } - if tc.addonDeploymentConfig != nil { - tc.managedClusterAddonBuilder.withAddonDeploymentConfig(tc.addonDeploymentConfig) - objs = append(objs, tc.addonDeploymentConfig) + t.Run(tc.name, func(t *testing.T) { + var managedClusterAddon *addonapiv1alpha1.ManagedClusterAddOn + var objs []runtime.Object + if tc.managedClusterAddonBuilder != nil { + if tc.addonTemplate != nil { + tc.managedClusterAddonBuilder.withAddonTemplate(tc.addonTemplate) + objs = append(objs, tc.addonTemplate) + } + if tc.addonDeploymentConfig != nil { + tc.managedClusterAddonBuilder.withAddonDeploymentConfig(tc.addonDeploymentConfig) + objs = append(objs, tc.addonDeploymentConfig) + } + managedClusterAddon = tc.managedClusterAddonBuilder.build() + objs = append(objs, managedClusterAddon) } - managedClusterAddon = tc.managedClusterAddonBuilder.build() - objs = append(objs, managedClusterAddon) - } - hubKubeClient := fakekube.NewSimpleClientset() - addonClient := fakeaddon.NewSimpleClientset(objs...) + hubKubeClient := fakekube.NewSimpleClientset() + addonClient := fakeaddon.NewSimpleClientset(objs...) - addonInformerFactory := addoninformers.NewSharedInformerFactory(addonClient, 30*time.Minute) - if managedClusterAddon != nil { - mcaStore := addonInformerFactory.Addon().V1alpha1().ManagedClusterAddOns().Informer().GetStore() - if err := mcaStore.Add(managedClusterAddon); err != nil { - t.Fatal(err) + addonInformerFactory := addoninformers.NewSharedInformerFactory(addonClient, 30*time.Minute) + if managedClusterAddon != nil { + mcaStore := addonInformerFactory.Addon().V1alpha1().ManagedClusterAddOns().Informer().GetStore() + if err := mcaStore.Add(managedClusterAddon); err != nil { + t.Fatal(err) + } } - } - if tc.addonTemplate != nil { - atStore := addonInformerFactory.Addon().V1alpha1().AddOnTemplates().Informer().GetStore() - if err := atStore.Add(tc.addonTemplate); err != nil { - t.Fatal(err) + if tc.addonTemplate != nil { + atStore := addonInformerFactory.Addon().V1alpha1().AddOnTemplates().Informer().GetStore() + if err := atStore.Add(tc.addonTemplate); err != nil { + t.Fatal(err) + } } - } - kubeInformers := kubeinformers.NewSharedInformerFactoryWithOptions(hubKubeClient, 10*time.Minute) + kubeInformers := kubeinformers.NewSharedInformerFactoryWithOptions(hubKubeClient, 10*time.Minute) - agentAddon := NewCRDTemplateAgentAddon( - ctx, - addonName, - hubKubeClient, - addonClient, - addonInformerFactory, - kubeInformers.Rbac().V1().RoleBindings().Lister(), - addonfactory.GetAddOnDeploymentConfigValues( - utils.NewAddOnDeploymentConfigGetter(addonClient), - addonfactory.ToAddOnCustomizedVariableValues, - ToAddOnNodePlacementPrivateValues, - ToAddOnRegistriesPrivateValues, - ), - ) + agentAddon := NewCRDTemplateAgentAddon( + ctx, + addonName, + hubKubeClient, + addonClient, + addonInformerFactory, + kubeInformers.Rbac().V1().RoleBindings().Lister(), + addonfactory.GetAddOnDeploymentConfigValues( + utils.NewAddOnDeploymentConfigGetter(addonClient), + addonfactory.ToAddOnCustomizedVariableValues, + ToAddOnNodePlacementPrivateValues, + ToAddOnRegistriesPrivateValues, + ToAddOnInstallNamespacePrivateValues, + ), + ) - objects, err := agentAddon.Manifests(tc.managedCluster, managedClusterAddon) - if err != nil { - assert.Equal(t, tc.expectedErr, err.Error(), tc.name) - } else { - tc.validateObjects(t, objects) - } + objects, err := agentAddon.Manifests(tc.managedCluster, managedClusterAddon) + if err != nil { + assert.Equal(t, tc.expectedErr, err.Error(), tc.name) + } else { + tc.validateObjects(t, objects) + } + }) } } diff --git a/pkg/addon/templateagent/values.go b/pkg/addon/templateagent/values.go index 3465631b3..ff33b7aa8 100644 --- a/pkg/addon/templateagent/values.go +++ b/pkg/addon/templateagent/values.go @@ -36,6 +36,15 @@ func ToAddOnRegistriesPrivateValues(config addonapiv1alpha1.AddOnDeploymentConfi }, nil } +func ToAddOnInstallNamespacePrivateValues(config addonapiv1alpha1.AddOnDeploymentConfig) (addonfactory.Values, error) { + if len(config.Spec.AgentInstallNamespace) == 0 { + return nil, nil + } + return addonfactory.Values{ + "INSTALL_NAMESPACE": config.Spec.AgentInstallNamespace, + }, nil +} + type keyValuePair struct { name string value string @@ -87,7 +96,7 @@ func (a *CRDTemplateAgentAddon) getValues( if err != nil { return presetValues, overrideValues, privateValues, nil } - overrideValues = addonfactory.MergeValues(overrideValues, builtinValues) + overrideValues = addonfactory.MergeValues(builtinValues, overrideValues) for k, v := range overrideValues { _, ok := v.(string) @@ -109,16 +118,10 @@ func (a *CRDTemplateAgentAddon) getValues( func (a *CRDTemplateAgentAddon) getBuiltinValues( cluster *clusterv1.ManagedCluster, - addon *addonapiv1alpha1.ManagedClusterAddOn) ([]string, addonfactory.Values, error) { + _ *addonapiv1alpha1.ManagedClusterAddOn) ([]string, addonfactory.Values, error) { builtinValues := templateCRDBuiltinValues{} builtinValues.ClusterName = cluster.GetName() - installNamespace := addon.Spec.InstallNamespace - if len(installNamespace) == 0 { - installNamespace = addonfactory.AddonDefaultInstallNamespace - } - builtinValues.AddonInstallNamespace = installNamespace - value, err := addonfactory.JsonStructToValues(builtinValues) if err != nil { return nil, nil, err @@ -127,8 +130,8 @@ func (a *CRDTemplateAgentAddon) getBuiltinValues( } func (a *CRDTemplateAgentAddon) getDefaultValues( - cluster *clusterv1.ManagedCluster, - addon *addonapiv1alpha1.ManagedClusterAddOn, + _ *clusterv1.ManagedCluster, + _ *addonapiv1alpha1.ManagedClusterAddOn, template *addonapiv1alpha1.AddOnTemplate) ([]string, addonfactory.Values, error) { defaultValues := templateCRDDefaultValues{} @@ -161,7 +164,7 @@ func hubKubeconfigPath() string { func GetAddOnRegistriesPrivateValuesFromClusterAnnotation( logger klog.Logger, cluster *clusterv1.ManagedCluster, - addon *addonapiv1alpha1.ManagedClusterAddOn) (addonfactory.Values, error) { + _ *addonapiv1alpha1.ManagedClusterAddOn) (addonfactory.Values, error) { values := map[string]interface{}{} annotations := cluster.GetAnnotations() logger.V(4).Info("Try to get image registries from annotation", diff --git a/test/e2e/addonmanagement_test.go b/test/e2e/addonmanagement_test.go index 774bf5473..f5ddf958d 100644 --- a/test/e2e/addonmanagement_test.go +++ b/test/e2e/addonmanagement_test.go @@ -595,7 +595,7 @@ func prepareImageOverrideAddOnDeploymentConfig(namespace, installNamespace strin return nil } -func prepareNodePlacementAddOnDeploymentConfig(namespace, installNamespae string) error { +func prepareNodePlacementAddOnDeploymentConfig(namespace, installNamespace string) error { _, err := t.AddOnClinet.AddonV1alpha1().AddOnDeploymentConfigs(namespace).Get( context.Background(), nodePlacementDeploymentConfigName, metav1.GetOptions{}) if errors.IsNotFound(err) { @@ -611,7 +611,7 @@ func prepareNodePlacementAddOnDeploymentConfig(namespace, installNamespae string NodeSelector: nodeSelector, Tolerations: tolerations, }, - AgentInstallNamespace: installNamespae, + AgentInstallNamespace: installNamespace, }, }, metav1.CreateOptions{},