Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run a cleanup plan when deleting instances #1108

Merged
merged 9 commits into from
Dec 5, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pkg/apis/kudo/v1beta1/instance_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,9 @@ const (

// UpdatePlanName is the name of the update plan
UpdatePlanName = "update"

// CleanupPlanName is the name of the cleanup plan
CleanupPlanName = "cleanup"
)

// IsTerminal returns true if the status is terminal (either complete, or in a nonrecoverable error)
Expand Down
98 changes: 92 additions & 6 deletions pkg/controller/instance/instance_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ import (
"sigs.k8s.io/controller-runtime/pkg/reconcile"
)

const instanceCleanupFinalizerName = "kudo.finalizers.instance.cleanup"
nfnt marked this conversation as resolved.
Show resolved Hide resolved

// Reconciler reconciles an Instance object.
type Reconciler struct {
client.Client
Expand Down Expand Up @@ -105,6 +107,12 @@ func (r *Reconciler) SetupWithManager(
// |
// v
// +-------------------------------+
// | Start cleanup plan if object |
// | is being deleted |
// +-------------------------------+
// |
// v
// +-------------------------------+
// | Start new plan if required |
// | and none is running |
// +-------------------------------+
Expand Down Expand Up @@ -141,12 +149,40 @@ func (r *Reconciler) Reconcile(request ctrl.Request) (ctrl.Result, error) {
return reconcile.Result{}, err // OV not found has to be retried because it can really have been created after Instance
}

// ---------- 2. First check if we should start execution of new plan ----------
// ---------- 2. Check if the object is being deleted, start cleanup plan ----------

planToBeExecuted, err := instance.GetPlanToBeExecuted(ov)
if err != nil {
return reconcile.Result{}, err
var planToBeExecuted *string

// A delete request is indicated by a non-zero 'metadata.deletionTimestamp',
// see https://kubernetes.io/docs/tasks/access-kubernetes-api/custom-resources/custom-resource-definitions/#finalizers
if instance.ObjectMeta.DeletionTimestamp.IsZero() {
nfnt marked this conversation as resolved.
Show resolved Hide resolved
if _, hasCleanupPlan := ov.Spec.Plans[kudov1beta1.CleanupPlanName]; hasCleanupPlan {
if err := r.addFinalizer(instance); err != nil {
return reconcile.Result{}, err
}
}
} else {
log.Printf("InstanceController: Instance %s/%s is being deleted", instance.Namespace, instance.Name)
if contains(instance.ObjectMeta.Finalizers, instanceCleanupFinalizerName) {
nfnt marked this conversation as resolved.
Show resolved Hide resolved
// Only start the cleanup plan if it isn't currently running or if
// it isn't already completed.
lastExecutedPlanStatus := instance.GetLastExecutedPlanStatus()
if lastExecutedPlanStatus == nil || lastExecutedPlanStatus.Name != kudov1beta1.CleanupPlanName {
cleanupPlan := kudov1beta1.CleanupPlanName
planToBeExecuted = &cleanupPlan
nfnt marked this conversation as resolved.
Show resolved Hide resolved
}
}
}

// ---------- 3. First check if we should start execution of new plan ----------
nfnt marked this conversation as resolved.
Show resolved Hide resolved

if planToBeExecuted == nil {
planToBeExecuted, err = instance.GetPlanToBeExecuted(ov)
if err != nil {
return reconcile.Result{}, err
}
}

if planToBeExecuted != nil {
log.Printf("InstanceController: Going to start execution of plan %s on instance %s/%s", kudo.StringValue(planToBeExecuted), instance.Namespace, instance.Name)
err = instance.StartPlanExecution(kudo.StringValue(planToBeExecuted), ov)
Expand All @@ -156,7 +192,7 @@ func (r *Reconciler) Reconcile(request ctrl.Request) (ctrl.Result, error) {
r.Recorder.Event(instance, "Normal", "PlanStarted", fmt.Sprintf("Execution of plan %s started", kudo.StringValue(planToBeExecuted)))
}

// ---------- 3. If there's currently active plan, continue with the execution ----------
// ---------- 4. If there's currently active plan, continue with the execution ----------

activePlanStatus := instance.GetPlanInProgress()
if activePlanStatus == nil { // we have no plan in progress
Expand All @@ -182,7 +218,7 @@ func (r *Reconciler) Reconcile(request ctrl.Request) (ctrl.Result, error) {
log.Printf("InstanceController: Going to proceed in execution of active plan %s on instance %s/%s", activePlan.Name, instance.Namespace, instance.Name)
newStatus, err := workflow.Execute(activePlan, metadata, r.Client, &renderer.KustomizeEnhancer{Scheme: r.Scheme}, time.Now())

// ---------- 4. Update status of instance after the execution proceeded ----------
// ---------- 5. Update status of instance after the execution proceeded ----------
if newStatus != nil {
instance.UpdateInstanceStatus(newStatus)
}
Expand All @@ -199,6 +235,13 @@ func (r *Reconciler) Reconcile(request ctrl.Request) (ctrl.Result, error) {

if instance.Status.AggregatedStatus.Status.IsTerminal() {
r.Recorder.Event(instance, "Normal", "PlanFinished", fmt.Sprintf("Execution of plan %s finished with status %s", activePlanStatus.Name, instance.Status.AggregatedStatus.Status))

lastExecutedPlanStatus := instance.GetLastExecutedPlanStatus()
if lastExecutedPlanStatus != nil && lastExecutedPlanStatus.Name == kudov1beta1.CleanupPlanName {
nfnt marked this conversation as resolved.
Show resolved Hide resolved
if err := r.removeFinalizer(instance); err != nil {
return reconcile.Result{}, err
}
}
}

return reconcile.Result{}, nil
Expand Down Expand Up @@ -348,3 +391,46 @@ func parameterDifference(old, new map[string]string) map[string]string {

return diff
}

func contains(values []string, s string) bool {
for _, value := range values {
if value == s {
return true
}
}
return false
}

func (r *Reconciler) addFinalizer(instance *kudov1beta1.Instance) error {
if !contains(instance.ObjectMeta.Finalizers, instanceCleanupFinalizerName) {
log.Printf("InstanceController: Adding finalizer on instance %s/%s", instance.Namespace, instance.Name)
instance.ObjectMeta.Finalizers = append(instance.ObjectMeta.Finalizers, instanceCleanupFinalizerName)
if err := r.Update(context.TODO(), instance); err != nil {
return err
}
}

return nil
}

func (r *Reconciler) removeFinalizer(instance *kudov1beta1.Instance) error {
remove := func(values []string, s string) (result []string) {
for _, value := range values {
if value == s {
continue
}
result = append(result, value)
}
return
}

if contains(instance.ObjectMeta.Finalizers, instanceCleanupFinalizerName) {
log.Printf("InstanceController: Removing finalizer on instance %s/%s", instance.Namespace, instance.Name)
instance.ObjectMeta.Finalizers = remove(instance.ObjectMeta.Finalizers, instanceCleanupFinalizerName)
if err := r.Update(context.TODO(), instance); err != nil {
return err
}
}

return nil
}
5 changes: 5 additions & 0 deletions test/e2e/cli-install-uninstall/02-errors.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,8 @@ apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx-deployment
---
apiVersion: apps/v1
kind: Deployment
metadata:
name: cleanup-deployment
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
apiVersion: kudo.dev/v1beta1
kind: TestStep
commands:
- command: bash -c 'kubectl get -A events | grep -q "Execution of plan cleanup finished with status COMPLETE"'
nfnt marked this conversation as resolved.
Show resolved Hide resolved
14 changes: 14 additions & 0 deletions test/e2e/cli-install-uninstall/first-operator/operator.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ tasks:
spec:
resources:
- deployment.yaml
- name: cleanup
kind: Apply
spec:
resources:
- cleanup.yaml
plans:
deploy:
strategy: serial
Expand All @@ -22,3 +27,12 @@ plans:
- name: everything
tasks:
- app
cleanup:
strategy: serial
phases:
- name: main
strategy: parallel
steps:
- name: everything
tasks:
- cleanup
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
apiVersion: apps/v1
kind: Deployment
metadata:
name: cleanup-deployment
spec:
selector:
matchLabels:
app: nginx
replicas: 1
template:
metadata:
labels:
app: nginx
spec:
containers:
- name: nginx
image: nginx:1.7.9
ports:
- containerPort: 80