Skip to content

Commit

Permalink
Add a default value for MaxReleaseHistory (#291)
Browse files Browse the repository at this point in the history
* default MaxReleaseHistory to ten

* make default history nullable

* rename maxHistory

* add gomega pointto in tests

---------

Co-authored-by: Joe Lanford <joe.lanford@gmail.com>
  • Loading branch information
Jay-Madden and joelanford authored Jul 8, 2024
1 parent 994b1d5 commit 20a5c31
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 9 deletions.
2 changes: 2 additions & 0 deletions pkg/reconciler/internal/values/values.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ import (
"github.com/operator-framework/helm-operator-plugins/pkg/values"
)

var DefaultMaxReleaseHistory = 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) {
Expand Down
21 changes: 14 additions & 7 deletions pkg/reconciler/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ type Reconciler struct {
skipDependentWatches bool
maxConcurrentReconciles int
reconcilePeriod time.Duration
maxHistory int
maxReleaseHistory *int
skipPrimaryGVKSchemeRegistration bool
controllerSetupFuncs []ControllerSetupFunc

Expand Down Expand Up @@ -347,13 +347,15 @@ 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 {
return errors.New("maximum Helm release history size must not be negative")
}
r.maxHistory = maxHistory
r.maxReleaseHistory = &maxHistory
return nil
}
}
Expand Down Expand Up @@ -745,9 +747,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
})
}
Expand Down Expand Up @@ -802,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
})
}
Expand Down Expand Up @@ -936,6 +938,11 @@ func (r *Reconciler) addDefaults(mgr ctrl.Manager, controllerName string) error
if r.valueMapper == nil {
r.valueMapper = internalvalues.DefaultMapper
}

if r.maxReleaseHistory == nil {
r.maxReleaseHistory = &internalvalues.DefaultMaxReleaseHistory
}

return nil
}

Expand Down
5 changes: 3 additions & 2 deletions pkg/reconciler/reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/go-logr/logr"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
. "github.com/onsi/gomega/gstruct"
sdkhandler "github.com/operator-framework/operator-lib/handler"
"helm.sh/helm/v3/pkg/action"
"helm.sh/helm/v3/pkg/chart"
Expand Down Expand Up @@ -216,11 +217,11 @@ var _ = Describe("Reconciler", func() {
_ = 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(PointTo(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(PointTo(Equal(0)))
})
It("should fail if value is less than 0", func() {
Expect(WithMaxReleaseHistory(-1)(r)).NotTo(Succeed())
Expand Down

0 comments on commit 20a5c31

Please sign in to comment.