diff --git a/design/vela-core/rollout-design.md b/design/vela-core/rollout-design.md index 9331bf746..f817eaeca 100644 --- a/design/vela-core/rollout-design.md +++ b/design/vela-core/rollout-design.md @@ -151,8 +151,9 @@ OAM rollout experience is different from flagger in some key areas and here are indicated in the [detailed rollout plan design](#rollout-plan-work-with-different-type-of-workloads). -## Notable implementation level details -Here are some high level implementation details based on the +## Notable implementation level design decisions +Here are some high level implementation design decisions that will impact the user experience of + rolling out. ### Rollout workflows As we mentioned in the introduction section, we will implement two rollout controllers that work @@ -169,7 +170,8 @@ When an appDeployment is used to do application level rollout, **the target appl is not reconciled by the application controller yet**. This is to make sure the appDeployment controller has the full control of the new application from the beginning. We will use a pre-defined annotation "app.oam.dev/rollout" that equals to "true" to facilitate - that. We expect any system uses an appDeployment object to follow this rule. + that. We expect any system, such as the [kubevela apiserver](APIServer-Catalog.md), that + utilizes an appDeployment object to follow this rule. - Upon creation, the appDeployment controller marks itself as the owner of the application. The application controller will have built-in logic to ignore any applications that has the "app.oam.dev/rollout" annotation set to true. diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/application_controller.go b/pkg/controller/core.oam.dev/v1alpha2/application/application_controller.go index 18250c0c7..fe6b616dc 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/application_controller.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/application_controller.go @@ -63,6 +63,11 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { return ctrl.Result{}, err } + // TODO: check finalizer + if app.DeletionTimestamp != nil { + return ctrl.Result{}, nil + } + // Check if the oam rollout annotation exists if _, exist := app.GetAnnotations()[oam.AnnotationAppRollout]; exist { applog.Info("The application is still in the process of rolling out") @@ -72,10 +77,6 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { return ctrl.Result{RequeueAfter: rolloutReconcileWaitTime}, r.Status().Update(ctx, app) } - if app.DeletionTimestamp != nil { - return ctrl.Result{}, nil - } - applog.Info("Start Rendering") app.Status.Phase = v1alpha2.ApplicationRendering diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/application_controller_test.go b/pkg/controller/core.oam.dev/v1alpha2/application/application_controller_test.go index ed78a1925..8cf1bde9d 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/application_controller_test.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/application_controller_test.go @@ -658,7 +658,7 @@ var _ = Describe("Test Application Controller", func() { Expect(k8sClient.Delete(ctx, app)).Should(BeNil()) }) - FIt("app with rolling out annotation", func() { + It("app with rolling out annotation", func() { By("crreat application with rolling out annotation") ns := &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/oam/labels.go b/pkg/oam/labels.go index f61f26749..844dc0107 100644 --- a/pkg/oam/labels.go +++ b/pkg/oam/labels.go @@ -49,6 +49,7 @@ const ( // resource for use in a three way diff during a patching apply AnnotationLastAppliedConfig = "app.oam.dev/last-applied-configuration" - // AnnotationAppRollout indicates that the application is rolling out - AnnotationAppRollout = "app.oam.dev/rollout" + // AnnotationAppRollout indicates that the application is still rolling out + // the application controller will not reconcile it yet + AnnotationAppRollout = "app.oam.dev/rollout-template" ) diff --git a/pkg/webhook/core.oam.dev/v1alpha2/application/validating_handler_test.go b/pkg/webhook/core.oam.dev/v1alpha2/application/validating_handler_test.go index 90293d7f3..6fb800694 100644 --- a/pkg/webhook/core.oam.dev/v1alpha2/application/validating_handler_test.go +++ b/pkg/webhook/core.oam.dev/v1alpha2/application/validating_handler_test.go @@ -70,7 +70,7 @@ var _ = Describe("Test Application Validator", func() { Expect(resp.Allowed).Should(BeFalse()) }) - FIt("Test Application Validator Forbid rollout annotation", func() { + It("Test Application Validator Forbid rollout annotation", func() { req := admission.Request{ AdmissionRequest: admissionv1beta1.AdmissionRequest{ Operation: admissionv1beta1.Update, diff --git a/pkg/webhook/core.oam.dev/v1alpha2/application/validation.go b/pkg/webhook/core.oam.dev/v1alpha2/application/validation.go index e95c0cd9f..48b1b80e5 100644 --- a/pkg/webhook/core.oam.dev/v1alpha2/application/validation.go +++ b/pkg/webhook/core.oam.dev/v1alpha2/application/validation.go @@ -21,12 +21,13 @@ func (h *ValidatingHandler) ValidateCreate(app *v1alpha2.Application) field.Erro // ValidateUpdate validates the Application on update func (h *ValidatingHandler) ValidateUpdate(newApp, oldApp *v1alpha2.Application) field.ErrorList { + // check if the newApp is valid componentErrs := h.ValidateCreate(newApp) // one can't add a rollout annotation to an existing application if _, exist := oldApp.GetAnnotations()[oam.AnnotationAppRollout]; !exist { if _, exist := newApp.GetAnnotations()[oam.AnnotationAppRollout]; exist { - componentErrs = append(componentErrs, field.Forbidden(field.NewPath("meta"), - "cannot add a rollout annotation")) + componentErrs = append(componentErrs, field.Forbidden(field.NewPath("meta").Child("annotation"), + "cannot add a rollout annotation on an existing application")) } } return componentErrs