add restriction for traffic configuration of partition-style step (#225)

Signed-off-by: yunbo <yunbo10124scut@gmail.com>
Co-authored-by: yunbo <yunbo10124scut@gmail.com>

pkg/webhook/rollout/validating/rollout_create_update_handler.go

@@ -18,6 +18,7 @@ package validating
 
 import (
 	"context"
+	"flag"
 	"fmt"
 	"net/http"
 	"reflect"
@@ -47,7 +48,27 @@ type RolloutCreateUpdateHandler struct {
 	Decoder *admission.Decoder
 }
 
-var _ admission.Handler = &RolloutCreateUpdateHandler{}
+var (
+	_ admission.Handler = &RolloutCreateUpdateHandler{}
+	// PartitionReplicasLimitWithTraffic represents the maximum percentage of replicas
+	// allowed for a step of partition-style release, if traffic/matches specified.
+	// If a step is configured with a number of replicas exceeding this percentage, the traffic strategy for that step
+	// must not be specified. If this rule is violated, the Rollout webhook will block the creation or modification of the Rollout.
+	// The default limit is set to 50%.
+	// Here is why we set this limit for partition style release:
+	// In rollback and continuous scenarios, usually we expect the Rollout to route all traffic to the stable version first.
+	// However, if the stable version's pods are relatively few (less than 1-PartitionReplicasLimitWithTraffic), this might overload the stable version's pods.
+	PartitionReplicasLimitWithTraffic = 50
+)
+
+func init() {
+	flag.IntVar(&PartitionReplicasLimitWithTraffic, "partition-percent-limit", 50, "represents the maximum percentage of replicas allowed for a step of partition-style release, if traffic/matches specified.")
+}
+
+// record upper level information about the rollout
+type validateContext struct {
+	style string
+}
 
 // Handle handles admission requests.
 func (h *RolloutCreateUpdateHandler) Handle(ctx context.Context, req admission.Request) admission.Response {
@@ -155,7 +176,7 @@ func (h *RolloutCreateUpdateHandler) validateRolloutUpdate(oldObj, newObj *appsv
 }
 
 func (h *RolloutCreateUpdateHandler) validateRollout(rollout *appsv1beta1.Rollout) field.ErrorList {
-	errList := validateRolloutSpec(rollout, field.NewPath("Spec"))
+	errList := validateRolloutSpec(GetContextFromv1beta1Rollout(rollout), rollout, field.NewPath("Spec"))
 	errList = append(errList, h.validateRolloutConflict(rollout, field.NewPath("Conflict Checker"))...)
 	return errList
 }
@@ -178,9 +199,9 @@ func (h *RolloutCreateUpdateHandler) validateRolloutConflict(rollout *appsv1beta
 	return nil
 }
 
-func validateRolloutSpec(rollout *appsv1beta1.Rollout, fldPath *field.Path) field.ErrorList {
+func validateRolloutSpec(c *validateContext, rollout *appsv1beta1.Rollout, fldPath *field.Path) field.ErrorList {
 	errList := validateRolloutSpecObjectRef(&rollout.Spec.WorkloadRef, fldPath.Child("ObjectRef"))
-	errList = append(errList, validateRolloutSpecStrategy(&rollout.Spec.Strategy, fldPath.Child("Strategy"))...)
+	errList = append(errList, validateRolloutSpecStrategy(c, &rollout.Spec.Strategy, fldPath.Child("Strategy"))...)
 	return errList
 }
 
@@ -196,7 +217,7 @@ func validateRolloutSpecObjectRef(workloadRef *appsv1beta1.ObjectRef, fldPath *f
 	return nil
 }
 
-func validateRolloutSpecStrategy(strategy *appsv1beta1.RolloutStrategy, fldPath *field.Path) field.ErrorList {
+func validateRolloutSpecStrategy(c *validateContext, strategy *appsv1beta1.RolloutStrategy, fldPath *field.Path) field.ErrorList {
 	if strategy.Canary == nil && strategy.BlueGreen == nil {
 		return field.ErrorList{field.Invalid(fldPath, nil, "Canary and BlueGreen cannot both be empty")}
 	}
@@ -204,25 +225,13 @@ func validateRolloutSpecStrategy(strategy *appsv1beta1.RolloutStrategy, fldPath
 		return field.ErrorList{field.Invalid(fldPath, nil, "Canary and BlueGreen cannot both be set")}
 	}
 	if strategy.BlueGreen != nil {
-		return validateRolloutSpecBlueGreenStrategy(strategy.BlueGreen, fldPath.Child("BlueGreen"))
+		return validateRolloutSpecBlueGreenStrategy(c, strategy.BlueGreen, fldPath.Child("BlueGreen"))
 	}
-	return validateRolloutSpecCanaryStrategy(strategy.Canary, fldPath.Child("Canary"))
+	return validateRolloutSpecCanaryStrategy(c, strategy.Canary, fldPath.Child("Canary"))
 }
 
-type TrafficRule string
+func validateRolloutSpecCanaryStrategy(c *validateContext, canary *appsv1beta1.CanaryStrategy, fldPath *field.Path) field.ErrorList {
-
+	errList := validateRolloutSpecCanarySteps(c, canary.Steps, fldPath.Child("Steps"))
-const (
-	TrafficRuleCanary    TrafficRule = "Canary"
-	TrafficRuleBlueGreen TrafficRule = "BlueGreen"
-	NoTraffic            TrafficRule = "NoTraffic"
-)
-
-func validateRolloutSpecCanaryStrategy(canary *appsv1beta1.CanaryStrategy, fldPath *field.Path) field.ErrorList {
-	trafficRule := NoTraffic
-	if len(canary.TrafficRoutings) > 0 {
-		trafficRule = TrafficRuleCanary
-	}
-	errList := validateRolloutSpecCanarySteps(canary.Steps, fldPath.Child("Steps"), trafficRule)
 	if len(canary.TrafficRoutings) > 1 {
 		errList = append(errList, field.Invalid(fldPath, canary.TrafficRoutings, "Rollout currently only support single TrafficRouting."))
 	}
@@ -232,12 +241,8 @@ func validateRolloutSpecCanaryStrategy(canary *appsv1beta1.CanaryStrategy, fldPa
 	return errList
 }
 
-func validateRolloutSpecBlueGreenStrategy(blueGreen *appsv1beta1.BlueGreenStrategy, fldPath *field.Path) field.ErrorList {
+func validateRolloutSpecBlueGreenStrategy(c *validateContext, blueGreen *appsv1beta1.BlueGreenStrategy, fldPath *field.Path) field.ErrorList {
-	trafficRule := NoTraffic
+	errList := validateRolloutSpecCanarySteps(c, blueGreen.Steps, fldPath.Child("Steps"))
-	if len(blueGreen.TrafficRoutings) > 0 {
-		trafficRule = TrafficRuleBlueGreen
-	}
-	errList := validateRolloutSpecCanarySteps(blueGreen.Steps, fldPath.Child("Steps"), trafficRule)
 	if len(blueGreen.TrafficRoutings) > 1 {
 		errList = append(errList, field.Invalid(fldPath, blueGreen.TrafficRoutings, "Rollout currently only support single TrafficRouting."))
 	}
@@ -275,7 +280,7 @@ func validateRolloutSpecCanaryTraffic(traffic appsv1beta1.TrafficRoutingRef, fld
 	return errList
 }
 
-func validateRolloutSpecCanarySteps(steps []appsv1beta1.CanaryStep, fldPath *field.Path, trafficRule TrafficRule) field.ErrorList {
+func validateRolloutSpecCanarySteps(c *validateContext, steps []appsv1beta1.CanaryStep, fldPath *field.Path) field.ErrorList {
 	stepCount := len(steps)
 	if stepCount == 0 {
 		return field.ErrorList{field.Invalid(fldPath, steps, "The number of Canary.Steps cannot be empty")}
@@ -293,13 +298,24 @@ func validateRolloutSpecCanarySteps(steps []appsv1beta1.CanaryStep, fldPath *fie
 			return field.ErrorList{field.Invalid(fldPath.Index(i).Child("Replicas"),
 				s.Replicas, `replicas must be positive number, or a percentage with "0%" < canaryReplicas <= "100%"`)}
 		}
-		if trafficRule == NoTraffic || s.Traffic == nil {
+		// no traffic strategy is configured for current step
+		if s.Traffic == nil && len(s.Matches) == 0 {
+			continue
+		}
+		// replicas is percentage
+		if c.style == string(appsv1beta1.PartitionRollingStyle) && IsPercentageCanaryReplicasType(s.Replicas) {
+			currCanaryReplicas, _ := intstr.GetScaledValueFromIntOrPercent(s.Replicas, 100, true)
+			if currCanaryReplicas > PartitionReplicasLimitWithTraffic {
+				return field.ErrorList{field.Invalid(fldPath.Index(i).Child("steps"), steps, `For partition style rollout: step[x].replicas must not greater than partition-percent-limit if traffic specified`)}
+			}
+		}
+		if s.Traffic == nil {
 			continue
 		}
 		is := intstr.FromString(*s.Traffic)
 		weight, err := intstr.GetScaledValueFromIntOrPercent(&is, 100, true)
-		switch trafficRule {
+		switch c.style {
-		case TrafficRuleBlueGreen:
+		case string(appsv1beta1.BlueGreenRollingStyle):
 			// traffic "0%" is allowed in blueGreen strategy
 			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%" in blueGreen strategy`)}
@@ -355,3 +371,14 @@ func (h *RolloutCreateUpdateHandler) InjectDecoder(d *admission.Decoder) error {
 	h.Decoder = d
 	return nil
 }
+
+func GetContextFromv1beta1Rollout(rollout *appsv1beta1.Rollout) *validateContext {
+	if rollout.Spec.Strategy.Canary == nil && rollout.Spec.Strategy.BlueGreen == nil {
+		return nil
+	}
+	style := rollout.Spec.Strategy.GetRollingStyle()
+	if appsv1beta1.IsRealPartition(rollout) {
+		style = appsv1beta1.PartitionRollingStyle
+	}
+	return &validateContext{style: string(style)}
+}

pkg/webhook/rollout/validating/rollout_create_update_handler_test.go

@@ -22,6 +22,7 @@ import (
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	rolloutapi "github.com/openkruise/rollouts/api"
+	appsv1alpha1 "github.com/openkruise/rollouts/api/v1alpha1"
 	appsv1beta1 "github.com/openkruise/rollouts/api/v1beta1"
 	apps "k8s.io/api/apps/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -102,6 +103,63 @@ var (
 			},
 		},
 	}
+	rolloutV1alpha1 = appsv1alpha1.Rollout{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: appsv1alpha1.SchemeGroupVersion.String(),
+			Kind:       "Rollout",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:        "rollout-demo",
+			Namespace:   "namespace-unit-test",
+			Annotations: map[string]string{},
+		},
+		Spec: appsv1alpha1.RolloutSpec{
+			ObjectRef: appsv1alpha1.ObjectRef{
+				WorkloadRef: &appsv1alpha1.WorkloadRef{
+					APIVersion: apps.SchemeGroupVersion.String(),
+					Kind:       "Deployment",
+					Name:       "deployment-demo",
+				},
+			},
+			Strategy: appsv1alpha1.RolloutStrategy{
+				Canary: &appsv1alpha1.CanaryStrategy{
+					Steps: []appsv1alpha1.CanaryStep{
+						{
+							TrafficRoutingStrategy: appsv1alpha1.TrafficRoutingStrategy{
+								Weight: utilpointer.Int32(10),
+							},
+							Pause: appsv1alpha1.RolloutPause{},
+						},
+						{
+							TrafficRoutingStrategy: appsv1alpha1.TrafficRoutingStrategy{
+								Weight: utilpointer.Int32(30),
+							},
+							Pause: appsv1alpha1.RolloutPause{Duration: utilpointer.Int32(1 * 24 * 60 * 60)},
+						},
+						{
+							TrafficRoutingStrategy: appsv1alpha1.TrafficRoutingStrategy{
+								Weight: utilpointer.Int32(100),
+							},
+						},
+					},
+					TrafficRoutings: []appsv1alpha1.TrafficRoutingRef{
+						{
+							Service: "service-demo",
+							Ingress: &appsv1alpha1.IngressTrafficRouting{
+								ClassType: "nginx",
+								Name:      "ingress-nginx-demo",
+							},
+						},
+					},
+				},
+			},
+		},
+		Status: appsv1alpha1.RolloutStatus{
+			CanaryStatus: &appsv1alpha1.CanaryStatus{
+				CurrentStepState: appsv1alpha1.CanaryStepStateCompleted,
+			},
+		},
+	}
 )
 
 func init() {
@@ -262,6 +320,74 @@ func TestRolloutValidateCreate(t *testing.T) {
 				return []client.Object{object}
 			},
 		},
+		{
+			Name:    "test with replicasLimitWithTraffic - 1",
+			Succeed: true,
+			GetObject: func() []client.Object {
+				object := rollout.DeepCopy()
+				n := len(object.Spec.Strategy.Canary.Steps)
+				object.Spec.Strategy.Canary.Steps[n-1].Replicas = &intstr.IntOrString{Type: intstr.String, StrVal: "30%"}
+				return []client.Object{object}
+			},
+		},
+		{
+			Name:    "test with replicasLimitWithTraffic - 2",
+			Succeed: false,
+			GetObject: func() []client.Object {
+				object := rollout.DeepCopy()
+				n := len(object.Spec.Strategy.Canary.Steps)
+				object.Spec.Strategy.Canary.Steps[n-1].Replicas = &intstr.IntOrString{Type: intstr.String, StrVal: "31%"}
+				return []client.Object{object}
+			},
+		},
+		{
+			Name:    "test with replicasLimitWithTraffic - 2 - canary style",
+			Succeed: true,
+			GetObject: func() []client.Object {
+				object := rollout.DeepCopy()
+				object.Spec.Strategy.Canary.EnableExtraWorkloadForCanary = true
+				n := len(object.Spec.Strategy.Canary.Steps)
+				object.Spec.Strategy.Canary.Steps[n-1].Replicas = &intstr.IntOrString{Type: intstr.String, StrVal: "31%"}
+				return []client.Object{object}
+			},
+		},
+		{
+			Name:    "test with replicasLimitWithTraffic - 2 - cloneset",
+			Succeed: false,
+			GetObject: func() []client.Object {
+				object := rollout.DeepCopy()
+				object.Spec.WorkloadRef = appsv1beta1.ObjectRef{
+					APIVersion: "apps.kruise.io/v1alpha1",
+					Kind:       "CloneSet",
+					Name:       "whatever",
+				}
+				n := len(object.Spec.Strategy.Canary.Steps)
+				object.Spec.Strategy.Canary.Steps[n-1].Replicas = &intstr.IntOrString{Type: intstr.String, StrVal: "31%"}
+				return []client.Object{object}
+			},
+		},
+		{
+			Name:    "test with replicasLimitWithTraffic - 3",
+			Succeed: true,
+			GetObject: func() []client.Object {
+				object := rollout.DeepCopy()
+				PartitionReplicasLimitWithTraffic = 100
+				n := len(object.Spec.Strategy.Canary.Steps)
+				object.Spec.Strategy.Canary.Steps[n-1].Replicas = &intstr.IntOrString{Type: intstr.String, StrVal: "100%"}
+				return []client.Object{object}
+			},
+		},
+		{
+			Name:    "test with replicasLimitWithTraffic - 4",
+			Succeed: false,
+			GetObject: func() []client.Object {
+				object := rollout.DeepCopy()
+				PartitionReplicasLimitWithTraffic = 50
+				n := len(object.Spec.Strategy.Canary.Steps)
+				object.Spec.Strategy.Canary.Steps[n-1].Replicas = &intstr.IntOrString{Type: intstr.String, StrVal: "51%"}
+				return []client.Object{object}
+			},
+		},
 		//{
 		//	Name:    "The last Steps.Traffic is not 100",
 		//	Succeed: false,
@@ -335,6 +461,133 @@ func TestRolloutValidateCreate(t *testing.T) {
 			errList := handler.validateRollout(objects[0].(*appsv1beta1.Rollout))
 			t.Log(errList)
 			Expect(len(errList) == 0).Should(Equal(cs.Succeed))
+			// restore PartitionReplicasLimitWithTraffic after each case
+			PartitionReplicasLimitWithTraffic = 30
+		})
+	}
+}
+
+func TestRolloutV1alpha1ValidateCreate(t *testing.T) {
+	RegisterFailHandler(Fail)
+
+	cases := []struct {
+		Name      string
+		Succeed   bool
+		GetObject func() []client.Object
+	}{
+		{
+			Name:    "Canary style",
+			Succeed: true,
+			GetObject: func() []client.Object {
+				obj := rolloutV1alpha1.DeepCopy()
+				return []client.Object{obj}
+			},
+		},
+		{
+			Name:    "Partition style without replicas - 1",
+			Succeed: false,
+			GetObject: func() []client.Object {
+				obj := rolloutV1alpha1.DeepCopy()
+				obj.Annotations[appsv1alpha1.RolloutStyleAnnotation] = string(appsv1alpha1.PartitionRollingStyle)
+				return []client.Object{obj}
+			},
+		},
+		{
+			Name:    "Partition style without replicas - 2",
+			Succeed: true,
+			GetObject: func() []client.Object {
+				obj := rolloutV1alpha1.DeepCopy()
+				PartitionReplicasLimitWithTraffic = 100
+				obj.Annotations[appsv1alpha1.RolloutStyleAnnotation] = string(appsv1alpha1.PartitionRollingStyle)
+				return []client.Object{obj}
+			},
+		},
+		{
+			Name:    "Partition style without replicas - 3",
+			Succeed: false,
+			GetObject: func() []client.Object {
+				obj := rolloutV1alpha1.DeepCopy()
+				obj.Spec.Strategy.Canary.Steps[len(obj.Spec.Strategy.Canary.Steps)-1].Weight = utilpointer.Int32(32)
+				PartitionReplicasLimitWithTraffic = 31
+				obj.Annotations[appsv1alpha1.RolloutStyleAnnotation] = string(appsv1alpha1.PartitionRollingStyle)
+				return []client.Object{obj}
+			},
+		},
+		{
+			Name:    "Partition style without replicas- 3 - canary style",
+			Succeed: true,
+			GetObject: func() []client.Object {
+				obj := rolloutV1alpha1.DeepCopy()
+				obj.Spec.Strategy.Canary.Steps[len(obj.Spec.Strategy.Canary.Steps)-1].Weight = utilpointer.Int32(32)
+				PartitionReplicasLimitWithTraffic = 31
+				return []client.Object{obj}
+			},
+		},
+		{
+			Name:    "Partition style without replicas - 3 - cloneset",
+			Succeed: false,
+			GetObject: func() []client.Object {
+				obj := rolloutV1alpha1.DeepCopy()
+				obj.Spec.ObjectRef.WorkloadRef = &appsv1alpha1.WorkloadRef{
+					APIVersion: "apps.kruise.io/v1alpha1",
+					Kind:       "CloneSet",
+					Name:       "whatever",
+				}
+				obj.Spec.Strategy.Canary.Steps[len(obj.Spec.Strategy.Canary.Steps)-1].Weight = utilpointer.Int32(32)
+				PartitionReplicasLimitWithTraffic = 31
+				obj.Annotations[appsv1alpha1.RolloutStyleAnnotation] = string(appsv1alpha1.PartitionRollingStyle)
+				return []client.Object{obj}
+			},
+		},
+		{
+			Name:    "Partition style with replicas - 1",
+			Succeed: false,
+			GetObject: func() []client.Object {
+				obj := rolloutV1alpha1.DeepCopy()
+				obj.Spec.Strategy.Canary.Steps[len(obj.Spec.Strategy.Canary.Steps)-1].Weight = utilpointer.Int32(50)
+				obj.Spec.Strategy.Canary.Steps[len(obj.Spec.Strategy.Canary.Steps)-1].Replicas = &intstr.IntOrString{Type: intstr.String, StrVal: "32%"}
+				PartitionReplicasLimitWithTraffic = 31
+				obj.Annotations[appsv1alpha1.RolloutStyleAnnotation] = string(appsv1alpha1.PartitionRollingStyle)
+				return []client.Object{obj}
+			},
+		},
+		{
+			Name:    "Partition style with replicas - 2",
+			Succeed: true,
+			GetObject: func() []client.Object {
+				obj := rolloutV1alpha1.DeepCopy()
+				obj.Spec.Strategy.Canary.Steps[len(obj.Spec.Strategy.Canary.Steps)-1].Weight = utilpointer.Int32(50)
+				obj.Spec.Strategy.Canary.Steps[len(obj.Spec.Strategy.Canary.Steps)-1].Replicas = &intstr.IntOrString{Type: intstr.String, StrVal: "31%"}
+				PartitionReplicasLimitWithTraffic = 31
+				obj.Annotations[appsv1alpha1.RolloutStyleAnnotation] = string(appsv1alpha1.PartitionRollingStyle)
+				return []client.Object{obj}
+			},
+		},
+		{
+			Name:    "Partition style with replicas - 3",
+			Succeed: true,
+			GetObject: func() []client.Object {
+				obj := rolloutV1alpha1.DeepCopy()
+				obj.Spec.Strategy.Canary.Steps[len(obj.Spec.Strategy.Canary.Steps)-1].Weight = nil
+				obj.Spec.Strategy.Canary.Steps[len(obj.Spec.Strategy.Canary.Steps)-1].Replicas = &intstr.IntOrString{Type: intstr.String, StrVal: "50%"}
+				obj.Annotations[appsv1alpha1.RolloutStyleAnnotation] = string(appsv1alpha1.PartitionRollingStyle)
+				return []client.Object{obj}
+			},
+		},
+	}
+
+	for _, cs := range cases {
+		t.Run(cs.Name, func(t *testing.T) {
+			objects := cs.GetObject()
+			cli := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).Build()
+			handler := RolloutCreateUpdateHandler{
+				Client: cli,
+			}
+			errList := handler.validateV1alpha1Rollout(objects[0].(*appsv1alpha1.Rollout))
+			t.Log(errList)
+			Expect(len(errList) == 0).Should(Equal(cs.Succeed))
+			// restore PartitionReplicasLimitWithTraffic after each case
+			PartitionReplicasLimitWithTraffic = 30
 		})
 	}
 }

pkg/webhook/rollout/validating/validate_v1alphal_rollout.go

@@ -25,6 +25,7 @@ import (
 	appsv1alpha1 "github.com/openkruise/rollouts/api/v1alpha1"
 	"github.com/openkruise/rollouts/pkg/util"
 	utilclient "github.com/openkruise/rollouts/pkg/util/client"
+	apps "k8s.io/api/apps/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/apimachinery/pkg/util/validation/field"
@@ -70,7 +71,7 @@ func (h *RolloutCreateUpdateHandler) validateV1alpha1RolloutUpdate(oldObj, newOb
 }
 
 func (h *RolloutCreateUpdateHandler) validateV1alpha1Rollout(rollout *appsv1alpha1.Rollout) field.ErrorList {
-	errList := validateV1alpha1RolloutSpec(rollout, field.NewPath("Spec"))
+	errList := validateV1alpha1RolloutSpec(GetContextFromv1alpha1Rollout(rollout), rollout, field.NewPath("Spec"))
 	errList = append(errList, h.validateV1alpha1RolloutConflict(rollout, field.NewPath("Conflict Checker"))...)
 	return errList
 }
@@ -93,10 +94,10 @@ func (h *RolloutCreateUpdateHandler) validateV1alpha1RolloutConflict(rollout *ap
 	return nil
 }
 
-func validateV1alpha1RolloutSpec(rollout *appsv1alpha1.Rollout, fldPath *field.Path) field.ErrorList {
+func validateV1alpha1RolloutSpec(c *validateContext, rollout *appsv1alpha1.Rollout, fldPath *field.Path) field.ErrorList {
 	errList := validateV1alpha1RolloutSpecObjectRef(&rollout.Spec.ObjectRef, fldPath.Child("ObjectRef"))
 	errList = append(errList, validateV1alpha1RolloutRollingStyle(rollout, field.NewPath("RollingStyle"))...)
-	errList = append(errList, validateV1alpha1RolloutSpecStrategy(&rollout.Spec.Strategy, fldPath.Child("Strategy"))...)
+	errList = append(errList, validateV1alpha1RolloutSpecStrategy(c, &rollout.Spec.Strategy, fldPath.Child("Strategy"))...)
 	return errList
 }
 
@@ -122,16 +123,16 @@ func validateV1alpha1RolloutSpecObjectRef(objectRef *appsv1alpha1.ObjectRef, fld
 	return nil
 }
 
-func validateV1alpha1RolloutSpecStrategy(strategy *appsv1alpha1.RolloutStrategy, fldPath *field.Path) field.ErrorList {
+func validateV1alpha1RolloutSpecStrategy(c *validateContext, strategy *appsv1alpha1.RolloutStrategy, fldPath *field.Path) field.ErrorList {
-	return validateV1alpha1RolloutSpecCanaryStrategy(strategy.Canary, fldPath.Child("Canary"))
+	return validateV1alpha1RolloutSpecCanaryStrategy(c, strategy.Canary, fldPath.Child("Canary"))
 }
 
-func validateV1alpha1RolloutSpecCanaryStrategy(canary *appsv1alpha1.CanaryStrategy, fldPath *field.Path) field.ErrorList {
+func validateV1alpha1RolloutSpecCanaryStrategy(c *validateContext, canary *appsv1alpha1.CanaryStrategy, fldPath *field.Path) field.ErrorList {
 	if canary == nil {
 		return field.ErrorList{field.Invalid(fldPath, nil, "Canary cannot be empty")}
 	}
 
-	errList := validateV1alpha1RolloutSpecCanarySteps(canary.Steps, fldPath.Child("Steps"), len(canary.TrafficRoutings) > 0)
+	errList := validateV1alpha1RolloutSpecCanarySteps(c, canary.Steps, fldPath.Child("Steps"), len(canary.TrafficRoutings) > 0)
 	if len(canary.TrafficRoutings) > 1 {
 		errList = append(errList, field.Invalid(fldPath, canary.TrafficRoutings, "Rollout currently only support single TrafficRouting."))
 	}
@@ -169,7 +170,7 @@ func validateV1alpha1RolloutSpecCanaryTraffic(traffic appsv1alpha1.TrafficRoutin
 	return errList
 }
 
-func validateV1alpha1RolloutSpecCanarySteps(steps []appsv1alpha1.CanaryStep, fldPath *field.Path, isTraffic bool) field.ErrorList {
+func validateV1alpha1RolloutSpecCanarySteps(c *validateContext, steps []appsv1alpha1.CanaryStep, fldPath *field.Path, isTraffic bool) field.ErrorList {
 	stepCount := len(steps)
 	if stepCount == 0 {
 		return field.ErrorList{field.Invalid(fldPath, steps, "The number of Canary.Steps cannot be empty")}
@@ -188,6 +189,25 @@ func validateV1alpha1RolloutSpecCanarySteps(steps []appsv1alpha1.CanaryStep, fld
 				return field.ErrorList{field.Invalid(fldPath.Index(i).Child("Replicas"),
 					s.Replicas, `replicas must be positive number, or a percentage with "0%" < canaryReplicas <= "100%"`)}
 			}
+			// is partiton-style release
+			// and has traffic strategy for this step
+			// and replicas is in percentage format and greater than replicasLimitWithTraffic
+			if c.style == string(appsv1alpha1.PartitionRollingStyle) &&
+				IsPercentageCanaryReplicasType(s.Replicas) && canaryReplicas > PartitionReplicasLimitWithTraffic &&
+				(s.Matches != nil || s.Weight != nil) {
+				return field.ErrorList{field.Invalid(fldPath.Index(i).Child("steps"), steps,
+					`For patition style rollout: step[x].replicas must not greater than replicasLimitWithTraffic if traffic or matches specified`)}
+			}
+		} else {
+			// replicas is nil, weight is not nil
+			if c.style == string(appsv1alpha1.PartitionRollingStyle) && *s.Weight > int32(PartitionReplicasLimitWithTraffic) {
+				return field.ErrorList{field.Invalid(fldPath.Index(i).Child("steps"), steps,
+					`For patition style rollout: step[x].weight must not greater than replicasLimitWithTraffic if replicas is not specified`)}
+			}
+			if *s.Weight <= 0 || *s.Weight > 100 {
+				return field.ErrorList{field.Invalid(fldPath.Index(i).Child("Weight"), s.Weight,
+					`weight must be positive number, and less than or equal to replicasLimitWithTraffic (defaults to 30)`)}
+			}
 		}
 	}
 
@@ -225,3 +245,19 @@ func IsSameV1alpha1WorkloadRefGVKName(a, b *appsv1alpha1.WorkloadRef) bool {
 	}
 	return reflect.DeepEqual(a, b)
 }
+
+func GetContextFromv1alpha1Rollout(rollout *appsv1alpha1.Rollout) *validateContext {
+	if rollout.Spec.Strategy.Canary == nil {
+		return nil
+	}
+	style := appsv1alpha1.PartitionRollingStyle
+	switch strings.ToLower(rollout.Annotations[appsv1alpha1.RolloutStyleAnnotation]) {
+	case "", strings.ToLower(string(appsv1alpha1.CanaryRollingStyle)):
+		targetRef := rollout.Spec.ObjectRef.WorkloadRef
+		if targetRef.APIVersion == apps.SchemeGroupVersion.String() && targetRef.Kind == reflect.TypeOf(apps.Deployment{}).Name() {
+			style = appsv1alpha1.CanaryRollingStyle
+		}
+	}
+
+	return &validateContext{style: string(style)}
+}

test/e2e/test_data/rollout/rollout_canary_base.yaml

@@ -1,3 +1,5 @@
+# we recommend that new test cases or modifications to existing test cases should
+# use v1beta1 Rollout, eg. use rollout_v1beta1_canary_base.yaml
 apiVersion: rollouts.kruise.io/v1alpha1
 kind: Rollout
 metadata:

test/e2e/test_data/rollout/rollout_v1beta1_partition_base.yaml

@@ -14,17 +14,13 @@ spec:
       - traffic: 20%
         replicas: 20%
         pause: {}
-      - traffic: 40%
+      - replicas: 40%
-        replicas: 40%
         pause: {duration: 10}
-      - traffic: 60%
+      - replicas: 60%
-        replicas: 60%
         pause: {duration: 10}
-      - traffic: 80%
+      - replicas: 80%
-        replicas: 80%
         pause: {duration: 10}
-      - traffic: 100%
+      - replicas: 100%
-        replicas: 100%
         pause: {duration: 0}
       trafficRoutings:
       - service: echoserver