Repository: fluxcd/flagger
Pull request #1405 (2023-04-05 18:40:42)

Fix canary to primary label promotion
This PR is to address issue https://github.com/fluxcd/flagger/issues/1403 where all canary labels are copied to the primary on promote.  This resulting in the following issues

- primary label was lost on promotion
- if no value was provided for `include-label-prefix` all labels would be copied over

Fix: #1403

codecov-commenter: ## [Codecov](https://codecov.io/gh/fluxcd/flagger/pull/1405?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fluxcd) Report
Patch coverage: **`100.00`**% and project coverage change: **`+0.02`** :tada:
> Comparison is base [(`5f63c4a`)](https://codecov.io/gh/fluxcd/flagger/commit/5f63c4ae63ef80925b0eac235edc55ef13a4d14b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fluxcd) 54.42% compared to head [(`785fc9d`)](https://codecov.io/gh/fluxcd/flagger/pull/1405?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fluxcd) 54.44%.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main    #1405      +/-   ##
==========================================
+ Coverage   54.42%   54.44%   +0.02%     
==========================================
  Files          84       84              
  Lines       10049    10048       -1     
==========================================
+ Hits         5469     5471       +2     
+ Misses       3925     3923       -2     
+ Partials      655      654       -1     
```


| [Impacted Files](https://codecov.io/gh/fluxcd/flagger/pull/1405?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fluxcd) | Coverage Δ | |
|---|---|---|
| [pkg/canary/deployment\_controller.go](https://codecov.io/gh/fluxcd/flagger/pull/1405?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fluxcd#diff-cGtnL2NhbmFyeS9kZXBsb3ltZW50X2NvbnRyb2xsZXIuZ28=) | `44.33% <100.00%> (+0.60%)` | :arrow_up: |
| [pkg/canary/util.go](https://codecov.io/gh/fluxcd/flagger/pull/1405?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fluxcd#diff-cGtnL2NhbmFyeS91dGlsLmdv) | `55.00% <100.00%> (ø)` | |
| [pkg/router/util.go](https://codecov.io/gh/fluxcd/flagger/pull/1405?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fluxcd#diff-cGtnL3JvdXRlci91dGlsLmdv) | `100.00% <100.00%> (ø)` | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fluxcd). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fluxcd)

</details>

[:umbrella: View full report in Codecov by Sentry](https://codecov.io/gh/fluxcd/flagger/pull/1405?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fluxcd).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fluxcd).

ta924: @stefanprodan and feedback on resolving the failed e2e test?  Seems to be a versioning issue with k8s

Patch:
diff --git a/pkg/canary/daemonset_controller.go b/pkg/canary/daemonset_controller.go
index 75de46c5..69ada2d5 100644
--- 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/deployment_controller.go b/pkg/canary/deployment_controller.go
index 5d176d4a..f35072c5 100644
--- 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/util.go b/pkg/canary/util.go
index 07bb50c2..b6ac9492 100644
--- 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/router/util.go b/pkg/router/util.go
index 72e08806..03267de3 100644
--- 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
 			}


Test patch:
diff --git a/pkg/canary/daemonset_controller_test.go b/pkg/canary/daemonset_controller_test.go
index 29585bb2..f886955d 100644
--- 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_test.go b/pkg/canary/deployment_controller_test.go
index c12a3379..24aa0c65 100644
--- 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_test.go b/pkg/canary/util_test.go
index b0ed41e5..b90b2cb9 100644
--- 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_test.go b/pkg/router/util_test.go
index 7f44436b..cddb92dd 100644
--- 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)
+}