From d1dcbdfa2a7499c1bfab389445cc3ca5e2976376 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20B=C3=A4umer?= Date: Tue, 18 May 2021 19:21:56 +0200 Subject: [PATCH 01/13] Fork helm-operator adjustments for CI and documentation (#4) --- .github/labeler.yml | 2 ++ .github/workflows/ci.yml | 6 +---- .github/workflows/deploy.yml | 19 ++++++++-------- README.md | 44 ++++++++++++++++++++++++++++++++++-- 4 files changed, 55 insertions(+), 16 deletions(-) diff --git a/.github/labeler.yml b/.github/labeler.yml index 4dc4ae9..6b2ba9e 100644 --- a/.github/labeler.yml +++ b/.github/labeler.yml @@ -1,3 +1,5 @@ +upstream-triage: + - "./*" area/main-binary: - changed-files: - any-glob-to-any-file: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3b538af..d52ba7e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,11 +1,7 @@ name: CI on: - merge_group: - push: - branches: - - 'main' - pull_request: + - push jobs: test: diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 204b18f..2c75ca0 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -1,14 +1,15 @@ name: Deploy -on: - merge_group: - push: - branches: - - 'main' - tags: - - 'v*' - pull_request: - branches: [ main ] +# Disabled as we don't need docker images to use the helm-operator as a library. +#on: +# merge_group: +# push: +# branches: +# - 'main' +# tags: +# - 'v*' +# pull_request: +# branches: [ main ] jobs: goreleaser: diff --git a/README.md b/README.md index 5123a9d..90dce68 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,6 @@ # helm-operator -![Build Status](https://github.com/operator-framework/helm-operator-plugins/workflows/CI/badge.svg?branch=main) -[![Coverage Status](https://coveralls.io/repos/github/operator-framework/helm-operator-plugins/badge.svg?branch=main)](https://coveralls.io/github/operator-framework/helm-operator-plugins?branch=main) +[![Build Status](https://github.com/stackrox/helm-operator/workflows/CI/badge.svg?branch=main) Reimplementation of the helm operator to enrich the Helm operator's reconciliation with custom Go code to create a hybrid operator. @@ -41,3 +40,44 @@ if err := reconciler.SetupWithManager(mgr); err != nil { panic(fmt.Sprintf("unable to create reconciler: %s", err)) } ``` + +## Why a fork? + +Initially the Helm operator type was designed to automate Helm chart operations +by mapping the [values](https://helm.sh/docs/chart_template_guide/values_files/) of a Helm chart exactly to a +`CustomResourceDefinition` and defining its watched resources in a `watches.yaml` +[configuration](https://sdk.operatorframework.io/docs/building-operators/helm/tutorial/#watch-the-nginx-cr). + +To write a [Level II+](https://sdk.operatorframework.io/docs/advanced-topics/operator-capabilities/operator-capabilities/) operator +which reuses an already existing Helm chart a [hybrid](https://github.com/operator-framework/operator-sdk/issues/670) +between the Go and Helm operator type is necessary. + +The hybrid approach allows to add customizations to the Helm operator like value mapping based on cluster state or +executing code in on specific events. + +### Quickstart + +Add this module as a replace directive to your `go.mod`: + +``` +go mod edit -replace=github.com/joelanford/helm-operator=github.com/stackrox/helm-operator@main +``` + +Example: + +``` +chart, err := loader.Load("path/to/chart") +if err != nil { + panic(err) +} + +reconciler := reconciler.New( + reconciler.WithChart(*chart), + reconciler.WithGroupVersionKind(gvk), +) + +if err := reconciler.SetupWithManager(mgr); err != nil { + panic(fmt.Sprintf("unable to create reconciler: %s", err)) +} +``` + From 02067ba71cdf44ed1efa7e57677addd66732a9d3 Mon Sep 17 00:00:00 2001 From: Gaurav Nelson Date: Thu, 20 May 2021 17:22:49 +1000 Subject: [PATCH 02/13] Updated README (#5)(#7) --- README.md | 53 +++++++++++++++++++++++++++-------------------------- 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index 90dce68..ab4a235 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # helm-operator -[![Build Status](https://github.com/stackrox/helm-operator/workflows/CI/badge.svg?branch=main) +![Build Status](https://github.com/stackrox/helm-operator/workflows/CI/badge.svg?branch=main) Reimplementation of the helm operator to enrich the Helm operator's reconciliation with custom Go code to create a hybrid operator. @@ -43,41 +43,42 @@ if err := reconciler.SetupWithManager(mgr); err != nil { ## Why a fork? -Initially the Helm operator type was designed to automate Helm chart operations +The Helm operator type automates Helm chart operations by mapping the [values](https://helm.sh/docs/chart_template_guide/values_files/) of a Helm chart exactly to a `CustomResourceDefinition` and defining its watched resources in a `watches.yaml` -[configuration](https://sdk.operatorframework.io/docs/building-operators/helm/tutorial/#watch-the-nginx-cr). +[configuration](https://sdk.operatorframework.io/docs/building-operators/helm/tutorial/#watch-the-nginx-cr) file. -To write a [Level II+](https://sdk.operatorframework.io/docs/advanced-topics/operator-capabilities/operator-capabilities/) operator -which reuses an already existing Helm chart a [hybrid](https://github.com/operator-framework/operator-sdk/issues/670) -between the Go and Helm operator type is necessary. +For creating a [Level II+](https://sdk.operatorframework.io/docs/advanced-topics/operator-capabilities/operator-capabilities/) operator +that reuses an already existing Helm chart, we need a [hybrid](https://github.com/operator-framework/operator-sdk/issues/670) +between the Go and Helm operator types. -The hybrid approach allows to add customizations to the Helm operator like value mapping based on cluster state or -executing code in on specific events. +The hybrid approach allows adding customizations to the Helm operator, such as: +- value mapping based on cluster state, or +- executing code in specific events. ### Quickstart -Add this module as a replace directive to your `go.mod`: +- Add this module as a replace directive to your `go.mod`: -``` -go mod edit -replace=github.com/joelanford/helm-operator=github.com/stackrox/helm-operator@main -``` + ``` + go mod edit -replace=github.com/joelanford/helm-operator=github.com/stackrox/helm-operator@main + ``` -Example: + For example: -``` -chart, err := loader.Load("path/to/chart") -if err != nil { - panic(err) -} + ```go + chart, err := loader.Load("path/to/chart") + if err != nil { + panic(err) + } -reconciler := reconciler.New( - reconciler.WithChart(*chart), - reconciler.WithGroupVersionKind(gvk), -) + reconciler := reconciler.New( + reconciler.WithChart(*chart), + reconciler.WithGroupVersionKind(gvk), + ) -if err := reconciler.SetupWithManager(mgr); err != nil { - panic(fmt.Sprintf("unable to create reconciler: %s", err)) -} -``` + if err := reconciler.SetupWithManager(mgr); err != nil { + panic(fmt.Sprintf("unable to create reconciler: %s", err)) + } + ``` From b4007abfac7f380ecc2750cb37ddf059a50fdda0 Mon Sep 17 00:00:00 2001 From: Malte Isberner <2822367+misberner@users.noreply.github.com> Date: Tue, 15 Jun 2021 12:16:55 +0200 Subject: [PATCH 03/13] Allow adding reconciliation extensions (#9) --- pkg/extensions/types.go | 12 +++++++ pkg/reconciler/reconciler.go | 69 ++++++++++++++++++++++++++++++++++-- 2 files changed, 79 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..689f91d --- /dev/null +++ b/pkg/extensions/types.go @@ -0,0 +1,12 @@ +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. +// 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 diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index dd64819..b8bc7b0 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -57,6 +57,7 @@ import ( "github.com/operator-framework/helm-operator-plugins/pkg/reconciler/internal/updater" internalvalues "github.com/operator-framework/helm-operator-plugins/pkg/reconciler/internal/values" "github.com/operator-framework/helm-operator-plugins/pkg/values" + "github.com/joelanford/helm-operator/pkg/extensions" ) const uninstallFinalizer = "uninstall-helm-release" @@ -71,6 +72,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 @@ -449,6 +453,20 @@ 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 { + 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 { @@ -458,6 +476,22 @@ 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). +// 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) + 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. @@ -622,6 +656,16 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } 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 { if err := r.handleDeletion(ctx, actionClient, obj, log); err != nil { return ctrl.Result{}, err @@ -684,6 +728,16 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } } + 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, "", "")), @@ -734,7 +788,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 } @@ -886,7 +940,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 { @@ -911,6 +965,17 @@ func (r *Reconciler) doUninstall(actionClient helmclient.ActionInterface, u *upd fmt.Println(diff.Generate(resp.Release.Manifest, "")) } } + + 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 ac3969587d768d5cec3d1bab4c455005d31c5fb0 Mon Sep 17 00:00:00 2001 From: Malte Isberner <2822367+misberner@users.noreply.github.com> Date: Tue, 29 Jun 2021 15:16:01 +0200 Subject: [PATCH 04/13] Allow marking releases stuck in a pending state as failed (#16) --- pkg/client/actionclient.go | 9 ++++ .../internal/conditions/conditions.go | 1 + pkg/reconciler/internal/fake/actionclient.go | 34 +++++++++---- pkg/reconciler/reconciler.go | 51 +++++++++++++++++++ 4 files changed, 84 insertions(+), 11 deletions(-) diff --git a/pkg/client/actionclient.go b/pkg/client/actionclient.go index 5cba3dd..6655ad7 100644 --- a/pkg/client/actionclient.go +++ b/pkg/client/actionclient.go @@ -54,6 +54,7 @@ type ActionInterface interface { Get(name string, opts ...GetOption) (*release.Release, error) Install(name, namespace string, chrt *chart.Chart, vals map[string]interface{}, opts ...InstallOption) (*release.Release, error) Upgrade(name, namespace string, chrt *chart.Chart, vals map[string]interface{}, opts ...UpgradeOption) (*release.Release, error) + MarkFailed(release *release.Release, reason string) error Uninstall(name string, opts ...UninstallOption) (*release.UninstallReleaseResponse, error) Reconcile(rel *release.Release) error } @@ -278,6 +279,14 @@ func (c *actionClient) Upgrade(name, namespace string, chrt *chart.Chart, vals m return rel, nil } +func (c *actionClient) MarkFailed(rel *release.Release, reason string) error { + infoCopy := *rel.Info + releaseCopy := *rel + releaseCopy.Info = &infoCopy + releaseCopy.SetStatus(release.StatusFailed, reason) + return c.conf.Releases.Update(&releaseCopy) +} + func (c *actionClient) rollback(name string, opts ...RollbackOption) error { rollback := action.NewRollback(c.conf) for _, o := range opts { diff --git a/pkg/reconciler/internal/conditions/conditions.go b/pkg/reconciler/internal/conditions/conditions.go index 55e2c65..86beb90 100644 --- a/pkg/reconciler/internal/conditions/conditions.go +++ b/pkg/reconciler/internal/conditions/conditions.go @@ -41,6 +41,7 @@ const ( ReasonUpgradeError = status.ConditionReason("UpgradeError") ReasonReconcileError = status.ConditionReason("ReconcileError") ReasonUninstallError = status.ConditionReason("UninstallError") + ReasonPendingError = status.ConditionReason("PendingError") ) func Initialized(stat corev1.ConditionStatus, reason status.ConditionReason, message interface{}) status.Condition { diff --git a/pkg/reconciler/internal/fake/actionclient.go b/pkg/reconciler/internal/fake/actionclient.go index e35cb3f..f5ce268 100644 --- a/pkg/reconciler/internal/fake/actionclient.go +++ b/pkg/reconciler/internal/fake/actionclient.go @@ -49,17 +49,19 @@ func (hcg *fakeActionClientGetter) ActionClientFor(_ context.Context, _ crclient } type ActionClient struct { - Gets []GetCall - Installs []InstallCall - Upgrades []UpgradeCall - Uninstalls []UninstallCall - Reconciles []ReconcileCall - - HandleGet func() (*release.Release, error) - HandleInstall func() (*release.Release, error) - HandleUpgrade func() (*release.Release, error) - HandleUninstall func() (*release.UninstallReleaseResponse, error) - HandleReconcile func() error + Gets []GetCall + Installs []InstallCall + Upgrades []UpgradeCall + MarkFaileds []MarkFailedCall + Uninstalls []UninstallCall + Reconciles []ReconcileCall + + HandleGet func() (*release.Release, error) + HandleInstall func() (*release.Release, error) + HandleUpgrade func() (*release.Release, error) + HandleMarkFailed func() error + HandleUninstall func() (*release.UninstallReleaseResponse, error) + HandleReconcile func() error } func NewActionClient() ActionClient { @@ -110,6 +112,11 @@ type UpgradeCall struct { Opts []client.UpgradeOption } +type MarkFailedCall struct { + Release *release.Release + Reason string +} + type UninstallCall struct { Name string Opts []client.UninstallOption @@ -134,6 +141,11 @@ func (c *ActionClient) Upgrade(name, namespace string, chrt *chart.Chart, vals m return c.HandleUpgrade() } +func (c *ActionClient) MarkFailed(rel *release.Release, reason string) error { + c.MarkFaileds = append(c.MarkFaileds, MarkFailedCall{rel, reason}) + return c.HandleMarkFailed() +} + func (c *ActionClient) Uninstall(name string, opts ...client.UninstallOption) (*release.UninstallReleaseResponse, error) { c.Uninstalls = append(c.Uninstalls, UninstallCall{name, opts}) return c.HandleUninstall() diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index b8bc7b0..72957fc 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -85,6 +85,7 @@ type Reconciler struct { reconcilePeriod time.Duration waitForDeletionTimeout time.Duration maxReleaseHistory *int + markFailedAfter time.Duration skipPrimaryGVKSchemeRegistration bool controllerSetupFuncs []ControllerSetupFunc @@ -380,6 +381,18 @@ func WithMaxReleaseHistory(maxHistory int) Option { } } +// WithMarkFailedAfter specifies the duration after which the reconciler will mark a release in a pending (locked) +// state as false in order to allow rolling forward. +func WithMarkFailedAfter(duration time.Duration) Option { + return func(r *Reconciler) error { + if duration < 0 { + return errors.New("auto-rollback after duration must not be negative") + } + r.markFailedAfter = duration + return nil + } +} + // WithInstallAnnotations is an Option that configures Install annotations // to enable custom action.Install fields to be set based on the value of // annotations found in the custom resource watched by this reconciler. @@ -693,6 +706,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu ) return ctrl.Result{}, err } + if state == statePending { + return r.handlePending(actionClient, rel, &u, log) + } + u.UpdateStatus(updater.EnsureCondition(conditions.Irreconcilable(corev1.ConditionFalse, "", ""))) for _, h := range r.preHooks { @@ -769,6 +786,7 @@ const ( stateNeedsInstall helmReleaseState = "needs install" stateNeedsUpgrade helmReleaseState = "needs upgrade" stateUnchanged helmReleaseState = "unchanged" + statePending helmReleaseState = "pending" stateError helmReleaseState = "error" ) @@ -818,6 +836,10 @@ func (r *Reconciler) getReleaseState(client helmclient.ActionInterface, obj meta return nil, stateNeedsInstall, nil } + if currentRelease.Info != nil && currentRelease.Info.Status.IsPending() { + return currentRelease, statePending, nil + } + var opts []helmclient.UpgradeOption if *r.maxReleaseHistory > 0 { opts = append(opts, func(u *action.Upgrade) error { @@ -913,6 +935,35 @@ func (r *Reconciler) doUpgrade(actionClient helmclient.ActionInterface, u *updat return rel, nil } +func (r *Reconciler) handlePending(actionClient helmclient.ActionInterface, rel *release.Release, u *updater.Updater, log logr.Logger) (ctrl.Result, error) { + err := r.doHandlePending(actionClient, rel, log) + if err == nil { + err = errors.New("unknown error handling pending release") + } + u.UpdateStatus( + updater.EnsureCondition(conditions.Irreconcilable(corev1.ConditionTrue, conditions.ReasonPendingError, err))) + return ctrl.Result{}, err +} + +func (r *Reconciler) doHandlePending(actionClient helmclient.ActionInterface, rel *release.Release, log logr.Logger) error { + if r.markFailedAfter <= 0 { + return errors.New("Release is in a pending (locked) state and cannot be modified. User intervention is required.") + } + if rel.Info == nil || rel.Info.LastDeployed.IsZero() { + return errors.New("Release is in a pending (locked) state and lacks 'last deployed' timestamp. User intervention is required.") + } + if pendingSince := time.Since(rel.Info.LastDeployed.Time); pendingSince < r.markFailedAfter { + return fmt.Errorf("Release is in a pending (locked) state and cannot currently be modified. Release will be marked failed to allow a roll-forward in %v.", r.markFailedAfter-pendingSince) + } + + log.Info("Marking release as failed", "releaseName", rel.Name) + err := actionClient.MarkFailed(rel, fmt.Sprintf("operator marked pending (locked) release as failed after state did not change for %v", r.markFailedAfter)) + if err != nil { + return fmt.Errorf("Failed to mark pending (locked) release as failed: %w", err) + } + return fmt.Errorf("marked release %s as failed to allow upgrade to succeed in next reconcile attempt", rel.Name) +} + func (r *Reconciler) reportOverrideEvents(obj runtime.Object) { for k, v := range r.overrideValues { r.eventRecorder.Eventf(obj, "Warning", "ValueOverridden", From eac78a5f5bc4f11d0e4fab2de78983b50c74af00 Mon Sep 17 00:00:00 2001 From: Malte Isberner <2822367+misberner@users.noreply.github.com> Date: Thu, 1 Jul 2021 12:01:09 +0200 Subject: [PATCH 05/13] Allow stripping manifest from the CR status (#18) --- pkg/reconciler/reconciler.go | 27 ++++++++++++++++++++++++--- pkg/reconciler/reconciler_test.go | 10 ++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 72957fc..16568ff 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -89,6 +89,8 @@ type Reconciler struct { skipPrimaryGVKSchemeRegistration bool controllerSetupFuncs []ControllerSetupFunc + stripManifestFromStatus bool + annotSetupOnce sync.Once annotations map[string]struct{} installAnnotations map[string]annotation.Install @@ -279,6 +281,17 @@ func SkipDependentWatches(skip bool) Option { } } +// StripManifestFromStatus is an Option that configures whether the manifest +// should be removed from the automatically populated status. +// This is recommended if the manifest might return sensitive data (i.e., +// secrets). +func StripManifestFromStatus(strip bool) Option { + return func(r *Reconciler) error { + r.stripManifestFromStatus = strip + return nil + } +} + // SkipPrimaryGVKSchemeRegistration is an Option that allows to disable the default behaviour of // registering unstructured.Unstructured as underlying type for the GVK scheme. // @@ -665,7 +678,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu if errors.Is(err, driver.ErrReleaseNotFound) { u.UpdateStatus(updater.EnsureCondition(conditions.Deployed(corev1.ConditionFalse, "", ""))) } else if err == nil { - ensureDeployedRelease(&u, rel) + r.ensureDeployedRelease(&u, rel) } u.UpdateStatus(updater.EnsureCondition(conditions.Initialized(corev1.ConditionTrue, "", ""))) @@ -755,7 +768,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } } - ensureDeployedRelease(&u, rel) + r.ensureDeployedRelease(&u, rel) u.UpdateStatus( updater.EnsureCondition(conditions.ReleaseFailed(corev1.ConditionFalse, "", "")), updater.EnsureCondition(conditions.Irreconcilable(corev1.ConditionFalse, "", "")), @@ -1137,7 +1150,7 @@ func (r *Reconciler) setupWatches(mgr ctrl.Manager, c controller.Controller) err return nil } -func ensureDeployedRelease(u *updater.Updater, rel *release.Release) { +func (r *Reconciler) ensureDeployedRelease(u *updater.Updater, rel *release.Release) { reason := conditions.ReasonInstallSuccessful message := "release was successfully installed" if rel.Version > 1 { @@ -1147,6 +1160,14 @@ func ensureDeployedRelease(u *updater.Updater, rel *release.Release) { if rel.Info != nil && len(rel.Info.Notes) > 0 { message = rel.Info.Notes } + + if r.stripManifestFromStatus { + relCopy := *rel + relCopy.Manifest = "" + rel = &relCopy + } + + u.Update(updater.EnsureFinalizer(uninstallFinalizer)) u.UpdateStatus( updater.EnsureCondition(conditions.Deployed(corev1.ConditionTrue, reason, message)), updater.EnsureDeployedRelease(rel), diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index cb8331b..3ff7780 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -198,6 +198,16 @@ var _ = Describe("Reconciler", func() { Expect(r.skipDependentWatches).To(BeTrue()) }) }) + _ = Describe("StripManifestFromStatus", func() { + It("should set to false", func() { + Expect(StripManifestFromStatus(false)(r)).To(Succeed()) + Expect(r.stripManifestFromStatus).To(Equal(false)) + }) + It("should set to true", func() { + Expect(StripManifestFromStatus(true)(r)).To(Succeed()) + Expect(r.stripManifestFromStatus).To(Equal(true)) + }) + }) _ = Describe("WithMaxConcurrentReconciles", func() { It("should set the reconciler max concurrent reconciled", func() { Expect(WithMaxConcurrentReconciles(1)(r)).To(Succeed()) From 0c607d24bf6732a5d8ce6dfac507c6dc7970f0e2 Mon Sep 17 00:00:00 2001 From: Malte Isberner <2822367+misberner@users.noreply.github.com> Date: Tue, 6 Jul 2021 02:52:54 +0200 Subject: [PATCH 06/13] ROX-7242: Make the operator preserve custom statuses, and allow updating custom status through extensions (#17) --- pkg/extensions/types.go | 7 +- pkg/reconciler/internal/updater/updater.go | 40 +++++++++-- .../internal/updater/updater_test.go | 70 ++++++++++++++++++- pkg/reconciler/reconciler.go | 6 +- 4 files changed, 111 insertions(+), 12 deletions(-) diff --git a/pkg/extensions/types.go b/pkg/extensions/types.go index 689f91d..fe449f8 100644 --- a/pkg/extensions/types.go +++ b/pkg/extensions/types.go @@ -2,11 +2,16 @@ package extensions import ( "context" + "github.com/go-logr/logr" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) +// UpdateStatusFunc is a function that updates an unstructured status. If the status has been modified, +// true must be returned, false otherwise. +type UpdateStatusFunc func(*unstructured.Unstructured) bool + // 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 +type ReconcileExtension func(context.Context, *unstructured.Unstructured, func(UpdateStatusFunc), logr.Logger) error diff --git a/pkg/reconciler/internal/updater/updater.go b/pkg/reconciler/internal/updater/updater.go index 1508c32..dccabe1 100644 --- a/pkg/reconciler/internal/updater/updater.go +++ b/pkg/reconciler/internal/updater/updater.go @@ -28,6 +28,7 @@ import ( "k8s.io/client-go/util/retry" "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/operator-framework/helm-operator/pkg/extensions" "github.com/operator-framework/helm-operator-plugins/internal/sdk/controllerutil" "github.com/operator-framework/helm-operator-plugins/pkg/internal/status" ) @@ -56,6 +57,21 @@ func (u *Updater) UpdateStatus(fs ...UpdateStatusFunc) { u.updateStatusFuncs = append(u.updateStatusFuncs, fs...) } +func (u *Updater) UpdateStatusCustom(f extensions.UpdateStatusFunc) { + updateFn := func(status *helmAppStatus) bool { + status.updateStatusObject() + + unstructuredStatus := unstructured.Unstructured{Object: status.StatusObject} + if !f(&unstructuredStatus) { + return false + } + _ = runtime.DefaultUnstructuredConverter.FromUnstructured(unstructuredStatus.Object, status) + status.StatusObject = unstructuredStatus.Object + return true + } + u.UpdateStatus(updateFn) +} + func (u *Updater) CancelUpdates() { u.isCanceled = true } @@ -94,12 +110,8 @@ func (u *Updater) Apply(ctx context.Context, obj *unstructured.Unstructured) err // we remove the finalizer, updating the status will fail // because the object and its status will be garbage-collected. if needsStatusUpdate { - uSt, err := runtime.DefaultUnstructuredConverter.ToUnstructured(st) - if err != nil { - return err - } - obj.Object["status"] = uSt - + st.updateStatusObject() + obj.Object["status"] = st.StatusObject if err := retryOnRetryableUpdateError(backoff, func() error { return u.client.Status().Update(ctx, obj) }); err != nil { @@ -166,10 +178,25 @@ func RemoveDeployedRelease() UpdateStatusFunc { } type helmAppStatus struct { + StatusObject map[string]interface{} `json:"-"` + Conditions status.Conditions `json:"conditions"` DeployedRelease *helmAppRelease `json:"deployedRelease,omitempty"` } +func (s *helmAppStatus) updateStatusObject() { + unstructuredHelmAppStatus, _ := runtime.DefaultUnstructuredConverter.ToUnstructured(s) + if s.StatusObject == nil { + s.StatusObject = make(map[string]interface{}) + } + s.StatusObject["conditions"] = unstructuredHelmAppStatus["conditions"] + if deployedRelease := unstructuredHelmAppStatus["deployedRelease"]; deployedRelease != nil { + s.StatusObject["deployedRelease"] = deployedRelease + } else { + delete(s.StatusObject, "deployedRelease") + } +} + type helmAppRelease struct { Name string `json:"name,omitempty"` Manifest string `json:"manifest,omitempty"` @@ -192,6 +219,7 @@ func statusFor(obj *unstructured.Unstructured) *helmAppStatus { case map[string]interface{}: out := &helmAppStatus{} _ = runtime.DefaultUnstructuredConverter.FromUnstructured(s, out) + out.StatusObject = s return out default: return &helmAppStatus{} diff --git a/pkg/reconciler/internal/updater/updater_test.go b/pkg/reconciler/internal/updater/updater_test.go index fa84e86..e42b1c5 100644 --- a/pkg/reconciler/internal/updater/updater_test.go +++ b/pkg/reconciler/internal/updater/updater_test.go @@ -108,6 +108,71 @@ var _ = Describe("Updater", func() { Expect((obj.Object["status"].(map[string]interface{}))["conditions"]).To(HaveLen(1)) Expect(obj.GetResourceVersion()).NotTo(Equal(resourceVersion)) }) + + It("should support a mix of standard and custom status updates", func() { + u.UpdateStatus(EnsureCondition(conditions.Deployed(corev1.ConditionTrue, "", ""))) + u.UpdateStatusCustom(func(uSt *unstructured.Unstructured) bool { + Expect(unstructured.SetNestedMap(uSt.Object, map[string]interface{}{"bar": "baz"}, "foo")).To(Succeed()) + return true + }) + u.UpdateStatus(EnsureCondition(conditions.Irreconcilable(corev1.ConditionFalse, "", ""))) + u.UpdateStatusCustom(func(uSt *unstructured.Unstructured) bool { + Expect(unstructured.SetNestedField(uSt.Object, "quux", "foo", "qux")).To(Succeed()) + return true + }) + u.UpdateStatus(EnsureCondition(conditions.Initialized(corev1.ConditionTrue, "", ""))) + + Expect(u.Apply(context.TODO(), obj)).To(Succeed()) + Expect(cl.Get(context.TODO(), types.NamespacedName{Namespace: "testNamespace", Name: "testDeployment"}, obj)).To(Succeed()) + Expect((obj.Object["status"].(map[string]interface{}))["conditions"]).To(HaveLen(3)) + _, found, err := unstructured.NestedFieldNoCopy(obj.Object, "status", "deployedRelease") + Expect(found).To(BeFalse()) + Expect(err).To(Not(HaveOccurred())) + + val, found, err := unstructured.NestedString(obj.Object, "status", "foo", "bar") + Expect(val).To(Equal("baz")) + Expect(found).To(BeTrue()) + Expect(err).To(Not(HaveOccurred())) + + val, found, err = unstructured.NestedString(obj.Object, "status", "foo", "qux") + Expect(val).To(Equal("quux")) + Expect(found).To(BeTrue()) + Expect(err).To(Not(HaveOccurred())) + }) + + It("should preserve any custom status across multiple apply calls", func() { + u.UpdateStatusCustom(func(uSt *unstructured.Unstructured) bool { + Expect(unstructured.SetNestedMap(uSt.Object, map[string]interface{}{"bar": "baz"}, "foo")).To(Succeed()) + return true + }) + Expect(u.Apply(context.TODO(), obj)).To(Succeed()) + + Expect(cl.Get(context.TODO(), types.NamespacedName{Namespace: "testNamespace", Name: "testDeployment"}, obj)).To(Succeed()) + + _, found, err := unstructured.NestedFieldNoCopy(obj.Object, "status", "deployedRelease") + Expect(found).To(BeFalse()) + Expect(err).To(Not(HaveOccurred())) + + val, found, err := unstructured.NestedString(obj.Object, "status", "foo", "bar") + Expect(val).To(Equal("baz")) + Expect(found).To(BeTrue()) + Expect(err).To(Succeed()) + + u.UpdateStatus(EnsureCondition(conditions.Deployed(corev1.ConditionTrue, "", ""))) + Expect(u.Apply(context.TODO(), obj)).To(Succeed()) + + Expect(cl.Get(context.TODO(), types.NamespacedName{Namespace: "testNamespace", Name: "testDeployment"}, obj)).To(Succeed()) + Expect((obj.Object["status"].(map[string]interface{}))["conditions"]).To(HaveLen(1)) + + _, found, err = unstructured.NestedFieldNoCopy(obj.Object, "status", "deployedRelease") + Expect(found).To(BeFalse()) + Expect(err).To(Not(HaveOccurred())) + + val, found, err = unstructured.NestedString(obj.Object, "status", "foo", "bar") + Expect(val).To(Equal("baz")) + Expect(found).To(BeTrue()) + Expect(err).To(Succeed()) + }) }) }) @@ -244,8 +309,9 @@ var _ = Describe("statusFor", func() { }) It("should handle map[string]interface{}", func() { - obj.Object["status"] = map[string]interface{}{} - Expect(statusFor(obj)).To(Equal(&helmAppStatus{})) + uSt := map[string]interface{}{} + obj.Object["status"] = uSt + Expect(statusFor(obj)).To(Equal(&helmAppStatus{StatusObject: uSt})) }) It("should handle arbitrary types", func() { diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 16568ff..bbdd5b1 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -683,7 +683,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu 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, u.UpdateStatusCustom, r.log); err != nil { u.UpdateStatus( updater.EnsureCondition(conditions.Irreconcilable(corev1.ConditionTrue, conditions.ReasonReconcileError, err)), updater.EnsureConditionUnknown(conditions.TypeReleaseFailed), @@ -759,7 +759,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } for _, ext := range r.postExtensions { - if err := ext(ctx, obj, r.log); err != nil { + if err := ext(ctx, obj, u.UpdateStatusCustom, r.log); err != nil { u.UpdateStatus( updater.EnsureCondition(conditions.Irreconcilable(corev1.ConditionTrue, conditions.ReasonReconcileError, err)), updater.EnsureConditionUnknown(conditions.TypeReleaseFailed), @@ -1031,7 +1031,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, u.UpdateStatusCustom, r.log); err != nil { u.UpdateStatus( updater.EnsureCondition(conditions.Irreconcilable(corev1.ConditionTrue, conditions.ReasonReconcileError, err)), updater.EnsureConditionUnknown(conditions.TypeReleaseFailed), From 991117d986687eb0d68cbfeb1951f3492bb1d50d Mon Sep 17 00:00:00 2001 From: Marcin Owsiany Date: Tue, 14 Dec 2021 06:55:05 +0100 Subject: [PATCH 07/13] ROX- 8130: Add WithExtraWatch option. (#22) --- pkg/reconciler/reconciler.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index bbdd5b1..7e1f12d 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -81,6 +81,7 @@ type Reconciler struct { selectorPredicate predicate.Predicate overrideValues map[string]string skipDependentWatches bool + extraWatchSources []source.Source maxConcurrentReconciles int reconcilePeriod time.Duration waitForDeletionTimeout time.Duration @@ -98,6 +99,12 @@ type Reconciler struct { uninstallAnnotations map[string]annotation.Uninstall } +type watchDescription struct { + src source.Source + predicates []predicate.Predicate + handler handler.EventHandler +} + // New creates a new Reconciler that reconciles custom resources that define a // Helm release. New takes variadic Option arguments that are used to configure // the Reconciler. @@ -555,6 +562,16 @@ func WithValueMapper(m values.Mapper) Option { } } +// WithExtraWatch is an Option that adds an extra event watch. +// Use this if you want your controller to respond to events other than coming from the primary custom resource, +// the helm release secret, or resources created by your helm chart. +func WithExtraWatch(src source.Source) Option { + return func(r *Reconciler) error { + r.extraWatchSources = append(r.extraWatchSources, src) + return nil + } +} + // WithSelector is an Option that configures the reconciler to creates a // predicate that is used to filter resources based on the specified selector func WithSelector(s metav1.LabelSelector) Option { @@ -1144,6 +1161,12 @@ func (r *Reconciler) setupWatches(mgr ctrl.Manager, c controller.Controller) err return err } + for _, s := range r.extraWatchSources { + if err := c.Watch(s); err != nil { + return err + } + } + if !r.skipDependentWatches { r.postHooks = append([]hook.PostHook{internalhook.NewDependentResourceWatcher(c, mgr.GetRESTMapper(), mgr.GetCache(), mgr.GetScheme())}, r.postHooks...) } From 5d45d80facfc43612778cba65ac0cc92cab6c669 Mon Sep 17 00:00:00 2001 From: Simon Baeumer Date: Thu, 5 May 2022 09:23:24 +0200 Subject: [PATCH 08/13] Fix updater test to use existing fields from deployment's status field --- pkg/reconciler/internal/updater/updater.go | 2 +- .../internal/updater/updater_test.go | 28 +++++++++++-------- pkg/reconciler/reconciler.go | 2 +- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/pkg/reconciler/internal/updater/updater.go b/pkg/reconciler/internal/updater/updater.go index dccabe1..19e84af 100644 --- a/pkg/reconciler/internal/updater/updater.go +++ b/pkg/reconciler/internal/updater/updater.go @@ -28,8 +28,8 @@ import ( "k8s.io/client-go/util/retry" "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/operator-framework/helm-operator/pkg/extensions" "github.com/operator-framework/helm-operator-plugins/internal/sdk/controllerutil" + "github.com/operator-framework/helm-operator-plugins/pkg/extensions" "github.com/operator-framework/helm-operator-plugins/pkg/internal/status" ) diff --git a/pkg/reconciler/internal/updater/updater_test.go b/pkg/reconciler/internal/updater/updater_test.go index e42b1c5..a4f8c3a 100644 --- a/pkg/reconciler/internal/updater/updater_test.go +++ b/pkg/reconciler/internal/updater/updater_test.go @@ -35,7 +35,11 @@ import ( "github.com/operator-framework/helm-operator-plugins/pkg/reconciler/internal/conditions" ) -const testFinalizer = "testFinalizer" +const ( + testFinalizer = "testFinalizer" + availableReplicasStatus = int64(3) + replicasStatus = int64(5) +) var _ = Describe("Updater", func() { var ( @@ -112,12 +116,12 @@ var _ = Describe("Updater", func() { It("should support a mix of standard and custom status updates", func() { u.UpdateStatus(EnsureCondition(conditions.Deployed(corev1.ConditionTrue, "", ""))) u.UpdateStatusCustom(func(uSt *unstructured.Unstructured) bool { - Expect(unstructured.SetNestedMap(uSt.Object, map[string]interface{}{"bar": "baz"}, "foo")).To(Succeed()) + Expect(unstructured.SetNestedField(uSt.Object, replicasStatus, "replicas")).To(Succeed()) return true }) u.UpdateStatus(EnsureCondition(conditions.Irreconcilable(corev1.ConditionFalse, "", ""))) u.UpdateStatusCustom(func(uSt *unstructured.Unstructured) bool { - Expect(unstructured.SetNestedField(uSt.Object, "quux", "foo", "qux")).To(Succeed()) + Expect(unstructured.SetNestedField(uSt.Object, availableReplicasStatus, "availableReplicas")).To(Succeed()) return true }) u.UpdateStatus(EnsureCondition(conditions.Initialized(corev1.ConditionTrue, "", ""))) @@ -129,20 +133,20 @@ var _ = Describe("Updater", func() { Expect(found).To(BeFalse()) Expect(err).To(Not(HaveOccurred())) - val, found, err := unstructured.NestedString(obj.Object, "status", "foo", "bar") - Expect(val).To(Equal("baz")) + val, found, err := unstructured.NestedInt64(obj.Object, "status", "replicas") + Expect(val).To(Equal(replicasStatus)) Expect(found).To(BeTrue()) Expect(err).To(Not(HaveOccurred())) - val, found, err = unstructured.NestedString(obj.Object, "status", "foo", "qux") - Expect(val).To(Equal("quux")) + val, found, err = unstructured.NestedInt64(obj.Object, "status", "availableReplicas") + Expect(val).To(Equal(availableReplicasStatus)) Expect(found).To(BeTrue()) Expect(err).To(Not(HaveOccurred())) }) It("should preserve any custom status across multiple apply calls", func() { u.UpdateStatusCustom(func(uSt *unstructured.Unstructured) bool { - Expect(unstructured.SetNestedMap(uSt.Object, map[string]interface{}{"bar": "baz"}, "foo")).To(Succeed()) + Expect(unstructured.SetNestedField(uSt.Object, int64(5), "replicas")).To(Succeed()) return true }) Expect(u.Apply(context.TODO(), obj)).To(Succeed()) @@ -153,8 +157,8 @@ var _ = Describe("Updater", func() { Expect(found).To(BeFalse()) Expect(err).To(Not(HaveOccurred())) - val, found, err := unstructured.NestedString(obj.Object, "status", "foo", "bar") - Expect(val).To(Equal("baz")) + val, found, err := unstructured.NestedInt64(obj.Object, "status", "replicas") + Expect(val).To(Equal(replicasStatus)) Expect(found).To(BeTrue()) Expect(err).To(Succeed()) @@ -168,8 +172,8 @@ var _ = Describe("Updater", func() { Expect(found).To(BeFalse()) Expect(err).To(Not(HaveOccurred())) - val, found, err = unstructured.NestedString(obj.Object, "status", "foo", "bar") - Expect(val).To(Equal("baz")) + val, found, err = unstructured.NestedInt64(obj.Object, "status", "replicas") + Expect(val).To(Equal(replicasStatus)) Expect(found).To(BeTrue()) Expect(err).To(Succeed()) }) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 7e1f12d..8b34f59 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -50,6 +50,7 @@ import ( "github.com/operator-framework/helm-operator-plugins/internal/sdk/controllerutil" "github.com/operator-framework/helm-operator-plugins/pkg/annotation" helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client" + "github.com/operator-framework/helm-operator-plugins/pkg/extensions" "github.com/operator-framework/helm-operator-plugins/pkg/hook" "github.com/operator-framework/helm-operator-plugins/pkg/reconciler/internal/conditions" "github.com/operator-framework/helm-operator-plugins/pkg/reconciler/internal/diff" @@ -57,7 +58,6 @@ import ( "github.com/operator-framework/helm-operator-plugins/pkg/reconciler/internal/updater" internalvalues "github.com/operator-framework/helm-operator-plugins/pkg/reconciler/internal/values" "github.com/operator-framework/helm-operator-plugins/pkg/values" - "github.com/joelanford/helm-operator/pkg/extensions" ) const uninstallFinalizer = "uninstall-helm-release" From 1e94c72556c9ad3b50b1a8fe6a4b2b2c35eda89a Mon Sep 17 00:00:00 2001 From: Moritz Clasmeier Date: Wed, 8 Jun 2022 09:28:09 +0200 Subject: [PATCH 09/13] Fix reconciler log format message (#25) Fix log format string interpolation --- pkg/reconciler/reconciler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 8b34f59..338e492 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -641,7 +641,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu obj.SetGroupVersionKind(*r.gvk) err := r.client.Get(ctx, req.NamespacedName, obj) if apierrors.IsNotFound(err) { - log.V(1).Info("Resource %s/%s not found, nothing to do", req.NamespacedName.Namespace, req.NamespacedName.Name) + log.V(1).Info(fmt.Sprintf("Resource %s/%s not found, nothing to do", req.NamespacedName.Namespace, req.NamespacedName.Name)) return ctrl.Result{}, nil } if err != nil { From 10aa91d2149e4ff369a622daa56c80d9772f11e2 Mon Sep 17 00:00:00 2001 From: Vlad Bologa Date: Mon, 19 Sep 2022 11:31:09 +0200 Subject: [PATCH 10/13] ROX-12219: Add support for pause-reconcile annotation (#29) Co-authored-by: Marcin Owsiany --- .../internal/conditions/conditions.go | 12 +++- pkg/reconciler/internal/updater/updater.go | 6 ++ pkg/reconciler/reconciler.go | 48 +++++++++++-- pkg/reconciler/reconciler_test.go | 67 +++++++++++++++++++ 4 files changed, 125 insertions(+), 8 deletions(-) diff --git a/pkg/reconciler/internal/conditions/conditions.go b/pkg/reconciler/internal/conditions/conditions.go index 86beb90..12ee6a4 100644 --- a/pkg/reconciler/internal/conditions/conditions.go +++ b/pkg/reconciler/internal/conditions/conditions.go @@ -29,10 +29,12 @@ const ( TypeDeployed = "Deployed" TypeReleaseFailed = "ReleaseFailed" TypeIrreconcilable = "Irreconcilable" + TypePaused = "Paused" - ReasonInstallSuccessful = status.ConditionReason("InstallSuccessful") - ReasonUpgradeSuccessful = status.ConditionReason("UpgradeSuccessful") - ReasonUninstallSuccessful = status.ConditionReason("UninstallSuccessful") + ReasonInstallSuccessful = status.ConditionReason("InstallSuccessful") + ReasonUpgradeSuccessful = status.ConditionReason("UpgradeSuccessful") + ReasonUninstallSuccessful = status.ConditionReason("UninstallSuccessful") + ReasonPauseReconcileAnnotationTrue = status.ConditionReason("PauseReconcileAnnotationTrue") ReasonErrorGettingClient = status.ConditionReason("ErrorGettingClient") ReasonErrorGettingValues = status.ConditionReason("ErrorGettingValues") @@ -60,6 +62,10 @@ func Irreconcilable(stat corev1.ConditionStatus, reason status.ConditionReason, return newCondition(TypeIrreconcilable, stat, reason, message) } +func Paused(stat corev1.ConditionStatus, reason status.ConditionReason, message interface{}) status.Condition { + return newCondition(TypePaused, stat, reason, message) +} + func newCondition(t status.ConditionType, s corev1.ConditionStatus, r status.ConditionReason, m interface{}) status.Condition { message := fmt.Sprintf("%s", m) return status.Condition{ diff --git a/pkg/reconciler/internal/updater/updater.go b/pkg/reconciler/internal/updater/updater.go index 19e84af..4c4bd31 100644 --- a/pkg/reconciler/internal/updater/updater.go +++ b/pkg/reconciler/internal/updater/updater.go @@ -158,6 +158,12 @@ func EnsureConditionUnknown(t status.ConditionType) UpdateStatusFunc { } } +func EnsureConditionAbsent(t status.ConditionType) UpdateStatusFunc { + return func(status *helmAppStatus) bool { + return status.Conditions.RemoveCondition(t) + } +} + func EnsureDeployedRelease(rel *release.Release) UpdateStatusFunc { return func(status *helmAppStatus) bool { newRel := helmAppReleaseFor(rel) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 338e492..4e055f7 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -92,11 +92,12 @@ type Reconciler struct { stripManifestFromStatus bool - annotSetupOnce sync.Once - annotations map[string]struct{} - installAnnotations map[string]annotation.Install - upgradeAnnotations map[string]annotation.Upgrade - uninstallAnnotations map[string]annotation.Uninstall + annotSetupOnce sync.Once + annotations map[string]struct{} + installAnnotations map[string]annotation.Install + upgradeAnnotations map[string]annotation.Upgrade + uninstallAnnotations map[string]annotation.Uninstall + pauseReconcileAnnotation string } type watchDescription struct { @@ -476,6 +477,18 @@ func WithUninstallAnnotations(as ...annotation.Uninstall) Option { } } +// WithPauseReconcileAnnotation is an Option that sets +// a PauseReconcile annotation. If the Custom Resource watched by this +// reconciler has the given annotation, and its value is set to `true`, +// then reconciliation for this CR will not be performed until this annotation +// is removed. +func WithPauseReconcileAnnotation(annotationName string) Option { + return func(r *Reconciler) error { + r.pauseReconcileAnnotation = annotationName + return nil + } +} + // WithPreHook is an Option that configures the reconciler to run the given // PreHook just before performing any actions (e.g. install, upgrade, uninstall, // or reconciliation). @@ -668,6 +681,31 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } }() + if r.pauseReconcileAnnotation != "" { + if v, ok := obj.GetAnnotations()[r.pauseReconcileAnnotation]; ok { + if v == "true" { + log.Info(fmt.Sprintf("Resource has '%s' annotation set to 'true', reconcile paused.", r.pauseReconcileAnnotation)) + u.UpdateStatus( + updater.EnsureCondition(conditions.Paused(corev1.ConditionTrue, conditions.ReasonPauseReconcileAnnotationTrue, "")), + updater.EnsureConditionUnknown(conditions.TypeIrreconcilable), + updater.EnsureConditionUnknown(conditions.TypeDeployed), + updater.EnsureConditionUnknown(conditions.TypeInitialized), + updater.EnsureConditionUnknown(conditions.TypeReleaseFailed), + updater.EnsureDeployedRelease(nil), + ) + return ctrl.Result{}, nil + } + } + } + + u.UpdateStatus( + // TODO(ROX-12637): change to updater.EnsureCondition(conditions.Paused(corev1.ConditionFalse, "", ""))) + // once stackrox operator with pause support is released. + // At that time also add `Paused` to the list of conditions expected in stackrox operator e2e tests. + // Otherwise, the number of conditions in the `status.conditions` list will vary depending on the version + // of used operator, which is cumbersome due to https://github.com/kudobuilder/kuttl/issues/76 + updater.EnsureConditionAbsent(conditions.TypePaused)) + actionClient, err := r.actionClientGetter.ActionClientFor(ctx, obj) if err != nil { u.UpdateStatus( diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index 3ff7780..bc57087 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -402,6 +402,13 @@ var _ = Describe("Reconciler", func() { })) }) }) + _ = Describe("WithPauseReconcileAnnotation", func() { + It("should set the pauseReconcileAnnotation field to the annotation name", func() { + a := "my.domain/pause-reconcile" + Expect(WithPauseReconcileAnnotation(a)(r)).To(Succeed()) + Expect(r.pauseReconcileAnnotation).To(Equal(a)) + }) + }) _ = Describe("WithPreHook", func() { It("should set a reconciler prehook", func() { called := false @@ -543,6 +550,7 @@ var _ = Describe("Reconciler", func() { WithInstallAnnotations(annotation.InstallDescription{}), WithUpgradeAnnotations(annotation.UpgradeDescription{}), WithUninstallAnnotations(annotation.UninstallDescription{}), + WithPauseReconcileAnnotation("my.domain/pause-reconcile"), WithOverrideValues(map[string]string{ "image.repository": "custom-nginx", }), @@ -557,6 +565,7 @@ var _ = Describe("Reconciler", func() { WithInstallAnnotations(annotation.InstallDescription{}), WithUpgradeAnnotations(annotation.UpgradeDescription{}), WithUninstallAnnotations(annotation.UninstallDescription{}), + WithPauseReconcileAnnotation("my.domain/pause-reconcile"), WithOverrideValues(map[string]string{ "image.repository": "custom-nginx", }), @@ -1382,6 +1391,64 @@ var _ = Describe("Reconciler", func() { verifyNoRelease(ctx, mgr.GetClient(), obj.GetNamespace(), obj.GetName(), currentRelease) }) + By("ensuring the finalizer is removed and the CR is deleted", func() { + err := mgr.GetAPIReader().Get(ctx, objKey, obj) + Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }) + }) + }) + When("pause-reconcile annotation is present", func() { + It("pauses reconciliation", func() { + By("adding the pause-reconcile annotation to the CR", func() { + Expect(mgr.GetClient().Get(ctx, objKey, obj)).To(Succeed()) + obj.SetAnnotations(map[string]string{"my.domain/pause-reconcile": "true"}) + obj.Object["spec"] = map[string]interface{}{"replicaCount": "666"} + Expect(mgr.GetClient().Update(ctx, obj)).To(Succeed()) + }) + + By("deleting the CR", func() { + Expect(mgr.GetClient().Delete(ctx, obj)).To(Succeed()) + }) + + By("successfully reconciling a request when paused", func() { + res, err := r.Reconcile(ctx, req) + Expect(res).To(Equal(reconcile.Result{})) + Expect(err).To(BeNil()) + }) + + By("getting the CR", func() { + Expect(mgr.GetAPIReader().Get(ctx, objKey, obj)).To(Succeed()) + }) + + By("verifying the CR status is Paused", func() { + objStat := &objStatus{} + Expect(runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, objStat)).To(Succeed()) + Expect(objStat.Status.Conditions.IsTrueFor(conditions.TypePaused)).To(BeTrue()) + }) + + By("verifying the release has not changed", func() { + rel, err := ac.Get(obj.GetName()) + Expect(err).To(BeNil()) + Expect(rel).NotTo(BeNil()) + Expect(*rel).To(Equal(*currentRelease)) + }) + + By("removing the pause-reconcile annotation from the CR", func() { + Expect(mgr.GetClient().Get(ctx, objKey, obj)).To(Succeed()) + obj.SetAnnotations(nil) + Expect(mgr.GetClient().Update(ctx, obj)).To(Succeed()) + }) + + By("successfully reconciling a request", func() { + res, err := r.Reconcile(ctx, req) + Expect(res).To(Equal(reconcile.Result{})) + Expect(err).To(BeNil()) + }) + + By("verifying the release is uninstalled", func() { + verifyNoRelease(ctx, mgr.GetClient(), obj.GetNamespace(), obj.GetName(), currentRelease) + }) + By("ensuring the finalizer is removed and the CR is deleted", func() { err := mgr.GetAPIReader().Get(ctx, objKey, obj) Expect(apierrors.IsNotFound(err)).To(BeTrue()) From 9b91dd7a2ed9b632d1e62c923604705398053ee2 Mon Sep 17 00:00:00 2001 From: Ludovic Cleroux Date: Fri, 4 Aug 2023 15:28:54 +0200 Subject: [PATCH 11/13] Fix rebase --- pkg/reconciler/reconciler.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 4e055f7..dfd78b1 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -1228,7 +1228,6 @@ func (r *Reconciler) ensureDeployedRelease(u *updater.Updater, rel *release.Rele rel = &relCopy } - u.Update(updater.EnsureFinalizer(uninstallFinalizer)) u.UpdateStatus( updater.EnsureCondition(conditions.Deployed(corev1.ConditionTrue, reason, message)), updater.EnsureDeployedRelease(rel), From 4f1b67d62ed973508a9539f9d4a85b17bcd04ca2 Mon Sep 17 00:00:00 2001 From: Aleksandr Kurlov Date: Fri, 25 Aug 2023 17:20:00 +0200 Subject: [PATCH 12/13] ROX-19221: Fix reconciling with label selector for multiple reconcilers (#42) * Add filter predicate to secret kind watch * Change fix to filter resource in the reconcile function * Fix doc string typo * Update description for setting broken action client --- pkg/reconciler/reconciler.go | 10 ++++++-- pkg/reconciler/reconciler_test.go | 39 +++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index dfd78b1..f31a37e 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -41,6 +41,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/source" @@ -277,7 +278,7 @@ func WithOverrideValues(overrides map[string]string) Option { } } -// WithDependentWatchesEnabled is an Option that configures whether the +// SkipDependentWatches is an Option that configures whether the // Reconciler will register watches for dependent objects in releases and // trigger reconciliations when they change. // @@ -608,7 +609,7 @@ func WithControllerSetupFunc(f ControllerSetupFunc) Option { } // ControllerSetup allows restricted access to the Controller using the WithControllerSetupFunc option. -// Currently the only supposed configuration is adding additional watchers do the controller. +// Currently, the only supposed configuration is adding additional watchers do the controller. type ControllerSetup interface { // Watch takes events provided by a Source and uses the EventHandler to // enqueue reconcile.Requests in response to the events. @@ -661,6 +662,11 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return ctrl.Result{}, err } + if r.selectorPredicate != nil && !r.selectorPredicate.Generic(event.GenericEvent{Object: obj}) { + log.V(1).Info("Label selector does not match, skipping reconcile") + return ctrl.Result{}, nil + } + // The finalizer must be present on the CR before we can do anything. Otherwise, if the reconciliation fails, // there might be resources created by the chart that will not be garbage-collected // (cluster-scoped resources or resources in other namespaces, which are not bound by an owner reference). diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index bc57087..a463952 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -501,6 +501,7 @@ var _ = Describe("Reconciler", func() { Expect(mgr.GetCache().WaitForCacheSync(ctx)).To(BeTrue()) obj = testutil.BuildTestCR(gvk) + obj.SetLabels(map[string]string{"foo": "bar"}) objKey = types.NamespacedName{Namespace: obj.GetNamespace(), Name: obj.GetName()} req = reconcile.Request{NamespacedName: objKey} }) @@ -533,6 +534,8 @@ var _ = Describe("Reconciler", func() { cancel() }) + selector := metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}} + // After migration to Ginkgo v2 this can be rewritten using e.g. DescribeTable. parameterizedReconcilerTests := func(opts reconcilerTestSuiteOpts) { BeforeEach(func() { @@ -551,6 +554,7 @@ var _ = Describe("Reconciler", func() { WithUpgradeAnnotations(annotation.UpgradeDescription{}), WithUninstallAnnotations(annotation.UninstallDescription{}), WithPauseReconcileAnnotation("my.domain/pause-reconcile"), + WithSelector(selector), WithOverrideValues(map[string]string{ "image.repository": "custom-nginx", }), @@ -566,6 +570,7 @@ var _ = Describe("Reconciler", func() { WithUpgradeAnnotations(annotation.UpgradeDescription{}), WithUninstallAnnotations(annotation.UninstallDescription{}), WithPauseReconcileAnnotation("my.domain/pause-reconcile"), + WithSelector(selector), WithOverrideValues(map[string]string{ "image.repository": "custom-nginx", }), @@ -1455,6 +1460,40 @@ var _ = Describe("Reconciler", func() { }) }) }) + When("label selector succeeds", func() { + It("reconciles only matching label", func() { + By("setting an invalid action client getter to assert different reconcile results", func() { + r.actionClientGetter = helmclient.ActionClientGetterFunc(func(context.Context, client.Object) (helmclient.ActionInterface, error) { + fakeClient := helmfake.NewActionClient() + return &fakeClient, nil + }) + }) + + By("setting not matching label to the CR", func() { + Expect(mgr.GetClient().Get(ctx, objKey, obj)).To(Succeed()) + obj.SetLabels(map[string]string{"foo": "baz"}) + Expect(mgr.GetClient().Update(ctx, obj)).To(Succeed()) + }) + + By("reconciling is skipped, action client was not called and no error returned", func() { + res, err := r.Reconcile(ctx, req) + Expect(res).To(Equal(reconcile.Result{})) + Expect(err).To(BeNil()) + }) + + By("setting matching label to the CR", func() { + Expect(mgr.GetClient().Get(ctx, objKey, obj)).To(Succeed()) + obj.SetLabels(map[string]string{"foo": "bar"}) + Expect(mgr.GetClient().Update(ctx, obj)).To(Succeed()) + }) + + By("reconciling is not skipped and error returned because of broken action client", func() { + res, err := r.Reconcile(ctx, req) + Expect(res).To(Equal(reconcile.Result{})) + Expect(err).To(MatchError("get not implemented")) + }) + }) + }) }) }) }) From a307b22ee0c93668717a099f660907e0ace02c4f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 15 Oct 2024 15:13:53 +0000 Subject: [PATCH 13/13] :seedling: Bump github.com/prometheus/client_golang Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.20.2 to 1.20.5. - [Release notes](https://github.com/prometheus/client_golang/releases) - [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md) - [Commits](https://github.com/prometheus/client_golang/compare/v1.20.2...v1.20.5) --- updated-dependencies: - dependency-name: github.com/prometheus/client_golang dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 852d62a..c90d9c3 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/onsi/gomega v1.34.2 github.com/operator-framework/operator-lib v0.14.0 github.com/pkg/errors v0.9.1 - github.com/prometheus/client_golang v1.20.2 + github.com/prometheus/client_golang v1.20.5 github.com/sergi/go-diff v1.3.1 github.com/spf13/cobra v1.8.1 github.com/spf13/pflag v1.0.5 diff --git a/go.sum b/go.sum index 2790f72..fb5c0fc 100644 --- a/go.sum +++ b/go.sum @@ -325,8 +325,8 @@ github.com/poy/onpar v1.1.2/go.mod h1:6X8FLNoxyr9kkmnlqpK6LSoiOtrO6MICtWwEuWkLjz github.com/prometheus/client_golang v0.9.1/go.mod h1:7SWBe2y4D6OKWSNQJUaRYU/AaXPKyh/dDVn+NZz0KFw= github.com/prometheus/client_golang v1.0.0/go.mod h1:db9x61etRT2tGnBNRi70OPL5FsnadC4Ky3P0J6CfImo= github.com/prometheus/client_golang v1.1.0/go.mod h1:I1FGZT9+L76gKKOs5djB6ezCbFQP1xR9D75/vuwEF3g= -github.com/prometheus/client_golang v1.20.2 h1:5ctymQzZlyOON1666svgwn3s6IKWgfbjsejTMiXIyjg= -github.com/prometheus/client_golang v1.20.2/go.mod h1:PIEt8X02hGcP8JWbeHyeZ53Y/jReSnHgO035n//V5WE= +github.com/prometheus/client_golang v1.20.5 h1:cxppBPuYhUnsO6yo/aoRol4L7q7UFfdm+bR9r+8l63Y= +github.com/prometheus/client_golang v1.20.5/go.mod h1:PIEt8X02hGcP8JWbeHyeZ53Y/jReSnHgO035n//V5WE= github.com/prometheus/client_model v0.0.0-20180712105110-5c3871d89910/go.mod h1:MbSGuTsp3dbXC40dX6PRTWyKYBIrTGTE9sqQNg2J8bo= github.com/prometheus/client_model v0.0.0-20190129233127-fd36f4220a90/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=