openkruise
/
rollouts
mirror of https://github.com/openkruise/rollouts.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

nextStepIndex default value from 0 to -1 

Signed-off-by: yunbo <yunbo10124scut@gmail.com>
This commit is contained in:
yunbo 2024-06-07 14:56:02 +08:00
parent ba359f32a4
commit 3b034869e5
9 changed files with 101 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
api/v1alpha1/deployment_types.go
View File

@ -61,8 +61,6 @@ const (
CanaryRollingStyle RollingStyleType = "Canary"
// BlueGreenRollingStyle means rolling in blue-green way, and will NOT create a canary Deployment.
BlueGreenRollingStyle RollingStyleType = "BlueGreen"
// Empty means both Canary and BlueGreen are empty
EmptyRollingStyle RollingStyleType = "Empty"
)
// DeploymentExtraStatus is extra status field for Advanced Deployment

18
api/v1beta1/deployment_types.go
View File

@ -38,9 +38,9 @@ const (
// which labels whether the deployment is controlled by advanced-deployment-controller.
AdvancedDeploymentControlLabel = "rollouts.kruise.io/controlled-by-advanced-deployment-controller"
// OriginalSettingAnnotation is annotation for workload in BlueGreen Release,
// it will storage the original setting of the workload, which will be used to restore the workload
OriginalSettingAnnotation = "rollouts.kruise.io/original-setting"
// OriginalDeploymentStrategyAnnotation is annotation for workload in BlueGreen Release,
// it will store the original setting of the workload, which will be used to restore the workload
OriginalDeploymentStrategyAnnotation = "rollouts.kruise.io/original-deployment-strategy"
// MaxProgressSeconds is the value we set for ProgressDeadlineSeconds
// MaxReadySeconds is the value we set for MinReadySeconds, which is one less than ProgressDeadlineSeconds
@ -62,12 +62,12 @@ type DeploymentStrategy struct {
Partition intstr.IntOrString `json:"partition,omitempty"`
}
// OriginalSetting storages part of the fileds of a workload,
// OriginalDeploymentStrategy stores part of the fileds of a workload,
// so that it can be restored when finalizing.
// It is only used for BlueGreen Release
// Similar to DeploymentStrategy, it is stored in workload annotation
// However, unlike DeploymentStrategy, it is only used to storage and restore
type OriginalSetting struct {
// Similar to DeploymentStrategy, it is an annotation used in workload
// However, unlike DeploymentStrategy, it is only used to store and restore the user's strategy
type OriginalDeploymentStrategy struct {
// The deployment strategy to use to replace existing pods with new ones.
// +optional
// +patchStrategy=retainKeys
@ -96,8 +96,6 @@ const (
CanaryRollingStyle RollingStyleType = "Canary"
// BlueGreenRollingStyle means rolling in blue-green way, and will NOT create a extra Deployment.
BlueGreenRollingStyle RollingStyleType = "BlueGreen"
// Empty means both Canary and BlueGreen are empty
EmptyRollingStyle RollingStyleType = "Empty"
)
// DeploymentExtraStatus is extra status field for Advanced Deployment
@ -141,7 +139,7 @@ func SetDefaultDeploymentStrategy(strategy *DeploymentStrategy) {
}
}
func SetDefaultSetting(setting *OriginalSetting) {
func SetDefaultSetting(setting *OriginalDeploymentStrategy) {
if setting.ProgressDeadlineSeconds == nil {
setting.ProgressDeadlineSeconds = new(int32)
*setting.ProgressDeadlineSeconds = 600

8
api/v1beta1/rollout_types.go
View File

@ -81,9 +81,6 @@ type RolloutStrategy struct {
// Get the rolling style based on the strategy
func (r *RolloutStrategy) GetRollingStyle() RollingStyleType {
if r.BlueGreen == nil && r.Canary == nil {
return EmptyRollingStyle
}
if r.BlueGreen != nil {
return BlueGreenRollingStyle
}
@ -105,11 +102,6 @@ func (r *RolloutStrategy) IsCanaryStragegy() bool {
return r.GetRollingStyle() == CanaryRollingStyle || r.GetRollingStyle() == PartitionRollingStyle
}
// r.GetRollingStyle() == EmptyRollingStyle
func (r *RolloutStrategy) IsEmptyRelease() bool {
return r.GetRollingStyle() == EmptyRollingStyle
}
// Get the steps based on the rolling style
func (r *RolloutStrategy) GetSteps() []CanaryStep {
switch r.GetRollingStyle() {

8
api/v1beta1/zz_generated.deepcopy.go
View File

@ -411,7 +411,7 @@ func (in *ObjectRef) DeepCopy() *ObjectRef {
}
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
func (in *OriginalSetting) DeepCopyInto(out *OriginalSetting) {
func (in *OriginalDeploymentStrategy) DeepCopyInto(out *OriginalDeploymentStrategy) {
*out = *in
if in.Strategy != nil {
in, out := &in.Strategy, &out.Strategy
@ -425,12 +425,12 @@ func (in *OriginalSetting) DeepCopyInto(out *OriginalSetting) {
}
}
// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OriginalSetting.
func (in *OriginalSetting) DeepCopy() *OriginalSetting {
// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OriginalDeploymentStrategy.
func (in *OriginalDeploymentStrategy) DeepCopy() *OriginalDeploymentStrategy {
if in == nil {
return nil
}
out := new(OriginalSetting)
out := new(OriginalDeploymentStrategy)
in.DeepCopyInto(out)
return out
}

4
pkg/controller/rollout/rollout_progressing_test.go
View File

@ -67,10 +67,6 @@ func TestReconcileRolloutProgressing(t *testing.T) {
s.CanaryStatus.StableRevision = "pod-template-hash-v1"
s.CanaryStatus.CanaryRevision = "6f8cc56547"
s.CanaryStatus.CurrentStepIndex = 1
// s.CanaryStatus.NextStepIndex will be initialized as 0 in ReconcileRolloutProgressing
// util.NextBatchIndex(rollout, s.CanaryStatus.CurrentStepIndex), which is 2 here.
// However the ReconcileRolloutProgressing won't update it, and thus expected value of it
// in the next cases will be 0 (zero in zero out), without update. This is as expected
s.CanaryStatus.NextStepIndex = 2
s.CanaryStatus.CurrentStepState = v1beta1.CanaryStepStateUpgrade
return s

2
pkg/util/rollout_utils.go
View File

@ -175,7 +175,7 @@ func NextBatchIndex(rollout *rolloutv1beta1.Rollout, CurrentStepIndex int32) int
// Patching NextStepIndex to 0 is meaningless for user, if doing so, nothing happen
// and step jump won't happen
if CurrentStepIndex >= allSteps {
return 0
return -1
}
return CurrentStepIndex + 1
}

70
pkg/webhook/rollout/validating/rollout_create_update_handler.go
View File

@ -133,12 +133,11 @@ func (h *RolloutCreateUpdateHandler) validateRolloutUpdate(oldObj, newObj *appsv
if !reflect.DeepEqual(oldObj.Spec.WorkloadRef, newObj.Spec.WorkloadRef) {
return field.ErrorList{field.Forbidden(field.NewPath("Spec.ObjectRef"), "Rollout 'ObjectRef' field is immutable")}
}
// canary strategy
if !reflect.DeepEqual(oldObj.Spec.Strategy.Canary.TrafficRoutings, newObj.Spec.Strategy.Canary.TrafficRoutings) {
return field.ErrorList{field.Forbidden(field.NewPath("Spec.Strategy.Canary.TrafficRoutings"), "Rollout 'Strategy.Canary.TrafficRoutings' field is immutable")}
if !reflect.DeepEqual(oldObj.Spec.Strategy.GetTrafficRouting(), newObj.Spec.Strategy.GetTrafficRouting()) {
return field.ErrorList{field.Forbidden(field.NewPath("Spec.Strategy.Canary|BlueGreen.TrafficRoutings"), "Rollout 'Strategy.Canary|BlueGreen.TrafficRoutings' field is immutable")}
}
if oldObj.Spec.Strategy.Canary.EnableExtraWorkloadForCanary != newObj.Spec.Strategy.Canary.EnableExtraWorkloadForCanary {
return field.ErrorList{field.Forbidden(field.NewPath("Spec.Strategy.Canary"), "Rollout enableExtraWorkloadForCanary is immutable")}
if oldObj.Spec.Strategy.GetRollingStyle() != newObj.Spec.Strategy.GetRollingStyle() {
return field.ErrorList{field.Forbidden(field.NewPath("Spec.Strategy.Canary|BlueGreen"), "Rollout style and enableExtraWorkloadForCanary are immutable")}
}
}
@ -198,15 +197,32 @@ func validateRolloutSpecObjectRef(workloadRef *appsv1beta1.ObjectRef, fldPath *f
}
func validateRolloutSpecStrategy(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")}
}
if strategy.Canary != nil && strategy.BlueGreen != nil {
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 validateRolloutSpecCanaryStrategy(strategy.Canary, fldPath.Child("Canary"))
}
func validateRolloutSpecCanaryStrategy(canary *appsv1beta1.CanaryStrategy, fldPath *field.Path) field.ErrorList {
if canary == nil {
return field.ErrorList{field.Invalid(fldPath, nil, "Canary cannot be empty")}
}
type TrafficRule string
errList := validateRolloutSpecCanarySteps(canary.Steps, fldPath.Child("Steps"), len(canary.TrafficRoutings) > 0)
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."))
}
@ -216,6 +232,21 @@ 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)
if len(blueGreen.TrafficRoutings) > 1 {
errList = append(errList, field.Invalid(fldPath, blueGreen.TrafficRoutings, "Rollout currently only support single TrafficRouting."))
}
for _, traffic := range blueGreen.TrafficRoutings {
errList = append(errList, validateRolloutSpecCanaryTraffic(traffic, fldPath.Child("TrafficRouting"))...)
}
return errList
}
func validateRolloutSpecCanaryTraffic(traffic appsv1beta1.TrafficRoutingRef, fldPath *field.Path) field.ErrorList {
errList := field.ErrorList{}
if len(traffic.Service) == 0 {
@ -240,7 +271,7 @@ func validateRolloutSpecCanaryTraffic(traffic appsv1beta1.TrafficRoutingRef, fld
return errList
}
func validateRolloutSpecCanarySteps(steps []appsv1beta1.CanaryStep, fldPath *field.Path, isTraffic bool) field.ErrorList {
func validateRolloutSpecCanarySteps(steps []appsv1beta1.CanaryStep, fldPath *field.Path, trafficRule TrafficRule) field.ErrorList {
stepCount := len(steps)
if stepCount == 0 {
return field.ErrorList{field.Invalid(fldPath, steps, "The number of Canary.Steps cannot be empty")}
@ -258,14 +289,21 @@ 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 !isTraffic {
if trafficRule == NoTraffic || s.Traffic == nil {
continue
}
if s.Traffic != nil {
is := intstr.FromString(*s.Traffic)
weight, err := intstr.GetScaledValueFromIntOrPercent(&is, 100, true)
is := intstr.FromString(*s.Traffic)
weight, err := intstr.GetScaledValueFromIntOrPercent(&is, 100, true)
switch trafficRule {
case TrafficRuleBlueGreen:
// 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`)}
}
default:
// traffic "0%" is not allowed in canary 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%"`)}
return field.ErrorList{field.Invalid(fldPath.Index(i).Child("steps"), steps, `traffic must be percentage with "0%" < traffic <= "100%" in canary strategy`)}
}
}
}

21
test/e2e/deployment_test.go
View File

@ -26,6 +26,16 @@ import (
var _ = SIGDescribe("Advanced Deployment", func() {
var namespace string
DumpAllResources := func() {
deploy := &apps.DeploymentList{}
k8sClient.List(context.TODO(), deploy, client.InNamespace(namespace))
fmt.Println(util.DumpJSON(deploy))
rs := &apps.ReplicaSetList{}
k8sClient.List(context.TODO(), rs, client.InNamespace(namespace))
fmt.Println(util.DumpJSON(rs))
}
defaultRetry := wait.Backoff{
Steps: 10,
Duration: 10 * time.Millisecond,
@ -132,7 +142,12 @@ var _ = SIGDescribe("Advanced Deployment", func() {
CheckReplicas := func(deployment *apps.Deployment, replicas, available, updated int32) {
var clone *apps.Deployment
start := time.Now()
Eventually(func() bool {
if start.Add(time.Minute * 2).Before(time.Now()) {
DumpAllResources()
Expect(true).Should(BeFalse())
}
clone = &apps.Deployment{}
err := GetObject(deployment.Namespace, deployment.Name, clone)
Expect(err).NotTo(HaveOccurred())
@ -239,6 +254,7 @@ var _ = SIGDescribe("Advanced Deployment", func() {
deployment.Namespace = namespace
Expect(ReadYamlToObject("./test_data/deployment/deployment.yaml", deployment)).ToNot(HaveOccurred())
CreateObject(deployment)
CheckReplicas(deployment, 5, 5, 5)
UpdateDeployment(deployment, "version2")
UpdatePartitionWithCheck(deployment, intstr.FromInt(0))
UpdatePartitionWithCheck(deployment, intstr.FromInt(1))
@ -255,6 +271,7 @@ var _ = SIGDescribe("Advanced Deployment", func() {
Expect(ReadYamlToObject("./test_data/deployment/deployment.yaml", deployment)).ToNot(HaveOccurred())
deployment.Spec.Replicas = pointer.Int32(10)
CreateObject(deployment)
CheckReplicas(deployment, 10, 10, 10)
UpdateDeployment(deployment, "version2")
UpdatePartitionWithCheck(deployment, intstr.FromString("0%"))
UpdatePartitionWithCheck(deployment, intstr.FromString("40%"))
@ -287,6 +304,7 @@ var _ = SIGDescribe("Advanced Deployment", func() {
`{"rollingStyle":"Partition","rollingUpdate":{"maxUnavailable":1,"maxSurge":0}}`
deployment.Spec.MinReadySeconds = 10
CreateObject(deployment)
CheckReplicas(deployment, 5, 5, 5)
UpdateDeployment(deployment, "version2")
UpdatePartitionWithCheck(deployment, intstr.FromInt(0))
UpdatePartitionWithoutCheck(deployment, intstr.FromInt(3))
@ -303,6 +321,7 @@ var _ = SIGDescribe("Advanced Deployment", func() {
deployment.Namespace = namespace
Expect(ReadYamlToObject("./test_data/deployment/deployment.yaml", deployment)).ToNot(HaveOccurred())
CreateObject(deployment)
CheckReplicas(deployment, 5, 5, 5)
UpdateDeployment(deployment, "version2")
UpdatePartitionWithCheck(deployment, intstr.FromInt(0))
UpdatePartitionWithCheck(deployment, intstr.FromInt(2))
@ -317,6 +336,7 @@ var _ = SIGDescribe("Advanced Deployment", func() {
deployment.Namespace = namespace
Expect(ReadYamlToObject("./test_data/deployment/deployment.yaml", deployment)).ToNot(HaveOccurred())
CreateObject(deployment)
CheckReplicas(deployment, 5, 5, 5)
UpdateDeployment(deployment, "version2")
UpdatePartitionWithCheck(deployment, intstr.FromInt(0))
UpdatePartitionWithCheck(deployment, intstr.FromInt(2))
@ -335,6 +355,7 @@ var _ = SIGDescribe("Advanced Deployment", func() {
Expect(ReadYamlToObject("./test_data/deployment/deployment.yaml", deployment)).ToNot(HaveOccurred())
deployment.Annotations["rollouts.kruise.io/deployment-strategy"] = `{"rollingUpdate":{"maxUnavailable":0,"maxSurge":1}}`
CreateObject(deployment)
CheckReplicas(deployment, 5, 5, 5)
UpdateDeployment(deployment, "version2", "busybox:not-exists")
UpdatePartitionWithoutCheck(deployment, intstr.FromInt(1))
CheckReplicas(deployment, 6, 5, 1)

19
test/e2e/rollout_test.go
View File

@ -204,18 +204,25 @@ var _ = SIGDescribe("Rollout", func() {
}
ResumeRolloutCanary := func(name string) {
clone := &v1alpha1.Rollout{}
Expect(GetObject(name, clone)).NotTo(HaveOccurred())
currentIndex := clone.Status.CanaryStatus.CurrentStepIndex
Eventually(func() bool {
clone := &v1alpha1.Rollout{}
Expect(GetObject(name, clone)).NotTo(HaveOccurred())
if clone.Status.CanaryStatus.CurrentStepState != v1alpha1.CanaryStepStatePaused {
if clone.Status.CanaryStatus.CurrentStepIndex == currentIndex && clone.Status.CanaryStatus.CurrentStepState == v1alpha1.CanaryStepStatePaused {
klog.Info("patch to stepReady")
body := fmt.Sprintf(`{"status":{"canaryStatus":{"currentStepState":"%s"}}}`, v1alpha1.CanaryStepStateReady)
Expect(k8sClient.Status().Patch(context.TODO(), clone, client.RawPatch(types.MergePatchType, []byte(body)))).NotTo(HaveOccurred())
return false
} else {
fmt.Println("resume rollout success, and CurrentStepState", util.DumpJSON(clone.Status))
return true
}
body := fmt.Sprintf(`{"status":{"canaryStatus":{"currentStepState":"%s"}}}`, v1alpha1.CanaryStepStateReady)
Expect(k8sClient.Status().Patch(context.TODO(), clone, client.RawPatch(types.MergePatchType, []byte(body)))).NotTo(HaveOccurred())
return false
}, 10*time.Second, time.Second).Should(BeTrue())
// interval was critical before:
// too small: StepReady could be overidden by StepPaused
// too big: StepReady could progress to StepPaused of next Step
}, 10*time.Second, 2*time.Second).Should(BeTrue())
}
WaitDeploymentAllPodsReady := func(deployment *apps.Deployment) {