From e82236c8430a414097c31a1510492980eb916c9d Mon Sep 17 00:00:00 2001 From: yunbo Date: Fri, 19 Jul 2024 13:46:38 +0800 Subject: [PATCH] add restriction for traffic configuration of partition-style step Signed-off-by: yunbo --- api/v1beta1/rollout_types.go | 8 + .../rollout_create_update_handler.go | 80 +++--- .../rollout_create_update_handler_test.go | 251 ++++++++++++++++++ .../validating/validate_v1alphal_rollout.go | 60 ++++- .../rollout/rollout_canary_base.yaml | 5 + 5 files changed, 366 insertions(+), 38 deletions(-) diff --git a/api/v1beta1/rollout_types.go b/api/v1beta1/rollout_types.go index c0b862e7..1c350df1 100644 --- a/api/v1beta1/rollout_types.go +++ b/api/v1beta1/rollout_types.go @@ -44,6 +44,14 @@ const ( // RollbackInBatchAnnotation is set to rollout annotations. // RollbackInBatchAnnotation allow use disable quick rollback, and will roll back in batch style. RollbackInBatchAnnotation = "rollouts.kruise.io/rollback-in-batch" + + // PartitionReplicasLimitWithTraffic is set to rollout annotations. + // PartitionReplicasLimitWithTraffic represents the maximum percentage of replicas + // allowed for a step of partition-style release, with 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 30%. + PartitionReplicasLimitWithTraffic = "rollouts.kruise.io/partition-replicas-limit" ) // RolloutSpec defines the desired state of Rollout diff --git a/pkg/webhook/rollout/validating/rollout_create_update_handler.go b/pkg/webhook/rollout/validating/rollout_create_update_handler.go index 08bfacf2..77c03aa5 100644 --- a/pkg/webhook/rollout/validating/rollout_create_update_handler.go +++ b/pkg/webhook/rollout/validating/rollout_create_update_handler.go @@ -49,6 +49,13 @@ type RolloutCreateUpdateHandler struct { var _ admission.Handler = &RolloutCreateUpdateHandler{} +const defaultReplicasLimitWithTraffic = 30 + +type validateContext struct { + replicasLimitWithTraffic int + style string +} + // Handle handles admission requests. func (h *RolloutCreateUpdateHandler) Handle(ctx context.Context, req admission.Request) admission.Response { switch req.Operation { @@ -155,7 +162,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 +185,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 +203,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 +211,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 - -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) +func validateRolloutSpecCanaryStrategy(c *validateContext, canary *appsv1beta1.CanaryStrategy, fldPath *field.Path) field.ErrorList { + errList := validateRolloutSpecCanarySteps(c, canary.Steps, fldPath.Child("Steps")) if len(canary.TrafficRoutings) > 1 { errList = append(errList, field.Invalid(fldPath, canary.TrafficRoutings, "Rollout currently only support single TrafficRouting.")) } @@ -232,12 +227,8 @@ func validateRolloutSpecCanaryStrategy(canary *appsv1beta1.CanaryStrategy, fldPa return errList } -func validateRolloutSpecBlueGreenStrategy(blueGreen *appsv1beta1.BlueGreenStrategy, fldPath *field.Path) field.ErrorList { - trafficRule := NoTraffic - if len(blueGreen.TrafficRoutings) > 0 { - trafficRule = TrafficRuleBlueGreen - } - errList := validateRolloutSpecCanarySteps(blueGreen.Steps, fldPath.Child("Steps"), trafficRule) +func validateRolloutSpecBlueGreenStrategy(c *validateContext, blueGreen *appsv1beta1.BlueGreenStrategy, fldPath *field.Path) field.ErrorList { + errList := validateRolloutSpecCanarySteps(c, blueGreen.Steps, fldPath.Child("Steps")) if len(blueGreen.TrafficRoutings) > 1 { errList = append(errList, field.Invalid(fldPath, blueGreen.TrafficRoutings, "Rollout currently only support single TrafficRouting.")) } @@ -275,7 +266,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 +284,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 > c.replicasLimitWithTraffic { + 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 specified`)} + } + } + if s.Traffic == nil { continue } is := intstr.FromString(*s.Traffic) weight, err := intstr.GetScaledValueFromIntOrPercent(&is, 100, true) - switch trafficRule { - case TrafficRuleBlueGreen: + switch c.style { + 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 +357,21 @@ 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 + } + replicasLimitWithTraffic := defaultReplicasLimitWithTraffic + if limit, ok := rollout.Annotations[appsv1beta1.PartitionReplicasLimitWithTraffic]; ok { + strVal := intstr.FromString(limit) + if val, err := intstr.GetScaledValueFromIntOrPercent(&strVal, 100, true); err == nil && 0 < val && val <= 100 { + replicasLimitWithTraffic = val + } + } + return &validateContext{style: string(style), replicasLimitWithTraffic: replicasLimitWithTraffic} +} 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 27ea4531..ec84499d 100644 --- a/pkg/webhook/rollout/validating/rollout_create_update_handler_test.go +++ b/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,65 @@ 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), + }, + // Replicas: &intstr.IntOrString{Type: intstr.Int, IntVal: int32(1)}, + 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), + }, + // Replicas: &intstr.IntOrString{Type: intstr.Int, IntVal: int32(20)}, + }, + }, + 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 +322,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() + object.Annotations[appsv1beta1.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() + object.Annotations[appsv1beta1.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, @@ -339,6 +467,129 @@ func TestRolloutValidateCreate(t *testing.T) { } } +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() + obj.Annotations[appsv1beta1.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) + obj.Annotations[appsv1beta1.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) + obj.Annotations[appsv1beta1.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) + obj.Annotations[appsv1beta1.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%"} + obj.Annotations[appsv1beta1.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%"} + obj.Annotations[appsv1beta1.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)) + }) + } +} + func TestRolloutValidateUpdate(t *testing.T) { RegisterFailHandler(Fail) diff --git a/pkg/webhook/rollout/validating/validate_v1alphal_rollout.go b/pkg/webhook/rollout/validating/validate_v1alphal_rollout.go index 19c230a7..aa680b92 100644 --- a/pkg/webhook/rollout/validating/validate_v1alphal_rollout.go +++ b/pkg/webhook/rollout/validating/validate_v1alphal_rollout.go @@ -23,8 +23,10 @@ import ( "strings" appsv1alpha1 "github.com/openkruise/rollouts/api/v1alpha1" + appsv1beta1 "github.com/openkruise/rollouts/api/v1beta1" "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 +72,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 +95,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 +124,16 @@ func validateV1alpha1RolloutSpecObjectRef(objectRef *appsv1alpha1.ObjectRef, fld return nil } -func validateV1alpha1RolloutSpecStrategy(strategy *appsv1alpha1.RolloutStrategy, fldPath *field.Path) field.ErrorList { - return validateV1alpha1RolloutSpecCanaryStrategy(strategy.Canary, fldPath.Child("Canary")) +func validateV1alpha1RolloutSpecStrategy(c *validateContext, strategy *appsv1alpha1.RolloutStrategy, fldPath *field.Path) field.ErrorList { + 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 +171,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 +190,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%"`)} } + // - partiton-style release + // - replicas is in percentage format and greater than replicasLimitWithTraffic + // - has traffic strategy for this step + if c.style == string(appsv1alpha1.PartitionRollingStyle) && + IsPercentageCanaryReplicasType(s.Replicas) && canaryReplicas > c.replicasLimitWithTraffic && + (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(c.replicasLimitWithTraffic) { + 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 +246,26 @@ 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 + } + } + + replicasLimitWithTraffic := defaultReplicasLimitWithTraffic + if limit, ok := rollout.Annotations[appsv1beta1.PartitionReplicasLimitWithTraffic]; ok { + strVal := intstr.FromString(limit) + if val, err := intstr.GetScaledValueFromIntOrPercent(&strVal, 100, true); err == nil && 0 < val && val <= 100 { + replicasLimitWithTraffic = val + } + } + return &validateContext{style: string(style), replicasLimitWithTraffic: replicasLimitWithTraffic} +} diff --git a/test/e2e/test_data/rollout/rollout_canary_base.yaml b/test/e2e/test_data/rollout/rollout_canary_base.yaml index fb60df39..de151198 100644 --- a/test/e2e/test_data/rollout/rollout_canary_base.yaml +++ b/test/e2e/test_data/rollout/rollout_canary_base.yaml @@ -2,6 +2,11 @@ apiVersion: rollouts.kruise.io/v1alpha1 kind: Rollout metadata: name: rollouts-demo + # add this annotation to ensure compatibility with v1alpha1 e2e test cases + # Test cases to add in the future SHOULD use v1beta1 rollout instead + # for example, use rollout_v1beta1_canary_base.yaml or rollout_v1beta1_blue_green_base.yaml + annotations: + rollouts.kruise.io/partition-replicas-limit: 100% spec: objectRef: workloadRef: