From b81c4b685d36e39ede64069b5ded58c0e91bfaef Mon Sep 17 00:00:00 2001 From: Kevin Snyder Date: Wed, 1 Oct 2025 15:58:11 -0700 Subject: [PATCH 01/10] Rework selector labels Addresses #1312, #1257, #1115 Retains all `matchLabels` in the primary `Deployment` or `DaemonSet`, instead of picking the first one that matches the `-selector-labels`. Recreates the primary `Deployment` or `DaemonSet` if the `matchLabels` of the existing object are not equal. This must be done since `matchLabels` is an immutable field. The recreate happens before promoting the canary to primary (i.e. no traffic is being routed to the primary). Still appends `-primary` to the value of the first label that matches `-selector-labels`. This ensures that the canary and primary have a unique set of selector labels. `Service` selector also uses the full set of `matchLabels` and overrides the value of the first matching `-selector-labels` to route to canary or primary. Signed-off-by: Kevin Snyder --- pkg/canary/controller.go | 2 +- pkg/canary/daemonset_controller.go | 64 +++++++++++++++++++++-------- pkg/canary/deployment_controller.go | 64 +++++++++++++++++++++-------- pkg/canary/knative_controller.go | 4 +- pkg/canary/service_controller.go | 4 +- pkg/controller/finalizer.go | 4 +- pkg/controller/scheduler.go | 4 +- pkg/router/factory.go | 3 +- pkg/router/kubernetes_default.go | 9 +++- 9 files changed, 115 insertions(+), 43 deletions(-) diff --git a/pkg/canary/controller.go b/pkg/canary/controller.go index 62c86470f..f37ebf725 100644 --- a/pkg/canary/controller.go +++ b/pkg/canary/controller.go @@ -23,7 +23,7 @@ import ( type Controller interface { IsPrimaryReady(canary *flaggerv1.Canary) (bool, error) IsCanaryReady(canary *flaggerv1.Canary) (bool, error) - GetMetadata(canary *flaggerv1.Canary) (string, string, map[string]int32, error) + GetMetadata(canary *flaggerv1.Canary) (map[string]string, string, string, map[string]int32, error) SyncStatus(canary *flaggerv1.Canary, status flaggerv1.CanaryStatus) error SetStatusFailedChecks(canary *flaggerv1.Canary, val int) error SetStatusWeight(canary *flaggerv1.Canary, val int) error diff --git a/pkg/canary/daemonset_controller.go b/pkg/canary/daemonset_controller.go index 5b8f92e87..af254c4c6 100644 --- a/pkg/canary/daemonset_controller.go +++ b/pkg/canary/daemonset_controller.go @@ -19,6 +19,7 @@ package canary import ( "context" "fmt" + "maps" "go.uber.org/zap" appsv1 "k8s.io/api/apps/v1" @@ -120,7 +121,7 @@ func (c *DaemonSetController) Promote(cd *flaggerv1.Canary) error { return fmt.Errorf("damonset %s.%s get query error: %v", targetName, cd.Namespace, err) } - label, labelValue, err := c.getSelectorLabel(canary) + matchLabels, label, labelValue, err := c.getSelectorLabel(canary) primaryLabelValue := fmt.Sprintf("%s-primary", labelValue) if err != nil { return fmt.Errorf("getSelectorLabel failed: %w", err) @@ -131,6 +132,10 @@ func (c *DaemonSetController) Promote(cd *flaggerv1.Canary) error { return fmt.Errorf("daemonset %s.%s get query error: %w", primaryName, cd.Namespace, err) } + if !maps.Equal(primary.Spec.Selector.MatchLabels, matchLabels) { + c.recreatePrimaryDaemonSet(cd, c.includeLabelPrefix) + } + // promote secrets and config maps configRefs, err := c.configTracker.GetTargetConfigs(cd) if err != nil { @@ -206,24 +211,24 @@ func (c *DaemonSetController) HasTargetChanged(cd *flaggerv1.Canary) (bool, erro } // GetMetadata returns the pod label selector and svc ports -func (c *DaemonSetController) GetMetadata(cd *flaggerv1.Canary) (string, string, map[string]int32, error) { +func (c *DaemonSetController) GetMetadata(cd *flaggerv1.Canary) (map[string]string, string, string, map[string]int32, error) { targetName := cd.Spec.TargetRef.Name canaryDae, err := c.kubeClient.AppsV1().DaemonSets(cd.Namespace).Get(context.TODO(), targetName, metav1.GetOptions{}) if err != nil { - return "", "", nil, fmt.Errorf("daemonset %s.%s get query error: %w", targetName, cd.Namespace, err) + return nil, "", "", nil, fmt.Errorf("daemonset %s.%s get query error: %w", targetName, cd.Namespace, err) } - label, labelValue, err := c.getSelectorLabel(canaryDae) + matchLabels, label, labelValue, err := c.getSelectorLabel(canaryDae) if err != nil { - return "", "", nil, fmt.Errorf("getSelectorLabel failed: %w", err) + return nil, "", "", nil, fmt.Errorf("getSelectorLabel failed: %w", err) } var ports map[string]int32 if cd.Spec.Service.PortDiscovery { ports = getPorts(cd, canaryDae.Spec.Template.Spec.Containers) } - return label, labelValue, ports, nil + return matchLabels, label, labelValue, ports, nil } func (c *DaemonSetController) createPrimaryDaemonSet(cd *flaggerv1.Canary, includeLabelPrefix []string) error { @@ -244,7 +249,7 @@ func (c *DaemonSetController) createPrimaryDaemonSet(cd *flaggerv1.Canary, inclu // Create the labels map but filter unwanted labels labels := includeLabelsByPrefix(canaryDae.Labels, includeLabelPrefix) - label, labelValue, err := c.getSelectorLabel(canaryDae) + matchLabels, label, labelValue, err := c.getSelectorLabel(canaryDae) primaryLabelValue := fmt.Sprintf("%s-primary", labelValue) if err != nil { return fmt.Errorf("getSelectorLabel failed: %w", err) @@ -265,6 +270,8 @@ func (c *DaemonSetController) createPrimaryDaemonSet(cd *flaggerv1.Canary, inclu return fmt.Errorf("makeAnnotations failed: %w", err) } + matchLabels[label] = primaryLabelValue + // create primary daemonset primaryDae = &appsv1.DaemonSet{ ObjectMeta: metav1.ObjectMeta{ @@ -285,9 +292,7 @@ func (c *DaemonSetController) createPrimaryDaemonSet(cd *flaggerv1.Canary, inclu RevisionHistoryLimit: canaryDae.Spec.RevisionHistoryLimit, UpdateStrategy: canaryDae.Spec.UpdateStrategy, Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - label: primaryLabelValue, - }, + MatchLabels: matchLabels, }, Template: corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ @@ -310,18 +315,45 @@ func (c *DaemonSetController) createPrimaryDaemonSet(cd *flaggerv1.Canary, inclu return nil } +func (c *DaemonSetController) recreatePrimaryDaemonSet(cd *flaggerv1.Canary, includeLabelPrefix []string) error { + targetName := cd.Spec.TargetRef.Name + primaryName := fmt.Sprintf("%s-primary", targetName) + + // TODO: Check if primary daemonset is scaled to zero and not receiving any traffic + + if err := c.kubeClient.AppsV1().DaemonSets(cd.Namespace).Delete(context.TODO(), primaryName, metav1.DeleteOptions{}); err != nil { + return fmt.Errorf("daemonset %s.%s delete error: %w", primaryName, cd.Namespace, err) + } + + if err := c.createPrimaryDaemonSet(cd, includeLabelPrefix); err != nil { + return fmt.Errorf("createPrimaryDaemonSet failed: %w", err) + } + + return nil +} + // getSelectorLabel returns the selector match label -func (c *DaemonSetController) getSelectorLabel(daemonSet *appsv1.DaemonSet) (string, string, error) { +func (c *DaemonSetController) getSelectorLabel(daemonSet *appsv1.DaemonSet) (map[string]string, string, string, error) { + label, labelValue := "", "" for _, l := range c.labels { if _, ok := daemonSet.Spec.Selector.MatchLabels[l]; ok { - return l, daemonSet.Spec.Selector.MatchLabels[l], nil + label, labelValue = l, daemonSet.Spec.Selector.MatchLabels[l] } } - return "", "", fmt.Errorf( - "daemonset %s.%s spec.selector.matchLabels must contain one of %v'", - daemonSet.Name, daemonSet.Namespace, c.labels, - ) + if label == "" { + return nil, "", "", fmt.Errorf( + "daemonset %s.%s spec.selector.matchLabels must contain one of %v", + daemonSet.Name, daemonSet.Namespace, c.labels, + ) + } + + matchLabels := map[string]string{} + for key, value := range daemonSet.Spec.Selector.MatchLabels { + matchLabels[key] = value + } + + return matchLabels, label, labelValue, nil } func (c *DaemonSetController) HaveDependenciesChanged(cd *flaggerv1.Canary) (bool, error) { diff --git a/pkg/canary/deployment_controller.go b/pkg/canary/deployment_controller.go index 708f4babe..7ba82d250 100644 --- a/pkg/canary/deployment_controller.go +++ b/pkg/canary/deployment_controller.go @@ -19,6 +19,7 @@ package canary import ( "context" "fmt" + "maps" "go.uber.org/zap" appsv1 "k8s.io/api/apps/v1" @@ -72,7 +73,7 @@ func (c *DeploymentController) Promote(cd *flaggerv1.Canary) error { return fmt.Errorf("deployment %s.%s get query error: %w", targetName, cd.Namespace, err) } - label, labelValue, err := c.getSelectorLabel(canary) + matchLabels, label, labelValue, err := c.getSelectorLabel(canary) primaryLabelValue := fmt.Sprintf("%s-primary", labelValue) if err != nil { return fmt.Errorf("getSelectorLabel failed: %w", err) @@ -83,6 +84,10 @@ func (c *DeploymentController) Promote(cd *flaggerv1.Canary) error { return fmt.Errorf("deployment %s.%s get query error: %w", primaryName, cd.Namespace, err) } + if !maps.Equal(primary.Spec.Selector.MatchLabels, matchLabels) { + c.recreatePrimaryDeployment(cd, c.includeLabelPrefix) + } + // promote secrets and config maps configRefs, err := c.configTracker.GetTargetConfigs(cd) if err != nil { @@ -219,17 +224,17 @@ func (c *DeploymentController) ScaleFromZero(cd *flaggerv1.Canary) error { } // GetMetadata returns the pod label selector and svc ports -func (c *DeploymentController) GetMetadata(cd *flaggerv1.Canary) (string, string, map[string]int32, error) { +func (c *DeploymentController) GetMetadata(cd *flaggerv1.Canary) (map[string]string, string, string, map[string]int32, error) { targetName := cd.Spec.TargetRef.Name canaryDep, err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Get(context.TODO(), targetName, metav1.GetOptions{}) if err != nil { - return "", "", nil, fmt.Errorf("deployment %s.%s get query error: %w", targetName, cd.Namespace, err) + return nil, "", "", nil, fmt.Errorf("deployment %s.%s get query error: %w", targetName, cd.Namespace, err) } - label, labelValue, err := c.getSelectorLabel(canaryDep) + matchLabels, label, labelValue, err := c.getSelectorLabel(canaryDep) if err != nil { - return "", "", nil, fmt.Errorf("getSelectorLabel failed: %w", err) + return nil, "", "", nil, fmt.Errorf("getSelectorLabel failed: %w", err) } var ports map[string]int32 @@ -237,7 +242,7 @@ func (c *DeploymentController) GetMetadata(cd *flaggerv1.Canary) (string, string ports = getPorts(cd, canaryDep.Spec.Template.Spec.Containers) } - return label, labelValue, ports, nil + return matchLabels, label, labelValue, ports, nil } func (c *DeploymentController) createPrimaryDeployment(cd *flaggerv1.Canary, includeLabelPrefix []string) error { targetName := cd.Spec.TargetRef.Name @@ -251,7 +256,7 @@ func (c *DeploymentController) createPrimaryDeployment(cd *flaggerv1.Canary, inc // Create the labels map but filter unwanted labels labels := includeLabelsByPrefix(canaryDep.Labels, includeLabelPrefix) - label, labelValue, err := c.getSelectorLabel(canaryDep) + matchLabels, label, labelValue, err := c.getSelectorLabel(canaryDep) primaryLabelValue := fmt.Sprintf("%s-primary", labelValue) if err != nil { return fmt.Errorf("getSelectorLabel failed: %w", err) @@ -277,6 +282,8 @@ func (c *DeploymentController) createPrimaryDeployment(cd *flaggerv1.Canary, inc replicas = *canaryDep.Spec.Replicas } + matchLabels[label] = primaryLabelValue + // create primary deployment primaryDep = &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ @@ -299,9 +306,7 @@ func (c *DeploymentController) createPrimaryDeployment(cd *flaggerv1.Canary, inc Replicas: int32p(replicas), Strategy: canaryDep.Spec.Strategy, Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - label: primaryLabelValue, - }, + MatchLabels: matchLabels, }, Template: corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ @@ -326,18 +331,45 @@ func (c *DeploymentController) createPrimaryDeployment(cd *flaggerv1.Canary, inc return nil } +func (c *DeploymentController) recreatePrimaryDeployment(cd *flaggerv1.Canary, includeLabelPrefix []string) error { + targetName := cd.Spec.TargetRef.Name + primaryName := fmt.Sprintf("%s-primary", targetName) + + // TODO: Check if primary deployment is scaled to zero and not receiving any traffic + + if err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Delete(context.TODO(), primaryName, metav1.DeleteOptions{}); err != nil { + return fmt.Errorf("deployment %s.%s delete error: %w", primaryName, cd.Namespace, err) + } + + if err := c.createPrimaryDeployment(cd, includeLabelPrefix); err != nil { + return fmt.Errorf("createPrimaryDeployment failed: %w", err) + } + + return nil +} + // getSelectorLabel returns the selector match label -func (c *DeploymentController) getSelectorLabel(deployment *appsv1.Deployment) (string, string, error) { +func (c *DeploymentController) getSelectorLabel(deployment *appsv1.Deployment) (map[string]string, string, string, error) { + label, labelValue := "", "" for _, l := range c.labels { if _, ok := deployment.Spec.Selector.MatchLabels[l]; ok { - return l, deployment.Spec.Selector.MatchLabels[l], nil + label, labelValue = l, deployment.Spec.Selector.MatchLabels[l] } } - return "", "", fmt.Errorf( - "deployment %s.%s spec.selector.matchLabels must contain one of %v", - deployment.Name, deployment.Namespace, c.labels, - ) + if label == "" { + return nil, "", "", fmt.Errorf( + "deployment %s.%s spec.selector.matchLabels must contain one of %v", + deployment.Name, deployment.Namespace, c.labels, + ) + } + + matchLabels := map[string]string{} + for key, value := range deployment.Spec.Selector.MatchLabels { + matchLabels[key] = value + } + + return matchLabels, label, labelValue, nil } func (c *DeploymentController) HaveDependenciesChanged(cd *flaggerv1.Canary) (bool, error) { diff --git a/pkg/canary/knative_controller.go b/pkg/canary/knative_controller.go index a69a33fb0..a5ece01f9 100644 --- a/pkg/canary/knative_controller.go +++ b/pkg/canary/knative_controller.go @@ -54,8 +54,8 @@ func (kc *KnativeController) IsCanaryReady(cd *flaggerv1.Canary) (bool, error) { return true, nil } -func (kc *KnativeController) GetMetadata(canary *flaggerv1.Canary) (string, string, map[string]int32, error) { - return "", "", make(map[string]int32), nil +func (kc *KnativeController) GetMetadata(canary *flaggerv1.Canary) (map[string]string, string, string, map[string]int32, error) { + return nil, "", "", make(map[string]int32), nil } // SyncStatus encodes list of revisions and updates the canary status diff --git a/pkg/canary/service_controller.go b/pkg/canary/service_controller.go index 97de616aa..7fa93c0a2 100644 --- a/pkg/canary/service_controller.go +++ b/pkg/canary/service_controller.go @@ -61,8 +61,8 @@ func (c *ServiceController) SetStatusPhase(cd *flaggerv1.Canary, phase flaggerv1 } // GetMetadata returns the pod label selector, label value and svc ports -func (c *ServiceController) GetMetadata(_ *flaggerv1.Canary) (string, string, map[string]int32, error) { - return "", "", nil, nil +func (c *ServiceController) GetMetadata(_ *flaggerv1.Canary) (map[string]string, string, string, map[string]int32, error) { + return nil, "", "", nil, nil } // Initialize creates or updates the primary and canary services to prepare for the canary release process targeted on the K8s service diff --git a/pkg/controller/finalizer.go b/pkg/controller/finalizer.go index 57ab04863..9b1d703de 100644 --- a/pkg/controller/finalizer.go +++ b/pkg/controller/finalizer.go @@ -93,13 +93,13 @@ func (c *Controller) finalize(old interface{}) error { return fmt.Errorf("canary not ready during finalizing: %w", err) } - labelSelector, labelValue, ports, err := canaryController.GetMetadata(canary) + matchLabels, labelSelector, labelValue, ports, err := canaryController.GetMetadata(canary) if err != nil { return fmt.Errorf("failed to get metadata for router finalizing: %w", err) } // Revert the Kubernetes service - router := c.routerFactory.KubernetesRouter(canary.Spec.TargetRef.Kind, labelSelector, labelValue, ports) + router := c.routerFactory.KubernetesRouter(canary.Spec.TargetRef.Kind, matchLabels, labelSelector, labelValue, ports) if err := router.Finalize(canary); err != nil { return fmt.Errorf("failed revert router: %w", err) } diff --git a/pkg/controller/scheduler.go b/pkg/controller/scheduler.go index 8f2e58b22..b7cfdaae6 100644 --- a/pkg/controller/scheduler.go +++ b/pkg/controller/scheduler.go @@ -181,7 +181,7 @@ func (c *Controller) advanceCanary(name string, namespace string) { // init controller based on target kind canaryController := c.canaryFactory.Controller(cd.Spec.TargetRef) - labelSelector, labelValue, ports, err := canaryController.GetMetadata(cd) + matchLabels, labelSelector, labelValue, ports, err := canaryController.GetMetadata(cd) if err != nil { c.recordEventWarningf(cd, "%v", err) return @@ -193,7 +193,7 @@ func (c *Controller) advanceCanary(name string, namespace string) { } // init Kubernetes router - kubeRouter := c.routerFactory.KubernetesRouter(cd.Spec.TargetRef.Kind, labelSelector, labelValue, ports) + kubeRouter := c.routerFactory.KubernetesRouter(cd.Spec.TargetRef.Kind, matchLabels, labelSelector, labelValue, ports) // reconcile the canary/primary services if err := kubeRouter.Initialize(cd); err != nil { diff --git a/pkg/router/factory.go b/pkg/router/factory.go index 838cd2bfe..3d276187f 100644 --- a/pkg/router/factory.go +++ b/pkg/router/factory.go @@ -62,7 +62,7 @@ func NewFactory(kubeConfig *restclient.Config, kubeClient kubernetes.Interface, } // KubernetesRouter returns a KubernetesRouter interface implementation -func (factory *Factory) KubernetesRouter(kind string, labelSelector string, labelValue string, ports map[string]int32) KubernetesRouter { +func (factory *Factory) KubernetesRouter(kind string, matchLabels map[string]string, labelSelector string, labelValue string, ports map[string]int32) KubernetesRouter { switch kind { case "Service": return &KubernetesNoopRouter{} @@ -71,6 +71,7 @@ func (factory *Factory) KubernetesRouter(kind string, labelSelector string, labe logger: factory.logger, flaggerClient: factory.flaggerClient, kubeClient: factory.kubeClient, + matchLabels: matchLabels, labelSelector: labelSelector, labelValue: labelValue, ports: ports, diff --git a/pkg/router/kubernetes_default.go b/pkg/router/kubernetes_default.go index 184b62331..a3472b87b 100644 --- a/pkg/router/kubernetes_default.go +++ b/pkg/router/kubernetes_default.go @@ -40,6 +40,7 @@ type KubernetesDefaultRouter struct { kubeClient kubernetes.Interface flaggerClient clientset.Interface logger *zap.SugaredLogger + matchLabels map[string]string labelSelector string labelValue string ports map[string]int32 @@ -100,10 +101,16 @@ func (c *KubernetesDefaultRouter) reconcileService(canary *flaggerv1.Canary, nam targetPort = canary.Spec.Service.TargetPort } + selector := map[string]string{} + for k, v := range c.matchLabels { + selector[k] = v + } + selector[c.labelSelector] = podSelector + // set pod selector and apex port svcSpec := corev1.ServiceSpec{ Type: corev1.ServiceTypeClusterIP, - Selector: map[string]string{c.labelSelector: podSelector}, + Selector: selector, Ports: []corev1.ServicePort{ { Name: portName, From e330e8ae09fd67a95f6d4d2bb5708b0bd35554a7 Mon Sep 17 00:00:00 2001 From: Kevin Snyder Date: Thu, 2 Oct 2025 13:48:43 -0700 Subject: [PATCH 02/10] Fix `matchLabels` comparison Signed-off-by: Kevin Snyder --- pkg/canary/daemonset_controller.go | 2 ++ pkg/canary/deployment_controller.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/pkg/canary/daemonset_controller.go b/pkg/canary/daemonset_controller.go index af254c4c6..4615916a0 100644 --- a/pkg/canary/daemonset_controller.go +++ b/pkg/canary/daemonset_controller.go @@ -127,6 +127,8 @@ func (c *DaemonSetController) Promote(cd *flaggerv1.Canary) error { return fmt.Errorf("getSelectorLabel failed: %w", err) } + matchLabels[label] = primaryLabelValue + primary, err := c.kubeClient.AppsV1().DaemonSets(cd.Namespace).Get(context.TODO(), primaryName, metav1.GetOptions{}) if err != nil { return fmt.Errorf("daemonset %s.%s get query error: %w", primaryName, cd.Namespace, err) diff --git a/pkg/canary/deployment_controller.go b/pkg/canary/deployment_controller.go index 7ba82d250..35624ecc8 100644 --- a/pkg/canary/deployment_controller.go +++ b/pkg/canary/deployment_controller.go @@ -79,6 +79,8 @@ func (c *DeploymentController) Promote(cd *flaggerv1.Canary) error { return fmt.Errorf("getSelectorLabel failed: %w", err) } + matchLabels[label] = primaryLabelValue + primary, err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Get(context.TODO(), primaryName, metav1.GetOptions{}) if err != nil { return fmt.Errorf("deployment %s.%s get query error: %w", primaryName, cd.Namespace, err) From fd8f780e2d620320d31c7574215c34a3f10f61a4 Mon Sep 17 00:00:00 2001 From: Kevin Snyder Date: Thu, 2 Oct 2025 14:19:25 -0700 Subject: [PATCH 03/10] Get `Deployment` or `DaemonSet` again after recreating Signed-off-by: Kevin Snyder --- pkg/canary/daemonset_controller.go | 6 ++++++ pkg/canary/deployment_controller.go | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/pkg/canary/daemonset_controller.go b/pkg/canary/daemonset_controller.go index 4615916a0..a0ed9451b 100644 --- a/pkg/canary/daemonset_controller.go +++ b/pkg/canary/daemonset_controller.go @@ -138,6 +138,12 @@ func (c *DaemonSetController) Promote(cd *flaggerv1.Canary) error { c.recreatePrimaryDaemonSet(cd, c.includeLabelPrefix) } + // Get primary daemonset again, in case it has changed + primary, err = c.kubeClient.AppsV1().DaemonSets(cd.Namespace).Get(context.TODO(), primaryName, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("daemonset %s.%s get query error: %w", primaryName, cd.Namespace, err) + } + // promote secrets and config maps configRefs, err := c.configTracker.GetTargetConfigs(cd) if err != nil { diff --git a/pkg/canary/deployment_controller.go b/pkg/canary/deployment_controller.go index 35624ecc8..2fde8813c 100644 --- a/pkg/canary/deployment_controller.go +++ b/pkg/canary/deployment_controller.go @@ -90,6 +90,12 @@ func (c *DeploymentController) Promote(cd *flaggerv1.Canary) error { c.recreatePrimaryDeployment(cd, c.includeLabelPrefix) } + // Get primary deployment again, in case it has changes + primary, err = c.kubeClient.AppsV1().Deployments(cd.Namespace).Get(context.TODO(), primaryName, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("deployment %s.%s get query error: %w", primaryName, cd.Namespace, err) + } + // promote secrets and config maps configRefs, err := c.configTracker.GetTargetConfigs(cd) if err != nil { From e025ccd3023bb618818e6b72050f8c0b85c72a42 Mon Sep 17 00:00:00 2001 From: Kevin Snyder Date: Thu, 2 Oct 2025 14:19:44 -0700 Subject: [PATCH 04/10] Update tests Signed-off-by: Kevin Snyder --- pkg/canary/daemonset_controller_test.go | 10 +++++++++- pkg/canary/daemonset_fixture_test.go | 15 +++++++++++++-- pkg/canary/deployment_controller_test.go | 10 +++++++++- pkg/canary/deployment_fixture_test.go | 1 + 4 files changed, 32 insertions(+), 4 deletions(-) diff --git a/pkg/canary/daemonset_controller_test.go b/pkg/canary/daemonset_controller_test.go index f3f73eb97..58de333ce 100644 --- a/pkg/canary/daemonset_controller_test.go +++ b/pkg/canary/daemonset_controller_test.go @@ -79,7 +79,9 @@ func TestDaemonSetController_Promote(t *testing.T) { require.NoError(t, err) dae2 := newDaemonSetControllerTestPodInfoV2() - _, err = mocks.kubeClient.AppsV1().DaemonSets("default").Update(context.TODO(), dae2, metav1.UpdateOptions{}) + err = mocks.kubeClient.AppsV1().DaemonSets("default").Delete(context.TODO(), dae2.ObjectMeta.Name, metav1.DeleteOptions{}) + require.NoError(t, err) + _, err = mocks.kubeClient.AppsV1().DaemonSets("default").Create(context.TODO(), dae2, metav1.CreateOptions{}) require.NoError(t, err) config2 := newDaemonSetControllerTestConfigMapV2() @@ -101,6 +103,12 @@ func TestDaemonSetController_Promote(t *testing.T) { assert.Equal(t, daeSourceLabels["app.kubernetes.io/test-label-1"], daePrimaryLabels["app.kubernetes.io/test-label-1"]) assert.Equal(t, "podinfo-primary", daePrimaryLabels["name"]) + daePrimaryMatchLabels := daePrimary.Spec.Selector.MatchLabels + daeSourceMatchLabels := dae2.Spec.Selector.MatchLabels + assert.Equal(t, len(daeSourceMatchLabels), len(daePrimaryMatchLabels)) + assert.Equal(t, daeSourceMatchLabels["app.kubernetes.io/test-label-1"], daePrimaryMatchLabels["app.kubernetes.io/test-label-1"]) + assert.Equal(t, "podinfo-primary", daePrimaryMatchLabels["name"]) + daePrimaryAnnotations := daePrimary.ObjectMeta.Annotations daeSourceAnnotations := dae2.ObjectMeta.Annotations assert.Equal(t, daeSourceAnnotations["app.kubernetes.io/test-annotation-1"], daePrimaryAnnotations["app.kubernetes.io/test-annotation-1"]) diff --git a/pkg/canary/daemonset_fixture_test.go b/pkg/canary/daemonset_fixture_test.go index 3fb4059f4..76cb986f8 100644 --- a/pkg/canary/daemonset_fixture_test.go +++ b/pkg/canary/daemonset_fixture_test.go @@ -637,17 +637,28 @@ func newDaemonSetControllerTestPodInfoV2() *appsv1.DaemonSet { ObjectMeta: metav1.ObjectMeta{ Namespace: "default", Name: "podinfo", + Annotations: map[string]string{ + "test-annotation-1": "test-annotation-value-1", + }, + Labels: map[string]string{ + "test-label-1": "test-label-value-1", + }, }, Spec: appsv1.DaemonSetSpec{ Selector: &metav1.LabelSelector{ MatchLabels: map[string]string{ - "name": "podinfo", + "name": "podinfo", + "test-label-1": "test-label-value-1", }, }, Template: corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - "name": "podinfo", + "name": "podinfo", + "test-label-1": "test-label-value-1", + }, + Annotations: map[string]string{ + "test-annotation-1": "test-annotation-value-1", }, }, Spec: corev1.PodSpec{ diff --git a/pkg/canary/deployment_controller_test.go b/pkg/canary/deployment_controller_test.go index 24aa0c65a..81e49f24f 100644 --- a/pkg/canary/deployment_controller_test.go +++ b/pkg/canary/deployment_controller_test.go @@ -75,7 +75,9 @@ func TestDeploymentController_Promote(t *testing.T) { mocks.initializeCanary(t) dep2 := newDeploymentControllerTestV2() - _, err := mocks.kubeClient.AppsV1().Deployments("default").Update(context.TODO(), dep2, metav1.UpdateOptions{}) + err := mocks.kubeClient.AppsV1().Deployments("default").Delete(context.TODO(), dep2.ObjectMeta.Name, metav1.DeleteOptions{}) + require.NoError(t, err) + _, err = mocks.kubeClient.AppsV1().Deployments("default").Create(context.TODO(), dep2, metav1.CreateOptions{}) require.NoError(t, err) config2 := newDeploymentControllerTestConfigMapV2() @@ -101,6 +103,12 @@ func TestDeploymentController_Promote(t *testing.T) { depSourceAnnotations := dep2.ObjectMeta.Annotations assert.Equal(t, depSourceAnnotations["app.kubernetes.io/test-annotation-1"], depPrimaryAnnotations["app.kubernetes.io/test-annotation-1"]) + depPrimaryMatchLabels := depPrimary.Spec.Selector.MatchLabels + depSourceMatchLabels := dep2.Spec.Selector.MatchLabels + assert.Equal(t, len(depSourceMatchLabels), len(depPrimaryMatchLabels)) + assert.Equal(t, depSourceMatchLabels["app.kubernetes.io/test-label-1"], depPrimaryMatchLabels["app.kubernetes.io/test-label-1"]) + assert.Equal(t, "podinfo-primary", depPrimaryMatchLabels["name"]) + configPrimary, err := mocks.kubeClient.CoreV1().ConfigMaps("default").Get(context.TODO(), "podinfo-config-env-primary", metav1.GetOptions{}) require.NoError(t, err) assert.Equal(t, config2.Data["color"], configPrimary.Data["color"]) diff --git a/pkg/canary/deployment_fixture_test.go b/pkg/canary/deployment_fixture_test.go index 0ebf07eea..2aab6eb44 100644 --- a/pkg/canary/deployment_fixture_test.go +++ b/pkg/canary/deployment_fixture_test.go @@ -776,6 +776,7 @@ func newDeploymentControllerTestV2() *appsv1.Deployment { Selector: &metav1.LabelSelector{ MatchLabels: map[string]string{ "name": "podinfo", + "test-label-1": "test-label-value-1", }, }, Template: corev1.PodTemplateSpec{ From 17a3b3cc4db7634ed158489f319b06f06df7930b Mon Sep 17 00:00:00 2001 From: Kevin Snyder Date: Fri, 3 Oct 2025 10:55:16 -0700 Subject: [PATCH 05/10] Add logging Signed-off-by: Kevin Snyder --- pkg/canary/deployment_controller.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pkg/canary/deployment_controller.go b/pkg/canary/deployment_controller.go index 2fde8813c..eb3ac2c64 100644 --- a/pkg/canary/deployment_controller.go +++ b/pkg/canary/deployment_controller.go @@ -87,6 +87,14 @@ func (c *DeploymentController) Promote(cd *flaggerv1.Canary) error { } if !maps.Equal(primary.Spec.Selector.MatchLabels, matchLabels) { + c.logger.With("canary", fmt.Sprintf(cd.Name, cd.Namespace)). + Infof( + "Primary deployment %s.%s matchLabels does not match canary deployment %s.%s, recreating...", + primaryName, + cd.Namespace, + targetName, + cd.Namespace, + ) c.recreatePrimaryDeployment(cd, c.includeLabelPrefix) } From 91b6f2ea5da31fb4376c13bece65bf14dd601cbd Mon Sep 17 00:00:00 2001 From: Kevin Snyder Date: Fri, 3 Oct 2025 10:58:36 -0700 Subject: [PATCH 06/10] Rework `createPrimaryDeployment` Signed-off-by: Kevin Snyder --- pkg/canary/deployment_controller.go | 150 ++++++++++++++-------------- 1 file changed, 73 insertions(+), 77 deletions(-) diff --git a/pkg/canary/deployment_controller.go b/pkg/canary/deployment_controller.go index eb3ac2c64..04d064fe5 100644 --- a/pkg/canary/deployment_controller.go +++ b/pkg/canary/deployment_controller.go @@ -47,8 +47,18 @@ type DeploymentController struct { // Initialize creates the primary deployment if it does not exist. func (c *DeploymentController) Initialize(cd *flaggerv1.Canary) (bool, error) { - if err := c.createPrimaryDeployment(cd, c.includeLabelPrefix); err != nil { - return true, fmt.Errorf("createPrimaryDeployment failed: %w", err) + targetName := cd.Spec.TargetRef.Name + primaryName := fmt.Sprintf("%s-primary", cd.Spec.TargetRef.Name) + + canaryDep, err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Get(context.TODO(), targetName, metav1.GetOptions{}) + if err != nil { + return true, fmt.Errorf("deployment %s.%s get query error: %w", targetName, cd.Namespace, err) + } + + if _, err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Get(context.TODO(), primaryName, metav1.GetOptions{}); errors.IsNotFound(err) { + if err := c.createPrimaryDeployment(cd, canaryDep, primaryName, c.includeLabelPrefix); err != nil { + return true, fmt.Errorf("createPrimaryDeployment failed: %w", err) + } } if cd.Status.Phase == "" || cd.Status.Phase == flaggerv1.CanaryPhaseInitializing { @@ -95,7 +105,7 @@ func (c *DeploymentController) Promote(cd *flaggerv1.Canary) error { targetName, cd.Namespace, ) - c.recreatePrimaryDeployment(cd, c.includeLabelPrefix) + c.recreatePrimaryDeployment(cd, canary, primaryName, c.includeLabelPrefix) } // Get primary deployment again, in case it has changes @@ -260,13 +270,23 @@ func (c *DeploymentController) GetMetadata(cd *flaggerv1.Canary) (map[string]str return matchLabels, label, labelValue, ports, nil } -func (c *DeploymentController) createPrimaryDeployment(cd *flaggerv1.Canary, includeLabelPrefix []string) error { - targetName := cd.Spec.TargetRef.Name - primaryName := fmt.Sprintf("%s-primary", cd.Spec.TargetRef.Name) - - canaryDep, err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Get(context.TODO(), targetName, metav1.GetOptions{}) +func (c *DeploymentController) createPrimaryDeployment(cd *flaggerv1.Canary, canaryDep *appsv1.Deployment, name string, includeLabelPrefix []string) error { + // create primary secrets and config maps + configRefs, err := c.configTracker.GetTargetConfigs(cd) if err != nil { - return fmt.Errorf("deployment %s.%s get query error: %w", targetName, cd.Namespace, err) + return fmt.Errorf("GetTargetConfigs failed: %w", err) + } + if err := c.configTracker.CreatePrimaryConfigs(cd, configRefs, c.includeLabelPrefix); err != nil { + return fmt.Errorf("CreatePrimaryConfigs failed: %w", err) + } + annotations, err := makeAnnotations(canaryDep.Spec.Template.Annotations) + if err != nil { + return fmt.Errorf("makeAnnotations failed: %w", err) + } + + replicas := int32(1) + if canaryDep.Spec.Replicas != nil && *canaryDep.Spec.Replicas > 0 { + replicas = *canaryDep.Spec.Replicas } // Create the labels map but filter unwanted labels @@ -278,86 +298,62 @@ func (c *DeploymentController) createPrimaryDeployment(cd *flaggerv1.Canary, inc return fmt.Errorf("getSelectorLabel failed: %w", err) } - primaryDep, err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Get(context.TODO(), primaryName, metav1.GetOptions{}) - if errors.IsNotFound(err) { - // create primary secrets and config maps - configRefs, err := c.configTracker.GetTargetConfigs(cd) - if err != nil { - return fmt.Errorf("GetTargetConfigs failed: %w", err) - } - if err := c.configTracker.CreatePrimaryConfigs(cd, configRefs, c.includeLabelPrefix); err != nil { - return fmt.Errorf("CreatePrimaryConfigs failed: %w", err) - } - annotations, err := makeAnnotations(canaryDep.Spec.Template.Annotations) - if err != nil { - return fmt.Errorf("makeAnnotations failed: %w", err) - } - - replicas := int32(1) - if canaryDep.Spec.Replicas != nil && *canaryDep.Spec.Replicas > 0 { - replicas = *canaryDep.Spec.Replicas - } - - matchLabels[label] = primaryLabelValue - - // create primary deployment - primaryDep = &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: primaryName, - Namespace: cd.Namespace, - Labels: makePrimaryLabels(labels, primaryLabelValue, label), - Annotations: filterMetadata(canaryDep.Annotations), - OwnerReferences: []metav1.OwnerReference{ - *metav1.NewControllerRef(cd, schema.GroupVersionKind{ - Group: flaggerv1.SchemeGroupVersion.Group, - Version: flaggerv1.SchemeGroupVersion.Version, - Kind: flaggerv1.CanaryKind, - }), - }, + matchLabels[label] = primaryLabelValue + + // create primary deployment + primaryDep := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: cd.Namespace, + Labels: makePrimaryLabels(labels, primaryLabelValue, label), + Annotations: filterMetadata(canaryDep.Annotations), + OwnerReferences: []metav1.OwnerReference{ + *metav1.NewControllerRef(cd, schema.GroupVersionKind{ + Group: flaggerv1.SchemeGroupVersion.Group, + Version: flaggerv1.SchemeGroupVersion.Version, + Kind: flaggerv1.CanaryKind, + }), }, - Spec: appsv1.DeploymentSpec{ - ProgressDeadlineSeconds: canaryDep.Spec.ProgressDeadlineSeconds, - MinReadySeconds: canaryDep.Spec.MinReadySeconds, - RevisionHistoryLimit: canaryDep.Spec.RevisionHistoryLimit, - Replicas: int32p(replicas), - Strategy: canaryDep.Spec.Strategy, - Selector: &metav1.LabelSelector{ - MatchLabels: matchLabels, - }, - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: makePrimaryLabels(canaryDep.Spec.Template.Labels, primaryLabelValue, label), - Annotations: annotations, - }, - // update spec with the primary secrets and config maps - Spec: c.getPrimaryDeploymentTemplateSpec(canaryDep, configRefs), + }, + Spec: appsv1.DeploymentSpec{ + ProgressDeadlineSeconds: canaryDep.Spec.ProgressDeadlineSeconds, + MinReadySeconds: canaryDep.Spec.MinReadySeconds, + RevisionHistoryLimit: canaryDep.Spec.RevisionHistoryLimit, + Replicas: int32p(replicas), + Strategy: canaryDep.Spec.Strategy, + Selector: &metav1.LabelSelector{ + MatchLabels: matchLabels, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: makePrimaryLabels(canaryDep.Spec.Template.Labels, primaryLabelValue, label), + Annotations: annotations, }, + // update spec with the primary secrets and config maps + Spec: c.getPrimaryDeploymentTemplateSpec(canaryDep, configRefs), }, - } - - _, err = c.kubeClient.AppsV1().Deployments(cd.Namespace).Create(context.TODO(), primaryDep, metav1.CreateOptions{}) - if err != nil { - return fmt.Errorf("creating deployment %s.%s failed: %w", primaryDep.Name, cd.Namespace, err) - } + }, + } - c.logger.With("canary", fmt.Sprintf("%s.%s", cd.Name, cd.Namespace)). - Infof("Deployment %s.%s created", primaryDep.GetName(), cd.Namespace) + _, err = c.kubeClient.AppsV1().Deployments(cd.Namespace).Create(context.TODO(), primaryDep, metav1.CreateOptions{}) + if err != nil { + return fmt.Errorf("creating deployment %s.%s failed: %w", primaryDep.Name, cd.Namespace, err) } + c.logger.With("canary", fmt.Sprintf("%s.%s", cd.Name, cd.Namespace)). + Infof("Deployment %s.%s created", primaryDep.GetName(), cd.Namespace) + return nil } -func (c *DeploymentController) recreatePrimaryDeployment(cd *flaggerv1.Canary, includeLabelPrefix []string) error { - targetName := cd.Spec.TargetRef.Name - primaryName := fmt.Sprintf("%s-primary", targetName) - - // TODO: Check if primary deployment is scaled to zero and not receiving any traffic +func (c *DeploymentController) recreatePrimaryDeployment(cd *flaggerv1.Canary, canary *appsv1.Deployment, name string, includeLabelPrefix []string) error { + // TODO: Make sure primary deployment is not receiving any traffic - if err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Delete(context.TODO(), primaryName, metav1.DeleteOptions{}); err != nil { - return fmt.Errorf("deployment %s.%s delete error: %w", primaryName, cd.Namespace, err) + if err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Delete(context.TODO(), name, metav1.DeleteOptions{}); err != nil { + return fmt.Errorf("deployment %s.%s delete error: %w", name, cd.Namespace, err) } - if err := c.createPrimaryDeployment(cd, includeLabelPrefix); err != nil { + if err := c.createPrimaryDeployment(cd, canary, name, includeLabelPrefix); err != nil { return fmt.Errorf("createPrimaryDeployment failed: %w", err) } From e804d9b3851c9de053e5ee62bb32c4da2ed98804 Mon Sep 17 00:00:00 2001 From: Kevin Snyder Date: Fri, 3 Oct 2025 12:08:31 -0700 Subject: [PATCH 07/10] Punt on recreating existing primary Signed-off-by: Kevin Snyder --- pkg/canary/daemonset_controller.go | 32 +------------------- pkg/canary/daemonset_controller_test.go | 10 +------ pkg/canary/daemonset_fixture_test.go | 15 ++-------- pkg/canary/deployment_controller.go | 37 +----------------------- pkg/canary/deployment_controller_test.go | 10 +------ pkg/canary/deployment_fixture_test.go | 1 - 6 files changed, 6 insertions(+), 99 deletions(-) diff --git a/pkg/canary/daemonset_controller.go b/pkg/canary/daemonset_controller.go index a0ed9451b..3769ecd3c 100644 --- a/pkg/canary/daemonset_controller.go +++ b/pkg/canary/daemonset_controller.go @@ -19,7 +19,6 @@ package canary import ( "context" "fmt" - "maps" "go.uber.org/zap" appsv1 "k8s.io/api/apps/v1" @@ -121,29 +120,17 @@ func (c *DaemonSetController) Promote(cd *flaggerv1.Canary) error { return fmt.Errorf("damonset %s.%s get query error: %v", targetName, cd.Namespace, err) } - matchLabels, label, labelValue, err := c.getSelectorLabel(canary) + _, label, labelValue, err := c.getSelectorLabel(canary) primaryLabelValue := fmt.Sprintf("%s-primary", labelValue) if err != nil { return fmt.Errorf("getSelectorLabel failed: %w", err) } - matchLabels[label] = primaryLabelValue - primary, err := c.kubeClient.AppsV1().DaemonSets(cd.Namespace).Get(context.TODO(), primaryName, metav1.GetOptions{}) if err != nil { return fmt.Errorf("daemonset %s.%s get query error: %w", primaryName, cd.Namespace, err) } - if !maps.Equal(primary.Spec.Selector.MatchLabels, matchLabels) { - c.recreatePrimaryDaemonSet(cd, c.includeLabelPrefix) - } - - // Get primary daemonset again, in case it has changed - primary, err = c.kubeClient.AppsV1().DaemonSets(cd.Namespace).Get(context.TODO(), primaryName, metav1.GetOptions{}) - if err != nil { - return fmt.Errorf("daemonset %s.%s get query error: %w", primaryName, cd.Namespace, err) - } - // promote secrets and config maps configRefs, err := c.configTracker.GetTargetConfigs(cd) if err != nil { @@ -323,23 +310,6 @@ func (c *DaemonSetController) createPrimaryDaemonSet(cd *flaggerv1.Canary, inclu return nil } -func (c *DaemonSetController) recreatePrimaryDaemonSet(cd *flaggerv1.Canary, includeLabelPrefix []string) error { - targetName := cd.Spec.TargetRef.Name - primaryName := fmt.Sprintf("%s-primary", targetName) - - // TODO: Check if primary daemonset is scaled to zero and not receiving any traffic - - if err := c.kubeClient.AppsV1().DaemonSets(cd.Namespace).Delete(context.TODO(), primaryName, metav1.DeleteOptions{}); err != nil { - return fmt.Errorf("daemonset %s.%s delete error: %w", primaryName, cd.Namespace, err) - } - - if err := c.createPrimaryDaemonSet(cd, includeLabelPrefix); err != nil { - return fmt.Errorf("createPrimaryDaemonSet failed: %w", err) - } - - return nil -} - // getSelectorLabel returns the selector match label func (c *DaemonSetController) getSelectorLabel(daemonSet *appsv1.DaemonSet) (map[string]string, string, string, error) { label, labelValue := "", "" diff --git a/pkg/canary/daemonset_controller_test.go b/pkg/canary/daemonset_controller_test.go index 58de333ce..f3f73eb97 100644 --- a/pkg/canary/daemonset_controller_test.go +++ b/pkg/canary/daemonset_controller_test.go @@ -79,9 +79,7 @@ func TestDaemonSetController_Promote(t *testing.T) { require.NoError(t, err) dae2 := newDaemonSetControllerTestPodInfoV2() - err = mocks.kubeClient.AppsV1().DaemonSets("default").Delete(context.TODO(), dae2.ObjectMeta.Name, metav1.DeleteOptions{}) - require.NoError(t, err) - _, err = mocks.kubeClient.AppsV1().DaemonSets("default").Create(context.TODO(), dae2, metav1.CreateOptions{}) + _, err = mocks.kubeClient.AppsV1().DaemonSets("default").Update(context.TODO(), dae2, metav1.UpdateOptions{}) require.NoError(t, err) config2 := newDaemonSetControllerTestConfigMapV2() @@ -103,12 +101,6 @@ func TestDaemonSetController_Promote(t *testing.T) { assert.Equal(t, daeSourceLabels["app.kubernetes.io/test-label-1"], daePrimaryLabels["app.kubernetes.io/test-label-1"]) assert.Equal(t, "podinfo-primary", daePrimaryLabels["name"]) - daePrimaryMatchLabels := daePrimary.Spec.Selector.MatchLabels - daeSourceMatchLabels := dae2.Spec.Selector.MatchLabels - assert.Equal(t, len(daeSourceMatchLabels), len(daePrimaryMatchLabels)) - assert.Equal(t, daeSourceMatchLabels["app.kubernetes.io/test-label-1"], daePrimaryMatchLabels["app.kubernetes.io/test-label-1"]) - assert.Equal(t, "podinfo-primary", daePrimaryMatchLabels["name"]) - daePrimaryAnnotations := daePrimary.ObjectMeta.Annotations daeSourceAnnotations := dae2.ObjectMeta.Annotations assert.Equal(t, daeSourceAnnotations["app.kubernetes.io/test-annotation-1"], daePrimaryAnnotations["app.kubernetes.io/test-annotation-1"]) diff --git a/pkg/canary/daemonset_fixture_test.go b/pkg/canary/daemonset_fixture_test.go index 76cb986f8..3fb4059f4 100644 --- a/pkg/canary/daemonset_fixture_test.go +++ b/pkg/canary/daemonset_fixture_test.go @@ -637,28 +637,17 @@ func newDaemonSetControllerTestPodInfoV2() *appsv1.DaemonSet { ObjectMeta: metav1.ObjectMeta{ Namespace: "default", Name: "podinfo", - Annotations: map[string]string{ - "test-annotation-1": "test-annotation-value-1", - }, - Labels: map[string]string{ - "test-label-1": "test-label-value-1", - }, }, Spec: appsv1.DaemonSetSpec{ Selector: &metav1.LabelSelector{ MatchLabels: map[string]string{ - "name": "podinfo", - "test-label-1": "test-label-value-1", + "name": "podinfo", }, }, Template: corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - "name": "podinfo", - "test-label-1": "test-label-value-1", - }, - Annotations: map[string]string{ - "test-annotation-1": "test-annotation-value-1", + "name": "podinfo", }, }, Spec: corev1.PodSpec{ diff --git a/pkg/canary/deployment_controller.go b/pkg/canary/deployment_controller.go index 04d064fe5..6b243e2aa 100644 --- a/pkg/canary/deployment_controller.go +++ b/pkg/canary/deployment_controller.go @@ -19,7 +19,6 @@ package canary import ( "context" "fmt" - "maps" "go.uber.org/zap" appsv1 "k8s.io/api/apps/v1" @@ -83,37 +82,17 @@ func (c *DeploymentController) Promote(cd *flaggerv1.Canary) error { return fmt.Errorf("deployment %s.%s get query error: %w", targetName, cd.Namespace, err) } - matchLabels, label, labelValue, err := c.getSelectorLabel(canary) + _, label, labelValue, err := c.getSelectorLabel(canary) primaryLabelValue := fmt.Sprintf("%s-primary", labelValue) if err != nil { return fmt.Errorf("getSelectorLabel failed: %w", err) } - matchLabels[label] = primaryLabelValue - primary, err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Get(context.TODO(), primaryName, metav1.GetOptions{}) if err != nil { return fmt.Errorf("deployment %s.%s get query error: %w", primaryName, cd.Namespace, err) } - if !maps.Equal(primary.Spec.Selector.MatchLabels, matchLabels) { - c.logger.With("canary", fmt.Sprintf(cd.Name, cd.Namespace)). - Infof( - "Primary deployment %s.%s matchLabels does not match canary deployment %s.%s, recreating...", - primaryName, - cd.Namespace, - targetName, - cd.Namespace, - ) - c.recreatePrimaryDeployment(cd, canary, primaryName, c.includeLabelPrefix) - } - - // Get primary deployment again, in case it has changes - primary, err = c.kubeClient.AppsV1().Deployments(cd.Namespace).Get(context.TODO(), primaryName, metav1.GetOptions{}) - if err != nil { - return fmt.Errorf("deployment %s.%s get query error: %w", primaryName, cd.Namespace, err) - } - // promote secrets and config maps configRefs, err := c.configTracker.GetTargetConfigs(cd) if err != nil { @@ -346,20 +325,6 @@ func (c *DeploymentController) createPrimaryDeployment(cd *flaggerv1.Canary, can return nil } -func (c *DeploymentController) recreatePrimaryDeployment(cd *flaggerv1.Canary, canary *appsv1.Deployment, name string, includeLabelPrefix []string) error { - // TODO: Make sure primary deployment is not receiving any traffic - - if err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Delete(context.TODO(), name, metav1.DeleteOptions{}); err != nil { - return fmt.Errorf("deployment %s.%s delete error: %w", name, cd.Namespace, err) - } - - if err := c.createPrimaryDeployment(cd, canary, name, includeLabelPrefix); err != nil { - return fmt.Errorf("createPrimaryDeployment failed: %w", err) - } - - return nil -} - // getSelectorLabel returns the selector match label func (c *DeploymentController) getSelectorLabel(deployment *appsv1.Deployment) (map[string]string, string, string, error) { label, labelValue := "", "" diff --git a/pkg/canary/deployment_controller_test.go b/pkg/canary/deployment_controller_test.go index 81e49f24f..24aa0c65a 100644 --- a/pkg/canary/deployment_controller_test.go +++ b/pkg/canary/deployment_controller_test.go @@ -75,9 +75,7 @@ func TestDeploymentController_Promote(t *testing.T) { mocks.initializeCanary(t) dep2 := newDeploymentControllerTestV2() - err := mocks.kubeClient.AppsV1().Deployments("default").Delete(context.TODO(), dep2.ObjectMeta.Name, metav1.DeleteOptions{}) - require.NoError(t, err) - _, err = mocks.kubeClient.AppsV1().Deployments("default").Create(context.TODO(), dep2, metav1.CreateOptions{}) + _, err := mocks.kubeClient.AppsV1().Deployments("default").Update(context.TODO(), dep2, metav1.UpdateOptions{}) require.NoError(t, err) config2 := newDeploymentControllerTestConfigMapV2() @@ -103,12 +101,6 @@ func TestDeploymentController_Promote(t *testing.T) { depSourceAnnotations := dep2.ObjectMeta.Annotations assert.Equal(t, depSourceAnnotations["app.kubernetes.io/test-annotation-1"], depPrimaryAnnotations["app.kubernetes.io/test-annotation-1"]) - depPrimaryMatchLabels := depPrimary.Spec.Selector.MatchLabels - depSourceMatchLabels := dep2.Spec.Selector.MatchLabels - assert.Equal(t, len(depSourceMatchLabels), len(depPrimaryMatchLabels)) - assert.Equal(t, depSourceMatchLabels["app.kubernetes.io/test-label-1"], depPrimaryMatchLabels["app.kubernetes.io/test-label-1"]) - assert.Equal(t, "podinfo-primary", depPrimaryMatchLabels["name"]) - configPrimary, err := mocks.kubeClient.CoreV1().ConfigMaps("default").Get(context.TODO(), "podinfo-config-env-primary", metav1.GetOptions{}) require.NoError(t, err) assert.Equal(t, config2.Data["color"], configPrimary.Data["color"]) diff --git a/pkg/canary/deployment_fixture_test.go b/pkg/canary/deployment_fixture_test.go index 2aab6eb44..0ebf07eea 100644 --- a/pkg/canary/deployment_fixture_test.go +++ b/pkg/canary/deployment_fixture_test.go @@ -776,7 +776,6 @@ func newDeploymentControllerTestV2() *appsv1.Deployment { Selector: &metav1.LabelSelector{ MatchLabels: map[string]string{ "name": "podinfo", - "test-label-1": "test-label-value-1", }, }, Template: corev1.PodTemplateSpec{ From cfcc5d0186e55d39363a6f69edcb45c4ceaaf142 Mon Sep 17 00:00:00 2001 From: Kevin Snyder Date: Fri, 3 Oct 2025 12:34:27 -0700 Subject: [PATCH 08/10] Add tests for `matchLabels` behavior Tests the `matchLabels` behavior for `Deployment`, `DaemonSet`, and `Service` Signed-off-by: Kevin Snyder --- pkg/canary/daemonset_controller_test.go | 15 +++++++++++++++ pkg/canary/daemonset_fixture_test.go | 3 ++- pkg/canary/deployment_controller_test.go | 14 ++++++++++++++ pkg/canary/deployment_fixture_test.go | 3 ++- pkg/router/kubernetes_default_test.go | 13 +++++++++++++ 5 files changed, 46 insertions(+), 2 deletions(-) diff --git a/pkg/canary/daemonset_controller_test.go b/pkg/canary/daemonset_controller_test.go index f3f73eb97..915e53877 100644 --- a/pkg/canary/daemonset_controller_test.go +++ b/pkg/canary/daemonset_controller_test.go @@ -72,6 +72,21 @@ func TestDaemonSetController_Sync_InconsistentNaming(t *testing.T) { assert.Equal(t, primarySelectorValue, fmt.Sprintf("%s-primary", sourceSelectorValue)) } +func TestDaemonSetController_Initialize(t *testing.T) { + dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"} + mocks := newDaemonSetFixture(dc) + _, err := mocks.controller.Initialize(mocks.canary) + require.NoError(t, err) + + daePrimary, err := mocks.kubeClient.AppsV1().DaemonSets("default").Get(context.TODO(), "podinfo-primary", metav1.GetOptions{}) + require.NoError(t, err) + + daePrimaryMatchLabels := daePrimary.Spec.Selector.MatchLabels + assert.Equal(t, 2, len(daePrimaryMatchLabels)) + assert.Equal(t, "podinfo-primary", daePrimaryMatchLabels["name"]) + assert.Equal(t, "test-label-value-1", daePrimaryMatchLabels["test-label-1"]) +} + func TestDaemonSetController_Promote(t *testing.T) { dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"} mocks := newDaemonSetFixture(dc) diff --git a/pkg/canary/daemonset_fixture_test.go b/pkg/canary/daemonset_fixture_test.go index 3fb4059f4..f0651f09d 100644 --- a/pkg/canary/daemonset_fixture_test.go +++ b/pkg/canary/daemonset_fixture_test.go @@ -379,7 +379,8 @@ func newDaemonSetControllerTestPodInfo(dc daemonsetConfigs) *appsv1.DaemonSet { Spec: appsv1.DaemonSetSpec{ Selector: &metav1.LabelSelector{ MatchLabels: map[string]string{ - dc.label: dc.labelValue, + dc.label: dc.labelValue, + "test-label-1": "test-label-value-1", }, }, Template: corev1.PodTemplateSpec{ diff --git a/pkg/canary/deployment_controller_test.go b/pkg/canary/deployment_controller_test.go index 24aa0c65a..c2b052706 100644 --- a/pkg/canary/deployment_controller_test.go +++ b/pkg/canary/deployment_controller_test.go @@ -69,6 +69,20 @@ func TestDeploymentController_Sync_InconsistentNaming(t *testing.T) { assert.Equal(t, primarySelectorValue, fmt.Sprintf("%s-primary", dc.labelValue)) } +func TestDeploymentController_Initialize(t *testing.T) { + dc := deploymentConfigs{name: "podinfo", label: "name", labelValue: "podinfo"} + mocks := newDeploymentFixture(dc) + mocks.initializeCanary(t) + + depPrimary, err := mocks.kubeClient.AppsV1().Deployments("default").Get(context.TODO(), "podinfo-primary", metav1.GetOptions{}) + require.NoError(t, err) + + depPrimaryMatchLabels := depPrimary.Spec.Selector.MatchLabels + assert.Equal(t, 2, len(depPrimaryMatchLabels)) + assert.Equal(t, "podinfo-primary", depPrimaryMatchLabels["name"]) + assert.Equal(t, "test-label-value-1", depPrimaryMatchLabels["test-label-1"]) +} + func TestDeploymentController_Promote(t *testing.T) { dc := deploymentConfigs{name: "podinfo", label: "name", labelValue: "podinfo"} mocks := newDeploymentFixture(dc) diff --git a/pkg/canary/deployment_fixture_test.go b/pkg/canary/deployment_fixture_test.go index 0ebf07eea..34d503c71 100644 --- a/pkg/canary/deployment_fixture_test.go +++ b/pkg/canary/deployment_fixture_test.go @@ -430,7 +430,8 @@ func newDeploymentControllerTest(dc deploymentConfigs) *appsv1.Deployment { Spec: appsv1.DeploymentSpec{ Selector: &metav1.LabelSelector{ MatchLabels: map[string]string{ - dc.label: dc.labelValue, + dc.label: dc.labelValue, + "test-label-1": "test-label-value-1", }, }, Template: corev1.PodTemplateSpec{ diff --git a/pkg/router/kubernetes_default_test.go b/pkg/router/kubernetes_default_test.go index b428c4a99..04c97c307 100644 --- a/pkg/router/kubernetes_default_test.go +++ b/pkg/router/kubernetes_default_test.go @@ -40,6 +40,12 @@ func TestServiceRouter_Create(t *testing.T) { kubeClient: mocks.kubeClient, flaggerClient: mocks.flaggerClient, logger: mocks.logger, + matchLabels: map[string]string{ + "name": "podinfo", + "test-label-1": "test-label-value-1", + }, + labelSelector: "name", + labelValue: "podinfo", } appProtocol := "http" @@ -52,6 +58,9 @@ func TestServiceRouter_Create(t *testing.T) { canarySvc, err := mocks.kubeClient.CoreV1().Services("default").Get(context.TODO(), "podinfo-canary", metav1.GetOptions{}) require.NoError(t, err) + assert.Equal(t, 2, len(canarySvc.Spec.Selector)) + assert.Equal(t, "podinfo", canarySvc.Spec.Selector["name"]) + assert.Equal(t, "test-label-value-1", canarySvc.Spec.Selector["test-label-1"]) assert.Equal(t, &appProtocol, canarySvc.Spec.Ports[0].AppProtocol) assert.Equal(t, "http", canarySvc.Spec.Ports[0].Name) assert.Equal(t, int32(9898), canarySvc.Spec.Ports[0].Port) @@ -59,6 +68,10 @@ func TestServiceRouter_Create(t *testing.T) { primarySvc, err := mocks.kubeClient.CoreV1().Services("default").Get(context.TODO(), "podinfo-primary", metav1.GetOptions{}) require.NoError(t, err) + + assert.Equal(t, 2, len(primarySvc.Spec.Selector)) + assert.Equal(t, "podinfo-primary", primarySvc.Spec.Selector["name"]) + assert.Equal(t, "test-label-value-1", primarySvc.Spec.Selector["test-label-1"]) assert.Equal(t, "http", primarySvc.Spec.Ports[0].Name) assert.Equal(t, int32(9898), primarySvc.Spec.Ports[0].Port) assert.Equal(t, "None", primarySvc.Spec.ClusterIP) From 7c873344de44085abc2a7eb0e16cf707f9a7ebb5 Mon Sep 17 00:00:00 2001 From: Kevin Snyder Date: Fri, 3 Oct 2025 12:37:39 -0700 Subject: [PATCH 09/10] Revert changes to `createPrimaryDeployment` Signed-off-by: Kevin Snyder --- pkg/canary/deployment_controller.go | 131 ++++++++++++++-------------- 1 file changed, 66 insertions(+), 65 deletions(-) diff --git a/pkg/canary/deployment_controller.go b/pkg/canary/deployment_controller.go index 6b243e2aa..b5975505d 100644 --- a/pkg/canary/deployment_controller.go +++ b/pkg/canary/deployment_controller.go @@ -46,18 +46,8 @@ type DeploymentController struct { // Initialize creates the primary deployment if it does not exist. func (c *DeploymentController) Initialize(cd *flaggerv1.Canary) (bool, error) { - targetName := cd.Spec.TargetRef.Name - primaryName := fmt.Sprintf("%s-primary", cd.Spec.TargetRef.Name) - - canaryDep, err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Get(context.TODO(), targetName, metav1.GetOptions{}) - if err != nil { - return true, fmt.Errorf("deployment %s.%s get query error: %w", targetName, cd.Namespace, err) - } - - if _, err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Get(context.TODO(), primaryName, metav1.GetOptions{}); errors.IsNotFound(err) { - if err := c.createPrimaryDeployment(cd, canaryDep, primaryName, c.includeLabelPrefix); err != nil { - return true, fmt.Errorf("createPrimaryDeployment failed: %w", err) - } + if err := c.createPrimaryDeployment(cd, c.includeLabelPrefix); err != nil { + return true, fmt.Errorf("createPrimaryDeployment failed: %w", err) } if cd.Status.Phase == "" || cd.Status.Phase == flaggerv1.CanaryPhaseInitializing { @@ -249,23 +239,13 @@ func (c *DeploymentController) GetMetadata(cd *flaggerv1.Canary) (map[string]str return matchLabels, label, labelValue, ports, nil } -func (c *DeploymentController) createPrimaryDeployment(cd *flaggerv1.Canary, canaryDep *appsv1.Deployment, name string, includeLabelPrefix []string) error { - // create primary secrets and config maps - configRefs, err := c.configTracker.GetTargetConfigs(cd) - if err != nil { - return fmt.Errorf("GetTargetConfigs failed: %w", err) - } - if err := c.configTracker.CreatePrimaryConfigs(cd, configRefs, c.includeLabelPrefix); err != nil { - return fmt.Errorf("CreatePrimaryConfigs failed: %w", err) - } - annotations, err := makeAnnotations(canaryDep.Spec.Template.Annotations) - if err != nil { - return fmt.Errorf("makeAnnotations failed: %w", err) - } +func (c *DeploymentController) createPrimaryDeployment(cd *flaggerv1.Canary, includeLabelPrefix []string) error { + targetName := cd.Spec.TargetRef.Name + primaryName := fmt.Sprintf("%s-primary", cd.Spec.TargetRef.Name) - replicas := int32(1) - if canaryDep.Spec.Replicas != nil && *canaryDep.Spec.Replicas > 0 { - replicas = *canaryDep.Spec.Replicas + canaryDep, err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Get(context.TODO(), targetName, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("deployment %s.%s get query error: %w", targetName, cd.Namespace, err) } // Create the labels map but filter unwanted labels @@ -279,48 +259,69 @@ func (c *DeploymentController) createPrimaryDeployment(cd *flaggerv1.Canary, can matchLabels[label] = primaryLabelValue - // create primary deployment - primaryDep := &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: cd.Namespace, - Labels: makePrimaryLabels(labels, primaryLabelValue, label), - Annotations: filterMetadata(canaryDep.Annotations), - OwnerReferences: []metav1.OwnerReference{ - *metav1.NewControllerRef(cd, schema.GroupVersionKind{ - Group: flaggerv1.SchemeGroupVersion.Group, - Version: flaggerv1.SchemeGroupVersion.Version, - Kind: flaggerv1.CanaryKind, - }), - }, - }, - Spec: appsv1.DeploymentSpec{ - ProgressDeadlineSeconds: canaryDep.Spec.ProgressDeadlineSeconds, - MinReadySeconds: canaryDep.Spec.MinReadySeconds, - RevisionHistoryLimit: canaryDep.Spec.RevisionHistoryLimit, - Replicas: int32p(replicas), - Strategy: canaryDep.Spec.Strategy, - Selector: &metav1.LabelSelector{ - MatchLabels: matchLabels, + primaryDep, err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Get(context.TODO(), primaryName, metav1.GetOptions{}) + if errors.IsNotFound(err) { + // create primary secrets and config maps + configRefs, err := c.configTracker.GetTargetConfigs(cd) + if err != nil { + return fmt.Errorf("GetTargetConfigs failed: %w", err) + } + if err := c.configTracker.CreatePrimaryConfigs(cd, configRefs, c.includeLabelPrefix); err != nil { + return fmt.Errorf("CreatePrimaryConfigs failed: %w", err) + } + annotations, err := makeAnnotations(canaryDep.Spec.Template.Annotations) + if err != nil { + return fmt.Errorf("makeAnnotations failed: %w", err) + } + + replicas := int32(1) + if canaryDep.Spec.Replicas != nil && *canaryDep.Spec.Replicas > 0 { + replicas = *canaryDep.Spec.Replicas + } + + // create primary deployment + primaryDep = &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: primaryName, + Namespace: cd.Namespace, + Labels: makePrimaryLabels(labels, primaryLabelValue, label), + Annotations: filterMetadata(canaryDep.Annotations), + OwnerReferences: []metav1.OwnerReference{ + *metav1.NewControllerRef(cd, schema.GroupVersionKind{ + Group: flaggerv1.SchemeGroupVersion.Group, + Version: flaggerv1.SchemeGroupVersion.Version, + Kind: flaggerv1.CanaryKind, + }), + }, }, - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: makePrimaryLabels(canaryDep.Spec.Template.Labels, primaryLabelValue, label), - Annotations: annotations, + Spec: appsv1.DeploymentSpec{ + ProgressDeadlineSeconds: canaryDep.Spec.ProgressDeadlineSeconds, + MinReadySeconds: canaryDep.Spec.MinReadySeconds, + RevisionHistoryLimit: canaryDep.Spec.RevisionHistoryLimit, + Replicas: int32p(replicas), + Strategy: canaryDep.Spec.Strategy, + Selector: &metav1.LabelSelector{ + MatchLabels: matchLabels, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: makePrimaryLabels(canaryDep.Spec.Template.Labels, primaryLabelValue, label), + Annotations: annotations, + }, + // update spec with the primary secrets and config maps + Spec: c.getPrimaryDeploymentTemplateSpec(canaryDep, configRefs), }, - // update spec with the primary secrets and config maps - Spec: c.getPrimaryDeploymentTemplateSpec(canaryDep, configRefs), }, - }, - } + } - _, err = c.kubeClient.AppsV1().Deployments(cd.Namespace).Create(context.TODO(), primaryDep, metav1.CreateOptions{}) - if err != nil { - return fmt.Errorf("creating deployment %s.%s failed: %w", primaryDep.Name, cd.Namespace, err) - } + _, err = c.kubeClient.AppsV1().Deployments(cd.Namespace).Create(context.TODO(), primaryDep, metav1.CreateOptions{}) + if err != nil { + return fmt.Errorf("creating deployment %s.%s failed: %w", primaryDep.Name, cd.Namespace, err) + } - c.logger.With("canary", fmt.Sprintf("%s.%s", cd.Name, cd.Namespace)). - Infof("Deployment %s.%s created", primaryDep.GetName(), cd.Namespace) + c.logger.With("canary", fmt.Sprintf("%s.%s", cd.Name, cd.Namespace)). + Infof("Deployment %s.%s created", primaryDep.GetName(), cd.Namespace) + } return nil } From 24ffc5b8728881bf470a0182cd591bac970efdb1 Mon Sep 17 00:00:00 2001 From: Kevin Snyder Date: Mon, 6 Oct 2025 12:26:51 -0700 Subject: [PATCH 10/10] Fix not matching first selector label Signed-off-by: Kevin Snyder --- pkg/canary/daemonset_controller.go | 1 + pkg/canary/daemonset_controller_test.go | 23 +++++++++++++++++++++++ pkg/canary/deployment_controller.go | 1 + pkg/canary/deployment_controller_test.go | 22 ++++++++++++++++++++++ 4 files changed, 47 insertions(+) diff --git a/pkg/canary/daemonset_controller.go b/pkg/canary/daemonset_controller.go index 3769ecd3c..89a011118 100644 --- a/pkg/canary/daemonset_controller.go +++ b/pkg/canary/daemonset_controller.go @@ -316,6 +316,7 @@ func (c *DaemonSetController) getSelectorLabel(daemonSet *appsv1.DaemonSet) (map for _, l := range c.labels { if _, ok := daemonSet.Spec.Selector.MatchLabels[l]; ok { label, labelValue = l, daemonSet.Spec.Selector.MatchLabels[l] + break } } diff --git a/pkg/canary/daemonset_controller_test.go b/pkg/canary/daemonset_controller_test.go index 915e53877..4ee5e74c2 100644 --- a/pkg/canary/daemonset_controller_test.go +++ b/pkg/canary/daemonset_controller_test.go @@ -72,6 +72,29 @@ func TestDaemonSetController_Sync_InconsistentNaming(t *testing.T) { assert.Equal(t, primarySelectorValue, fmt.Sprintf("%s-primary", sourceSelectorValue)) } +func TestDaemonSetController_GetMetadata(t *testing.T) { + dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"} + mocks := newDaemonSetFixture(dc) + _, err := mocks.controller.Initialize(mocks.canary) + require.NoError(t, err) + + matchLabels, label, labelValue, _, err := mocks.controller.GetMetadata(mocks.canary) + require.NoError(t, err) + + assert.Equal(t, map[string]string{"name": "podinfo", "test-label-1": "test-label-value-1"}, matchLabels) + assert.Equal(t, "name", label) + assert.Equal(t, "podinfo", labelValue) + + mocks.controller.labels = []string{"app", "name", "test-label-1"} + + matchLabels, label, labelValue, _, err = mocks.controller.GetMetadata(mocks.canary) + require.NoError(t, err) + + assert.Equal(t, map[string]string{"name": "podinfo", "test-label-1": "test-label-value-1"}, matchLabels) + assert.Equal(t, "name", label) + assert.Equal(t, "podinfo", labelValue) +} + func TestDaemonSetController_Initialize(t *testing.T) { dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"} mocks := newDaemonSetFixture(dc) diff --git a/pkg/canary/deployment_controller.go b/pkg/canary/deployment_controller.go index b5975505d..f92147963 100644 --- a/pkg/canary/deployment_controller.go +++ b/pkg/canary/deployment_controller.go @@ -332,6 +332,7 @@ func (c *DeploymentController) getSelectorLabel(deployment *appsv1.Deployment) ( for _, l := range c.labels { if _, ok := deployment.Spec.Selector.MatchLabels[l]; ok { label, labelValue = l, deployment.Spec.Selector.MatchLabels[l] + break } } diff --git a/pkg/canary/deployment_controller_test.go b/pkg/canary/deployment_controller_test.go index c2b052706..b6e687844 100644 --- a/pkg/canary/deployment_controller_test.go +++ b/pkg/canary/deployment_controller_test.go @@ -69,6 +69,28 @@ func TestDeploymentController_Sync_InconsistentNaming(t *testing.T) { assert.Equal(t, primarySelectorValue, fmt.Sprintf("%s-primary", dc.labelValue)) } +func TestDeploymentController_GetMetadata(t *testing.T) { + dc := deploymentConfigs{name: "podinfo", label: "name", labelValue: "podinfo"} + mocks := newDeploymentFixture(dc) + mocks.initializeCanary(t) + + matchLabels, label, labelValue, _, err := mocks.controller.GetMetadata(mocks.canary) + require.NoError(t, err) + + assert.Equal(t, map[string]string{"name": "podinfo", "test-label-1": "test-label-value-1"}, matchLabels) + assert.Equal(t, "name", label) + assert.Equal(t, "podinfo", labelValue) + + mocks.controller.labels = []string{"app", "name", "test-label-1"} + + matchLabels, label, labelValue, _, err = mocks.controller.GetMetadata(mocks.canary) + require.NoError(t, err) + + assert.Equal(t, map[string]string{"name": "podinfo", "test-label-1": "test-label-value-1"}, matchLabels) + assert.Equal(t, "name", label) + assert.Equal(t, "podinfo", labelValue) +} + func TestDeploymentController_Initialize(t *testing.T) { dc := deploymentConfigs{name: "podinfo", label: "name", labelValue: "podinfo"} mocks := newDeploymentFixture(dc)