Check deletion event matches ClusterSet before Multi-cluster cleanup (#…

…5638) When handling a ClusterSet deletion event in a member cluster, check the deleted ClusterSet and clean up Multi-cluster resources only when the current ClusterSet is deleted. Also fix and enhance the ClusterSet deletion unit test. Signed-off-by: Jianjun Shen <[email protected]>
antrea-io · Oct 30, 2023 · e5f044d · e5f044d
1 parent 55e731d
commit e5f044d
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 3 deletions.
diff --git a/multicluster/controllers/multicluster/member/clusterset_controller.go b/multicluster/controllers/multicluster/member/clusterset_controller.go
@@ -101,6 +101,10 @@ func (r *MemberClusterSetReconciler) Reconcile(ctx context.Context, req ctrl.Req
 		if !apierrors.IsNotFound(err) {
 			return ctrl.Result{}, err
 		}
+		if r.clusterSetID != common.ClusterSetID(req.Name) {
+			// Not the current ClusterSet.
+			return ctrl.Result{}, nil
+		}
 		clusterSetNotFound = true
 	}
 

diff --git a/multicluster/controllers/multicluster/member/clusterset_controller_test.go b/multicluster/controllers/multicluster/member/clusterset_controller_test.go
@@ -50,7 +50,32 @@ func TestMemberClusterDelete(t *testing.T) {
 	reconciler := MemberClusterSetReconciler{
 		Client:           fakeClient,
 		remoteCommonArea: commonArea,
+		clusterSetID:     common.ClusterSetID("clusterset1"),
 	}
+
+	// Delete a different ClusterSet.
+	if _, err := reconciler.Reconcile(common.TestCtx, reconcile.Request{
+		NamespacedName: types.NamespacedName{
+			Namespace: "default",
+			Name:      "clusterset2",
+		}}); err != nil {
+		t.Errorf("Member ClusterSet Reconciler should handle delete event successfully but got error = %v", err)
+	} else {
+		memberClusterAnnounce := &mcv1alpha1.MemberClusterAnnounce{}
+		err := fakeRemoteClient.Get(common.TestCtx, types.NamespacedName{
+			Namespace: "default",
+			Name:      "member-announce-from-cluster-a",
+		}, memberClusterAnnounce)
+		if err != nil {
+			if apierrors.IsNotFound(err) {
+				t.Errorf("Member ClusterSet Reconciler should not remove MemberClusterAnnounce")
+			} else {
+				t.Errorf("Get MemberClusterAnnounce returned error = %v", err)
+			}
+		}
+	}
+
+	// Delete the current ClusterSet.
 	if _, err := reconciler.Reconcile(common.TestCtx, reconcile.Request{
 		NamespacedName: types.NamespacedName{
 			Namespace: "default",
@@ -59,12 +84,14 @@ func TestMemberClusterDelete(t *testing.T) {
 		t.Errorf("Member ClusterSet Reconciler should handle delete event successfully but got error = %v", err)
 	} else {
 		memberClusterAnnounce := &mcv1alpha1.MemberClusterAnnounce{}
-		err := fakeClient.Get(common.TestCtx, types.NamespacedName{
+		err := fakeRemoteClient.Get(common.TestCtx, types.NamespacedName{
 			Namespace: "default",
 			Name:      "member-announce-from-cluster-a",
 		}, memberClusterAnnounce)
-		if !apierrors.IsNotFound(err) {
-			t.Errorf("Member ClusterSet Reconciler should remove MemberClusterAnnounce successfully but got error = %v", err)
+		if err == nil {
+			t.Errorf("Member ClusterSet Reconciler should remove MemberClusterAnnounce but not")
+		} else if !apierrors.IsNotFound(err) {
+			t.Errorf("Get MemberClusterAnnounce returned error = %v", err)
 		}
 	}
 }