use managedclustraddon install namespace when addondeploymentconfig not set

Signed-off-by: zhujian <[email protected]>

Author: zhujian7

pkg/addon/templateagent/decorator.go

@@ -29,7 +29,7 @@ type namespaceDecorator struct {
 	paths map[string][]string
 }
 
-func newNamespaceDecorator(privateValues addonfactory.Values) *namespaceDecorator {
+func newNamespaceDecorator(defaultNs string, privateValues addonfactory.Values) *namespaceDecorator {
 	decorator := &namespaceDecorator{
 		paths: map[string][]string{
 			"ClusterRoleBinding": {"subjects", "namespace"},
@@ -40,6 +40,8 @@ func newNamespaceDecorator(privateValues addonfactory.Values) *namespaceDecorato
 	namespace, ok := privateValues[InstallNamespacePrivateValueKey]
 	if ok {
 		decorator.installNamespace = namespace.(string)
+	} else {
+		decorator.installNamespace = defaultNs
 	}
 
 	return decorator

pkg/addon/templateagent/decorator_test.go

@@ -19,6 +19,7 @@ import (
 )
 
 func TestNamespaceDecorator(t *testing.T) {
+	defaultNs := "ocm-addon"
 	tests := []struct {
 		name           string
 		namespace      string
@@ -29,7 +30,7 @@ func TestNamespaceDecorator(t *testing.T) {
 			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")
+				testingcommon.AssertEqualNameNamespace(t, obj.GetName(), obj.GetNamespace(), "test", defaultNs)
 			},
 		},
 		{
@@ -153,7 +154,7 @@ func TestNamespaceDecorator(t *testing.T) {
 				values[InstallNamespacePrivateValueKey] = tc.namespace
 			}
 
-			d := newNamespaceDecorator(values)
+			d := newNamespaceDecorator(defaultNs, values)
 			result, err := d.decorate(tc.object)
 			if err != nil {
 				t.Fatal(err)

pkg/addon/templateagent/template_agent.go

@@ -189,14 +189,14 @@ func (a *CRDTemplateAgentAddon) renderObjects(
 			return objects, err
 		}
 
-		object, err = a.decorateObject(template, object, presetValues, privateValues)
+		object, err = a.decorateObject(addon, template, object, presetValues, privateValues)
 		if err != nil {
 			return objects, err
 		}
 		objects = append(objects, object)
 	}
 
-	additionalObjects, err := a.injectAdditionalObjects(template, presetValues, privateValues)
+	additionalObjects, err := a.injectAdditionalObjects(addon, template, presetValues, privateValues)
 	if err != nil {
 		return objects, err
 	}
@@ -206,14 +206,16 @@ func (a *CRDTemplateAgentAddon) renderObjects(
 }
 
 func (a *CRDTemplateAgentAddon) decorateObject(
+	addon *addonapiv1alpha1.ManagedClusterAddOn,
 	template *addonapiv1alpha1.AddOnTemplate,
 	obj *unstructured.Unstructured,
 	orderedValues orderedValues,
-	privateValues addonfactory.Values) (*unstructured.Unstructured, error) {
+	privateValues addonfactory.Values,
+) (*unstructured.Unstructured, error) {
 	decorators := []decorator{
 		newDeploymentDecorator(a.logger, a.addonName, template, orderedValues, privateValues),
 		newDaemonSetDecorator(a.logger, a.addonName, template, orderedValues, privateValues),
-		newNamespaceDecorator(privateValues),
+		newNamespaceDecorator(addon.Spec.InstallNamespace, privateValues),
 	}
 
 	var err error
@@ -228,6 +230,7 @@ func (a *CRDTemplateAgentAddon) decorateObject(
 }
 
 func (a *CRDTemplateAgentAddon) injectAdditionalObjects(
+	addon *addonapiv1alpha1.ManagedClusterAddOn,
 	template *addonapiv1alpha1.AddOnTemplate,
 	orderedValues orderedValues,
 	privateValues addonfactory.Values) ([]runtime.Object, error) {
@@ -237,7 +240,7 @@ func (a *CRDTemplateAgentAddon) injectAdditionalObjects(
 
 	decorators := []decorator{
 		// decorate the namespace of the additional objects
-		newNamespaceDecorator(privateValues),
+		newNamespaceDecorator(addon.Spec.InstallNamespace, privateValues),
 	}
 
 	var objs []runtime.Object
@@ -296,47 +299,12 @@ func (a *CRDTemplateAgentAddon) getDesiredAddOnTemplateInner(
 	return template.DeepCopy(), nil
 }
 
-// TemplateAgentRegistrationNamespaceFunc reads deployment/daemonset resources in the manifests and use that namespace
+// TemplateAgentRegistrationNamespaceFunc reads the managedclusteraddon.spec.installnamespace
 // as the default registration namespace. If addonDeploymentConfig is set, uses the namespace in it.
 func (a *CRDTemplateAgentAddon) TemplateAgentRegistrationNamespaceFunc(
 	addon *addonapiv1alpha1.ManagedClusterAddOn) (string, error) {
-	template, err := a.getDesiredAddOnTemplateInner(addon.Name, addon.Status.ConfigReferences)
-	if err != nil {
-		return "", err
-	}
-	if template == nil {
-		return "", fmt.Errorf("addon %s template not found in status", addon.Name)
-	}
-
-	// pick the namespace of the first deployment, if there is no deployment, pick the namespace of the first daemonset
-	var desiredNS = "open-cluster-management-agent-addon"
-	var firstDeploymentNamespace, firstDaemonSetNamespace string
-	for _, manifest := range template.Spec.AgentSpec.Workload.Manifests {
-		object := &unstructured.Unstructured{}
-		if err := object.UnmarshalJSON(manifest.Raw); err != nil {
-			a.logger.Error(err, "failed to extract the object")
-			continue
-		}
-
-		if firstDeploymentNamespace == "" {
-			if _, err = utils.ConvertToDeployment(object); err == nil {
-				firstDeploymentNamespace = object.GetNamespace()
-				break
-			}
-		}
-		if firstDaemonSetNamespace == "" {
-			if _, err = utils.ConvertToDaemonSet(object); err == nil {
-				firstDaemonSetNamespace = object.GetNamespace()
-			}
-		}
-	}
-
-	if firstDeploymentNamespace != "" {
-		desiredNS = firstDeploymentNamespace
-	} else if firstDaemonSetNamespace != "" {
-		desiredNS = firstDaemonSetNamespace
-	}
 
+	desiredNS := addon.Spec.InstallNamespace
 	overrideNs, err := utils.AgentInstallNamespaceFromDeploymentConfigFunc(
 		utils.NewAddOnDeploymentConfigGetter(a.addonClient))(addon)
 	if err != nil {

pkg/addon/templateagent/template_agent_test.go

@@ -151,6 +151,7 @@ func TestAddonTemplateAgentManifests(t *testing.T) {
 		}
 	}
 
+	defaultInstallNs := "ocm-addon"
 	cases := []struct {
 		name                  string
 		addonTemplatePath     string
@@ -474,7 +475,7 @@ func TestAddonTemplateAgentManifests(t *testing.T) {
 				if err != nil {
 					t.Fatal(err)
 				}
-				if daemonSet.Namespace != "open-cluster-management-agent-addon-ds" { // first daemonset ns
+				if daemonSet.Namespace != defaultInstallNs {
 					t.Errorf("unexpected namespace %s", daemonSet.Namespace)
 				}
 				validatePodTemplate(t, daemonSet.Spec.Template,
@@ -496,13 +497,13 @@ func TestAddonTemplateAgentManifests(t *testing.T) {
 				if err != nil {
 					t.Fatal(err)
 				}
-				if clusterRoleBinding.Subjects[0].Namespace != "open-cluster-management-agent-addon-ds" {
+				if clusterRoleBinding.Subjects[0].Namespace != defaultInstallNs {
 					t.Errorf("clusterRolebinding namespace does not match, got %s", clusterRoleBinding.Subjects[0].Namespace)
 				}
 			},
 			validateAgentOptions: func(t *testing.T, o agent.AgentAddonOptions,
 				mca *addonapiv1alpha1.ManagedClusterAddOn) {
-				validateAgentOptions(t, o, mca, "open-cluster-management-agent-addon-ds")
+				validateAgentOptions(t, o, mca, defaultInstallNs)
 			},
 		},
 		{
@@ -640,6 +641,9 @@ func TestAddonTemplateAgentManifests(t *testing.T) {
 						Name:      addonName,
 						Namespace: clusterName,
 					},
+					Spec: addonapiv1alpha1.ManagedClusterAddOnSpec{
+						InstallNamespace: defaultInstallNs,
+					},
 				},
 			)