diff --git a/multicluster/controllers/multicluster/stale_controller.go b/multicluster/controllers/multicluster/stale_controller.go index 5c1b031d48a..a84bfbc2abf 100644 --- a/multicluster/controllers/multicluster/stale_controller.go +++ b/multicluster/controllers/multicluster/stale_controller.go @@ -339,7 +339,7 @@ func (c *StaleResCleanupController) cleanupLabelIdentityResourceExport(commonAre if !ok { continue } - normalizedLabel := podNSlabel + "&pod:" + labels.FormatLabels(p.Labels) + normalizedLabel := podNSlabel + "&pod:" + labels.Set(p.Labels).String() delete(staleResExpItems, normalizedLabel) } for _, r := range staleResExpItems { diff --git a/multicluster/controllers/multicluster/stale_controller_test.go b/multicluster/controllers/multicluster/stale_controller_test.go index 97949ebd5ca..a3f7a47738a 100644 --- a/multicluster/controllers/multicluster/stale_controller_test.go +++ b/multicluster/controllers/multicluster/stale_controller_test.go @@ -312,10 +312,18 @@ func TestStaleController_CleanupResourceExport(t *testing.T) { }, }, }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-ns", + Name: "test-pod-empty-labels", + }, + }, }, } labelNormalizedExist := "ns:kubernetes.io/metadata.name=test-ns,purpose=test&pod:app=web" labelNormalizedNonExist := "ns:kubernetes.io/metadata.name=test-ns,purpose=test&pod:app=db" + labelNormalizedEmptyLabels := "ns:kubernetes.io/metadata.name=test-ns,purpose=test&pod:" + labelNormalizedEmptyLabelsStale := "ns:kubernetes.io/metadata.name=test-ns,purpose=test&pod:" toKeepLabelResExport := mcsv1alpha1.ResourceExport{ ObjectMeta: metav1.ObjectMeta{ Namespace: "default", @@ -332,6 +340,22 @@ func TestStaleController_CleanupResourceExport(t *testing.T) { }, }, } + toKeepLabelResExportEmptyLabels := mcsv1alpha1.ResourceExport{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "cluster-a-" + common.HashLabelIdentity(labelNormalizedEmptyLabels), + Labels: map[string]string{ + constants.SourceKind: constants.LabelIdentityKind, + constants.SourceClusterID: "cluster-a", + }, + }, + Spec: mcsv1alpha1.ResourceExportSpec{ + Kind: constants.LabelIdentityKind, + LabelIdentity: &mcsv1alpha1.LabelIdentityExport{ + NormalizedLabel: labelNormalizedEmptyLabels, + }, + }, + } toDeleteLabelResExport := mcsv1alpha1.ResourceExport{ ObjectMeta: metav1.ObjectMeta{ Namespace: "default", @@ -348,6 +372,22 @@ func TestStaleController_CleanupResourceExport(t *testing.T) { }, }, } + toDeleteLabelResExportEmptyLabels := mcsv1alpha1.ResourceExport{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "cluster-a-" + common.HashLabelIdentity(labelNormalizedEmptyLabelsStale), + Labels: map[string]string{ + constants.SourceKind: constants.LabelIdentityKind, + constants.SourceClusterID: "cluster-a", + }, + }, + Spec: mcsv1alpha1.ResourceExportSpec{ + Kind: constants.LabelIdentityKind, + LabelIdentity: &mcsv1alpha1.LabelIdentityExport{ + NormalizedLabel: labelNormalizedEmptyLabelsStale, + }, + }, + } tests := []struct { name string existSvcList *corev1.ServiceList @@ -375,6 +415,8 @@ func TestStaleController_CleanupResourceExport(t *testing.T) { toKeepSvcResExport, toKeepLabelResExport, toDeleteLabelResExport, + toKeepLabelResExportEmptyLabels, + toDeleteLabelResExportEmptyLabels, svcResExportFromOtherCluster, }, }, @@ -396,8 +438,8 @@ func TestStaleController_CleanupResourceExport(t *testing.T) { err := fakeRemoteClient.List(context.TODO(), resExpList, &client.ListOptions{}) resExpLen := len(resExpList.Items) if err == nil { - if resExpLen != 3 { - t.Errorf("Should only THREE valid ResourceExports left but got %v", resExpLen) + if resExpLen != 4 { + t.Errorf("Should only FOUR valid ResourceExports left but got %v", resExpLen) } } else { t.Errorf("Should list ResourceExport successfully but got err = %v", err) diff --git a/pkg/controller/labelidentity/label_group_index.go b/pkg/controller/labelidentity/label_group_index.go index 79d3d4d633a..7de65a845ea 100644 --- a/pkg/controller/labelidentity/label_group_index.go +++ b/pkg/controller/labelidentity/label_group_index.go @@ -124,7 +124,12 @@ func (l *labelIdentityMatch) matches(s *selectorItem) bool { // constructMapFromLabelString parses label string of format "app=client,env=dev" into a map. func constructMapFromLabelString(s string) map[string]string { m := map[string]string{} - if s == "" { + // Before https://github.com/antrea-io/antrea/issues/5403 is fixed, LabelIdentities created + // for Pods with an empty label set will include a string. Handling for such LabelIdentities + // are needed, as in multi-cluster controller upgrade cases, these LabelIdentities created by + // previous controller still need to be processed, but will be cleaned up by the stale controller + // eventually. + if s == "" || s == "" { return m } kvs := strings.Split(s, ",") diff --git a/pkg/controller/labelidentity/label_group_index_test.go b/pkg/controller/labelidentity/label_group_index_test.go index 651118a2fef..b710d95c8aa 100644 --- a/pkg/controller/labelidentity/label_group_index_test.go +++ b/pkg/controller/labelidentity/label_group_index_test.go @@ -65,10 +65,11 @@ var ( selectorItemG = &selectorItem{ selector: selectorG, } - labelA = "ns:kubernetes.io/metadata.name=testing,purpose=test&pod:app=web" - labelB = "ns:kubernetes.io/metadata.name=testing,purpose=test&pod:app=db" - labelC = "ns:kubernetes.io/metadata.name=nomatch,purpose=nomatch&pod:app=db" - labelD = "ns:kubernetes.io/metadata.name=testing,purpose=test&pod:" + labelA = "ns:kubernetes.io/metadata.name=testing,purpose=test&pod:app=web" + labelB = "ns:kubernetes.io/metadata.name=testing,purpose=test&pod:app=db" + labelC = "ns:kubernetes.io/metadata.name=nomatch,purpose=nomatch&pod:app=db" + labelD = "ns:kubernetes.io/metadata.name=testing,purpose=test&pod:" + labelStale = "ns:kubernetes.io/metadata.name=testing,purpose=test&pod:" ) func TestLabelIdentityMatch(t *testing.T) { @@ -172,6 +173,11 @@ func TestLabelIdentityMatch(t *testing.T) { selector: selectorItemG, expectMatch: true, }, + { + label: labelStale, + selector: selectorItemA, + expectMatch: false, + }, } for _, tt := range tests { labelMatch := newLabelIdentityMatch(tt.label, 1)