From 8b4aa861e4d80e0bdfa8c881a05f89dbd45e002c Mon Sep 17 00:00:00 2001
From: "liheng.zms" <liheng.zms@alibaba-inc.com>
Date: Tue, 4 Jul 2023 15:36:14 +0800
Subject: [PATCH] rollout support patchPodTemplateMetadata

Signed-off-by: liheng.zms <liheng.zms@alibaba-inc.com>
---
 api/v1alpha1/batchrelease_plan_types.go       |  4 ++
 api/v1alpha1/rollout_types.go                 | 11 ++++++
 api/v1alpha1/zz_generated.deepcopy.go         | 39 +++++++++++++++++++
 .../rollouts.kruise.io_batchreleases.yaml     | 16 ++++++++
 .../bases/rollouts.kruise.io_rollouts.yaml    | 16 ++++++++
 .../trafficrouting_ingress/mse.lua            |  5 +++
 .../control/canarystyle/deployment/canary.go  | 29 ++++++++++++--
 .../control/canarystyle/deployment/control.go |  2 +-
 .../canarystyle/deployment/control_test.go    |  2 +-
 pkg/controller/rollout/rollout_canary.go      | 10 ++---
 pkg/trafficrouting/manager.go                 | 16 +++++---
 .../network/ingress/ingress_test.go           |  6 +++
 pkg/util/controller_finder.go                 |  3 --
 pkg/util/workloads_utils.go                   | 19 +++++++++
 test/e2e/rollout_test.go                      | 12 ++++++
 15 files changed, 170 insertions(+), 20 deletions(-)

diff --git a/api/v1alpha1/batchrelease_plan_types.go b/api/v1alpha1/batchrelease_plan_types.go
index fd33c903..0cf37e8c 100644
--- a/api/v1alpha1/batchrelease_plan_types.go
+++ b/api/v1alpha1/batchrelease_plan_types.go
@@ -50,6 +50,10 @@ type ReleasePlan struct {
 	// FinalizingPolicy define the behavior of controller when phase enter Finalizing
 	// Defaults to "Immediate"
 	FinalizingPolicy FinalizingPolicyType `json:"finalizingPolicy,omitempty"`
+	// PatchPodTemplateMetadata indicates patch configuration(e.g. labels, annotations) to the canary deployment podTemplateSpec.metadata
+	// only support for canary deployment
+	// +optional
+	PatchPodTemplateMetadata *PatchPodTemplateMetadata `json:"patchPodTemplateMetadata,omitempty"`
 }
 
 type FinalizingPolicyType string
diff --git a/api/v1alpha1/rollout_types.go b/api/v1alpha1/rollout_types.go
index 9136e20f..1a8bea18 100644
--- a/api/v1alpha1/rollout_types.go
+++ b/api/v1alpha1/rollout_types.go
@@ -117,6 +117,17 @@ type CanaryStrategy struct {
 	// FailureThreshold.
 	// Defaults to nil.
 	FailureThreshold *intstr.IntOrString `json:"failureThreshold,omitempty"`
+	// PatchPodTemplateMetadata indicates patch configuration(e.g. labels, annotations) to the canary deployment podTemplateSpec.metadata
+	// only support for canary deployment
+	// +optional
+	PatchPodTemplateMetadata *PatchPodTemplateMetadata `json:"patchPodTemplateMetadata,omitempty"`
+}
+
+type PatchPodTemplateMetadata struct {
+	// annotations
+	Annotations map[string]string `json:"annotations,omitempty"`
+	// labels
+	Labels map[string]string `json:"labels,omitempty"`
 }
 
 // CanaryStep defines a step of a canary workload.
diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
index 6940c32b..29d6019a 100644
--- a/api/v1alpha1/zz_generated.deepcopy.go
+++ b/api/v1alpha1/zz_generated.deepcopy.go
@@ -239,6 +239,11 @@ func (in *CanaryStrategy) DeepCopyInto(out *CanaryStrategy) {
 		*out = new(intstr.IntOrString)
 		**out = **in
 	}
+	if in.PatchPodTemplateMetadata != nil {
+		in, out := &in.PatchPodTemplateMetadata, &out.PatchPodTemplateMetadata
+		*out = new(PatchPodTemplateMetadata)
+		(*in).DeepCopyInto(*out)
+	}
 }
 
 // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CanaryStrategy.
