KEP-20 part 2: trigger plans manually (#1352)

Summary:
- current `ValidationWebhookConfiguration` is replaced by `MutatingWebhookConfiguration`. Instance admission webhook is now setting the `Spec.PlanExecution.PlanName` on updates/creates
- `kudo-manager-instance-validation-webhook-config` webhook (when active) is enforcing all rules around triggering plans directly (manually) and indirectly (through Instance updates), making sure that no conflicting plans are running
- new CLI command `$ kubectl kudo plan trigger --name deploy --instance dummy-instance` is available to trigger a plan manually
- manually triggering plans is only possible when webhooks are enabled (`kudo init --webhook=InstanceValidation ...`)

**Notes**:
- **change**: Instance CRD got updated
- **heavy change**: `ValidationWebhookConfiguration` from previous KUDO version (in case webhooks were used) has to be removed manually

<!--  Thanks for sending a pull request!  Here are some tips for you:

1. If this is your first time, please read our contributor guidelines: https://github.com/kudobuilder/kudo/blob/master/CONTRIBUTING.md
2. Make sure you have added and ran the tests before submitting your PR
3. If the PR is unfinished, start it as a Draft PR: https://github.blog/2019-02-14-introducing-draft-pull-requests/
-->

**What this PR does / why we need it**:

<!-- 
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->
Fixes #649 #741

cmd/manager/main.go

@@ -60,60 +60,57 @@ func main() {
 	log.Printf("KUDO Version: %#v", version.Get())
 
 	// create new controller-runtime manager
-
 	syncPeriod, err := parseSyncPeriod()
 	if err != nil {
-		log.Printf("unable to parse manager sync period variable: %v", err)
+		log.Printf("Unable to parse manager sync period variable: %v", err)
 		os.Exit(1)
 	}
 
 	if syncPeriod != nil {
-		log.Print(fmt.Sprintf("setting up manager, sync-period is %v", syncPeriod))
+		log.Print(fmt.Sprintf("Setting up manager, sync-period is %v:", syncPeriod))
 	} else {
-		log.Print("setting up manager")
+		log.Print("Setting up manager")
 	}
 
 	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
 		CertDir:    "/tmp/cert",
 		SyncPeriod: syncPeriod,
 	})
 	if err != nil {
-		log.Printf("unable to start manager: %v", err)
+		log.Printf("Unable to start manager: %v", err)
 		os.Exit(1)
 	}
 
 	log.Print("Registering Components")
 
-	log.Print("setting up scheme")
 	if err := apis.AddToScheme(mgr.GetScheme()); err != nil {
-		log.Printf("unable to add APIs to scheme: %v", err)
+		log.Printf("Unable to add APIs to scheme: %v", err)
 	}
+	log.Print("Scheme initialization")
 
 	if err := apiextenstionsv1beta1.AddToScheme(mgr.GetScheme()); err != nil {
-		log.Printf("unable to add extension APIs to scheme: %v", err)
+		log.Printf("Unable to add extension APIs to scheme: %v", err)
 	}
 
 	// Setup all Controllers
-
-	log.Print("Setting up operator controller")
 	err = (&operator.Reconciler{
 		Client: mgr.GetClient(),
 	}).SetupWithManager(mgr)
 	if err != nil {
-		log.Printf("unable to register operator controller to the manager: %v", err)
+		log.Printf("Unable to register operator controller to the manager: %v", err)
 		os.Exit(1)
 	}
+	log.Print("Operator controller set up")
 
-	log.Print("Setting up operator version controller")
 	err = (&operatorversion.Reconciler{
 		Client: mgr.GetClient(),
 	}).SetupWithManager(mgr)
 	if err != nil {
-		log.Printf("unable to register operator controller to the manager: %v", err)
+		log.Printf("Unable to register operator controller to the manager: %v", err)
 		os.Exit(1)
 	}
+	log.Print("OperatorVersion controller set up")
 
