address comments and fix CI

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.

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

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{

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"
 )

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,

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