@@ -412,6 +417,35 @@ func (in *ObjectRef) DeepCopy() *ObjectRef {
 	return out
 }
 
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *PatchPodTemplateMetadata) DeepCopyInto(out *PatchPodTemplateMetadata) {
+	*out = *in
+	if in.Annotations != nil {
+		in, out := &in.Annotations, &out.Annotations
+		*out = make(map[string]string, len(*in))
+		for key, val := range *in {
+			(*out)[key] = val
+		}
+	}
+	if in.Labels != nil {
+		in, out := &in.Labels, &out.Labels
+		*out = make(map[string]string, len(*in))
+		for key, val := range *in {
+			(*out)[key] = val
+		}
+	}
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PatchPodTemplateMetadata.
+func (in *PatchPodTemplateMetadata) DeepCopy() *PatchPodTemplateMetadata {
+	if in == nil {
+		return nil
+	}
+	out := new(PatchPodTemplateMetadata)
+	in.DeepCopyInto(out)
+	return out
+}
+
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *Pod) DeepCopyInto(out *Pod) {
 	*out = *in
@@ -461,6 +495,11 @@ func (in *ReleasePlan) DeepCopyInto(out *ReleasePlan) {
 		*out = new(intstr.IntOrString)
 		**out = **in
 	}
+	if in.PatchPodTemplateMetadata != nil {
+		in, out := &in.PatchPodTemplateMetadata, &out.PatchPodTemplateMetadata
+		*out = new(PatchPodTemplateMetadata)
+		(*in).DeepCopyInto(*out)
+	}
 }
 
 // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ReleasePlan.
diff --git a/config/crd/bases/rollouts.kruise.io_batchreleases.yaml b/config/crd/bases/rollouts.kruise.io_batchreleases.yaml
index 61752ad3..421b07f4 100644
--- a/config/crd/bases/rollouts.kruise.io_batchreleases.yaml
+++ b/config/crd/bases/rollouts.kruise.io_batchreleases.yaml
@@ -102,6 +102,22 @@ spec:
                     description: FinalizingPolicy define the behavior of controller
                       when phase enter Finalizing Defaults to "Immediate"
                     type: string
+                  patchPodTemplateMetadata:
+                    description: PatchPodTemplateMetadata indicates patch configuration(e.g.
+                      labels, annotations) to the canary deployment podTemplateSpec.metadata
+                      only support for canary deployment
+                    properties:
+                      annotations:
+                        additionalProperties:
+                          type: string
+                        description: annotations
+                        type: object
+                      labels:
+                        additionalProperties:
+                          type: string
+                        description: labels
+                        type: object
+                    type: object
                   rolloutID:
                     description: RolloutID indicates an id for each rollout progress
                     type: string
diff --git a/config/crd/bases/rollouts.kruise.io_rollouts.yaml b/config/crd/bases/rollouts.kruise.io_rollouts.yaml
index 93e20a0a..5e189d67 100644
--- a/config/crd/bases/rollouts.kruise.io_rollouts.yaml
+++ b/config/crd/bases/rollouts.kruise.io_rollouts.yaml
@@ -109,6 +109,22 @@ spec:
                           is nil, Rollout will use the MaxUnavailable of workload
                           as its FailureThreshold. Defaults to nil.
                         x-kubernetes-int-or-string: true
+                      patchPodTemplateMetadata:
+                        description: PatchPodTemplateMetadata indicates patch configuration(e.g.
+                          labels, annotations) to the canary deployment podTemplateSpec.metadata
+                          only support for canary deployment
+                        properties:
+                          annotations:
+                            additionalProperties:
+                              type: string
+                            description: annotations
+                            type: object
+                          labels:
+                            additionalProperties:
+                              type: string
+                            description: labels
+                            type: object
+                        type: object
                       steps:
                         description: Steps define the order of phases to execute release
                           in batches(20%, 40%, 60%, 80%, 100%)
diff --git a/lua_configuration/trafficrouting_ingress/mse.lua b/lua_configuration/trafficrouting_ingress/mse.lua
index ecaf3a00..46b0a010 100644
--- a/lua_configuration/trafficrouting_ingress/mse.lua
+++ b/lua_configuration/trafficrouting_ingress/mse.lua
@@ -15,6 +15,11 @@ if ( obj.weight ~= "-1" )
 then
     annotations["nginx.ingress.kubernetes.io/canary-weight"] = obj.weight
 end
+if ( annotations["mse.ingress.kubernetes.io/service-subset"] )
+then
+    annotations["mse.ingress.kubernetes.io/service-subset"] = "gray"
+end
+
 if ( obj.requestHeaderModifier )
 then
     local str = ''
diff --git a/pkg/controller/batchrelease/control/canarystyle/deployment/canary.go b/pkg/controller/batchrelease/control/canarystyle/deployment/canary.go
index 9e3f7884..811db7d5 100644
--- a/pkg/controller/batchrelease/control/canarystyle/deployment/canary.go
+++ b/pkg/controller/batchrelease/control/canarystyle/deployment/canary.go
@@ -132,7 +132,19 @@ func (r *realCanaryController) create(release *v1alpha1.BatchRelease, template *
 
 	// spec
 	canary.Spec = *template.Spec.DeepCopy()
-	// todo, patch canary pod metadata
+	// patch canary pod metadata
+	if release.Spec.ReleasePlan.PatchPodTemplateMetadata != nil {
+		patch := release.Spec.ReleasePlan.PatchPodTemplateMetadata
+		for k, v := range patch.Labels {
+			canary.Spec.Template.Labels[k] = v
+		}
+		if canary.Spec.Template.Annotations == nil {
+			canary.Spec.Template.Annotations = map[string]string{}
+		}
+		for k, v := range patch.Annotations {
+			canary.Spec.Template.Annotations[k] = v
+		}
+	}
 	canary.Spec.Replicas = pointer.Int32Ptr(0)
 	canary.Spec.Paused = false
 
@@ -169,7 +181,7 @@ func (r *realCanaryController) listDeployment(release *v1alpha1.BatchRelease, op
 }
 
 // return the latest deployment with the newer creation time
-func filterCanaryDeployment(ds []*apps.Deployment, template *corev1.PodTemplateSpec) *apps.Deployment {
+func filterCanaryDeployment(release *v1alpha1.BatchRelease, ds []*apps.Deployment, template *corev1.PodTemplateSpec) *apps.Deployment {
 	if len(ds) == 0 {
 		return nil
 	}
@@ -179,9 +191,18 @@ func filterCanaryDeployment(ds []*apps.Deployment, template *corev1.PodTemplateS
 	if template == nil {
 		return ds[0]
 	}
+	var ignoreLabels, ignoreAnno = make([]string, 0), make([]string, 0)
+	if release.Spec.ReleasePlan.PatchPodTemplateMetadata != nil {
+		patch := release.Spec.ReleasePlan.PatchPodTemplateMetadata
+		for k := range patch.Labels {
+			ignoreLabels = append(ignoreLabels, k)
+		}
+		for k := range patch.Annotations {
+			ignoreAnno = append(ignoreAnno, k)
+		}
+	}
 	for _, d := range ds {
-		// todo, remove the canary pod metadata
-		if util.EqualIgnoreHash(template, &d.Spec.Template) {
+		if util.EqualIgnoreSpecifyMetadata(template, &d.Spec.Template, ignoreLabels, ignoreAnno) {
 			return d
 		}
 	}
diff --git a/pkg/controller/batchrelease/control/canarystyle/deployment/control.go b/pkg/controller/batchrelease/control/canarystyle/deployment/control.go
index 04938d0e..15f46021 100644
--- a/pkg/controller/batchrelease/control/canarystyle/deployment/control.go
+++ b/pkg/controller/batchrelease/control/canarystyle/deployment/control.go
@@ -73,7 +73,7 @@ func (rc *realController) BuildCanaryController(release *v1alpha1.BatchRelease)
 	if client.IgnoreNotFound(err) != nil {
 		return rc, err
 	}
-	rc.canaryObject = filterCanaryDeployment(util.FilterActiveDeployment(ds), template)
+	rc.canaryObject = filterCanaryDeployment(release, util.FilterActiveDeployment(ds), template)
 	if rc.canaryObject == nil {
 		return rc, control.GenerateNotFoundError(fmt.Sprintf("%v-canary", rc.stableKey), "Deployment")
 	}
diff --git a/pkg/controller/batchrelease/control/canarystyle/deployment/control_test.go b/pkg/controller/batchrelease/control/canarystyle/deployment/control_test.go
index 6846f829..2eaf31a0 100644
--- a/pkg/controller/batchrelease/control/canarystyle/deployment/control_test.go
+++ b/pkg/controller/batchrelease/control/canarystyle/deployment/control_test.go
@@ -316,7 +316,7 @@ func getCanaryDeployment(release *v1alpha1.BatchRelease, stable *apps.Deployment
 	if len(ds) == 0 {
 		return nil
 	}
-	return filterCanaryDeployment(ds, &stable.Spec.Template)
+	return filterCanaryDeployment(release, ds, &stable.Spec.Template)
 }
 
 func checkWorkloadInfo(stableInfo *util.WorkloadInfo, deployment *apps.Deployment) {
diff --git a/pkg/controller/rollout/rollout_canary.go b/pkg/controller/rollout/rollout_canary.go
index 4ae89517..9cd18e13 100644
--- a/pkg/controller/rollout/rollout_canary.go
+++ b/pkg/controller/rollout/rollout_canary.go
@@ -361,11 +361,11 @@ func createBatchRelease(rollout *v1alpha1.Rollout, rolloutID string, batch int32
 				},
 			},
 			ReleasePlan: v1alpha1.ReleasePlan{
-				Batches:          batches,
-				RolloutID:        rolloutID,
-				BatchPartition:   utilpointer.Int32Ptr(batch),
-				FailureThreshold: rollout.Spec.Strategy.Canary.FailureThreshold,
-				// PatchPodTemplateMetadata: rollout.Spec.Strategy.Canary.PatchPodTemplateMetadata,
+				Batches:                  batches,
+				RolloutID:                rolloutID,
+				BatchPartition:           utilpointer.Int32Ptr(batch),
+				FailureThreshold:         rollout.Spec.Strategy.Canary.FailureThreshold,
+				PatchPodTemplateMetadata: rollout.Spec.Strategy.Canary.PatchPodTemplateMetadata,
 			},
 		},
 	}
diff --git a/pkg/trafficrouting/manager.go b/pkg/trafficrouting/manager.go
index d11e14e0..1f77dd2b 100644
--- a/pkg/trafficrouting/manager.go
+++ b/pkg/trafficrouting/manager.go
@@ -247,13 +247,17 @@ func (m *Manager) FinalisingTrafficRouting(c *TrafficRoutingContext, onlyRestore
 	if err = trController.Finalise(context.TODO()); err != nil {
 		return false, err
 	}
-	// remove canary service
-	err = m.Delete(context.TODO(), cService)
-	if err != nil && !errors.IsNotFound(err) {
-		klog.Errorf("%s remove canary service(%s) failed: %s", c.Key, cService.Name, err.Error())
-		return false, err
+	// end to end deployment, don't remove the canary service;
+	// because canary service is stable service
+	if !c.OnlyTrafficRouting {
+		// remove canary service
+		err = m.Delete(context.TODO(), cService)
+		if err != nil && !errors.IsNotFound(err) {
+			klog.Errorf("%s remove canary service(%s) failed: %s", c.Key, cService.Name, err.Error())
+			return false, err
+		}
+		klog.Infof("%s remove canary service(%s) success", c.Key, cService.Name)
 	}
-	klog.Infof("%s remove canary service(%s) success", c.Key, cService.Name)
 	return true, nil
 }
 
diff --git a/pkg/trafficrouting/network/ingress/ingress_test.go b/pkg/trafficrouting/network/ingress/ingress_test.go
index 2cec56ca..95e1ec99 100644
--- a/pkg/trafficrouting/network/ingress/ingress_test.go
+++ b/pkg/trafficrouting/network/ingress/ingress_test.go
@@ -61,6 +61,10 @@ var (
 				then
 					annotations["nginx.ingress.kubernetes.io/canary-weight"] = obj.weight
 				end
+				if ( annotations["mse.ingress.kubernetes.io/service-subset"] )
+				then
+					annotations["mse.ingress.kubernetes.io/service-subset"] = "gray"
+				end
 				if ( obj.requestHeaderModifier )
                 then
 					local str = ''
@@ -317,6 +321,7 @@ func TestEnsureRoutes(t *testing.T) {
 				canary.Name = "echoserver-canary"
 				canary.Annotations["nginx.ingress.kubernetes.io/canary"] = "true"
 				canary.Annotations["nginx.ingress.kubernetes.io/canary-weight"] = "0"
+				canary.Annotations["mse.ingress.kubernetes.io/service-subset"] = ""
 				canary.Spec.Rules[0].HTTP.Paths = canary.Spec.Rules[0].HTTP.Paths[:1]
 				canary.Spec.Rules[0].HTTP.Paths[0].Backend.Service.Name = "echoserver-canary"
 				canary.Spec.Rules[1].HTTP.Paths[0].Backend.Service.Name = "echoserver-canary"
@@ -369,6 +374,7 @@ func TestEnsureRoutes(t *testing.T) {
 				expect.Annotations["nginx.ingress.kubernetes.io/canary-by-header"] = "user_id"
 				expect.Annotations["nginx.ingress.kubernetes.io/canary-by-header-value"] = "123456"
 				expect.Annotations["mse.ingress.kubernetes.io/request-header-control-update"] = "gray blue\ngray green\n"
+				expect.Annotations["mse.ingress.kubernetes.io/service-subset"] = "gray"
 				expect.Spec.Rules[0].HTTP.Paths = expect.Spec.Rules[0].HTTP.Paths[:1]
 				expect.Spec.Rules[0].HTTP.Paths[0].Backend.Service.Name = "echoserver-canary"
 				expect.Spec.Rules[1].HTTP.Paths[0].Backend.Service.Name = "echoserver-canary"
diff --git a/pkg/util/controller_finder.go b/pkg/util/controller_finder.go
index dd15519a..cc16baf8 100644
--- a/pkg/util/controller_finder.go
+++ b/pkg/util/controller_finder.go
@@ -50,8 +50,6 @@ type Workload struct {
 	PodTemplateHash string
 	// Revision hash key
 	RevisionLabelKey string
-	// label selector
-	Selector *metav1.LabelSelector
 
 	// Is it in rollback phase
 	IsInRollback bool
@@ -311,7 +309,6 @@ func (r *ControllerFinder) getDeployment(namespace string, ref *rolloutv1alpha1.
 		StableRevision:     stableRs.Labels[apps.DefaultDeploymentUniqueLabelKey],
 		CanaryRevision:     ComputeHash(&stable.Spec.Template, nil),
 		RevisionLabelKey:   apps.DefaultDeploymentUniqueLabelKey,
-		Selector:           stable.Spec.Selector,
 	}
 
 	// not in rollout progressing
diff --git a/pkg/util/workloads_utils.go b/pkg/util/workloads_utils.go
index 493f8907..86bc8659 100644
--- a/pkg/util/workloads_utils.go
+++ b/pkg/util/workloads_utils.go
@@ -159,6 +159,25 @@ func SafeEncodeString(s string) string {
 	return string(r)
 }
 
+func EqualIgnoreSpecifyMetadata(template1, template2 *v1.PodTemplateSpec, ignoreLabels, ignoreAnno []string) bool {
+	t1Copy := template1.DeepCopy()
+	t2Copy := template2.DeepCopy()
+	if ignoreLabels == nil {
+		ignoreLabels = make([]string, 0)
+	}
+	// default remove the hash label
+	ignoreLabels = append(ignoreLabels, apps.DefaultDeploymentUniqueLabelKey)
+	for _, k := range ignoreLabels {
+		delete(t1Copy.Labels, k)
+		delete(t2Copy.Labels, k)
+	}
+	for _, k := range ignoreAnno {
+		delete(t1Copy.Annotations, k)
+		delete(t2Copy.Annotations, k)
+	}
+	return apiequality.Semantic.DeepEqual(t1Copy, t2Copy)
+}
+
 // EqualIgnoreHash compare template without pod-template-hash label
 func EqualIgnoreHash(template1, template2 *v1.PodTemplateSpec) bool {
 	t1Copy := template1.DeepCopy()
diff --git a/test/e2e/rollout_test.go b/test/e2e/rollout_test.go
index 2ad7f175..e158b688 100644
--- a/test/e2e/rollout_test.go
+++ b/test/e2e/rollout_test.go
@@ -442,6 +442,10 @@ var _ = SIGDescribe("Rollout", func() {
 					},
 				},
 			}
+			rollout.Spec.Strategy.Canary.PatchPodTemplateMetadata = &v1alpha1.PatchPodTemplateMetadata{
+				Labels:      map[string]string{"pod": "canary"},
+				Annotations: map[string]string{"pod": "canary"},
+			}
 			CreateObject(rollout)
 
 			By("Creating workload and waiting for all pods ready...")
@@ -457,6 +461,10 @@ var _ = SIGDescribe("Rollout", func() {
 			workload := &apps.Deployment{}
 			Expect(ReadYamlToObject("./test_data/rollout/deployment.yaml", workload)).ToNot(HaveOccurred())
 			workload.Spec.Replicas = utilpointer.Int32(3)
+			workload.Spec.Template.Labels["pod"] = "stable"
+			workload.Spec.Template.Annotations = map[string]string{
+				"pod": "stable",
+			}
 			CreateObject(workload)
 			WaitDeploymentAllPodsReady(workload)
 
@@ -484,6 +492,10 @@ var _ = SIGDescribe("Rollout", func() {
 			cIngress := &netv1.Ingress{}
 			Expect(GetObject(service.Name+"-canary", cIngress)).NotTo(HaveOccurred())
 			Expect(cIngress.Annotations[fmt.Sprintf("%s/canary-weight", nginxIngressAnnotationDefaultPrefix)]).Should(Equal("20"))
+			canaryWorkload, err := GetCanaryDeployment(workload)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(canaryWorkload.Spec.Template.Annotations["pod"]).Should(Equal("canary"))
+			Expect(canaryWorkload.Spec.Template.Labels["pod"]).Should(Equal("canary"))
 
 			// resume rollout canary
 			ResumeRolloutCanary(rollout.Name)