rollout support patchPodTemplateMetadata (#157)

Signed-off-by: liheng.zms <[email protected]>

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

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.

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

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

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 = ''

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

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

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

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

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
 }
 

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"

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

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

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)