address issue with all canary labels copied to primary on promote

address issue with all canary labels copied to primary on promote Signed-off-by: [email protected] <[email protected]> address review comments
jiri-pinkava · Apr 10, 2023 · 44363d5 · 44363d5
1 parent f3f6266
commit 44363d5
Show file tree

Hide file tree

Showing 8 changed files with 32 additions and 10 deletions.
diff --git a/pkg/canary/daemonset_controller.go b/pkg/canary/daemonset_controller.go
@@ -175,11 +175,8 @@ func (c *DaemonSetController) Promote(cd *flaggerv1.Canary) error {
 			primaryCopy.ObjectMeta.Annotations[k] = v
 		}
 		// update ds labels
-		primaryCopy.ObjectMeta.Labels = make(map[string]string)
 		filteredLabels := includeLabelsByPrefix(canary.ObjectMeta.Labels, c.includeLabelPrefix)
-		for k, v := range filteredLabels {
-			primaryCopy.ObjectMeta.Labels[k] = v
-		}
+		primaryCopy.ObjectMeta.Labels = makePrimaryLabels(filteredLabels, primaryLabelValue, label)
 
 		// apply update
 		_, err = c.kubeClient.AppsV1().DaemonSets(cd.Namespace).Update(context.TODO(), primaryCopy, metav1.UpdateOptions{})

diff --git a/pkg/canary/daemonset_controller_test.go b/pkg/canary/daemonset_controller_test.go
@@ -99,6 +99,7 @@ func TestDaemonSetController_Promote(t *testing.T) {
 	daePrimaryLabels := daePrimary.ObjectMeta.Labels
 	daeSourceLabels := dae2.ObjectMeta.Labels
 	assert.Equal(t, daeSourceLabels["app.kubernetes.io/test-label-1"], daePrimaryLabels["app.kubernetes.io/test-label-1"])
+	assert.Equal(t, "podinfo-primary", daePrimaryLabels["name"])
 
 	daePrimaryAnnotations := daePrimary.ObjectMeta.Annotations
 	daeSourceAnnotations := dae2.ObjectMeta.Annotations

diff --git a/pkg/canary/deployment_controller.go b/pkg/canary/deployment_controller.go
@@ -129,11 +129,8 @@ func (c *DeploymentController) Promote(cd *flaggerv1.Canary) error {
 			primaryCopy.ObjectMeta.Annotations[k] = v
 		}
 		// update deploy labels
-		primaryCopy.ObjectMeta.Labels = make(map[string]string)
 		filteredLabels := includeLabelsByPrefix(canary.ObjectMeta.Labels, c.includeLabelPrefix)
-		for k, v := range filteredLabels {
-			primaryCopy.ObjectMeta.Labels[k] = v
-		}
+		primaryCopy.ObjectMeta.Labels = makePrimaryLabels(filteredLabels, primaryLabelValue, label)
 
 		// apply update
 		_, err = c.kubeClient.AppsV1().Deployments(cd.Namespace).Update(context.TODO(), primaryCopy, metav1.UpdateOptions{})

diff --git a/pkg/canary/deployment_controller_test.go b/pkg/canary/deployment_controller_test.go
@@ -95,6 +95,7 @@ func TestDeploymentController_Promote(t *testing.T) {
 	depPrimaryLabels := depPrimary.ObjectMeta.Labels
 	depSourceLabels := dep2.ObjectMeta.Labels
 	assert.Equal(t, depSourceLabels["app.kubernetes.io/test-label-1"], depPrimaryLabels["app.kubernetes.io/test-label-1"])
+	assert.Equal(t, "podinfo-primary", depPrimaryLabels["name"])
 
 	depPrimaryAnnotations := depPrimary.ObjectMeta.Annotations
 	depSourceAnnotations := dep2.ObjectMeta.Annotations

diff --git a/pkg/canary/util.go b/pkg/canary/util.go
@@ -115,7 +115,7 @@ func includeLabelsByPrefix(labels map[string]string, includeLabelPrefixes []stri
 			continue
 		}
 		for _, includeLabelPrefix := range includeLabelPrefixes {
-			if includeLabelPrefix == "*" || strings.HasPrefix(key, includeLabelPrefix) {
+			if includeLabelPrefix == "*" || (includeLabelPrefix != "" && strings.HasPrefix(key, includeLabelPrefix)) {
 				filteredLabels[key] = value
 				break
 			}

diff --git a/pkg/canary/util_test.go b/pkg/canary/util_test.go
@@ -56,6 +56,19 @@ func TestIncludeLabelsByPrefixWithWildcard(t *testing.T) {
 	})
 }
 
+func TestIncludeLabelsNoIncludes(t *testing.T) {
+	labels := map[string]string{
+		"foo":   "foo-value",
+		"bar":   "bar-value",
+		"lorem": "ipsum",
+	}
+	includeLabelPrefix := []string{""}
+
+	filteredLabels := includeLabelsByPrefix(labels, includeLabelPrefix)
+
+	assert.Equal(t, map[string]string{}, filteredLabels)
+}
+
 func TestMakePrimaryLabels(t *testing.T) {
 	labels := map[string]string{
 		"lorem": "ipsum",

diff --git a/pkg/router/util.go b/pkg/router/util.go
@@ -17,7 +17,7 @@ func includeLabelsByPrefix(labels map[string]string, includeLabelPrefixes []stri
 			continue
 		}
 		for _, includeLabelPrefix := range includeLabelPrefixes {
-			if includeLabelPrefix == "*" || strings.HasPrefix(key, includeLabelPrefix) {
+			if includeLabelPrefix == "*" || (includeLabelPrefix != "" && strings.HasPrefix(key, includeLabelPrefix)) {
 				filteredLabels[key] = value
 				break
 			}

diff --git a/pkg/router/util_test.go b/pkg/router/util_test.go
@@ -56,3 +56,16 @@ func TestIncludeLabelsByPrefixWithWildcard(t *testing.T) {
 		"lorem": "ipsum",
 	})
 }
+
+func TestIncludeLabelsNoIncludes(t *testing.T) {
+	labels := map[string]string{
+		"foo":   "foo-value",
+		"bar":   "bar-value",
+		"lorem": "ipsum",
+	}
+	includeLabelPrefix := []string{""}
+
+	filteredLabels := includeLabelsByPrefix(labels, includeLabelPrefix)
+
+	assert.Equal(t, map[string]string{}, filteredLabels)
+}