Updates based on review comments
Signed-off-by: Anton Bretting <sajfer@gmail.com>
This commit is contained in:
		
							parent
							
								
									5b88006e86
								
							
						
					
					
						commit
						3c0456c577
					
				|  | @ -15,7 +15,6 @@ import ( | ||||||
| 
 | 
 | ||||||
| 	"github.com/variantdev/vals" | 	"github.com/variantdev/vals" | ||||||
| 	"go.uber.org/zap" | 	"go.uber.org/zap" | ||||||
| 	"golang.org/x/exp/slices" |  | ||||||
| 
 | 
 | ||||||
| 	"github.com/helmfile/helmfile/pkg/argparser" | 	"github.com/helmfile/helmfile/pkg/argparser" | ||||||
| 	"github.com/helmfile/helmfile/pkg/filesystem" | 	"github.com/helmfile/helmfile/pkg/filesystem" | ||||||
|  | @ -1324,11 +1323,7 @@ func (a *App) apply(r *Run, c ApplyConfigProvider) (bool, bool, []error) { | ||||||
| 		} | 		} | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	releasesWithPreApply := getReleasesWithPreApply(toApplyWithNeeds) | 	if releasesToBeDeleted == nil && releasesToBeUpdated == nil { | ||||||
| 
 |  | ||||||
| 	infoMsg = preApplyInfoMsg(releasesWithPreApply, infoMsg) |  | ||||||
| 
 |  | ||||||
| 	if releasesToBeDeleted == nil && releasesToBeUpdated == nil && releasesWithPreApply == nil { |  | ||||||
| 		if infoMsg != nil { | 		if infoMsg != nil { | ||||||
| 			logger := c.Logger() | 			logger := c.Logger() | ||||||
| 			logger.Infof("") | 			logger.Infof("") | ||||||
|  | @ -1357,8 +1352,8 @@ Do you really want to apply? | ||||||
| 	if !interactive || interactive && r.askForConfirmation(confMsg) { | 	if !interactive || interactive && r.askForConfirmation(confMsg) { | ||||||
| 		r.helm.SetExtraArgs(argparser.GetArgs(c.Args(), r.state)...) | 		r.helm.SetExtraArgs(argparser.GetArgs(c.Args(), r.state)...) | ||||||
| 
 | 
 | ||||||
| 		for _, release := range releasesWithPreApply { | 		for _, release := range st.Releases { | ||||||
| 			a.Logger.Infof("\nRunning preapply hook for %s:", release.Name) | 			// a.Logger.Infof("\nRunning preapply hook for %s:", release.Name)
 | ||||||
| 			if _, err := st.TriggerPreapplyEvent(&release, "apply"); err != nil { | 			if _, err := st.TriggerPreapplyEvent(&release, "apply"); err != nil { | ||||||
| 				applyErrs = append(applyErrs, err) | 				applyErrs = append(applyErrs, err) | ||||||
| 				continue | 				continue | ||||||
|  | @ -1421,35 +1416,6 @@ Do you really want to apply? | ||||||
| 	return true, true, applyErrs | 	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) { | func (a *App) delete(r *Run, purge bool, c DestroyConfigProvider) (bool, []error) { | ||||||
| 	st := r.state | 	st := r.state | ||||||
| 	helm := r.helm | 	helm := r.helm | ||||||
|  |  | ||||||
|  | @ -8,14 +8,10 @@ import ( | ||||||
| 	"testing" | 	"testing" | ||||||
| 
 | 
 | ||||||
| 	"github.com/google/go-cmp/cmp" | 	"github.com/google/go-cmp/cmp" | ||||||
| 	"github.com/stretchr/testify/require" |  | ||||||
| 	"github.com/variantdev/vals" | 	"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/exectest" | ||||||
| 	"github.com/helmfile/helmfile/pkg/helmexec" | 	"github.com/helmfile/helmfile/pkg/helmexec" | ||||||
| 	"github.com/helmfile/helmfile/pkg/state" |  | ||||||
| 	"github.com/helmfile/helmfile/pkg/testhelper" | 	"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) |  | ||||||
| 	} |  | ||||||
| } |  | ||||||
|  |  | ||||||
|  | @ -2,8 +2,6 @@ | ||||||
| hook[prepare] logs | foo | hook[prepare] logs | foo | ||||||
| hook[prepare] logs |  | hook[prepare] logs |  | ||||||
| 
 | 
 | ||||||
| Running preapply hook for foo: |  | ||||||
| 
 |  | ||||||
| hook[preapply] logs | foo | hook[preapply] logs | foo | ||||||
| hook[preapply] logs |  | hook[preapply] logs |  | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -1,6 +1,4 @@ | ||||||
| 
 | 
 | ||||||
| Running preapply hook for foo: |  | ||||||
| 
 |  | ||||||
| hook[preapply] logs | foo | hook[preapply] logs | foo | ||||||
| hook[preapply] logs |  | hook[preapply] logs |  | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue