From 338a862a8e3a2ba631cb4779c4eb2c2ec0e842a1 Mon Sep 17 00:00:00 2001 From: "liheng.zms" Date: Tue, 12 Dec 2023 14:32:12 +0800 Subject: [PATCH] webhook validate v1beta1 rollout Signed-off-by: liheng.zms --- config/webhook/manifests.yaml | 1 + pkg/controller/rollout/rollout_canary.go | 11 +++++- .../rollout_create_update_handler.go | 26 +++---------- .../rollout_create_update_handler_test.go | 4 +- pkg/webhook/rollout/validating/webhooks.go | 2 +- test/e2e/rollout_test.go | 37 ++++++++++++++++++- 6 files changed, 54 insertions(+), 27 deletions(-) diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 1c74221c..64ccd607 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -152,6 +152,7 @@ webhooks: - rollouts.kruise.io apiVersions: - v1alpha1 + - v1beta1 operations: - CREATE - UPDATE diff --git a/pkg/controller/rollout/rollout_canary.go b/pkg/controller/rollout/rollout_canary.go index fa5145ed..3537c036 100644 --- a/pkg/controller/rollout/rollout_canary.go +++ b/pkg/controller/rollout/rollout_canary.go @@ -62,9 +62,14 @@ func (m *canaryReleaseManager) runCanary(c *RolloutContext) error { // update podTemplateHash, Why is this position assigned? // Because If workload is deployment, only after canary pod already was created, // we can get the podTemplateHash from pod.annotations[pod-template-hash] - if canaryStatus.PodTemplateHash == "" { + // PodTemplateHash is used to select a new version of the Pod. + + // Note: + // In a scenario of successive releases v1->v2->v3, the value could be v2 before v3 has been pulled up. + // this is not as expected. So, the most accurate way to get PodTemplateHash is really after stepUpgrade. + /*if canaryStatus.PodTemplateHash == "" { canaryStatus.PodTemplateHash = c.Workload.PodTemplateHash - } + }*/ // When the first batch is trafficRouting rolling and the next steps are rolling release, // We need to clean up the canary-related resources first and then rollout the rest of the batch. currentStep := c.Rollout.Spec.Strategy.Canary.Steps[canaryStatus.CurrentStepIndex-1] @@ -185,6 +190,8 @@ func (m *canaryReleaseManager) doCanaryUpgrade(c *RolloutContext) (bool, error) m.recorder.Eventf(c.Rollout, corev1.EventTypeNormal, "Progressing", fmt.Sprintf("upgrade step(%d) canary pods with new versions done", canaryStatus.CurrentStepIndex)) klog.Infof("rollout(%s/%s) batch(%s) state(%s), and success", c.Rollout.Namespace, c.Rollout.Name, util.DumpJSON(br.Status), br.Status.CanaryStatus.CurrentBatchState) + // set the latest PodTemplateHash to selector the latest pods. + canaryStatus.PodTemplateHash = c.Workload.PodTemplateHash return true, nil } diff --git a/pkg/webhook/rollout/validating/rollout_create_update_handler.go b/pkg/webhook/rollout/validating/rollout_create_update_handler.go index 40cbb36e..0d2c78b9 100644 --- a/pkg/webhook/rollout/validating/rollout_create_update_handler.go +++ b/pkg/webhook/rollout/validating/rollout_create_update_handler.go @@ -181,23 +181,10 @@ func (h *RolloutCreateUpdateHandler) validateRolloutConflict(rollout *appsv1beta func validateRolloutSpec(rollout *appsv1beta1.Rollout, fldPath *field.Path) field.ErrorList { errList := validateRolloutSpecObjectRef(&rollout.Spec.WorkloadRef, fldPath.Child("ObjectRef")) - errList = append(errList, validateRolloutRollingStyle(rollout, field.NewPath("RollingStyle"))...) errList = append(errList, validateRolloutSpecStrategy(&rollout.Spec.Strategy, fldPath.Child("Strategy"))...) return errList } -func validateRolloutRollingStyle(rollout *appsv1beta1.Rollout, fldPath *field.Path) field.ErrorList { - workloadRef := rollout.Spec.WorkloadRef - if workloadRef.Kind == util.ControllerKindDep.Kind { - return nil // Deployment support all rolling styles, no need to validate. - } - if rollout.Spec.Strategy.Canary.EnableExtraWorkloadForCanary { - return field.ErrorList{field.Invalid(fldPath, rollout.Spec.Strategy.Canary.EnableExtraWorkloadForCanary, - "Only Deployment can set enableExtraWorkloadForCanary=true")} - } - return nil -} - func validateRolloutSpecObjectRef(workloadRef *appsv1beta1.ObjectRef, fldPath *field.Path) field.ErrorList { if workloadRef == nil { return field.ErrorList{field.Invalid(fldPath.Child("WorkloadRef"), workloadRef, "WorkloadRef is required")} @@ -274,13 +261,12 @@ func validateRolloutSpecCanarySteps(steps []appsv1beta1.CanaryStep, fldPath *fie if !isTraffic { continue } - if s.Traffic == nil { - return field.ErrorList{field.Invalid(fldPath.Index(i).Child("steps"), steps, `traffic cannot be empty`)} - } - is := intstr.FromString(*s.Traffic) - weight, err := intstr.GetScaledValueFromIntOrPercent(&is, 100, true) - if err != nil || weight <= 0 || weight > 100 { - return field.ErrorList{field.Invalid(fldPath.Index(i).Child("steps"), steps, `traffic must be percentage with "0%" < traffic <= "100%"`)} + if s.Traffic != nil { + is := intstr.FromString(*s.Traffic) + weight, err := intstr.GetScaledValueFromIntOrPercent(&is, 100, true) + if err != nil || weight <= 0 || weight > 100 { + return field.ErrorList{field.Invalid(fldPath.Index(i).Child("steps"), steps, `traffic must be percentage with "0%" < traffic <= "100%"`)} + } } } diff --git a/pkg/webhook/rollout/validating/rollout_create_update_handler_test.go b/pkg/webhook/rollout/validating/rollout_create_update_handler_test.go index 959d7536..07eacd57 100644 --- a/pkg/webhook/rollout/validating/rollout_create_update_handler_test.go +++ b/pkg/webhook/rollout/validating/rollout_create_update_handler_test.go @@ -250,8 +250,8 @@ func TestRolloutValidateCreate(t *testing.T) { }, }, { - Name: "Miss matched rolling style", - Succeed: false, + Name: "matched rolling style", + Succeed: true, GetObject: func() []client.Object { object := rollout.DeepCopy() object.Spec.Strategy.Canary.EnableExtraWorkloadForCanary = true diff --git a/pkg/webhook/rollout/validating/webhooks.go b/pkg/webhook/rollout/validating/webhooks.go index d105deb5..bc211648 100644 --- a/pkg/webhook/rollout/validating/webhooks.go +++ b/pkg/webhook/rollout/validating/webhooks.go @@ -20,7 +20,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) -// +kubebuilder:webhook:path=/validate-rollouts-kruise-io-rollout,mutating=false,failurePolicy=fail,sideEffects=None,admissionReviewVersions=v1;v1beta1,groups=rollouts.kruise.io,resources=rollouts,verbs=create;update,versions=v1alpha1,name=vrollout.kb.io +// +kubebuilder:webhook:path=/validate-rollouts-kruise-io-rollout,mutating=false,failurePolicy=fail,sideEffects=None,admissionReviewVersions=v1;v1beta1,groups=rollouts.kruise.io,resources=rollouts,verbs=create;update,versions=v1alpha1;v1beta1,name=vrollout.kb.io var ( // HandlerMap contains admission webhook handlers diff --git a/test/e2e/rollout_test.go b/test/e2e/rollout_test.go index 31151158..66e894c9 100644 --- a/test/e2e/rollout_test.go +++ b/test/e2e/rollout_test.go @@ -881,6 +881,36 @@ var _ = SIGDescribe("Rollout", func() { By("Creating Rollout...") rollout := &v1alpha1.Rollout{} Expect(ReadYamlToObject("./test_data/rollout/rollout_canary_base.yaml", rollout)).ToNot(HaveOccurred()) + rollout.Spec.Strategy.Canary.Steps = []v1alpha1.CanaryStep{ + { + TrafficRoutingStrategy: v1alpha1.TrafficRoutingStrategy{ + Weight: utilpointer.Int32(20), + }, + }, + { + TrafficRoutingStrategy: v1alpha1.TrafficRoutingStrategy{ + Weight: utilpointer.Int32(40), + }, + }, + { + TrafficRoutingStrategy: v1alpha1.TrafficRoutingStrategy{ + Weight: utilpointer.Int32(60), + }, + Pause: v1alpha1.RolloutPause{Duration: utilpointer.Int32(10)}, + }, + { + TrafficRoutingStrategy: v1alpha1.TrafficRoutingStrategy{ + Weight: utilpointer.Int32(80), + }, + Pause: v1alpha1.RolloutPause{Duration: utilpointer.Int32(10)}, + }, + { + TrafficRoutingStrategy: v1alpha1.TrafficRoutingStrategy{ + Weight: utilpointer.Int32(100), + }, + Pause: v1alpha1.RolloutPause{Duration: utilpointer.Int32(1)}, + }, + } CreateObject(rollout) By("Creating workload and waiting for all pods ready...") @@ -942,8 +972,8 @@ var _ = SIGDescribe("Rollout", func() { // resume rollout canary ResumeRolloutCanary(rollout.Name) - By("check rollout canary status success, resume rollout, and wait rollout canary complete") - time.Sleep(time.Second * 15) + // wait step 1 complete + WaitRolloutCanaryStepPaused(rollout.Name, 2) // v1 -> v2 -> v3, continuous release newEnvs = mergeEnvVar(workload.Spec.Template.Spec.Containers[0].Env, v1.EnvVar{Name: "NODE_NAME", Value: "version3"}) @@ -994,6 +1024,9 @@ var _ = SIGDescribe("Rollout", func() { // resume rollout canary ResumeRolloutCanary(rollout.Name) + // wait step 1 complete + WaitRolloutCanaryStepPaused(rollout.Name, 2) + ResumeRolloutCanary(rollout.Name) By("check rollout canary status success, resume rollout, and wait rollout canary complete") WaitRolloutStatusPhase(rollout.Name, v1alpha1.RolloutPhaseHealthy) klog.Infof("rollout(%s) completed, and check", namespace)