From 5718c4e4d081b3bcc1c294048aea6db300cd500e Mon Sep 17 00:00:00 2001 From: Alena Varkockova Date: Thu, 15 Aug 2019 10:47:10 +0200 Subject: [PATCH 01/20] Add reference to PE inside instance controller --- .../instance/instance_controller.go | 39 ++++++++++++++----- .../planexecution/planexecution_controller.go | 15 ------- 2 files changed, 29 insertions(+), 25 deletions(-) diff --git a/pkg/controller/instance/instance_controller.go b/pkg/controller/instance/instance_controller.go index 8ed3daec1..36834b823 100644 --- a/pkg/controller/instance/instance_controller.go +++ b/pkg/controller/instance/instance_controller.go @@ -309,26 +309,45 @@ func createPlan(r *ReconcileInstance, planName string, instance *kudov1alpha1.In ctx := context.TODO() planExecution := newPlanExecution(instance, planName, r.scheme) - return createPlanExecution(ctx, instance, planExecution, r, planName) + err := createPlanExecution(ctx, instance, &planExecution, r, planName) + if err != nil { + return err + } + err = addOwnerReference(r, &planExecution, instance) + if err != nil { + r.recorder.Event(instance, "Warning", "UpdateError", fmt.Sprintf("Could not update the ActivePlan for (%v): %v", planExecution.Spec.Instance.Name, err)) + } + return err +} + +func addOwnerReference(r *ReconcileInstance, planExecution *kudov1alpha1.PlanExecution, instance *kudov1alpha1.Instance) error { + instance.Status.ActivePlan = corev1.ObjectReference{ + Name: planExecution.Name, + Kind: planExecution.Kind, + Namespace: planExecution.Namespace, + APIVersion: planExecution.APIVersion, + UID: planExecution.UID, + } + return r.Update(context.TODO(), instance) } // createPlanExecution takes all the k8s objects needed to create the actual plan -func createPlanExecution(ctx context.Context, instance *kudov1alpha1.Instance, plan kudov1alpha1.PlanExecution, r *ReconcileInstance, planName string) error { - log.Printf("Creating PlanExecution of plan %s for instance %s", planName, instance.Name) - r.recorder.Event(instance, "Normal", "CreatePlanExecution", fmt.Sprintf("Creating \"%v\" plan execution", planName)) +func createPlanExecution(ctx context.Context, instance *kudov1alpha1.Instance, planExecution *kudov1alpha1.PlanExecution, r *ReconcileInstance, planName string) error { + log.Printf("Creating PlanExecution of planExecution %s for instance %s", planName, instance.Name) + r.recorder.Event(instance, "Normal", "CreatePlanExecution", fmt.Sprintf("Creating \"%v\" planExecution execution", planName)) // Make this instance the owner of the PlanExecution - if err := controllerutil.SetControllerReference(instance, &plan, r.scheme); err != nil { + if err := controllerutil.SetControllerReference(instance, planExecution, r.scheme); err != nil { log.Printf("InstanceController: Error setting ControllerReference") return err } - if err := r.Create(ctx, &plan); err != nil { - log.Printf("InstanceController: Error creating planexecution \"%v\": %v", plan.Name, err) - r.recorder.Event(instance, "Warning", "CreatePlanExecution", fmt.Sprintf("Error creating planexecution \"%v\": %v", plan.Name, err)) + if err := r.Create(ctx, planExecution); err != nil { + log.Printf("InstanceController: Error creating planexecution \"%v\": %v", planExecution.Name, err) + r.recorder.Event(instance, "Warning", "CreatePlanExecution", fmt.Sprintf("Error creating planexecution \"%v\": %v", planExecution.Name, err)) return err } - log.Printf("Created PlanExecution of plan %s for instance %s", planName, instance.Name) - r.recorder.Event(instance, "Normal", "PlanCreated", fmt.Sprintf("PlanExecution \"%v\" created", plan.Name)) + log.Printf("Created PlanExecution of planExecution %s for instance %s", planName, instance.Name) + r.recorder.Event(instance, "Normal", "PlanCreated", fmt.Sprintf("PlanExecution \"%v\" created", planExecution.Name)) return nil } diff --git a/pkg/controller/planexecution/planexecution_controller.go b/pkg/controller/planexecution/planexecution_controller.go index 37de1d643..61d95066a 100644 --- a/pkg/controller/planexecution/planexecution_controller.go +++ b/pkg/controller/planexecution/planexecution_controller.go @@ -33,7 +33,6 @@ import ( "github.com/kudobuilder/kudo/pkg/util/health" appsv1 "k8s.io/api/apps/v1" batchv1 "k8s.io/api/batch/v1" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -245,20 +244,6 @@ func (r *ReconcilePlanExecution) Reconcile(request reconcile.Request) (reconcile // Before returning from this function, update the status defer r.Update(context.Background(), planExecution) - // Need to add ownerReference as the Instance. - instance.Status.ActivePlan = corev1.ObjectReference{ - Name: planExecution.Name, - Kind: planExecution.Kind, - Namespace: planExecution.Namespace, - APIVersion: planExecution.APIVersion, - UID: planExecution.UID, - } - err = r.Update(context.TODO(), instance) - if err != nil { - r.recorder.Event(planExecution, "Warning", "UpdateError", fmt.Sprintf("Could not update the ActivePlan for (%v): %v", planExecution.Spec.Instance.Name, err)) - log.Printf("PlanExecutionController: Upate of instance with ActivePlan errored: %v", err) - } - // Get associated OperatorVersion operatorVersion := &kudov1alpha1.OperatorVersion{} err = r.Get(context.TODO(), From e70cf0982cff8e42fdff07dca44b3f07cb797f03 Mon Sep 17 00:00:00 2001 From: Alena Varkockova Date: Thu, 15 Aug 2019 11:08:55 +0200 Subject: [PATCH 02/20] Support owners in createPlanOld --- pkg/controller/instance/instance_controller.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/controller/instance/instance_controller.go b/pkg/controller/instance/instance_controller.go index 36834b823..0e26384c4 100644 --- a/pkg/controller/instance/instance_controller.go +++ b/pkg/controller/instance/instance_controller.go @@ -313,14 +313,10 @@ func createPlan(r *ReconcileInstance, planName string, instance *kudov1alpha1.In if err != nil { return err } - err = addOwnerReference(r, &planExecution, instance) - if err != nil { - r.recorder.Event(instance, "Warning", "UpdateError", fmt.Sprintf("Could not update the ActivePlan for (%v): %v", planExecution.Spec.Instance.Name, err)) - } - return err + return addOwnerReference(r.Client, r.recorder, &planExecution, instance) } -func addOwnerReference(r *ReconcileInstance, planExecution *kudov1alpha1.PlanExecution, instance *kudov1alpha1.Instance) error { +func addOwnerReference(c client.Client, r record.EventRecorder, planExecution *kudov1alpha1.PlanExecution, instance *kudov1alpha1.Instance) error { instance.Status.ActivePlan = corev1.ObjectReference{ Name: planExecution.Name, Kind: planExecution.Kind, @@ -328,7 +324,11 @@ func addOwnerReference(r *ReconcileInstance, planExecution *kudov1alpha1.PlanExe APIVersion: planExecution.APIVersion, UID: planExecution.UID, } - return r.Update(context.TODO(), instance) + err := c.Update(context.TODO(), instance) + if err != nil { + r.Event(instance, "Warning", "UpdateError", fmt.Sprintf("Could not update the ActivePlan for (%v): %v", planExecution.Spec.Instance.Name, err)) + } + return err } // createPlanExecution takes all the k8s objects needed to create the actual plan @@ -374,7 +374,7 @@ func createPlanOld(mgr manager.Manager, planName string, instance *kudov1alpha1. } log.Printf("Created PlanExecution of plan %s for instance %s", planName, instance.Name) recorder.Event(instance, "Normal", "PlanCreated", fmt.Sprintf("PlanExecution \"%v\" created", planExecution.Name)) - return nil + return addOwnerReference(mgr.GetClient(), recorder, &planExecution, instance) } var _ reconcile.Reconciler = &ReconcileInstance{} From 55853c2c989231fa482b80322349076e7d3bac6e Mon Sep 17 00:00:00 2001 From: Alena Varkockova Date: Thu, 15 Aug 2019 11:26:17 +0200 Subject: [PATCH 03/20] Fill GVK --- pkg/controller/instance/instance_controller.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/controller/instance/instance_controller.go b/pkg/controller/instance/instance_controller.go index 0e26384c4..d3c4cf430 100644 --- a/pkg/controller/instance/instance_controller.go +++ b/pkg/controller/instance/instance_controller.go @@ -519,5 +519,10 @@ func newPlanExecution(instance *kudov1alpha1.Instance, planName string, scheme * Template: instance.Spec, }, } + + gvk, _ = apiutil.GVKForObject(&planExecution, scheme) + planExecution.Kind = gvk.Kind + planExecution.APIVersion = gvk.Version + return planExecution } From 3cb71c7315eaf73ffef3861104cdcbc8833be47e Mon Sep 17 00:00:00 2001 From: Alena Varkockova Date: Thu, 15 Aug 2019 11:29:18 +0200 Subject: [PATCH 04/20] Format --- pkg/controller/instance/instance_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/instance/instance_controller.go b/pkg/controller/instance/instance_controller.go index d3c4cf430..484d5e6b7 100644 --- a/pkg/controller/instance/instance_controller.go +++ b/pkg/controller/instance/instance_controller.go @@ -523,6 +523,6 @@ func newPlanExecution(instance *kudov1alpha1.Instance, planName string, scheme * gvk, _ = apiutil.GVKForObject(&planExecution, scheme) planExecution.Kind = gvk.Kind planExecution.APIVersion = gvk.Version - + return planExecution } From c6d6c70b9bd45f2e034547b907bcc47565b3bae5 Mon Sep 17 00:00:00 2001 From: Alena Varkockova Date: Thu, 15 Aug 2019 11:37:18 +0200 Subject: [PATCH 05/20] TypeMeta --- pkg/controller/instance/instance_controller.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/controller/instance/instance_controller.go b/pkg/controller/instance/instance_controller.go index 484d5e6b7..0f05279b0 100644 --- a/pkg/controller/instance/instance_controller.go +++ b/pkg/controller/instance/instance_controller.go @@ -504,6 +504,10 @@ func newPlanExecution(instance *kudov1alpha1.Instance, planName string, scheme * Namespace: instance.Namespace, } planExecution := kudov1alpha1.PlanExecution{ + TypeMeta: metav1.TypeMeta{ + Kind: "PlanExecution", + APIVersion: "kudo.dev/v1alpha1", + }, ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("%v-%v-%v", instance.Name, planName, time.Now().Nanosecond()), Namespace: instance.GetNamespace(), @@ -520,9 +524,5 @@ func newPlanExecution(instance *kudov1alpha1.Instance, planName string, scheme * }, } - gvk, _ = apiutil.GVKForObject(&planExecution, scheme) - planExecution.Kind = gvk.Kind - planExecution.APIVersion = gvk.Version - return planExecution } From 219bf80b9684be8d6255efd24613ed08871bbe95 Mon Sep 17 00:00:00 2001 From: Alena Varkockova Date: Thu, 15 Aug 2019 11:58:00 +0200 Subject: [PATCH 06/20] Logging --- pkg/controller/instance/instance_controller.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/pkg/controller/instance/instance_controller.go b/pkg/controller/instance/instance_controller.go index 0f05279b0..aa9a32f9a 100644 --- a/pkg/controller/instance/instance_controller.go +++ b/pkg/controller/instance/instance_controller.go @@ -309,14 +309,14 @@ func createPlan(r *ReconcileInstance, planName string, instance *kudov1alpha1.In ctx := context.TODO() planExecution := newPlanExecution(instance, planName, r.scheme) - err := createPlanExecution(ctx, instance, &planExecution, r, planName) + err := createPlanExecution(ctx, instance, planExecution, r, planName) if err != nil { return err } - return addOwnerReference(r.Client, r.recorder, &planExecution, instance) + return addActivePlanReference(r.Client, r.recorder, planExecution, instance) } -func addOwnerReference(c client.Client, r record.EventRecorder, planExecution *kudov1alpha1.PlanExecution, instance *kudov1alpha1.Instance) error { +func addActivePlanReference(c client.Client, r record.EventRecorder, planExecution *kudov1alpha1.PlanExecution, instance *kudov1alpha1.Instance) error { instance.Status.ActivePlan = corev1.ObjectReference{ Name: planExecution.Name, Kind: planExecution.Kind, @@ -324,6 +324,7 @@ func addOwnerReference(c client.Client, r record.EventRecorder, planExecution *k APIVersion: planExecution.APIVersion, UID: planExecution.UID, } + fmt.Printf("Updating reference: %v", instance.Status.ActivePlan) err := c.Update(context.TODO(), instance) if err != nil { r.Event(instance, "Warning", "UpdateError", fmt.Sprintf("Could not update the ActivePlan for (%v): %v", planExecution.Spec.Instance.Name, err)) @@ -362,19 +363,19 @@ func createPlanOld(mgr manager.Manager, planName string, instance *kudov1alpha1. planExecution := newPlanExecution(instance, planName, mgr.GetScheme()) // Make this instance the owner of the PlanExecution - if err := controllerutil.SetControllerReference(instance, &planExecution, mgr.GetScheme()); err != nil { + if err := controllerutil.SetControllerReference(instance, planExecution, mgr.GetScheme()); err != nil { log.Printf("InstanceController: Error setting ControllerReference") return err } - if err := mgr.GetClient().Create(ctx, &planExecution); err != nil { + if err := mgr.GetClient().Create(ctx, planExecution); err != nil { log.Printf("InstanceController: Error creating planexecution \"%v\": %v", planExecution.Name, err) recorder.Event(instance, "Warning", "CreatePlanExecution", fmt.Sprintf("Error creating planexecution \"%v\": %v", planExecution.Name, err)) return err } log.Printf("Created PlanExecution of plan %s for instance %s", planName, instance.Name) recorder.Event(instance, "Normal", "PlanCreated", fmt.Sprintf("PlanExecution \"%v\" created", planExecution.Name)) - return addOwnerReference(mgr.GetClient(), recorder, &planExecution, instance) + return addActivePlanReference(mgr.GetClient(), recorder, planExecution, instance) } var _ reconcile.Reconciler = &ReconcileInstance{} @@ -496,7 +497,7 @@ func parameterDifference(old, new map[string]string) map[string]string { } // newPlanExecution creates a PlanExecution based on the kind and instance for a given planName -func newPlanExecution(instance *kudov1alpha1.Instance, planName string, scheme *runtime.Scheme) kudov1alpha1.PlanExecution { +func newPlanExecution(instance *kudov1alpha1.Instance, planName string, scheme *runtime.Scheme) *kudov1alpha1.PlanExecution { gvk, _ := apiutil.GVKForObject(instance, scheme) ref := corev1.ObjectReference{ Kind: gvk.Kind, @@ -524,5 +525,5 @@ func newPlanExecution(instance *kudov1alpha1.Instance, planName string, scheme * }, } - return planExecution + return &planExecution } From 31216b32508fa4c7dbc868397a464e3a35a59fab Mon Sep 17 00:00:00 2001 From: Alena Varkockova Date: Thu, 15 Aug 2019 12:06:27 +0200 Subject: [PATCH 07/20] More logging --- pkg/controller/instance/instance_controller.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/controller/instance/instance_controller.go b/pkg/controller/instance/instance_controller.go index aa9a32f9a..11ac92c4f 100644 --- a/pkg/controller/instance/instance_controller.go +++ b/pkg/controller/instance/instance_controller.go @@ -525,5 +525,7 @@ func newPlanExecution(instance *kudov1alpha1.Instance, planName string, scheme * }, } + println("APIV: %s", planExecution.APIVersion) + return &planExecution } From 7e837524a143c21de679274cdaf8da2a2ba1c00e Mon Sep 17 00:00:00 2001 From: Alena Varkockova Date: Thu, 15 Aug 2019 12:14:07 +0200 Subject: [PATCH 08/20] Log --- pkg/controller/instance/instance_controller.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/controller/instance/instance_controller.go b/pkg/controller/instance/instance_controller.go index 11ac92c4f..ac33b64aa 100644 --- a/pkg/controller/instance/instance_controller.go +++ b/pkg/controller/instance/instance_controller.go @@ -324,7 +324,7 @@ func addActivePlanReference(c client.Client, r record.EventRecorder, planExecuti APIVersion: planExecution.APIVersion, UID: planExecution.UID, } - fmt.Printf("Updating reference: %v", instance.Status.ActivePlan) + fmt.Printf("Updating reference: %s %s", instance.Status.ActivePlan.APIVersion, instance.Status.ActivePlan.Kind) err := c.Update(context.TODO(), instance) if err != nil { r.Event(instance, "Warning", "UpdateError", fmt.Sprintf("Could not update the ActivePlan for (%v): %v", planExecution.Spec.Instance.Name, err)) @@ -525,7 +525,5 @@ func newPlanExecution(instance *kudov1alpha1.Instance, planName string, scheme * }, } - println("APIV: %s", planExecution.APIVersion) - return &planExecution } From b93f869baf14426cdfeceebd5eccaa70811fcc19 Mon Sep 17 00:00:00 2001 From: Alena Varkockova Date: Thu, 15 Aug 2019 12:20:46 +0200 Subject: [PATCH 09/20] More --- pkg/controller/instance/instance_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/instance/instance_controller.go b/pkg/controller/instance/instance_controller.go index ac33b64aa..6082a4a03 100644 --- a/pkg/controller/instance/instance_controller.go +++ b/pkg/controller/instance/instance_controller.go @@ -324,7 +324,7 @@ func addActivePlanReference(c client.Client, r record.EventRecorder, planExecuti APIVersion: planExecution.APIVersion, UID: planExecution.UID, } - fmt.Printf("Updating reference: %s %s", instance.Status.ActivePlan.APIVersion, instance.Status.ActivePlan.Kind) + fmt.Printf("Updating reference: %s %s %s %s", instance.Status.ActivePlan.APIVersion, planExecution.APIVersion, instance.Status.ActivePlan.Kind, planExecution.Kind) err := c.Update(context.TODO(), instance) if err != nil { r.Event(instance, "Warning", "UpdateError", fmt.Sprintf("Could not update the ActivePlan for (%v): %v", planExecution.Spec.Instance.Name, err)) From d80182126cbd19a1c55568111fd6245853bda3b2 Mon Sep 17 00:00:00 2001 From: Alena Varkockova Date: Thu, 15 Aug 2019 12:31:09 +0200 Subject: [PATCH 10/20] Switch order --- pkg/controller/instance/instance_controller.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/controller/instance/instance_controller.go b/pkg/controller/instance/instance_controller.go index 6082a4a03..40b3edfe5 100644 --- a/pkg/controller/instance/instance_controller.go +++ b/pkg/controller/instance/instance_controller.go @@ -309,11 +309,12 @@ func createPlan(r *ReconcileInstance, planName string, instance *kudov1alpha1.In ctx := context.TODO() planExecution := newPlanExecution(instance, planName, r.scheme) + _ = addActivePlanReference(r.Client, r.recorder, planExecution, instance) err := createPlanExecution(ctx, instance, planExecution, r, planName) if err != nil { return err } - return addActivePlanReference(r.Client, r.recorder, planExecution, instance) + return nil } func addActivePlanReference(c client.Client, r record.EventRecorder, planExecution *kudov1alpha1.PlanExecution, instance *kudov1alpha1.Instance) error { @@ -361,6 +362,7 @@ func createPlanOld(mgr manager.Manager, planName string, instance *kudov1alpha1. recorder.Event(instance, "Normal", "CreatePlanExecution", fmt.Sprintf("Creating \"%v\" plan execution", planName)) planExecution := newPlanExecution(instance, planName, mgr.GetScheme()) + _ = addActivePlanReference(mgr.GetClient(), recorder, planExecution, instance) // Make this instance the owner of the PlanExecution if err := controllerutil.SetControllerReference(instance, planExecution, mgr.GetScheme()); err != nil { @@ -375,7 +377,7 @@ func createPlanOld(mgr manager.Manager, planName string, instance *kudov1alpha1. } log.Printf("Created PlanExecution of plan %s for instance %s", planName, instance.Name) recorder.Event(instance, "Normal", "PlanCreated", fmt.Sprintf("PlanExecution \"%v\" created", planExecution.Name)) - return addActivePlanReference(mgr.GetClient(), recorder, planExecution, instance) + return nil } var _ reconcile.Reconciler = &ReconcileInstance{} From 3a4ce8078c4400c4ede0bd2aaa888871cc04193a Mon Sep 17 00:00:00 2001 From: Alena Varkockova Date: Thu, 15 Aug 2019 12:39:46 +0200 Subject: [PATCH 11/20] Logs --- pkg/controller/instance/instance_controller.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/controller/instance/instance_controller.go b/pkg/controller/instance/instance_controller.go index 40b3edfe5..0307b21ae 100644 --- a/pkg/controller/instance/instance_controller.go +++ b/pkg/controller/instance/instance_controller.go @@ -364,17 +364,23 @@ func createPlanOld(mgr manager.Manager, planName string, instance *kudov1alpha1. planExecution := newPlanExecution(instance, planName, mgr.GetScheme()) _ = addActivePlanReference(mgr.GetClient(), recorder, planExecution, instance) + fmt.Printf("1: %s \n", planExecution.Kind) + // Make this instance the owner of the PlanExecution if err := controllerutil.SetControllerReference(instance, planExecution, mgr.GetScheme()); err != nil { log.Printf("InstanceController: Error setting ControllerReference") return err } + fmt.Printf("2: %s \n", planExecution.Kind) + if err := mgr.GetClient().Create(ctx, planExecution); err != nil { log.Printf("InstanceController: Error creating planexecution \"%v\": %v", planExecution.Name, err) recorder.Event(instance, "Warning", "CreatePlanExecution", fmt.Sprintf("Error creating planexecution \"%v\": %v", planExecution.Name, err)) return err } + + fmt.Printf("3: %s \n", planExecution.Kind) log.Printf("Created PlanExecution of plan %s for instance %s", planName, instance.Name) recorder.Event(instance, "Normal", "PlanCreated", fmt.Sprintf("PlanExecution \"%v\" created", planExecution.Name)) return nil From 0185fe0ffb6abbb545b38182d04ac590e52260b0 Mon Sep 17 00:00:00 2001 From: Alena Varkockova Date: Thu, 15 Aug 2019 12:55:02 +0200 Subject: [PATCH 12/20] Hardcode the values, do not rely on PE --- pkg/controller/instance/instance_controller.go | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/pkg/controller/instance/instance_controller.go b/pkg/controller/instance/instance_controller.go index 0307b21ae..354b33f4e 100644 --- a/pkg/controller/instance/instance_controller.go +++ b/pkg/controller/instance/instance_controller.go @@ -320,12 +320,11 @@ func createPlan(r *ReconcileInstance, planName string, instance *kudov1alpha1.In func addActivePlanReference(c client.Client, r record.EventRecorder, planExecution *kudov1alpha1.PlanExecution, instance *kudov1alpha1.Instance) error { instance.Status.ActivePlan = corev1.ObjectReference{ Name: planExecution.Name, - Kind: planExecution.Kind, + Kind: "PlanExecution", Namespace: planExecution.Namespace, - APIVersion: planExecution.APIVersion, + APIVersion: "kudo.dev/v1alpha1", UID: planExecution.UID, } - fmt.Printf("Updating reference: %s %s %s %s", instance.Status.ActivePlan.APIVersion, planExecution.APIVersion, instance.Status.ActivePlan.Kind, planExecution.Kind) err := c.Update(context.TODO(), instance) if err != nil { r.Event(instance, "Warning", "UpdateError", fmt.Sprintf("Could not update the ActivePlan for (%v): %v", planExecution.Spec.Instance.Name, err)) @@ -362,9 +361,6 @@ func createPlanOld(mgr manager.Manager, planName string, instance *kudov1alpha1. recorder.Event(instance, "Normal", "CreatePlanExecution", fmt.Sprintf("Creating \"%v\" plan execution", planName)) planExecution := newPlanExecution(instance, planName, mgr.GetScheme()) - _ = addActivePlanReference(mgr.GetClient(), recorder, planExecution, instance) - - fmt.Printf("1: %s \n", planExecution.Kind) // Make this instance the owner of the PlanExecution if err := controllerutil.SetControllerReference(instance, planExecution, mgr.GetScheme()); err != nil { @@ -372,18 +368,15 @@ func createPlanOld(mgr manager.Manager, planName string, instance *kudov1alpha1. return err } - fmt.Printf("2: %s \n", planExecution.Kind) - if err := mgr.GetClient().Create(ctx, planExecution); err != nil { log.Printf("InstanceController: Error creating planexecution \"%v\": %v", planExecution.Name, err) recorder.Event(instance, "Warning", "CreatePlanExecution", fmt.Sprintf("Error creating planexecution \"%v\": %v", planExecution.Name, err)) return err } - fmt.Printf("3: %s \n", planExecution.Kind) log.Printf("Created PlanExecution of plan %s for instance %s", planName, instance.Name) recorder.Event(instance, "Normal", "PlanCreated", fmt.Sprintf("PlanExecution \"%v\" created", planExecution.Name)) - return nil + return addActivePlanReference(mgr.GetClient(), recorder, planExecution, instance) } var _ reconcile.Reconciler = &ReconcileInstance{} From d171bf7e5d0d68e3c5ba32c6d7801f33970900a7 Mon Sep 17 00:00:00 2001 From: Alena Varkockova Date: Thu, 15 Aug 2019 13:53:56 +0200 Subject: [PATCH 13/20] Retry when update fails --- pkg/controller/planexecution/planexecution_controller.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/controller/planexecution/planexecution_controller.go b/pkg/controller/planexecution/planexecution_controller.go index 61d95066a..ecd16b589 100644 --- a/pkg/controller/planexecution/planexecution_controller.go +++ b/pkg/controller/planexecution/planexecution_controller.go @@ -415,6 +415,7 @@ func (r *ReconcilePlanExecution) Reconcile(request reconcile.Request) (reconcile err = r.Client.Update(context.TODO(), instance) if err != nil { log.Printf("Error updating instance status to %v: %v\n", instance.Status.Status, err) + return reconcile.Result{}, err } return reconcile.Result{}, nil From 58f4ae85f921dd0672873f5ea84620d28f557538 Mon Sep 17 00:00:00 2001 From: Alena Varkockova Date: Thu, 15 Aug 2019 14:14:13 +0200 Subject: [PATCH 14/20] Filter out PE without reference --- pkg/controller/planexecution/planexecution_controller.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/controller/planexecution/planexecution_controller.go b/pkg/controller/planexecution/planexecution_controller.go index ecd16b589..99c6ea4ae 100644 --- a/pkg/controller/planexecution/planexecution_controller.go +++ b/pkg/controller/planexecution/planexecution_controller.go @@ -227,6 +227,11 @@ func (r *ReconcilePlanExecution) Reconcile(request reconcile.Request) (reconcile return reconcile.Result{}, err } + if instance.Status.ActivePlan.Name != planExecution.Name || instance.Status.ActivePlan.Namespace != planExecution.Namespace { + // this can happen for newly created PlanExecution where ActivePlan was not yet set to point to this instance + return reconcile.Result{}, fmt.Errorf("Instance %s does not have ActivePlan pointing to PlanExecution %s, %s. Instead %s, %s. Retrying", instance.Name, planExecution.Name, planExecution.Namespace, instance.Status.ActivePlan.Name, instance.Status.ActivePlan.Namespace) + } + // Check for Suspend set. if planExecution.Spec.Suspend != nil && *planExecution.Spec.Suspend { planExecution.Status.State = kudov1alpha1.PhaseStateSuspend From 004b3cdff7c0e31fab1d58339e35fff5c985c7a3 Mon Sep 17 00:00:00 2001 From: Alena Varkockova Date: Thu, 15 Aug 2019 14:29:57 +0200 Subject: [PATCH 15/20] Format --- pkg/controller/planexecution/planexecution_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/planexecution/planexecution_controller.go b/pkg/controller/planexecution/planexecution_controller.go index 99c6ea4ae..cb358b479 100644 --- a/pkg/controller/planexecution/planexecution_controller.go +++ b/pkg/controller/planexecution/planexecution_controller.go @@ -229,7 +229,7 @@ func (r *ReconcilePlanExecution) Reconcile(request reconcile.Request) (reconcile if instance.Status.ActivePlan.Name != planExecution.Name || instance.Status.ActivePlan.Namespace != planExecution.Namespace { // this can happen for newly created PlanExecution where ActivePlan was not yet set to point to this instance - return reconcile.Result{}, fmt.Errorf("Instance %s does not have ActivePlan pointing to PlanExecution %s, %s. Instead %s, %s. Retrying", instance.Name, planExecution.Name, planExecution.Namespace, instance.Status.ActivePlan.Name, instance.Status.ActivePlan.Namespace) + return reconcile.Result{}, fmt.Errorf("instance %s does not have ActivePlan pointing to PlanExecution %s, %s. Instead %s, %s. Retrying", instance.Name, planExecution.Name, planExecution.Namespace, instance.Status.ActivePlan.Name, instance.Status.ActivePlan.Namespace) } // Check for Suspend set. From 96efe480c04812b981c6f430fa0f606fc1cd0b18 Mon Sep 17 00:00:00 2001 From: Alena Varkockova Date: Thu, 15 Aug 2019 14:54:22 +0200 Subject: [PATCH 16/20] Have only one createPlan method --- .../instance/instance_controller.go | 76 +++++-------------- 1 file changed, 20 insertions(+), 56 deletions(-) diff --git a/pkg/controller/instance/instance_controller.go b/pkg/controller/instance/instance_controller.go index 354b33f4e..f0859c79d 100644 --- a/pkg/controller/instance/instance_controller.go +++ b/pkg/controller/instance/instance_controller.go @@ -101,7 +101,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { instance.Status.ActivePlan.Name == "" { log.Printf("InstanceController: Creating a deploy execution plan for the instance %v", instance.Name) - err = createPlanOld(mgr, "deploy", &instance) + err = createPlan(mgr.GetClient(), mgr.GetEventRecorderFor("instance-controller"), mgr.GetScheme(), "deploy", &instance) if err != nil { log.Printf("InstanceController: Error creating \"%v\" object for \"deploy\": %v", instance.Name, err) } @@ -264,7 +264,7 @@ func instanceEventFilter(mgr manager.Manager) predicate.Funcs { } } - if err = createPlanOld(mgr, planName, new); err != nil { + if err = createPlan(mgr.GetClient(), mgr.GetEventRecorderFor("instance-controller"), mgr.GetScheme(), planName, new); err != nil { log.Printf("InstanceController: Error creating PlanExecution \"%v\" for instance \"%v\": %v", planName, new.Name, err) } } @@ -305,16 +305,27 @@ func instanceEventFilter(mgr manager.Manager) predicate.Funcs { } } -func createPlan(r *ReconcileInstance, planName string, instance *kudov1alpha1.Instance) error { +func createPlan(c client.Client, r record.EventRecorder, scheme *runtime.Scheme, planName string, instance *kudov1alpha1.Instance) error { ctx := context.TODO() - planExecution := newPlanExecution(instance, planName, r.scheme) - _ = addActivePlanReference(r.Client, r.recorder, planExecution, instance) - err := createPlanExecution(ctx, instance, planExecution, r, planName) - if err != nil { + planExecution := newPlanExecution(instance, planName, scheme) + log.Printf("Creating PlanExecution of planExecution %s for instance %s", planName, instance.Name) + r.Event(instance, "Normal", "CreatePlanExecution", fmt.Sprintf("Creating \"%v\" planExecution execution", planName)) + + // Make this instance the owner of the PlanExecution + if err := controllerutil.SetControllerReference(instance, planExecution, scheme); err != nil { + log.Printf("InstanceController: Error setting ControllerReference") return err } - return nil + if err := c.Create(ctx, planExecution); err != nil { + log.Printf("InstanceController: Error creating planexecution \"%v\": %v", planExecution.Name, err) + r.Event(instance, "Warning", "CreatePlanExecution", fmt.Sprintf("Error creating planexecution \"%v\": %v", planExecution.Name, err)) + return err + } + log.Printf("Created PlanExecution of planExecution %s for instance %s", planName, instance.Name) + r.Event(instance, "Normal", "PlanCreated", fmt.Sprintf("PlanExecution \"%v\" created", planExecution.Name)) + + return addActivePlanReference(c, r, planExecution, instance) } func addActivePlanReference(c client.Client, r record.EventRecorder, planExecution *kudov1alpha1.PlanExecution, instance *kudov1alpha1.Instance) error { @@ -332,53 +343,6 @@ func addActivePlanReference(c client.Client, r record.EventRecorder, planExecuti return err } -// createPlanExecution takes all the k8s objects needed to create the actual plan -func createPlanExecution(ctx context.Context, instance *kudov1alpha1.Instance, planExecution *kudov1alpha1.PlanExecution, r *ReconcileInstance, planName string) error { - log.Printf("Creating PlanExecution of planExecution %s for instance %s", planName, instance.Name) - r.recorder.Event(instance, "Normal", "CreatePlanExecution", fmt.Sprintf("Creating \"%v\" planExecution execution", planName)) - - // Make this instance the owner of the PlanExecution - if err := controllerutil.SetControllerReference(instance, planExecution, r.scheme); err != nil { - log.Printf("InstanceController: Error setting ControllerReference") - return err - } - if err := r.Create(ctx, planExecution); err != nil { - log.Printf("InstanceController: Error creating planexecution \"%v\": %v", planExecution.Name, err) - r.recorder.Event(instance, "Warning", "CreatePlanExecution", fmt.Sprintf("Error creating planexecution \"%v\": %v", planExecution.Name, err)) - return err - } - log.Printf("Created PlanExecution of planExecution %s for instance %s", planName, instance.Name) - r.recorder.Event(instance, "Normal", "PlanCreated", fmt.Sprintf("PlanExecution \"%v\" created", planExecution.Name)) - return nil -} - -//todo: REMOVE this... we do not want to create a plan outside of reconcile -// most likely... any use of this function is wrong... all plans should be created in reconcile -func createPlanOld(mgr manager.Manager, planName string, instance *kudov1alpha1.Instance) error { - ctx := context.TODO() - recorder := mgr.GetEventRecorderFor("instance-controller") - log.Printf("Creating PlanExecution of plan %s for instance %s", planName, instance.Name) - recorder.Event(instance, "Normal", "CreatePlanExecution", fmt.Sprintf("Creating \"%v\" plan execution", planName)) - - planExecution := newPlanExecution(instance, planName, mgr.GetScheme()) - - // Make this instance the owner of the PlanExecution - if err := controllerutil.SetControllerReference(instance, planExecution, mgr.GetScheme()); err != nil { - log.Printf("InstanceController: Error setting ControllerReference") - return err - } - - if err := mgr.GetClient().Create(ctx, planExecution); err != nil { - log.Printf("InstanceController: Error creating planexecution \"%v\": %v", planExecution.Name, err) - recorder.Event(instance, "Warning", "CreatePlanExecution", fmt.Sprintf("Error creating planexecution \"%v\": %v", planExecution.Name, err)) - return err - } - - log.Printf("Created PlanExecution of plan %s for instance %s", planName, instance.Name) - recorder.Event(instance, "Normal", "PlanCreated", fmt.Sprintf("PlanExecution \"%v\" created", planExecution.Name)) - return addActivePlanReference(mgr.GetClient(), recorder, planExecution, instance) -} - var _ reconcile.Reconciler = &ReconcileInstance{} // ReconcileInstance reconciles an Instance object. @@ -413,7 +377,7 @@ func (r *ReconcileInstance) Reconcile(request reconcile.Request) (reconcile.Resu // if this is new create and create and assign a planexecution and return if isNewInstance(instance) { - err = createPlan(r, "deploy", instance) + err = createPlan(r.Client, r.recorder, r.scheme, "deploy", instance) // err or not we return. If err == nil the controller is done, else requeue return reconcile.Result{}, err } From 063ca4085bac3372dbff42ef7fc7023c07b23552 Mon Sep 17 00:00:00 2001 From: Alena Varkockova Date: Tue, 27 Aug 2019 12:24:15 +0200 Subject: [PATCH 17/20] Register to updates for root instances --- .../planexecution/planexecution_controller.go | 35 ++++++++++++++++--- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/pkg/controller/planexecution/planexecution_controller.go b/pkg/controller/planexecution/planexecution_controller.go index cb358b479..d3d0d1bea 100644 --- a/pkg/controller/planexecution/planexecution_controller.go +++ b/pkg/controller/planexecution/planexecution_controller.go @@ -75,7 +75,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { // Define a mapping from the object in the event to one or more objects to // Reconcile. Specifically this calls for a reconciliation of any owned // objects. - mapFn := handler.ToRequestsFunc( + mapToOwningInstanceActivePlan := handler.ToRequestsFunc( func(a handler.MapObject) []reconcile.Request { owners := a.Meta.GetOwnerReferences() requests := make([]reconcile.Request, 0) @@ -133,7 +133,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { err = c.Watch( &source.Kind{Type: &appsv1.StatefulSet{}}, &handler.EnqueueRequestsFromMapFunc{ - ToRequests: mapFn, + ToRequests: mapToOwningInstanceActivePlan, }, p) if err != nil { @@ -142,7 +142,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { err = c.Watch( &source.Kind{Type: &appsv1.Deployment{}}, &handler.EnqueueRequestsFromMapFunc{ - ToRequests: mapFn, + ToRequests: mapToOwningInstanceActivePlan, }, p) if err != nil { @@ -151,17 +151,42 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { err = c.Watch( &source.Kind{Type: &batchv1.Job{}}, &handler.EnqueueRequestsFromMapFunc{ - ToRequests: mapFn, + ToRequests: mapToOwningInstanceActivePlan, }, p) if err != nil { return err } + // for instances we're interested in updates of instances owned by some planexecution (instance was created as part of PE) + // but also root instances of an operator that might have been updated with new activeplan err = c.Watch( &source.Kind{Type: &kudov1alpha1.Instance{}}, &handler.EnqueueRequestsFromMapFunc{ - ToRequests: mapFn, + ToRequests: handler.ToRequestsFunc( + func(a handler.MapObject) []reconcile.Request { + requests := mapToOwningInstanceActivePlan(a) + if len(requests) == 0 { + inst := &kudov1alpha1.Instance{} + err = mgr.GetClient().Get(context.TODO(), client.ObjectKey{ + Name: a.Meta.GetName(), + Namespace: a.Meta.GetNamespace(), + }, inst) + + if err != nil { + // for every updated/added instance also trigger reconcile for its active plan + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: inst.Status.ActivePlan.Name, + Namespace: inst.Status.ActivePlan.Namespace, + }, + }) + } else { + log.Printf("PlanExecutionController: received event from Instance %s/%s but instance of that name does not exist", a.Meta.GetNamespace(), a.Meta.GetName()) + } + } + return requests + }), }, p) if err != nil { From 7e9c866951ac85dbb5a685bfae1162e181df57ae Mon Sep 17 00:00:00 2001 From: Alena Varkockova Date: Wed, 28 Aug 2019 11:31:08 +0200 Subject: [PATCH 18/20] PR fixes --- pkg/controller/instance/instance_controller.go | 8 ++++---- pkg/controller/planexecution/planexecution_controller.go | 4 +++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/pkg/controller/instance/instance_controller.go b/pkg/controller/instance/instance_controller.go index f0859c79d..76c667630 100644 --- a/pkg/controller/instance/instance_controller.go +++ b/pkg/controller/instance/instance_controller.go @@ -101,7 +101,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { instance.Status.ActivePlan.Name == "" { log.Printf("InstanceController: Creating a deploy execution plan for the instance %v", instance.Name) - err = createPlan(mgr.GetClient(), mgr.GetEventRecorderFor("instance-controller"), mgr.GetScheme(), "deploy", &instance) + err = createPlanAndUpdateReference(mgr.GetClient(), mgr.GetEventRecorderFor("instance-controller"), mgr.GetScheme(), "deploy", &instance) if err != nil { log.Printf("InstanceController: Error creating \"%v\" object for \"deploy\": %v", instance.Name, err) } @@ -264,7 +264,7 @@ func instanceEventFilter(mgr manager.Manager) predicate.Funcs { } } - if err = createPlan(mgr.GetClient(), mgr.GetEventRecorderFor("instance-controller"), mgr.GetScheme(), planName, new); err != nil { + if err = createPlanAndUpdateReference(mgr.GetClient(), mgr.GetEventRecorderFor("instance-controller"), mgr.GetScheme(), planName, new); err != nil { log.Printf("InstanceController: Error creating PlanExecution \"%v\" for instance \"%v\": %v", planName, new.Name, err) } } @@ -305,7 +305,7 @@ func instanceEventFilter(mgr manager.Manager) predicate.Funcs { } } -func createPlan(c client.Client, r record.EventRecorder, scheme *runtime.Scheme, planName string, instance *kudov1alpha1.Instance) error { +func createPlanAndUpdateReference(c client.Client, r record.EventRecorder, scheme *runtime.Scheme, planName string, instance *kudov1alpha1.Instance) error { ctx := context.TODO() planExecution := newPlanExecution(instance, planName, scheme) @@ -377,7 +377,7 @@ func (r *ReconcileInstance) Reconcile(request reconcile.Request) (reconcile.Resu // if this is new create and create and assign a planexecution and return if isNewInstance(instance) { - err = createPlan(r.Client, r.recorder, r.scheme, "deploy", instance) + err = createPlanAndUpdateReference(r.Client, r.recorder, r.scheme, "deploy", instance) // err or not we return. If err == nil the controller is done, else requeue return reconcile.Result{}, err } diff --git a/pkg/controller/planexecution/planexecution_controller.go b/pkg/controller/planexecution/planexecution_controller.go index d3d0d1bea..04ba4b87c 100644 --- a/pkg/controller/planexecution/planexecution_controller.go +++ b/pkg/controller/planexecution/planexecution_controller.go @@ -254,7 +254,9 @@ func (r *ReconcilePlanExecution) Reconcile(request reconcile.Request) (reconcile if instance.Status.ActivePlan.Name != planExecution.Name || instance.Status.ActivePlan.Namespace != planExecution.Namespace { // this can happen for newly created PlanExecution where ActivePlan was not yet set to point to this instance - return reconcile.Result{}, fmt.Errorf("instance %s does not have ActivePlan pointing to PlanExecution %s, %s. Instead %s, %s. Retrying", instance.Name, planExecution.Name, planExecution.Namespace, instance.Status.ActivePlan.Name, instance.Status.ActivePlan.Namespace) + // this will get retried thanks to a watch set up for instance updates + log.Printf("instance %s does not have ActivePlan pointing to PlanExecution %s, %s. Instead %s, %s", instance.Name, planExecution.Name, planExecution.Namespace, instance.Status.ActivePlan.Name, instance.Status.ActivePlan.Namespace) + return reconcile.Result{}, nil } // Check for Suspend set. From 57645d45bffc363927fa1ca09945afec16fa7cff Mon Sep 17 00:00:00 2001 From: Alena Varkockova Date: Wed, 28 Aug 2019 12:15:58 +0200 Subject: [PATCH 19/20] Add log --- pkg/controller/planexecution/planexecution_controller.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/controller/planexecution/planexecution_controller.go b/pkg/controller/planexecution/planexecution_controller.go index 04ba4b87c..b7270b946 100644 --- a/pkg/controller/planexecution/planexecution_controller.go +++ b/pkg/controller/planexecution/planexecution_controller.go @@ -167,6 +167,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { func(a handler.MapObject) []reconcile.Request { requests := mapToOwningInstanceActivePlan(a) if len(requests) == 0 { + log.Printf("Found root instance %s", a.Meta.GetName()) inst := &kudov1alpha1.Instance{} err = mgr.GetClient().Get(context.TODO(), client.ObjectKey{ Name: a.Meta.GetName(), From 905e61f8ca71329fc2c27fa99e0343e2467a565b Mon Sep 17 00:00:00 2001 From: Alena Varkockova Date: Wed, 28 Aug 2019 12:33:42 +0200 Subject: [PATCH 20/20] Fix if-else --- pkg/controller/planexecution/planexecution_controller.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/controller/planexecution/planexecution_controller.go b/pkg/controller/planexecution/planexecution_controller.go index b7270b946..fae0c632e 100644 --- a/pkg/controller/planexecution/planexecution_controller.go +++ b/pkg/controller/planexecution/planexecution_controller.go @@ -167,14 +167,13 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { func(a handler.MapObject) []reconcile.Request { requests := mapToOwningInstanceActivePlan(a) if len(requests) == 0 { - log.Printf("Found root instance %s", a.Meta.GetName()) inst := &kudov1alpha1.Instance{} err = mgr.GetClient().Get(context.TODO(), client.ObjectKey{ Name: a.Meta.GetName(), Namespace: a.Meta.GetNamespace(), }, inst) - if err != nil { + if err == nil { // for every updated/added instance also trigger reconcile for its active plan requests = append(requests, reconcile.Request{ NamespacedName: types.NamespacedName{