From 89705868311a4885abc167ac5905b52c7ec9af4a Mon Sep 17 00:00:00 2001 From: Bryce-Huang <44901318+Bryce-huang@users.noreply.github.com> Date: Fri, 15 Sep 2023 10:41:10 +0800 Subject: [PATCH] Add CanaryStrategy instead of CanarySteps Signed-off-by: Bryce-Huang <44901318+Bryce-huang@users.noreply.github.com> Signed-off-by: Bryce-Huang <44901318+Bryce-huang@users.noreply.github.com> --- apis/core/v1beta2/function_types.go | 18 +++- apis/core/v1beta2/function_webhook.go | 12 +-- apis/core/v1beta2/function_webhook_test.go | 86 +++++++++++++++---- apis/core/v1beta2/zz_generated.deepcopy.go | 37 ++++++-- config/bundle.yaml | 39 ++++++--- .../bases/core.openfunction.io_functions.yaml | 39 ++++++--- controllers/core/function_controller.go | 18 +++- 7 files changed, 186 insertions(+), 63 deletions(-) diff --git a/apis/core/v1beta2/function_types.go b/apis/core/v1beta2/function_types.go index 0d583a91..d9703387 100644 --- a/apis/core/v1beta2/function_types.go +++ b/apis/core/v1beta2/function_types.go @@ -134,12 +134,23 @@ type FunctionSpec struct { Build *BuildImpl `json:"build,omitempty"` // Information needed to run a function. The serving step will be skipped if `Serving` is nil. Serving *ServingImpl `json:"serving,omitempty"` + // Canary strategy for a function + // +optional + CanaryStrategy *CanaryStrategy `json:"canaryStrategy,omitempty"` +} +type CanaryStrategy struct { // Canary release steps for a function + // +optional CanarySteps []CanaryStep `json:"canarySteps,omitempty"` } + type CanaryStep struct { + // Weight indicate how many percentage of traffic the canary function should receive + // +optional Weight *int32 `json:"weight,omitempty"` - Pause Pause `json:"pause,omitempty"` + // Pause defines a pause stage for a canary step, manual or auto + // +optional + Pause Pause `json:"pause,omitempty"` } type Pause struct { // Duration the amount of time to wait before moving to the next step. @@ -240,8 +251,9 @@ type FunctionStatus struct { CanaryStatus *CanaryStatus `json:"canaryStatus,omitempty"` // Addresses holds the addresses that used to access the Function. // +optional - Addresses []FunctionAddress `json:"addresses,omitempty"` - Revision *Revision `json:"revision,omitempty"` + Addresses []FunctionAddress `json:"addresses,omitempty"` + Revision *Revision `json:"revision,omitempty"` + StableRevision *Revision `json:"stableRevision,omitempty"` // Sources holds the results emitted from the step definition // of different sources // diff --git a/apis/core/v1beta2/function_webhook.go b/apis/core/v1beta2/function_webhook.go index 0f80cb75..0cb9ad56 100644 --- a/apis/core/v1beta2/function_webhook.go +++ b/apis/core/v1beta2/function_webhook.go @@ -232,8 +232,8 @@ func (r *Function) Validate() error { "must be specified when `spec.build` is not enabled") } - if r.Spec.CanarySteps != nil && len(r.Spec.CanarySteps) > 0 { - if err := r.ValidateCanarySteps(field.NewPath("spec", "canarySteps")); err != nil { + if r.Spec.CanaryStrategy != nil && len(r.Spec.CanaryStrategy.CanarySteps) > 0 { + if err := r.ValidCanaryStrategy(field.NewPath("spec", "canaryStrategy")); err != nil { return err } } @@ -292,15 +292,15 @@ func (r *Function) ValidateBuild() error { return nil } -func (r *Function) ValidateCanarySteps(fldPath *field.Path) error { - steps := r.Spec.CanarySteps +func (r *Function) ValidCanaryStrategy(fldPath *field.Path) error { + steps := r.Spec.CanaryStrategy.CanarySteps for i, step := range steps { weight := step.Weight if weight == nil { - return field.Invalid(fldPath.Index(i).Child("weight"), steps, `Weight cannot be empty`) + return field.Invalid(fldPath.Index(i).Child("canarySteps").Child("weight"), steps, `Weight cannot be empty`) } if *weight < 1 || *weight > 100 { - return field.Invalid(fldPath.Index(i).Child("weight"), steps, `Weight cannot be less than 1 or greater than 100`) + return field.Invalid(fldPath.Index(i).Child("canarySteps").Child("weight"), steps, `Weight cannot be less than 1 or greater than 100`) } if step.Pause.Duration != nil { if *step.Pause.Duration < 1 { diff --git a/apis/core/v1beta2/function_webhook_test.go b/apis/core/v1beta2/function_webhook_test.go index 100c1cb1..e00c204f 100644 --- a/apis/core/v1beta2/function_webhook_test.go +++ b/apis/core/v1beta2/function_webhook_test.go @@ -720,17 +720,19 @@ func Test_Validate(t *testing.T) { Serving: &ServingImpl{ Triggers: &Triggers{Http: &HttpTrigger{Engine: (*Engine)(utilpointer.String(string(HttpEngineKnative)))}}, }, - CanarySteps: []CanaryStep{ - { - Weight: utilpointer.Int32(10), - Pause: Pause{ - Duration: utilpointer.Int32(10000), + CanaryStrategy: &CanaryStrategy{ + CanarySteps: []CanaryStep{ + { + Weight: utilpointer.Int32(10), + Pause: Pause{ + Duration: utilpointer.Int32(10000), + }, }, - }, - { - Weight: utilpointer.Int32(20), - Pause: Pause{ - Duration: utilpointer.Int32(10000), + { + Weight: utilpointer.Int32(20), + Pause: Pause{ + Duration: utilpointer.Int32(10000), + }, }, }, }, @@ -745,9 +747,47 @@ func Test_Validate(t *testing.T) { Serving: &ServingImpl{ Triggers: &Triggers{Http: &HttpTrigger{Engine: (*Engine)(utilpointer.String(string(HttpEngineKnative)))}}, }, - CanarySteps: []CanaryStep{ - { - Weight: utilpointer.Int32(-1), + CanaryStrategy: &CanaryStrategy{ + CanarySteps: []CanaryStep{ + { + Weight: utilpointer.Int32(10), + Pause: Pause{ + Duration: utilpointer.Int32(10000), + }, + }, + { + Weight: utilpointer.Int32(20), + Pause: Pause{ + Duration: utilpointer.Int32(10000), + }, + }, + }, + }, + }, + }, + wantErr: false, + }, { + name: "function.spec.canarySteps.weight-error", + r: Function{ + Spec: FunctionSpec{ + Image: "test", + Serving: &ServingImpl{ + Triggers: &Triggers{Http: &HttpTrigger{Engine: (*Engine)(utilpointer.String(string(HttpEngineKnative)))}}, + }, + CanaryStrategy: &CanaryStrategy{ + CanarySteps: []CanaryStep{ + { + Weight: utilpointer.Int32(20), + Pause: Pause{ + Duration: utilpointer.Int32(10000), + }, + }, + { + Weight: utilpointer.Int32(10), + Pause: Pause{ + Duration: utilpointer.Int32(10000), + }, + }, }, }, }, @@ -761,15 +801,25 @@ func Test_Validate(t *testing.T) { Serving: &ServingImpl{ Triggers: &Triggers{Http: &HttpTrigger{Engine: (*Engine)(utilpointer.String(string(HttpEngineKnative)))}}, }, - CanarySteps: []CanaryStep{ - { - Weight: utilpointer.Int32(10), - Pause: Pause{Duration: utilpointer.Int32(-1)}, + CanaryStrategy: &CanaryStrategy{ + CanarySteps: []CanaryStep{ + { + Weight: utilpointer.Int32(10), + Pause: Pause{ + Duration: utilpointer.Int32(10000), + }, + }, + { + Weight: utilpointer.Int32(20), + Pause: Pause{ + Duration: utilpointer.Int32(10000), + }, + }, }, }, }, }, - wantErr: true, + wantErr: false, }, } diff --git a/apis/core/v1beta2/zz_generated.deepcopy.go b/apis/core/v1beta2/zz_generated.deepcopy.go index 93c87096..76492026 100644 --- a/apis/core/v1beta2/zz_generated.deepcopy.go +++ b/apis/core/v1beta2/zz_generated.deepcopy.go @@ -302,6 +302,28 @@ func (in *CanaryStep) DeepCopy() *CanaryStep { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CanaryStrategy) DeepCopyInto(out *CanaryStrategy) { + *out = *in + if in.CanarySteps != nil { + in, out := &in.CanarySteps, &out.CanarySteps + *out = make([]CanaryStep, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CanaryStrategy. +func (in *CanaryStrategy) DeepCopy() *CanaryStrategy { + if in == nil { + return nil + } + out := new(CanaryStrategy) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *CommonRouteSpec) DeepCopyInto(out *CommonRouteSpec) { *out = *in @@ -541,12 +563,10 @@ func (in *FunctionSpec) DeepCopyInto(out *FunctionSpec) { *out = new(ServingImpl) (*in).DeepCopyInto(*out) } - if in.CanarySteps != nil { - in, out := &in.CanarySteps, &out.CanarySteps - *out = make([]CanaryStep, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } + if in.CanaryStrategy != nil { + in, out := &in.CanaryStrategy, &out.CanaryStrategy + *out = new(CanaryStrategy) + (*in).DeepCopyInto(*out) } } @@ -600,6 +620,11 @@ func (in *FunctionStatus) DeepCopyInto(out *FunctionStatus) { *out = new(Revision) **out = **in } + if in.StableRevision != nil { + in, out := &in.StableRevision, &out.StableRevision + *out = new(Revision) + **out = **in + } if in.Sources != nil { in, out := &in.Sources, &out.Sources *out = make([]SourceResult, len(*in)) diff --git a/config/bundle.yaml b/config/bundle.yaml index eb74b5cb..65220213 100644 --- a/config/bundle.yaml +++ b/config/bundle.yaml @@ -11557,23 +11557,31 @@ spec: required: - srcRepo type: object - canarySteps: - description: Canary release steps for a function - items: - properties: - pause: + canaryStrategy: + description: Canary strategy for a function + properties: + canarySteps: + description: Canary release steps for a function + items: properties: - duration: - description: Duration the amount of time to wait before - moving to the next step. + pause: + description: Pause defines a pause stage for a canary step, + manual or auto + properties: + duration: + description: Duration the amount of time to wait before + moving to the next step. + format: int32 + type: integer + type: object + weight: + description: Weight indicate how many percentage of traffic + the canary function should receive format: int32 type: integer type: object - weight: - format: int32 - type: integer - type: object - type: array + type: array + type: object image: description: Function image name type: string @@ -21356,6 +21364,11 @@ spec: - name type: object type: array + stableRevision: + properties: + imageDigest: + type: string + type: object stableServing: properties: buildDuration: diff --git a/config/crd/bases/core.openfunction.io_functions.yaml b/config/crd/bases/core.openfunction.io_functions.yaml index 3f93833f..60ec2e42 100644 --- a/config/crd/bases/core.openfunction.io_functions.yaml +++ b/config/crd/bases/core.openfunction.io_functions.yaml @@ -9835,23 +9835,31 @@ spec: required: - srcRepo type: object - canarySteps: - description: Canary release steps for a function - items: - properties: - pause: + canaryStrategy: + description: Canary strategy for a function + properties: + canarySteps: + description: Canary release steps for a function + items: properties: - duration: - description: Duration the amount of time to wait before - moving to the next step. + pause: + description: Pause defines a pause stage for a canary step, + manual or auto + properties: + duration: + description: Duration the amount of time to wait before + moving to the next step. + format: int32 + type: integer + type: object + weight: + description: Weight indicate how many percentage of traffic + the canary function should receive format: int32 type: integer type: object - weight: - format: int32 - type: integer - type: object - type: array + type: array + type: object image: description: Function image name type: string @@ -19634,6 +19642,11 @@ spec: - name type: object type: array + stableRevision: + properties: + imageDigest: + type: string + type: object stableServing: properties: buildDuration: diff --git a/controllers/core/function_controller.go b/controllers/core/function_controller.go index 8a37ed13..b1a1fd8c 100644 --- a/controllers/core/function_controller.go +++ b/controllers/core/function_controller.go @@ -157,10 +157,11 @@ func (r *FunctionReconciler) updateCanaryRelease(fn *openfunction.Function) (*ti log := r.Log.WithName("UpdateCanaryRelease") if !hasCanaryReleasePlan(fn) { - fn.Status.StableServing = fn.Status.Serving.DeepCopy() if fn.Status.CanaryStatus == nil { return nil, nil } + fn.Status.StableServing = fn.Status.Serving.DeepCopy() + fn.Status.StableRevision = fn.Status.Revision // no plan no status fn.Status.CanaryStatus = nil if err := r.Status().Update(r.ctx, fn); err != nil { @@ -196,7 +197,7 @@ func (r *FunctionReconciler) updateCanaryRelease(fn *openfunction.Function) (*ti return nil, nil } var recheckTime *time.Time - steps := fn.Spec.CanarySteps + steps := fn.Spec.CanaryStrategy.CanarySteps currentStep := steps[status.CurrentStepIndex] if currentStep.Pause.Duration != nil && status.CurrentStepState == openfunction.CanaryStepStatePaused { duration := time.Second * time.Duration(*currentStep.Pause.Duration) @@ -218,6 +219,7 @@ func (r *FunctionReconciler) updateCanaryRelease(fn *openfunction.Function) (*ti if len(steps) == int(status.CurrentStepIndex)+1 { //complete step fn.Status.StableServing = fn.Status.Serving.DeepCopy() + fn.Status.StableRevision = fn.Status.Revision status.CurrentStepState = openfunction.CanaryStepStateCompleted status.Phase = openfunction.CanaryPhaseHealthy status.LastUpdateTime = &metav1.Time{Time: time.Now()} @@ -241,7 +243,7 @@ func (r *FunctionReconciler) updateCanaryRelease(fn *openfunction.Function) (*ti return recheckTime, nil } func hasCanaryReleasePlan(fn *openfunction.Function) bool { - if fn.Spec.CanarySteps == nil || len(fn.Spec.CanarySteps) == 0 { + if fn.Spec.CanaryStrategy == nil || len(fn.Spec.CanaryStrategy.CanarySteps) == 0 { return false } @@ -509,6 +511,12 @@ func (r *FunctionReconciler) createServing(fn *openfunction.Function) error { } if fn.Status.Serving.ResourceHash == fn.Status.StableServing.ResourceHash { fn.Status.StableServing = fn.Status.Serving + fn.Status.StableRevision = fn.Status.Revision + err := r.Status().Update(r.ctx, fn) + if err != nil { + log.Error(err, "Failed to update function serving status") + return err + } return nil } if err := r.updateFuncWithServingStatus(fn, fn.Status.StableServing); err != nil { @@ -557,6 +565,7 @@ func (r *FunctionReconciler) createServing(fn *openfunction.Function) error { fn.Status.CanaryStatus.Phase = openfunction.CanaryPhaseHealthy fn.Status.CanaryStatus.CurrentStepState = openfunction.CanaryStepStatePaused fn.Status.CanaryStatus.Message = "Canary progressing has been cancelled" + fn.Status.Revision = fn.Status.StableRevision if err := r.Status().Update(r.ctx, fn); err != nil { log.Error(err, "Failed to update function canary status") return err @@ -625,6 +634,7 @@ func (r *FunctionReconciler) createServing(fn *openfunction.Function) error { if fn.Status.StableServing == nil || !hasCanaryReleasePlan(fn) { // create stable serving status fn.Status.StableServing = servingStatus.DeepCopy() + fn.Status.StableRevision = fn.Status.Revision } if err := r.Status().Update(r.ctx, fn); err != nil { @@ -896,7 +906,7 @@ func (r *FunctionReconciler) createOrUpdateHTTPRoute(fn *openfunction.Function) var port k8sgatewayapiv1beta1.PortNumber if hasCanaryReleasePlan(fn) && fn.Status.CanaryStatus != nil && fn.Status.CanaryStatus.Phase == openfunction.CanaryPhaseProgressing { - step := fn.Spec.CanarySteps[fn.Status.CanaryStatus.CurrentStepIndex] + step := fn.Spec.CanaryStrategy.CanarySteps[fn.Status.CanaryStatus.CurrentStepIndex] weight = step.Weight } else { weight = pointer.Int32(100)