-	log.Print("Setting up instance controller")
 	discoveryClient, err := utils.GetDiscoveryClient(mgr)
 	if err != nil {
 		log.Println(err)
@@ -128,32 +125,38 @@ func main() {
 		Scheme:    mgr.GetScheme(),
 	}).SetupWithManager(mgr)
 	if err != nil {
-		log.Printf("unable to register instance controller to the manager: %v", err)
+		log.Printf("Unable to register instance controller to the manager: %v", err)
 		os.Exit(1)
 	}
+	log.Print("Instance controller set up")
 
 	if strings.ToLower(os.Getenv("ENABLE_WEBHOOKS")) == "true" {
-		log.Printf("Setting up webhooks")
+		log.Printf("🔸 Setting up webhooks")
 
-		if err := registerWebhook("/validate", &v1beta1.Instance{}, &webhook.Admission{Handler: &kudohook.InstanceAdmission{}}, mgr); err != nil {
-			log.Printf("unable to create instance validation webhook: %v", err)
+		if err := registerWebhook("/admit", &v1beta1.Instance{}, &webhook.Admission{Handler: &kudohook.InstanceAdmission{}}, mgr); err != nil {
+			log.Printf("Unable to create instance admission webhook: %v", err)
 			os.Exit(1)
 		}
+		log.Printf("Instance admission webhook")
 
 		// Add more webhooks below using the above registerWebhook method
 	}
 
 	// Start the KUDO manager
-	log.Print("Starting KUDO manager")
+	log.Print("Done! Everything is setup, starting KUDO manager now")
 	if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil {
-		log.Printf("unable to run the manager: %v", err)
+		log.Printf("Unable to run the manager: %v", err)
 		os.Exit(1)
 	}
 }
 
 // registerWebhook method registers passed webhook using a give prefix (e.g. "/validate") and runtime object
 // (e.g. v1beta1.Instance) to generate a webhook path e.g. "/validate-kudo-dev-v1beta1-instances". Webhook
 // has to implement http.Handler interface (see v1beta1.InstanceAdmission for an example)
+//
+// NOTE: generated webhook path HAS to match the one used in the webhook configuration. See for example how
+// MutatingWebhookConfiguration.Webhooks[0].ClientConfig.Service.Path is set in
+// pkg/kudoctl/kudoinit/prereq/webhook.go::instanceAdmissionWebhook method
 func registerWebhook(prefix string, obj runtime.Object, hook http.Handler, mgr manager.Manager) error {
 	path, err := webhookPath(prefix, obj, mgr)
 	if err != nil {

config/crds/kudo.dev_instances.yaml

@@ -80,20 +80,26 @@ spec:
             planExecution:
               description: 'There are two ways a plan execution can be triggered:  1)
                 indirectly through update of a corresponding parameter in the InstanceSpec.Parameters
-                map  2) directly through setting of the InstanceSpec.PlanExecution
+                map  2) directly through setting of the InstanceSpec.PlanExecution.PlanName
                 field While indirect (1) triggers happens every time a user changes
                 a parameter, a directly (2) triggered plan is reserved for the situations
                 when parameters doesn''t change e.g. a periodic backup is triggered
                 overriding the existing backup file. Additionally, this opens room
                 for canceling and overriding currently running plans in the future.
                 Note: PlanExecution field defines plan name and corresponding parameters
                 that IS CURRENTLY executed. Once the instance controller (IC) is done
-                with the execution, this field will be cleared.'
+                with the execution, this field will be cleared. Each plan execution
+                has a unique UID so should the same plan be re-triggered it will have
+                a new UID'
               properties:
                 planName:
                   type: string
-              required:
-              - planName
+                uid:
+                  description: UID is a type that holds unique ID values, including
+                    UUIDs.  Because we don't ONLY use UUIDs, this is an alias to string.  Being
+                    a type captures intent and helps make sure that UIDs and names
+                    do not get conflated.
+                  type: string
               type: object
           type: object
         status:

go.mod

