Skip to content

Commit

Permalink
webhook validate v1beta1 rollout (#188)
Browse files Browse the repository at this point in the history
Signed-off-by: liheng.zms <[email protected]>
  • Loading branch information
zmberg authored Dec 18, 2023
1 parent 9dcf365 commit 75b1b90
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 25 deletions.
1 change: 1 addition & 0 deletions config/webhook/manifests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ webhooks:
- rollouts.kruise.io
apiVersions:
- v1alpha1
- v1beta1
operations:
- CREATE
- UPDATE
Expand Down
7 changes: 7 additions & 0 deletions pkg/controller/rollout/rollout_canary.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}

Expand Down
26 changes: 6 additions & 20 deletions pkg/webhook/rollout/validating/rollout_create_update_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")}
Expand Down Expand Up @@ -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%"`)}
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/webhook/rollout/validating/webhooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
37 changes: 35 additions & 2 deletions test/e2e/rollout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...")
Expand Down Expand Up @@ -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"})
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 75b1b90

Please sign in to comment.