Skip to content
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.

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

Automated cherry pick of #5449: Handle stale LabelIdentities created for empty Pod labels #5459

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion multicluster/controllers/multicluster/stale_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
46 changes: 44 additions & 2 deletions multicluster/controllers/multicluster/stale_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:<none>"
toKeepLabelResExport := mcsv1alpha1.ResourceExport{
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Expand All @@ -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",
Expand All @@ -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
Expand Down Expand Up @@ -375,6 +415,8 @@ func TestStaleController_CleanupResourceExport(t *testing.T) {
toKeepSvcResExport,
toKeepLabelResExport,
toDeleteLabelResExport,
toKeepLabelResExportEmptyLabels,
toDeleteLabelResExportEmptyLabels,
svcResExportFromOtherCluster,
},
},
Expand All @@ -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)
Expand Down
7 changes: 6 additions & 1 deletion pkg/controller/labelidentity/label_group_index.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 <none> 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 == "<none>" {
return m
}
kvs := strings.Split(s, ",")
Expand Down
14 changes: 10 additions & 4 deletions pkg/controller/labelidentity/label_group_index_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:<none>"
)

func TestLabelIdentityMatch(t *testing.T) {
Expand Down Expand Up @@ -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)
Expand Down