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..ab4a235 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,45 @@ if err := reconciler.SetupWithManager(mgr); err != nil { panic(fmt.Sprintf("unable to create reconciler: %s", err)) } ``` + +## Why a fork? + +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) file. + +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 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`: + + ``` + go mod edit -replace=github.com/joelanford/helm-operator=github.com/stackrox/helm-operator@main + ``` + + For example: + + ```go + 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)) + } + ``` + 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= 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/extensions/types.go b/pkg/extensions/types.go new file mode 100644 index 0000000..fe449f8 --- /dev/null +++ b/pkg/extensions/types.go @@ -0,0 +1,17 @@ +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, func(UpdateStatusFunc), logr.Logger) error diff --git a/pkg/reconciler/internal/conditions/conditions.go b/pkg/reconciler/internal/conditions/conditions.go index 55e2c65..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") @@ -41,6 +43,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 { @@ -59,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/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/internal/updater/updater.go b/pkg/reconciler/internal/updater/updater.go index 1508c32..4c4bd31 100644 --- a/pkg/reconciler/internal/updater/updater.go +++ b/pkg/reconciler/internal/updater/updater.go @@ -29,6 +29,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "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" ) @@ -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 { @@ -146,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) @@ -166,10 +184,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 +225,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..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 ( @@ -108,6 +112,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.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, availableReplicasStatus, "availableReplicas")).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.NestedInt64(obj.Object, "status", "replicas") + Expect(val).To(Equal(replicasStatus)) + Expect(found).To(BeTrue()) + Expect(err).To(Not(HaveOccurred())) + + 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.SetNestedField(uSt.Object, int64(5), "replicas")).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.NestedInt64(obj.Object, "status", "replicas") + Expect(val).To(Equal(replicasStatus)) + 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.NestedInt64(obj.Object, "status", "replicas") + Expect(val).To(Equal(replicasStatus)) + Expect(found).To(BeTrue()) + Expect(err).To(Succeed()) + }) }) }) @@ -244,8 +313,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 dd64819..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" @@ -50,6 +51,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" @@ -71,24 +73,38 @@ type Reconciler struct { preHooks []hook.PreHook postHooks []hook.PostHook + preExtensions []extensions.ReconcileExtension + postExtensions []extensions.ReconcileExtension + log logr.Logger gvk *schema.GroupVersionKind chrt *chart.Chart selectorPredicate predicate.Predicate overrideValues map[string]string skipDependentWatches bool + extraWatchSources []source.Source maxConcurrentReconciles int reconcilePeriod time.Duration waitForDeletionTimeout time.Duration maxReleaseHistory *int + markFailedAfter time.Duration skipPrimaryGVKSchemeRegistration bool controllerSetupFuncs []ControllerSetupFunc - annotSetupOnce sync.Once - annotations map[string]struct{} - installAnnotations map[string]annotation.Install - upgradeAnnotations map[string]annotation.Upgrade - uninstallAnnotations map[string]annotation.Uninstall + stripManifestFromStatus bool + + 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 { + src source.Source + predicates []predicate.Predicate + handler handler.EventHandler } // New creates a new Reconciler that reconciles custom resources that define a @@ -262,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. // @@ -274,6 +290,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. // @@ -376,6 +403,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. @@ -439,6 +478,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). @@ -449,6 +500,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 +523,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. @@ -495,6 +576,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 { @@ -518,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. @@ -564,13 +655,18 @@ 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 { 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). @@ -591,6 +687,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( @@ -618,10 +739,20 @@ 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, "", ""))) + for _, ext := range r.preExtensions { + 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), + ) + return ctrl.Result{}, err + } + } + if obj.GetDeletionTimestamp() != nil { if err := r.handleDeletion(ctx, actionClient, obj, log); err != nil { return ctrl.Result{}, err @@ -649,6 +780,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 { @@ -684,7 +819,17 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } } - ensureDeployedRelease(&u, rel) + for _, ext := range r.postExtensions { + 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), + ) + return ctrl.Result{}, err + } + } + + r.ensureDeployedRelease(&u, rel) u.UpdateStatus( updater.EnsureCondition(conditions.ReleaseFailed(corev1.ConditionFalse, "", "")), updater.EnsureCondition(conditions.Irreconcilable(corev1.ConditionFalse, "", "")), @@ -715,6 +860,7 @@ const ( stateNeedsInstall helmReleaseState = "needs install" stateNeedsUpgrade helmReleaseState = "needs upgrade" stateUnchanged helmReleaseState = "unchanged" + statePending helmReleaseState = "pending" stateError helmReleaseState = "error" ) @@ -734,7 +880,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 } @@ -764,6 +910,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 { @@ -859,6 +1009,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", @@ -886,7 +1065,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 +1090,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, u.UpdateStatusCustom, 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, "", "")), @@ -1015,13 +1205,19 @@ 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...) } 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 { @@ -1031,6 +1227,13 @@ 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.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..a463952 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()) @@ -392,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 @@ -484,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} }) @@ -516,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() { @@ -533,6 +553,8 @@ var _ = Describe("Reconciler", func() { WithInstallAnnotations(annotation.InstallDescription{}), WithUpgradeAnnotations(annotation.UpgradeDescription{}), WithUninstallAnnotations(annotation.UninstallDescription{}), + WithPauseReconcileAnnotation("my.domain/pause-reconcile"), + WithSelector(selector), WithOverrideValues(map[string]string{ "image.repository": "custom-nginx", }), @@ -547,6 +569,8 @@ var _ = Describe("Reconciler", func() { WithInstallAnnotations(annotation.InstallDescription{}), WithUpgradeAnnotations(annotation.UpgradeDescription{}), WithUninstallAnnotations(annotation.UninstallDescription{}), + WithPauseReconcileAnnotation("my.domain/pause-reconcile"), + WithSelector(selector), WithOverrideValues(map[string]string{ "image.repository": "custom-nginx", }), @@ -1378,6 +1402,98 @@ var _ = Describe("Reconciler", func() { }) }) }) + 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()) + }) + }) + }) + 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")) + }) + }) + }) }) }) })