@@ -17,6 +17,7 @@ require (
 	github.com/go-bindata/go-bindata v3.1.2+incompatible
 	github.com/go-openapi/jsonreference v0.19.3 // indirect
 	github.com/go-openapi/spec v0.19.3 // indirect
+	github.com/go-openapi/validate v0.19.2
 	github.com/gogo/protobuf v1.3.1 // indirect
 	github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef // indirect
 	github.com/google/btree v1.0.0 // indirect

pkg/apis/kudo/v1beta1/instance_types.go

@@ -36,15 +36,17 @@ type InstanceSpec struct {
 
 // There are two ways a plan execution can be triggered:
 //  1) indirectly through update of a corresponding parameter in the InstanceSpec.Parameters map
-//  2) directly through setting of the InstanceSpec.PlanExecution field
+//  2) directly through setting of the InstanceSpec.PlanExecution.PlanName field
 // While indirect (1) triggers happens every time a user changes a parameter, a directly (2) triggered
 // plan is reserved for the situations when parameters doesn't change e.g. a periodic backup is triggered
 // overriding the existing backup file. Additionally, this opens room for canceling and overriding
 // currently running plans in the future.
 // Note: PlanExecution field defines plan name and corresponding parameters that IS CURRENTLY executed.
 // Once the instance controller (IC) is done with the execution, this field will be cleared.
+// Each plan execution has a unique UID so should the same plan be re-triggered it will have a new UID
 type PlanExecution struct {
-	PlanName string `json:"planName"  validate:"required"`
+	PlanName string                `json:"planName,omitempty"`
+	UID      apimachinerytypes.UID `json:"uid,omitempty"`
 
 	// Future PE options like Force: bool. Not needed for now
 }

pkg/apis/kudo/v1beta1/instance_types_helpers.go

@@ -1,12 +1,20 @@
 package v1beta1
 
 import (
+	"encoding/json"
 	"fmt"
+	"log"
 
+	"github.com/thoas/go-funk"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/uuid"
 )
 
+const (
+	instanceCleanupFinalizerName = "kudo.dev.instance.cleanup"
+	snapshotAnnotation           = "kudo.dev/last-applied-instance-state"
+)
+
 // GetPlanInProgress returns plan status of currently active plan or nil if no plan is running
 func (i *Instance) GetPlanInProgress() *PlanStatus {
 	for _, p := range i.Status.PlanStatus {
@@ -76,7 +84,12 @@ func (i *Instance) ResetPlanStatus(plan string, updatedTimestamp *metav1.Time) e
 
 	// reset plan's phases and steps by setting them to ExecutionPending
 	planStatus.Set(ExecutionPending)
-	planStatus.UID = uuid.NewUUID()
+	// when using webhooks, instance admission webhook already generates an UID for current plan, otherwise, we generate a new one.
+	if i.Spec.PlanExecution.UID != "" {
+		planStatus.UID = i.Spec.PlanExecution.UID
+	} else {
+		planStatus.UID = uuid.NewUUID()
+	}
 
 	for i, ph := range planStatus.Phases {
 		planStatus.Phases[i].Set(ExecutionPending)
@@ -86,7 +99,7 @@ func (i *Instance) ResetPlanStatus(plan string, updatedTimestamp *metav1.Time) e
 		}
 	}
 
-	// update instance aggregated status
+	// update plan status and instance aggregated status
 	i.UpdateInstanceStatus(planStatus, updatedTimestamp)
 	return nil
 }
@@ -118,6 +131,87 @@ func (i *Instance) PlanStatus(plan string) *PlanStatus {
 	return nil
 }
 
+// annotateSnapshot stores the current spec of Instance into the snapshot annotation
+// this information is used when executing update/upgrade plans, this overrides any snapshot that existed before
+func (i *Instance) AnnotateSnapshot() error {
+	jsonBytes, err := json.Marshal(i.Spec)
+	if err != nil {
+		return err
+	}
+	if i.Annotations == nil {
+		i.Annotations = make(map[string]string)
+	}
+	i.Annotations[snapshotAnnotation] = string(jsonBytes)
+	return nil
+}
+
+func (i *Instance) SnapshotSpec() (*InstanceSpec, error) {
+	if i.Annotations != nil {
+		snapshot, ok := i.Annotations[snapshotAnnotation]
+		if ok {
+			var spec *InstanceSpec
+			err := json.Unmarshal([]byte(snapshot), &spec)
+			if err != nil {
+				return nil, err
+			}
+			return spec, nil
+		}
+	}
+	return nil, nil
+}
+
+func (i *Instance) HasCleanupFinalizer() bool {
+	return funk.ContainsString(i.ObjectMeta.Finalizers, instanceCleanupFinalizerName)
+}
+
+// TryAddFinalizer adds the cleanup finalizer to an instance if the finalizer
+// hasn't been added yet, the instance has a cleanup plan and the cleanup plan
+// didn't run yet. Returns true if the cleanup finalizer has been added.
+func (i *Instance) TryAddFinalizer() bool {
+	if !i.HasCleanupFinalizer() {
+		planStatus := i.PlanStatus(CleanupPlanName)
+		// avoid adding a finalizer multiple times: we only add it if the corresponding
+		// plan.Status is nil (meaning the plan never ran) or if it exists but equals ExecutionNeverRun
+		if planStatus == nil || planStatus.Status == ExecutionNeverRun {
+			i.ObjectMeta.Finalizers = append(i.ObjectMeta.Finalizers, instanceCleanupFinalizerName)
+			return true
+		}
+	}
+
+	return false
+}
+
+// TryRemoveFinalizer removes the cleanup finalizer of an instance if it has
+// been added, the instance has a cleanup plan and the cleanup plan *successfully* finished.
+// Returns true if the cleanup finalizer has been removed.
+func (i *Instance) TryRemoveFinalizer() bool {
+	if funk.ContainsString(i.ObjectMeta.Finalizers, instanceCleanupFinalizerName) {
+		if planStatus := i.PlanStatus(CleanupPlanName); planStatus != nil {
+			// we check IsFinished and *not* IsTerminal here so that the finalizer is not removed in the FatalError
+			// case. This way a human operator has to intervene and we don't leave garbage in the cluster.
+			if planStatus.Status.IsFinished() {
+				log.Printf("Removing finalizer on instance %s/%s, cleanup plan is finished", i.Namespace, i.Name)
+				i.ObjectMeta.Finalizers = remove(i.ObjectMeta.Finalizers, instanceCleanupFinalizerName)
+				return true
+			}
+		} else {
+			// We have a finalizer but no cleanup plan. This could be due to an updated instance.
+			// Let's remove the finalizer.
+			log.Printf("Removing finalizer on instance %s/%s because there is no cleanup plan", i.Namespace, i.Name)
+			i.ObjectMeta.Finalizers = remove(i.ObjectMeta.Finalizers, instanceCleanupFinalizerName)
+			return true
+		}
+	}
+
+	return false
+}
+
+func remove(values []string, s string) []string {
+	return funk.FilterString(values, func(str string) bool {
+		return str != s
+	})
+}
+
 // wasRunAfter returns true if p1 was run after p2
 func wasRunAfter(p1 PlanStatus, p2 PlanStatus) bool {
 	if p1.Status == ExecutionNeverRun || p2.Status == ExecutionNeverRun || p1.LastUpdatedTimestamp == nil || p2.LastUpdatedTimestamp == nil {
@@ -160,6 +254,13 @@ func ParameterDiff(old, new map[string]string) map[string]string {
 	return diff
 }
 
+func CleanupPlanExists(ov *OperatorVersion) bool { return PlanExists(CleanupPlanName, ov) }
+
+func PlanExists(plan string, ov *OperatorVersion) bool {
+	_, ok := ov.Spec.Plans[plan]
+	return ok
+}
+
 // SelectPlan returns nil if none of the plan exists, otherwise the first one in list that exists
 func SelectPlan(possiblePlans []string, ov *OperatorVersion) *string {
 	for _, plan := range possiblePlans {