Implemented webhook rules around the deletion life-cycle phase of an …

…instance

additionally, instance controller now properly handles the cases where the webhooks are enabled and where they are disabled.

Signed-off-by: Aleksey Dukhovniy <alex.dukhovniy@googlemail.com>

cmd/manager/main.go

@@ -69,7 +69,7 @@ func main() {
 	if syncPeriod != nil {
 		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{
@@ -81,12 +81,12 @@ func main() {
 		os.Exit(1)
 	}
 
-	log.Print("⌛ Registering Components")
+	log.Print("✨ Registering Components")
 
 	if err := apis.AddToScheme(mgr.GetScheme()); err != nil {
 		log.Printf("❌ unable to add APIs to scheme: %v", err)
 	}
-	log.Print("✅ Scheme initialization")
+	log.Print("💎 Scheme initialization")
 
 	if err := apiextenstionsv1beta1.AddToScheme(mgr.GetScheme()); err != nil {
 		log.Printf("❌ unable to add extension APIs to scheme: %v", err)
@@ -100,7 +100,7 @@ func main() {
 		log.Printf("❌ unable to register operator controller to the manager: %v", err)
 		os.Exit(1)
 	}
-	log.Print("✅ Operator controller")
+	log.Print("📗 Operator controller")
 
 	err = (&operatorversion.Reconciler{
 		Client: mgr.GetClient(),
@@ -109,7 +109,7 @@ func main() {
 		log.Printf("❌ unable to register operator controller to the manager: %v", err)
 		os.Exit(1)
 	}
-	log.Print("✅ OperatorVersion controller")
+	log.Print("📘 OperatorVersion controller")
 
 	discoveryClient, err := utils.GetDiscoveryClient(mgr)
 	if err != nil {
@@ -128,28 +128,24 @@ func main() {
 		log.Printf("❌ unable to register instance controller to the manager: %v", err)
 		os.Exit(1)
 	}
-	log.Print("✅ Instance controller")
+	log.Print("📙 Instance controller")
 
 	if strings.ToLower(os.Getenv("ENABLE_WEBHOOKS")) == "true" {
-		log.Printf("⌛ Setting up webhooks")
+		log.Printf("🔸 Setting up webhooks")
 
-		// TODO (ad/an): this introduces a new mutating webhook instead of the old, validating one. However, old configuration
-		// has to be removed from the cluster, or every request will fail:
-		// $ k delete validatingwebhookconfigurations.admissionregistration.k8s.io kudo-manager-instance-validation-webhook-config
-		// This is either a breaking change, or we need to figure out a way to handle this as a part of an upgrade process.
 		if err := registerWebhook("/admit", &v1beta1.Instance{}, &webhook.Admission{Handler: &kudohook.InstanceAdmission{}}, mgr); err != nil {
 			log.Printf("❌ unable to create instance validation webhook: %v", err)
 			os.Exit(1)
 		}
-		log.Printf("✅ Instance admission webhook")
+		log.Printf("🧲 Instance admission webhook")
 
 		// Add more webhooks below using the above registerWebhook method
 	}
 
 	// Start the 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)
 	}
 }

config/crds/kudo.dev_instances.yaml

@@ -89,7 +89,8 @@ spec:
                 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 even if'
+                has a unique UID so should the same plan be re-triggered it will have
+                a new UID'
               properties:
                 planName:
                   type: string

pkg/apis/kudo/v1beta1/instance_types.go

@@ -42,7 +42,7 @@ type InstanceSpec struct {
 // 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 even if
+// 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,omitempty"`
 	UID      apimachinerytypes.UID `json:"uid,omitempty"`

pkg/apis/kudo/v1beta1/instance_types_helpers.go

@@ -1,13 +1,20 @@
 package v1beta1
 
 import (
+	"encoding/json"
 	"fmt"
 
+	"github.com/thoas/go-funk"
 	"k8s.io/apimachinery/pkg/util/uuid"
 
 	"github.com/kudobuilder/kudo/pkg/util/kudo"
 )
 
+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 +83,12 @@ func (i *Instance) ResetPlanStatus(plan string) error {
 
 	// 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 generate 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 +98,7 @@ func (i *Instance) ResetPlanStatus(plan string) error {
 		}
 	}
 
-	// update instance aggregated status
+	// update plan status and instance aggregated status
 	i.UpdateInstanceStatus(planStatus)
 	return nil
 }
@@ -116,6 +128,85 @@ 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 i.HasCleanupFinalizer() {
+		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() {
+				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.
+			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.LastFinishedRun == nil || p2.LastFinishedRun == nil {
@@ -158,6 +249,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 _, n := range possiblePlans {