From 3c0456c577b7e13fe3e2e52075bb108312594835 Mon Sep 17 00:00:00 2001 From: Anton Bretting Date: Sun, 18 Sep 2022 09:42:15 +0200 Subject: [PATCH] Updates based on review comments Signed-off-by: Anton Bretting --- pkg/app/app.go | 40 +--- pkg/app/app_apply_hooks_test.go | 176 ------------------ .../apply_release_with_preapply_hook#01/log | 2 - .../apply_release_with_preapply_hook/log | 2 - 4 files changed, 3 insertions(+), 217 deletions(-) diff --git a/pkg/app/app.go b/pkg/app/app.go index d5c4e7a3..b02e9370 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -15,7 +15,6 @@ import ( "github.com/variantdev/vals" "go.uber.org/zap" - "golang.org/x/exp/slices" "github.com/helmfile/helmfile/pkg/argparser" "github.com/helmfile/helmfile/pkg/filesystem" @@ -1324,11 +1323,7 @@ func (a *App) apply(r *Run, c ApplyConfigProvider) (bool, bool, []error) { } } - releasesWithPreApply := getReleasesWithPreApply(toApplyWithNeeds) - - infoMsg = preApplyInfoMsg(releasesWithPreApply, infoMsg) - - if releasesToBeDeleted == nil && releasesToBeUpdated == nil && releasesWithPreApply == nil { + if releasesToBeDeleted == nil && releasesToBeUpdated == nil { if infoMsg != nil { logger := c.Logger() logger.Infof("") @@ -1357,8 +1352,8 @@ Do you really want to apply? if !interactive || interactive && r.askForConfirmation(confMsg) { r.helm.SetExtraArgs(argparser.GetArgs(c.Args(), r.state)...) - for _, release := range releasesWithPreApply { - a.Logger.Infof("\nRunning preapply hook for %s:", release.Name) + for _, release := range st.Releases { + // a.Logger.Infof("\nRunning preapply hook for %s:", release.Name) if _, err := st.TriggerPreapplyEvent(&release, "apply"); err != nil { applyErrs = append(applyErrs, err) continue @@ -1421,35 +1416,6 @@ Do you really want to apply? return true, true, applyErrs } -func getReleasesWithPreApply(releases []state.ReleaseSpec) []state.ReleaseSpec { - var releasesWithPreApply []state.ReleaseSpec - for _, r := range releases { - release := r - for _, hook := range release.Hooks { - if slices.Contains(hook.Events, "preapply") { - releasesWithPreApply = append(releasesWithPreApply, release) - break - } - } - } - return releasesWithPreApply -} - -func preApplyInfoMsg(releasesWithPreApply []state.ReleaseSpec, infoMsg *string) *string { - if len(releasesWithPreApply) > 0 { - msg := "Releases with preapply hooks: \n" - if infoMsg != nil { - msg = fmt.Sprintf("%s\n%s", *infoMsg, msg) - } - infoMsg = &msg - } - for _, release := range releasesWithPreApply { - tmp := fmt.Sprintf("%s %s (%s)\n", *infoMsg, release.Name, release.Chart) - infoMsg = &tmp - } - return infoMsg -} - func (a *App) delete(r *Run, purge bool, c DestroyConfigProvider) (bool, []error) { st := r.state helm := r.helm diff --git a/pkg/app/app_apply_hooks_test.go b/pkg/app/app_apply_hooks_test.go index abd57676..ad880715 100644 --- a/pkg/app/app_apply_hooks_test.go +++ b/pkg/app/app_apply_hooks_test.go @@ -8,14 +8,10 @@ import ( "testing" "github.com/google/go-cmp/cmp" - "github.com/stretchr/testify/require" "github.com/variantdev/vals" - "k8s.io/utils/pointer" - "github.com/helmfile/helmfile/pkg/event" "github.com/helmfile/helmfile/pkg/exectest" "github.com/helmfile/helmfile/pkg/helmexec" - "github.com/helmfile/helmfile/pkg/state" "github.com/helmfile/helmfile/pkg/testhelper" ) @@ -266,175 +262,3 @@ releases: }) }) } - -func TestGetReleasesWithPreApply(t *testing.T) { - tests := []struct { - releases []state.ReleaseSpec - preApplyreleases []state.ReleaseSpec - }{ - { - []state.ReleaseSpec{ - { - Chart: "foo/bar", - Name: "foobar", - Namespace: "foobar", - Hooks: []event.Hook{ - { - Events: []string{ - "prepare", - "preapply", - }, - }, - }, - }, - { - Chart: "foo/foo", - Name: "foo", - Namespace: "foo", - Hooks: []event.Hook{ - { - Events: []string{ - "prepare", - "presync", - }, - }, - }, - }, - { - Chart: "bar/bar", - Name: "bar", - Namespace: "bar", - Hooks: []event.Hook{ - { - Events: []string{ - "preapply", - }, - }, - }, - }, - }, - []state.ReleaseSpec{ - { - Chart: "foo/bar", - Name: "foobar", - Namespace: "foobar", - Hooks: []event.Hook{ - { - Events: []string{ - "prepare", - "preapply", - }, - }, - }, - }, - { - Chart: "bar/bar", - Name: "bar", - Namespace: "bar", - Hooks: []event.Hook{ - { - Events: []string{ - "preapply", - }, - }, - }, - }, - }, - }, - } - - for _, test := range tests { - preApply := getReleasesWithPreApply(test.releases) - require.Equal(t, test.preApplyreleases, preApply) - } -} - -func TestPreApplyInfoMsg(t *testing.T) { - tests := []struct { - preApplyreleases []state.ReleaseSpec - infoMsg *string - expected *string - }{ - { - []state.ReleaseSpec{ - { - Chart: "foo/bar", - Name: "foobar", - Namespace: "foobar", - Hooks: []event.Hook{ - { - Events: []string{ - "prepare", - "preapply", - }, - }, - }, - }, - { - Chart: "bar/bar", - Name: "bar", - Namespace: "bar", - Hooks: []event.Hook{ - { - Events: []string{ - "preapply", - }, - }, - }, - }, - }, - pointer.String("infoMsg\n"), - pointer.String(`infoMsg - -Releases with preapply hooks: - foobar (foo/bar) - bar (bar/bar) -`), - }, - { - []state.ReleaseSpec{ - { - Chart: "foo/bar", - Name: "foobar", - Namespace: "foobar", - Hooks: []event.Hook{ - { - Events: []string{ - "prepare", - "preapply", - }, - }, - }, - }, - { - Chart: "bar/bar", - Name: "bar", - Namespace: "bar", - Hooks: []event.Hook{ - { - Events: []string{ - "preapply", - }, - }, - }, - }, - }, - pointer.String(""), - pointer.String(` -Releases with preapply hooks: - foobar (foo/bar) - bar (bar/bar) -`), - }, - { - []state.ReleaseSpec{}, - pointer.String(""), - pointer.String(""), - }, - } - - for _, test := range tests { - infoMsg := preApplyInfoMsg(test.preApplyreleases, test.infoMsg) - require.Equal(t, *test.expected, *infoMsg) - } -} diff --git a/pkg/app/testdata/testapply_hooks/apply_release_with_preapply_hook#01/log b/pkg/app/testdata/testapply_hooks/apply_release_with_preapply_hook#01/log index bcc2fb64..0c6b5308 100644 --- a/pkg/app/testdata/testapply_hooks/apply_release_with_preapply_hook#01/log +++ b/pkg/app/testdata/testapply_hooks/apply_release_with_preapply_hook#01/log @@ -2,8 +2,6 @@ hook[prepare] logs | foo hook[prepare] logs | -Running preapply hook for foo: - hook[preapply] logs | foo hook[preapply] logs | diff --git a/pkg/app/testdata/testapply_hooks/apply_release_with_preapply_hook/log b/pkg/app/testdata/testapply_hooks/apply_release_with_preapply_hook/log index 5c8158c0..e6233234 100644 --- a/pkg/app/testdata/testapply_hooks/apply_release_with_preapply_hook/log +++ b/pkg/app/testdata/testapply_hooks/apply_release_with_preapply_hook/log @@ -1,6 +1,4 @@ -Running preapply hook for foo: - hook[preapply] logs | foo hook[preapply] logs |