Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 11, 2025
+298 −1
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
268 changes: 268 additions & 0 deletions ...rator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
}
31 changes: 30 additions & 1 deletion ...tor/operators/klusterlet/controllers/klusterletcontroller/klusterlet_managed_reconcile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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
}
Loading