From 443719cf250f299aa6809b5355870d38e63ddd92 Mon Sep 17 00:00:00 2001 From: Malte Isberner Date: Tue, 8 Jun 2021 02:31:00 +0200 Subject: [PATCH 1/5] Allow adding reconciliation extensions --- pkg/extensions/types.go | 11 +++++++ pkg/reconciler/reconciler.go | 64 ++++++++++++++++++++++++++++++++++-- 2 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 pkg/extensions/types.go diff --git a/pkg/extensions/types.go b/pkg/extensions/types.go new file mode 100644 index 0000000..1becfa3 --- /dev/null +++ b/pkg/extensions/types.go @@ -0,0 +1,11 @@ +package extensions + +import ( + "context" + "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +// ReconcileExtension is an arbitrary extension that can be implemented to run either before +// or after the main Helm reconciliation action. +type ReconcileExtension func(context.Context, unstructured.Unstructured, logr.Logger) error diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 6bbf1df..b3fd941 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -20,6 +20,7 @@ import ( "context" "errors" "fmt" + "github.com/joelanford/helm-operator/pkg/extensions" "strings" "sync" "time" @@ -67,6 +68,9 @@ type Reconciler struct { preHooks []hook.PreHook postHooks []hook.PostHook + preExtensions []extensions.ReconcileExtension + postExtensions []extensions.ReconcileExtension + log logr.Logger gvk *schema.GroupVersionKind chrt *chart.Chart @@ -354,6 +358,18 @@ func WithPreHook(h hook.PreHook) Option { } } +// WithPreExtension is an Option that configures the reconciler to run the given +// extension before performing any reconciliation steps (including values translation). +// The extension will be invoked with the raw object state; meaning it is responsible +// for determining if a deletion needs to be performed by checking the deletionTimestamp +// field. +func WithPreExtension(e extensions.ReconcileExtension) Option { + return func(r *Reconciler) error { + r.preExtensions = append(r.preExtensions, e) + return nil + } +} + // WithPostHook is an Option that configures the reconciler to run the given // PostHook just after performing any non-uninstall release actions. func WithPostHook(h hook.PostHook) Option { @@ -363,6 +379,19 @@ func WithPostHook(h hook.PostHook) Option { } } +// WithPostExtension is an Option that configures the reconciler to run the given +// extension after performing any reconciliation steps (including uninstall of the release, +// but not removal of the finalizer). +// The extension will be invoked with the raw object state; meaning it is responsible +// for determining if a deletion needs to be performed by checking the deletionTimestamp +// field. +func WithPostExtension(e extensions.ReconcileExtension) Option { + return func(r *Reconciler) error { + r.postExtensions = append(r.postExtensions, e) + return nil + } +} + // WithValueTranslator is an Option that configures a function that translates a // custom resource to the values passed to Helm. // Use this if you need to customize the logic that translates your custom resource to Helm values. @@ -466,6 +495,16 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl. } u.UpdateStatus(updater.EnsureCondition(conditions.Initialized(corev1.ConditionTrue, "", ""))) + for _, ext := range r.preExtensions { + if err := ext(ctx, *obj, r.log); err != nil { + u.UpdateStatus( + updater.EnsureCondition(conditions.Irreconcilable(corev1.ConditionTrue, conditions.ReasonReconcileError, err)), + updater.EnsureConditionUnknown(conditions.TypeReleaseFailed), + ) + return ctrl.Result{}, err + } + } + if obj.GetDeletionTimestamp() != nil { err := r.handleDeletion(ctx, actionClient, obj, log) return ctrl.Result{}, err @@ -525,6 +564,16 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl. } } + for _, ext := range r.postExtensions { + if err := ext(ctx, *obj, r.log); err != nil { + u.UpdateStatus( + updater.EnsureCondition(conditions.Irreconcilable(corev1.ConditionTrue, conditions.ReasonReconcileError, err)), + updater.EnsureConditionUnknown(conditions.TypeReleaseFailed), + ) + return ctrl.Result{}, err + } + } + ensureDeployedRelease(&u, rel) u.UpdateStatus( updater.EnsureCondition(conditions.ReleaseFailed(corev1.ConditionFalse, "", "")), @@ -580,7 +629,7 @@ func (r *Reconciler) handleDeletion(ctx context.Context, actionClient helmclient err = applyErr } }() - return r.doUninstall(actionClient, &uninstallUpdater, obj, log) + return r.doUninstall(ctx, actionClient, &uninstallUpdater, obj, log) }(); err != nil { return err } @@ -697,7 +746,7 @@ func (r *Reconciler) doReconcile(actionClient helmclient.ActionInterface, u *upd return nil } -func (r *Reconciler) doUninstall(actionClient helmclient.ActionInterface, u *updater.Updater, obj *unstructured.Unstructured, log logr.Logger) error { +func (r *Reconciler) doUninstall(ctx context.Context, actionClient helmclient.ActionInterface, u *updater.Updater, obj *unstructured.Unstructured, log logr.Logger) error { var opts []helmclient.UninstallOption for name, annot := range r.uninstallAnnotations { if v, ok := obj.GetAnnotations()[name]; ok { @@ -717,6 +766,17 @@ func (r *Reconciler) doUninstall(actionClient helmclient.ActionInterface, u *upd } else { log.Info("Release uninstalled", "name", resp.Release.Name, "version", resp.Release.Version) } + + for _, ext := range r.postExtensions { + if err := ext(ctx, *obj, r.log); err != nil { + u.UpdateStatus( + updater.EnsureCondition(conditions.Irreconcilable(corev1.ConditionTrue, conditions.ReasonReconcileError, err)), + updater.EnsureConditionUnknown(conditions.TypeReleaseFailed), + ) + return err + } + } + u.Update(updater.RemoveFinalizer(uninstallFinalizer)) u.UpdateStatus( updater.EnsureCondition(conditions.ReleaseFailed(corev1.ConditionFalse, "", "")), From add9575e34f2a332100c7a0f345bc65b628b1646 Mon Sep 17 00:00:00 2001 From: Malte Isberner Date: Tue, 8 Jun 2021 02:42:37 +0200 Subject: [PATCH 2/5] pointer --- pkg/extensions/types.go | 2 +- pkg/reconciler/reconciler.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/extensions/types.go b/pkg/extensions/types.go index 1becfa3..ef7a02d 100644 --- a/pkg/extensions/types.go +++ b/pkg/extensions/types.go @@ -8,4 +8,4 @@ import ( // ReconcileExtension is an arbitrary extension that can be implemented to run either before // or after the main Helm reconciliation action. -type ReconcileExtension func(context.Context, unstructured.Unstructured, logr.Logger) error +type ReconcileExtension func(context.Context, *unstructured.Unstructured, logr.Logger) error diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index b3fd941..7c0d594 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -496,7 +496,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl. u.UpdateStatus(updater.EnsureCondition(conditions.Initialized(corev1.ConditionTrue, "", ""))) for _, ext := range r.preExtensions { - if err := ext(ctx, *obj, r.log); err != nil { + if err := ext(ctx, obj, r.log); err != nil { u.UpdateStatus( updater.EnsureCondition(conditions.Irreconcilable(corev1.ConditionTrue, conditions.ReasonReconcileError, err)), updater.EnsureConditionUnknown(conditions.TypeReleaseFailed), @@ -565,7 +565,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl. } for _, ext := range r.postExtensions { - if err := ext(ctx, *obj, r.log); err != nil { + if err := ext(ctx, obj, r.log); err != nil { u.UpdateStatus( updater.EnsureCondition(conditions.Irreconcilable(corev1.ConditionTrue, conditions.ReasonReconcileError, err)), updater.EnsureConditionUnknown(conditions.TypeReleaseFailed), @@ -768,7 +768,7 @@ func (r *Reconciler) doUninstall(ctx context.Context, actionClient helmclient.Ac } for _, ext := range r.postExtensions { - if err := ext(ctx, *obj, r.log); err != nil { + if err := ext(ctx, obj, r.log); err != nil { u.UpdateStatus( updater.EnsureCondition(conditions.Irreconcilable(corev1.ConditionTrue, conditions.ReasonReconcileError, err)), updater.EnsureConditionUnknown(conditions.TypeReleaseFailed), From a30b47c52cfd045e8c074a98265970d9e3b885d7 Mon Sep 17 00:00:00 2001 From: Malte Isberner <2822367+misberner@users.noreply.github.com> Date: Tue, 15 Jun 2021 12:03:12 +0200 Subject: [PATCH 3/5] Update pkg/extensions/types.go Co-authored-by: Marcin Owsiany --- pkg/extensions/types.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/extensions/types.go b/pkg/extensions/types.go index ef7a02d..689f91d 100644 --- a/pkg/extensions/types.go +++ b/pkg/extensions/types.go @@ -8,4 +8,5 @@ import ( // ReconcileExtension is an arbitrary extension that can be implemented to run either before // or after the main Helm reconciliation action. +// An error returned by a ReconcileExtension will cause the Reconcile to fail, unlike a hook error. type ReconcileExtension func(context.Context, *unstructured.Unstructured, logr.Logger) error From 4fac4d7529c602537eb0facfddb7d489dc5ab6d0 Mon Sep 17 00:00:00 2001 From: Malte Isberner <2822367+misberner@users.noreply.github.com> Date: Tue, 15 Jun 2021 12:05:53 +0200 Subject: [PATCH 4/5] Update pkg/reconciler/reconciler.go Co-authored-by: Marcin Owsiany --- pkg/reconciler/reconciler.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 7c0d594..7a526c1 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -360,9 +360,8 @@ func WithPreHook(h hook.PreHook) Option { // WithPreExtension is an Option that configures the reconciler to run the given // extension before performing any reconciliation steps (including values translation). -// The extension will be invoked with the raw object state; meaning it is responsible -// for determining if a deletion needs to be performed by checking the deletionTimestamp -// field. +// The extension will be invoked with the raw object state; meaning it needs to be careful +// to check for existence of the deletionTimestamp field. func WithPreExtension(e extensions.ReconcileExtension) Option { return func(r *Reconciler) error { r.preExtensions = append(r.preExtensions, e) From e1b8540b5eb5f0a97e5c6a60deb727110ac52656 Mon Sep 17 00:00:00 2001 From: Malte Isberner Date: Tue, 15 Jun 2021 12:10:26 +0200 Subject: [PATCH 5/5] review comments --- pkg/reconciler/reconciler.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 7a526c1..f34d110 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -20,7 +20,6 @@ import ( "context" "errors" "fmt" - "github.com/joelanford/helm-operator/pkg/extensions" "strings" "sync" "time" @@ -47,6 +46,7 @@ import ( "github.com/joelanford/helm-operator/pkg/annotation" helmclient "github.com/joelanford/helm-operator/pkg/client" + "github.com/joelanford/helm-operator/pkg/extensions" "github.com/joelanford/helm-operator/pkg/hook" "github.com/joelanford/helm-operator/pkg/internal/sdk/controllerutil" "github.com/joelanford/helm-operator/pkg/reconciler/internal/conditions" @@ -360,6 +360,9 @@ func WithPreHook(h hook.PreHook) Option { // WithPreExtension is an Option that configures the reconciler to run the given // extension before performing any reconciliation steps (including values translation). +// An error returned from the extension will cause the reconciliation to fail. +// This should be preferred to WithPreHook in most cases, except for when the logic +// depends on the translated Helm values. // The extension will be invoked with the raw object state; meaning it needs to be careful // to check for existence of the deletionTimestamp field. func WithPreExtension(e extensions.ReconcileExtension) Option { @@ -381,9 +384,12 @@ func WithPostHook(h hook.PostHook) Option { // WithPostExtension is an Option that configures the reconciler to run the given // extension after performing any reconciliation steps (including uninstall of the release, // but not removal of the finalizer). -// The extension will be invoked with the raw object state; meaning it is responsible -// for determining if a deletion needs to be performed by checking the deletionTimestamp -// field. +// An error returned from the extension will cause the reconciliation to fail, which might +// prevent the finalizer from getting removed. +// This should be preferred to WithPostHook in most cases, except for when the logic +// depends on the translated Helm values. +// The extension will be invoked with the raw object state; meaning it needs to be careful +// to check for existence of the deletionTimestamp field. func WithPostExtension(e extensions.ReconcileExtension) Option { return func(r *Reconciler) error { r.postExtensions = append(r.postExtensions, e)