Rework selector labels

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

pkg/canary/daemonset_controller.go

@@ -120,7 +120,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)
+		_, label, labelValue, err := c.getSelectorLabel(canary)
 		primaryLabelValue := fmt.Sprintf("%s-primary", labelValue)
 		if err != nil {
 			return fmt.Errorf("getSelectorLabel failed: %w", err)
@@ -206,24 +206,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 +244,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 +265,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 +287,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{
@@ -311,17 +311,28 @@ func (c *DaemonSetController) createPrimaryDaemonSet(cd *flaggerv1.Canary, inclu
 }
 
 // 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]
+			break
 		}
 	}
 
-	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) {

pkg/canary/daemonset_controller_test.go

@@ -72,6 +72,44 @@ 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)
+	_, 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)

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{

pkg/canary/deployment_controller.go

@@ -72,7 +72,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)
+		_, label, labelValue, err := c.getSelectorLabel(canary)
 		primaryLabelValue := fmt.Sprintf("%s-primary", labelValue)
 		if err != nil {
 			return fmt.Errorf("getSelectorLabel failed: %w", err)
@@ -219,25 +219,25 @@ 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
 	if cd.Spec.Service.PortDiscovery {
 		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,12 +251,14 @@ 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)
 	}
 
+	matchLabels[label] = primaryLabelValue
+
 	primaryDep, err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Get(context.TODO(), primaryName, metav1.GetOptions{})
 	if errors.IsNotFound(err) {
 		// create primary secrets and config maps
@@ -299,9 +301,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{
@@ -327,17 +327,28 @@ func (c *DeploymentController) createPrimaryDeployment(cd *flaggerv1.Canary, inc
 }
 
 // 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]
+			break
 		}
 	}
 
-	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) {

pkg/canary/deployment_controller_test.go

@@ -69,6 +69,42 @@ 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)
+	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)

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{

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

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

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)
 	}

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 {

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,