From 8b4aa861e4d80e0bdfa8c881a05f89dbd45e002c Mon Sep 17 00:00:00 2001 From: "liheng.zms" Date: Tue, 4 Jul 2023 15:36:14 +0800 Subject: [PATCH] rollout support patchPodTemplateMetadata Signed-off-by: liheng.zms --- 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)