From 44363d5d99c09db319a92443a6e54b54509b10ff Mon Sep 17 00:00:00 2001 From: "ta924@yahoo.com" Date: Wed, 5 Apr 2023 13:31:32 -0500 Subject: [PATCH] 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: ta924@yahoo.com address review comments --- pkg/canary/daemonset_controller.go | 5 +---- pkg/canary/daemonset_controller_test.go | 1 + pkg/canary/deployment_controller.go | 5 +---- pkg/canary/deployment_controller_test.go | 1 + pkg/canary/util.go | 2 +- pkg/canary/util_test.go | 13 +++++++++++++ pkg/router/util.go | 2 +- pkg/router/util_test.go | 13 +++++++++++++ 8 files changed, 32 insertions(+), 10 deletions(-) diff --git a/pkg/canary/daemonset_controller.go b/pkg/canary/daemonset_controller.go index 75de46c54..69ada2d50 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/daemonset_controller_test.go b/pkg/canary/daemonset_controller_test.go index 29585bb2a..f886955da 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.go b/pkg/canary/deployment_controller.go index 5d176d4af..f35072c5a 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/deployment_controller_test.go b/pkg/canary/deployment_controller_test.go index c12a3379e..24aa0c65a 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.go b/pkg/canary/util.go index 07bb50c22..b6ac9492a 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/canary/util_test.go b/pkg/canary/util_test.go index b0ed41e5f..b90b2cb9c 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.go b/pkg/router/util.go index 72e088064..03267de36 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 } diff --git a/pkg/router/util_test.go b/pkg/router/util_test.go index 7f44436b5..cddb92ddc 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) +}