🐛 add wlp annotation if necessary

Signed-off-by: Yang Le <yangle@redhat.com>

deploy/klusterlet/config/rbac/cluster_role.yaml

@@ -15,7 +15,7 @@ rules:
   verbs: ["create"]
 - apiGroups: [""]
   resources: ["namespaces"]
-  verbs: ["create", "get", "list", "watch", "delete"]
+  verbs: ["create", "get", "list", "update", "watch", "patch", "delete"]
 - apiGroups: [""]
   resources: ["nodes"]
   verbs: ["get", "list", "watch"]

deploy/klusterlet/olm-catalog/klusterlet/manifests/klusterlet.clusterserviceversion.yaml

@@ -136,7 +136,9 @@ spec:
           - create
           - get
           - list
+          - update
           - watch
+          - patch
           - delete
         - apiGroups:
           - ""

pkg/operator/operators/klusterlet/controllers/klusterletcontroller/client_builder.go

@@ -77,7 +77,7 @@ func (m *managedClusterClientsBuilder) build(ctx context.Context) (*managedClust
 	// Ensure the agent namespace for users to create the external-managed-kubeconfig secret in this
 	// namespace, so that in the next reconcile loop the controller can get the secret successfully after
 	// the secret was created.
-	if err := ensureAgentNamespace(ctx, m.kubeClient, m.secretNamespace); err != nil {
+	if err := ensureNamespaceWithAnnotation(ctx, m.kubeClient, m.secretNamespace, wlpAnnotationName, wlpAnnotationValue); err != nil {
 		return nil, err
 	}
 

pkg/operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go

@@ -45,6 +45,8 @@ const (
 	hubConnectionDegraded                 = "HubConnectionDegraded"
 	hubKubeConfigSecretMissing            = "HubKubeConfigSecretMissing" // #nosec G101
 	managedResourcesEvictionTimestampAnno = "operator.open-cluster-management.io/managed-resources-eviction-timestamp"
+	wlpAnnotationName                     = "workload.openshift.io/allowed"
+	wlpAnnotationValue                    = "management"
 )
 
 type klusterletController struct {
@@ -333,20 +335,42 @@ func getManagedKubeConfig(ctx context.Context, kubeClient kubernetes.Interface,
 	return helpers.LoadClientConfigFromSecret(managedKubeconfigSecret)
 }
 
-// ensureAgentNamespace create agent namespace if it is not exist
-func ensureAgentNamespace(ctx context.Context, kubeClient kubernetes.Interface, namespace string) error {
-	_, err := kubeClient.CoreV1().Namespaces().Get(ctx, namespace, metav1.GetOptions{})
+// ensureNamespaceWithAnnotation create the namespace if necessary and make sure it has the
+// given annotation
+func ensureNamespaceWithAnnotation(ctx context.Context, kubeClient kubernetes.Interface, namespace, annotationName,
+	annotationValue string) error {
+	ns, err := kubeClient.CoreV1().Namespaces().Get(ctx, namespace, metav1.GetOptions{})
 	if errors.IsNotFound(err) {
 		_, createErr := kubeClient.CoreV1().Namespaces().Create(ctx, &corev1.Namespace{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: namespace,
 				Annotations: map[string]string{
-					"workload.openshift.io/allowed": "management",
+					annotationName: annotationValue,
 				},
 			},
 		}, metav1.CreateOptions{})
-		return createErr
+		if createErr != nil {
+			return createErr
+		}
+		return nil
+	}
+	if err != nil {
+		return err
+	}
+
+	// add the annotation if necessary
+	if ns.Annotations[annotationName] == annotationValue {
+		return nil
+	}
+
+	nsCopy := ns.DeepCopy()
+	if nsCopy.Annotations == nil {
+		nsCopy.Annotations = map[string]string{}
 	}
+	nsCopy.Annotations[annotationName] = annotationValue
+
+	nsPatcher := patcher.NewPatcher[*corev1.Namespace, corev1.NamespaceSpec, corev1.NamespaceStatus](kubeClient.CoreV1().Namespaces())
+	_, err = nsPatcher.PatchLabelAnnotations(ctx, ns, nsCopy.ObjectMeta, ns.ObjectMeta)
 	return err
 }
 
@@ -375,30 +399,15 @@ func syncPullSecret(ctx context.Context, sourceClient, targetClient kubernetes.I
 }
 
 func ensureNamespace(ctx context.Context, kubeClient kubernetes.Interface, klusterlet *operatorapiv1.Klusterlet, namespace string) error {
-	_, err := kubeClient.CoreV1().Namespaces().Get(ctx, namespace, metav1.GetOptions{})
-	switch {
-	case errors.IsNotFound(err):
-		_, createErr := kubeClient.CoreV1().Namespaces().Create(ctx, &corev1.Namespace{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: namespace,
-				Annotations: map[string]string{
-					"workload.openshift.io/allowed": "management",
-				},
-			},
-		}, metav1.CreateOptions{})
-		meta.SetStatusCondition(&klusterlet.Status.Conditions, metav1.Condition{
-			Type: klusterletApplied, Status: metav1.ConditionFalse, Reason: "KlusterletApplyFailed",
-			Message: fmt.Sprintf("Failed to create namespace %q: %v", namespace, createErr)})
-		return createErr
-
-	case err != nil:
-		meta.SetStatusCondition(&klusterlet.Status.Conditions, metav1.Condition{
-			Type: klusterletApplied, Status: metav1.ConditionFalse, Reason: "KlusterletApplyFailed",
-			Message: fmt.Sprintf("Failed to get namespace %q: %v", namespace, err)})
-		return err
+	err := ensureNamespaceWithAnnotation(ctx, kubeClient, namespace, wlpAnnotationName, wlpAnnotationValue)
+	if err == nil {
+		return nil
 	}
 
-	return nil
+	meta.SetStatusCondition(&klusterlet.Status.Conditions, metav1.Condition{
+		Type: klusterletApplied, Status: metav1.ConditionFalse, Reason: "KlusterletApplyFailed",
+		Message: fmt.Sprintf("Failed to ensure namespace %q: %v", namespace, err)})
+	return err
 }
 
 func serviceAccountName(suffix string, klusterlet *operatorapiv1.Klusterlet) string {

pkg/operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller_test.go

@@ -1087,3 +1087,85 @@ func (f *fakeManagedClusterBuilder) build(_ context.Context) (*managedClusterCli
 		},
 	}, nil
 }
+
+func TestEnsureNamespaceWithAnnotation(t *testing.T) {
+	annotationName := "type"
+	annotationValue := "workload"
+	namespaceName := "test-ns"
+	cases := []struct {
+		name      string
+		namespace runtime.Object
+		validate  func(t *testing.T, actions []clienttesting.Action)
+	}{
+		{
+			name: "create ns",
+			validate: func(t *testing.T, actions []clienttesting.Action) {
+				testingcommon.AssertAction(t, actions[1], "create")
+			},
+		},
+		{
+			name: "patch ns",
+			namespace: &corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: namespaceName,
+				},
+			},
+			validate: func(t *testing.T, actions []clienttesting.Action) {
+				testingcommon.AssertAction(t, actions[1], "patch")
+				patch := actions[1].(clienttesting.PatchAction).GetPatch()
+				ns := &corev1.Namespace{}
+				err := json.Unmarshal(patch, ns)
+				if err != nil {
+					t.Fatal(err)
+				}
+				if !equality.Semantic.DeepEqual(ns.Annotations, map[string]string{annotationName: annotationValue}) {
+					t.Errorf("not patched correctly got %v", ns.Annotations)
+				}
+			},
+		},
+		{
+			name: "use exsiting ns",
+			namespace: &corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: namespaceName,
+					Annotations: map[string]string{
+						annotationName: annotationValue,
+					},
+				},
+			},
+			validate: func(t *testing.T, actions []clienttesting.Action) {
+				if len(actions) != 1 {
+					t.Errorf("Unexpected number of actions: %d\n%+v", len(actions), actions)
+				}
+			},
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			objects := []runtime.Object{}
+			if c.namespace != nil {
+				objects = append(objects, c.namespace)
+			}
+
+			fakeKubeClient := fakekube.NewSimpleClientset(objects...)
+			err := ensureNamespaceWithAnnotation(context.Background(), fakeKubeClient, namespaceName, annotationName, annotationValue)
+			if err != nil {
+				t.Errorf("Expected non error when sync, %v", err)
+			}
+
+			if c.validate != nil {
+				c.validate(t, fakeKubeClient.Actions())
+			}
+
+			// check the annotation
+			ns, err := fakeKubeClient.CoreV1().Namespaces().Get(context.Background(), namespaceName, metav1.GetOptions{})
+			if err != nil {
+				t.Errorf("Expected non error when sync, %v", err)
+			}
+			if ns.Annotations[annotationName] != annotationValue {
+				t.Errorf("Expected %q annotation %q, but got %q", annotationName, annotationValue, ns.Annotations[annotationName])
+			}
+		})
+	}
+}