From 924a2bd57c870706af247a43b6ed8855ad48e1ba Mon Sep 17 00:00:00 2001 From: Jay Cox Date: Tue, 30 Jan 2024 16:01:13 -0600 Subject: [PATCH 1/4] default MaxReleaseHistory to ten --- pkg/reconciler/reconciler.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 9602a719..494c2951 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -348,7 +348,9 @@ func WithReconcilePeriod(rp time.Duration) Option { } // WithMaxReleaseHistory specifies the maximum size of the Helm release history maintained -// on upgrades/rollbacks. Zero (default) means unlimited. +// on upgrades/rollbacks. Zero means unlimited. +// +// Defaults is 10 func WithMaxReleaseHistory(maxHistory int) Option { return func(r *Reconciler) error { if maxHistory < 0 { @@ -936,6 +938,9 @@ func (r *Reconciler) addDefaults(mgr ctrl.Manager, controllerName string) error if r.valueMapper == nil { r.valueMapper = internalvalues.DefaultMapper } + + r.maxHistory = 10 + return nil } From 521b766563e528fb90297f38f83cf6de2085efaf Mon Sep 17 00:00:00 2001 From: Jay Cox Date: Tue, 30 Jan 2024 16:18:15 -0600 Subject: [PATCH 2/4] make default history nullable --- pkg/reconciler/internal/values/values.go | 2 ++ pkg/reconciler/reconciler.go | 16 +++++++++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/pkg/reconciler/internal/values/values.go b/pkg/reconciler/internal/values/values.go index 28d23438..76478ee3 100644 --- a/pkg/reconciler/internal/values/values.go +++ b/pkg/reconciler/internal/values/values.go @@ -28,6 +28,8 @@ import ( "github.com/operator-framework/helm-operator-plugins/pkg/values" ) +var DefaultMaxHistory = 10 + var DefaultMapper = values.MapperFunc(func(v chartutil.Values) chartutil.Values { return v }) var DefaultTranslator = values.TranslatorFunc(func(ctx context.Context, u *unstructured.Unstructured) (chartutil.Values, error) { diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 494c2951..3ebe26d2 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -79,7 +79,7 @@ type Reconciler struct { skipDependentWatches bool maxConcurrentReconciles int reconcilePeriod time.Duration - maxHistory int + maxHistory *int skipPrimaryGVKSchemeRegistration bool controllerSetupFuncs []ControllerSetupFunc @@ -356,7 +356,7 @@ func WithMaxReleaseHistory(maxHistory int) Option { if maxHistory < 0 { return errors.New("maximum Helm release history size must not be negative") } - r.maxHistory = maxHistory + r.maxHistory = &maxHistory return nil } } @@ -748,9 +748,9 @@ func (r *Reconciler) getReleaseState(client helmclient.ActionInterface, obj meta } var opts []helmclient.UpgradeOption - if r.maxHistory > 0 { + if *r.maxHistory > 0 { opts = append(opts, func(u *action.Upgrade) error { - u.MaxHistory = r.maxHistory + u.MaxHistory = *r.maxHistory return nil }) } @@ -804,9 +804,9 @@ func (r *Reconciler) doInstall(actionClient helmclient.ActionInterface, u *updat func (r *Reconciler) doUpgrade(actionClient helmclient.ActionInterface, u *updater.Updater, obj *unstructured.Unstructured, vals map[string]interface{}, log logr.Logger) (*release.Release, error) { var opts []helmclient.UpgradeOption - if r.maxHistory > 0 { + if *r.maxHistory > 0 { opts = append(opts, func(u *action.Upgrade) error { - u.MaxHistory = r.maxHistory + u.MaxHistory = *r.maxHistory return nil }) } @@ -939,7 +939,9 @@ func (r *Reconciler) addDefaults(mgr ctrl.Manager, controllerName string) error r.valueMapper = internalvalues.DefaultMapper } - r.maxHistory = 10 + if r.maxHistory == nil { + r.maxHistory = &internalvalues.DefaultMaxHistory + } return nil } From a591ffef675e55741882e3e3584299b1dd0d94dd Mon Sep 17 00:00:00 2001 From: Jay Cox Date: Tue, 30 Jan 2024 16:50:22 -0600 Subject: [PATCH 3/4] rename maxHistory --- pkg/reconciler/internal/values/values.go | 2 +- pkg/reconciler/reconciler.go | 16 ++++++++-------- pkg/reconciler/reconciler_test.go | 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/pkg/reconciler/internal/values/values.go b/pkg/reconciler/internal/values/values.go index 76478ee3..cddcd0e6 100644 --- a/pkg/reconciler/internal/values/values.go +++ b/pkg/reconciler/internal/values/values.go @@ -28,7 +28,7 @@ import ( "github.com/operator-framework/helm-operator-plugins/pkg/values" ) -var DefaultMaxHistory = 10 +var DefaultMaxReleaseHistory = 10 var DefaultMapper = values.MapperFunc(func(v chartutil.Values) chartutil.Values { return v }) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 3ebe26d2..8ec5439f 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -79,7 +79,7 @@ type Reconciler struct { skipDependentWatches bool maxConcurrentReconciles int reconcilePeriod time.Duration - maxHistory *int + maxReleaseHistory *int skipPrimaryGVKSchemeRegistration bool controllerSetupFuncs []ControllerSetupFunc @@ -356,7 +356,7 @@ func WithMaxReleaseHistory(maxHistory int) Option { if maxHistory < 0 { return errors.New("maximum Helm release history size must not be negative") } - r.maxHistory = &maxHistory + r.maxReleaseHistory = &maxHistory return nil } } @@ -748,9 +748,9 @@ func (r *Reconciler) getReleaseState(client helmclient.ActionInterface, obj meta } var opts []helmclient.UpgradeOption - if *r.maxHistory > 0 { + if *r.maxReleaseHistory > 0 { opts = append(opts, func(u *action.Upgrade) error { - u.MaxHistory = *r.maxHistory + u.MaxHistory = *r.maxReleaseHistory return nil }) } @@ -804,9 +804,9 @@ func (r *Reconciler) doInstall(actionClient helmclient.ActionInterface, u *updat func (r *Reconciler) doUpgrade(actionClient helmclient.ActionInterface, u *updater.Updater, obj *unstructured.Unstructured, vals map[string]interface{}, log logr.Logger) (*release.Release, error) { var opts []helmclient.UpgradeOption - if *r.maxHistory > 0 { + if *r.maxReleaseHistory > 0 { opts = append(opts, func(u *action.Upgrade) error { - u.MaxHistory = *r.maxHistory + u.MaxHistory = *r.maxReleaseHistory return nil }) } @@ -939,8 +939,8 @@ func (r *Reconciler) addDefaults(mgr ctrl.Manager, controllerName string) error r.valueMapper = internalvalues.DefaultMapper } - if r.maxHistory == nil { - r.maxHistory = &internalvalues.DefaultMaxHistory + if r.maxReleaseHistory == nil { + r.maxReleaseHistory = &internalvalues.DefaultMaxReleaseHistory } return nil diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index 968f5fa5..32a6b4a2 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -217,11 +217,11 @@ var _ = Describe("Reconciler", func() { var _ = Describe("WithMaxReleaseHistory", func() { It("should set the max history size", func() { Expect(WithMaxReleaseHistory(10)(r)).To(Succeed()) - Expect(r.maxHistory).To(Equal(10)) + Expect(r.maxReleaseHistory).To(Equal(10)) }) It("should allow setting the history to unlimited", func() { Expect(WithMaxReleaseHistory(0)(r)).To(Succeed()) - Expect(r.maxHistory).To(Equal(0)) + Expect(r.maxReleaseHistory).To(Equal(0)) }) It("should fail if value is less than 0", func() { Expect(WithMaxReleaseHistory(-1)(r)).NotTo(Succeed()) From 4523eef9965e857a0649c910149b96bad0650b55 Mon Sep 17 00:00:00 2001 From: Jay Cox Date: Mon, 26 Feb 2024 16:25:10 -0600 Subject: [PATCH 4/4] add gomega pointto in tests --- pkg/reconciler/reconciler_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index 32a6b4a2..91f111cb 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -27,6 +27,7 @@ import ( "github.com/go-logr/logr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + . "github.com/onsi/gomega/gstruct" "helm.sh/helm/v3/pkg/action" "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chartutil" @@ -217,11 +218,11 @@ var _ = Describe("Reconciler", func() { var _ = Describe("WithMaxReleaseHistory", func() { It("should set the max history size", func() { Expect(WithMaxReleaseHistory(10)(r)).To(Succeed()) - Expect(r.maxReleaseHistory).To(Equal(10)) + Expect(r.maxReleaseHistory).To(PointTo(Equal(10))) }) It("should allow setting the history to unlimited", func() { Expect(WithMaxReleaseHistory(0)(r)).To(Succeed()) - Expect(r.maxReleaseHistory).To(Equal(0)) + Expect(r.maxReleaseHistory).To(PointTo(Equal(0))) }) It("should fail if value is less than 0", func() { Expect(WithMaxReleaseHistory(-1)(r)).NotTo(Succeed())