🐛 fix addon namespace deletion in multi-hub scenarios (#1153)

Signed-off-by: Yang Le <yangle@redhat.com>
2026-08-23 22:26:49 +00:00 · 2025-09-11 07:54:39 +00:00
parent e4530eeb50
commit 6d4561ca9a
2 changed files with 298 additions and 1 deletions
@@ -1621,3 +1621,271 @@ func (f *fakeManagedClusterBuilder) build(_ context.Context) (*managedClusterCli
 		kubeconfigSecretCreationTime: creationTime,
 	}, nil
 }
+
+// TestHasActiveKlusterletAgentNamespaces tests the hasActiveKlusterletAgentNamespaces helper function
+func TestHasActiveKlusterletAgentNamespaces(t *testing.T) {
+	tests := []struct {
+		name                  string
+		currentAgentNamespace string
+		existingNamespaces    []runtime.Object
+		expectedResult        bool
+		expectError           bool
+	}{
+		{
+			name:                  "no other klusterlet agent namespaces exist",
+			currentAgentNamespace: "open-cluster-management-agent",
+			existingNamespaces: []runtime.Object{
+				&corev1.Namespace{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "open-cluster-management-agent",
+						Labels: map[string]string{
+							klusterletNamespaceLabelKey: "klusterlet",
+						},
+					},
+				},
+			},
+			expectedResult: false,
+			expectError:    false,
+		},
+		{
+			name:                  "other klusterlet agent namespaces exist",
+			currentAgentNamespace: "open-cluster-management-agent",
+			existingNamespaces: []runtime.Object{
+				&corev1.Namespace{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "open-cluster-management-agent",
+						Labels: map[string]string{
+							klusterletNamespaceLabelKey: "klusterlet",
+						},
+					},
+				},
+				&corev1.Namespace{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "open-cluster-management-agent-hosted",
+						Labels: map[string]string{
+							klusterletNamespaceLabelKey: "klusterlet-hosted",
+						},
+					},
+				},
+			},
+			expectedResult: true,
+			expectError:    false,
+		},
+		{
+			name:                  "multiple other klusterlet agent namespaces exist",
+			currentAgentNamespace: "open-cluster-management-agent-hosted1",
+			existingNamespaces: []runtime.Object{
+				&corev1.Namespace{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "open-cluster-management-agent",
+						Labels: map[string]string{
+							klusterletNamespaceLabelKey: "klusterlet",
+						},
+					},
+				},
+				&corev1.Namespace{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "open-cluster-management-agent-hosted1",
+						Labels: map[string]string{
+							klusterletNamespaceLabelKey: "klusterlet-hosted1",
+						},
+					},
+				},
+				&corev1.Namespace{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "open-cluster-management-agent-hosted2",
+						Labels: map[string]string{
+							klusterletNamespaceLabelKey: "klusterlet-hosted2",
+						},
+					},
+				},
+			},
+			expectedResult: true,
+			expectError:    false,
+		},
+		{
+			name:                  "no klusterlet namespaces with labels exist",
+			currentAgentNamespace: "open-cluster-management-agent",
+			existingNamespaces: []runtime.Object{
+				&corev1.Namespace{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "some-other-namespace",
+					},
+				},
+			},
+			expectedResult: false,
+			expectError:    false,
+		},
+		{
+			name:                  "namespace without klusterlet label should be ignored",
+			currentAgentNamespace: "open-cluster-management-agent",
+			existingNamespaces: []runtime.Object{
+				&corev1.Namespace{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "open-cluster-management-agent",
+						Labels: map[string]string{
+							klusterletNamespaceLabelKey: "klusterlet",
+						},
+					},
+				},
+				&corev1.Namespace{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "some-namespace-without-label",
+					},
+				},
+			},
+			expectedResult: false,
+			expectError:    false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx := context.TODO()
+			fakeKubeClient := fakekube.NewSimpleClientset(tt.existingNamespaces...)
+
+			managedClusterClients := &managedClusterClients{
+				kubeClient: fakeKubeClient,
+			}
+
+			managedReconciler := &managedReconcile{
+				managedClusterClients: managedClusterClients,
+			}
+
+			hasActive, err := managedReconciler.hasActiveKlusterletAgentNamespaces(ctx, tt.currentAgentNamespace)
+
+			if tt.expectError {
+				if err == nil {
+					t.Errorf("Expected an error but got none")
+				}
+			} else {
+				if err != nil {
+					t.Errorf("Expected no error but got: %v", err)
+				}
+				if hasActive != tt.expectedResult {
+					t.Errorf("Expected hasActiveKlusterletAgentNamespaces to return %t, but got %t", tt.expectedResult, hasActive)
+				}
+			}
+		})
+	}
+}
+
+// TestCleanWithMultipleKlusterletAgentNamespaces tests the clean function behavior when multiple klusterlet agent namespaces exist
+func TestCleanWithMultipleKlusterletAgentNamespaces(t *testing.T) {
+	tests := []struct {
+		name                       string
+		klusterlet                 *operatorapiv1.Klusterlet
+		existingNamespaces         []runtime.Object
+		shouldDeleteAddonNamespace bool
+		description                string
+	}{
+		{
+			name:       "should not delete addon namespace when other klusterlet agents exist",
+			klusterlet: newKlusterlet("klusterlet", "open-cluster-management-agent", "cluster1"),
+			existingNamespaces: []runtime.Object{
+				&corev1.Namespace{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "open-cluster-management-agent",
+						Labels: map[string]string{
+							klusterletNamespaceLabelKey: "klusterlet",
+						},
+					},
+				},
+				&corev1.Namespace{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "open-cluster-management-agent-hosted",
+						Labels: map[string]string{
+							klusterletNamespaceLabelKey: "klusterlet-hosted",
+						},
+					},
+				},
+				&corev1.Namespace{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: helpers.DefaultAddonNamespace,
+					},
+				},
+			},
+			shouldDeleteAddonNamespace: false,
+			description:                "Other klusterlet agents still exist, so addon namespace should be preserved",
+		},
+		{
+			name:       "should delete addon namespace when no other klusterlet agents exist",
+			klusterlet: newKlusterlet("klusterlet", "open-cluster-management-agent", "cluster1"),
+			existingNamespaces: []runtime.Object{
+				&corev1.Namespace{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "open-cluster-management-agent",
+						Labels: map[string]string{
+							klusterletNamespaceLabelKey: "klusterlet",
+						},
+					},
+				},
+				&corev1.Namespace{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: helpers.DefaultAddonNamespace,
+					},
+				},
+			},
+			shouldDeleteAddonNamespace: true,
+			description:                "No other klusterlet agents exist, so addon namespace should be deleted",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx := context.TODO()
+			syncContext := testingcommon.NewFakeSyncContext(t, "klusterlet")
+
+			// Mark klusterlet for deletion
+			now := metav1.Now()
+			tt.klusterlet.DeletionTimestamp = &now
+
+			controller := newTestController(t, tt.klusterlet, syncContext.Recorder(), nil, false, tt.existingNamespaces...)
+
+			// Call the clean function through the cleanup controller
+			err := controller.cleanupController.sync(ctx, syncContext)
+			if err != nil {
+				t.Errorf("Expected no error from cleanup sync, but got: %v", err)
+			}
+
+			// Check the namespace deletion actions
+			var deletedNamespaces []string
+			for _, action := range controller.kubeClient.Actions() {
+				if action.GetVerb() == deleteVerb && action.GetResource().Resource == "namespaces" {
+					deletedNamespaces = append(deletedNamespaces, action.(clienttesting.DeleteActionImpl).Name)
+				}
+			}
+
+			// Verify addon namespace deletion behavior
+			addonNamespaceDeleted := false
+			for _, ns := range deletedNamespaces {
+				if ns == helpers.DefaultAddonNamespace {
+					addonNamespaceDeleted = true
+					break
+				}
+			}
+
+			if tt.shouldDeleteAddonNamespace {
+				if !addonNamespaceDeleted {
+					t.Errorf("Expected addon namespace to be deleted but it wasn't. %s", tt.description)
+				}
+			} else {
+				if addonNamespaceDeleted {
+					t.Errorf("Expected addon namespace NOT to be deleted but it was. %s", tt.description)
+				}
+			}
+
+			// The klusterlet's own agent namespace should always be deleted
+			agentNamespaceDeleted := false
+			for _, ns := range deletedNamespaces {
+				if ns == tt.klusterlet.Spec.Namespace {
+					agentNamespaceDeleted = true
+					break
+				}
+			}
+			if !agentNamespaceDeleted {
+				t.Errorf("Expected klusterlet agent namespace %s to be deleted but it wasn't", tt.klusterlet.Spec.Namespace)
+			}
+		})
+	}
+}
@@ -185,7 +185,16 @@ func (r *managedReconcile) clean(ctx context.Context, klusterlet *operatorapiv1.
 	// For now, whether in Default or Hosted mode, the addons could be deployed on the managed cluster.
 	namespaces := []string{config.KlusterletNamespace}
 	if !config.DisableAddonNamespace {
-		namespaces = append(namespaces, helpers.DefaultAddonNamespace)
+		// check if other klusterlet agent namespaces still exist before deleting addon namespace
+		hasOtherAgents, err := r.hasActiveKlusterletAgentNamespaces(ctx, config.KlusterletNamespace)
+		if err != nil {
+			return klusterlet, reconcileStop, fmt.Errorf("failed to check for active klusterlet agent namespaces: %v", err)
+		}
+
+		// only delete addon namespace if no other klusterlet agents exist
+		if !hasOtherAgents {
+			namespaces = append(namespaces, helpers.DefaultAddonNamespace)
+		}
 	}
 	for _, namespace := range namespaces {
 		if err := r.managedClusterClients.kubeClient.CoreV1().Namespaces().Delete(
@@ -230,3 +239,23 @@ func (r *managedReconcile) cleanUpAppliedManifestWorks(ctx context.Context, klus
 	}
 	return utilerrors.NewAggregate(errs)
 }
+
+// hasActiveKlusterletAgentNamespaces checks if there are any other klusterlet agent namespaces
+// on the managed cluster besides the one being deleted. This prevents premature deletion of
+// the shared addon namespace when multiple klusterlets (from different hubs) are using it.
+func (r *managedReconcile) hasActiveKlusterletAgentNamespaces(ctx context.Context, currentAgentNamespace string) (bool, error) {
+	// Look for namespaces with klusterlet labels
+	namespaces, err := r.managedClusterClients.kubeClient.CoreV1().Namespaces().List(ctx, metav1.ListOptions{
+		LabelSelector: klusterletNamespaceLabelKey,
+	})
+	if err != nil {
+		return false, fmt.Errorf("failed to list klusterlet agent namespaces: %v", err)
+	}
+
+	// check if there exist namespaces other than the one being deleted
+	if len(namespaces.Items) > 1 || len(namespaces.Items) == 1 && namespaces.Items[0].Name != currentAgentNamespace {
+		return true, nil
+	}
+
+	return false, nil
+}