Merge pull request #1405 from ta924/main

avoid copying canary labels to primary on promotion
fluxcd · Apr 10, 2023 · f1def19 · f1def19
2 parents f3f6266 + 44363d5
commit f1def19
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)
+}