webhook validate v1beta1 rollout (#188)

Signed-off-by: liheng.zms <liheng.zms@alibaba-inc.com>

config/webhook/manifests.yaml

@@ -152,6 +152,7 @@ webhooks:
     - rollouts.kruise.io
     apiVersions:
     - v1alpha1
+    - v1beta1
     operations:
     - CREATE
     - UPDATE

pkg/controller/rollout/rollout_canary.go

@@ -62,6 +62,11 @@ 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]
+	// PodTemplateHash is used to select a new version of the Pod.
+
+	// Note:
+	// In a scenario of successive releases v1->v2->v3, It is possible that this is the PodTemplateHash value of v2,
+	// so it needs to be set later in the stepUpgrade
 	if canaryStatus.PodTemplateHash == "" {
 		canaryStatus.PodTemplateHash = c.Workload.PodTemplateHash
 	}
@@ -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
 }
 

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 {
+		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)
-		is := intstr.FromString(*s.Traffic)
+			if err != nil || weight <= 0 || weight > 100 {
-		weight, err := intstr.GetScaledValueFromIntOrPercent(&is, 100, true)
+				return field.ErrorList{field.Invalid(fldPath.Index(i).Child("steps"), steps, `traffic must be percentage with "0%" < traffic <= "100%"`)}
-		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%"`)}
 		}
 	}
 

pkg/webhook/rollout/validating/rollout_create_update_handler_test.go

@@ -250,8 +250,8 @@ func TestRolloutValidateCreate(t *testing.T) {
 			},
 		},
 		{
-			Name:    "Miss matched rolling style",
+			Name:    "matched rolling style",
-			Succeed: false,
+			Succeed: true,
 			GetObject: func() []client.Object {
 				object := rollout.DeepCopy()
 				object.Spec.Strategy.Canary.EnableExtraWorkloadForCanary = true

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

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")
+			// wait step 1 complete
-			time.Sleep(time.Second * 15)
+			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)