fix: address PR review comments for sync --interactive diff preview
- Pass detailed-exitcode flag to diff preview so changes are properly detected and labeled - Wire up --detailed-exitcode flag on sync to return exit code 2 when changes are detected and synced (mirrors apply behavior) - Set extra args before diff preview so --args/helmDefaults.args affect the preview - Assert helm.Diffed was called in interactive test cases - Fix typo releaes -> releases in test error messages Signed-off-by: vomba <hani.harzallah@elastisys.com>
This commit is contained in:
vomba
parent
58fa8b1c37
commit
940a364fdc
|
|
@ -484,7 +484,11 @@ func (a *App) Fetch(c FetchConfigProvider) error {
|
|||
}
|
||||
|
||||
func (a *App) Sync(c SyncConfigProvider) error {
|
||||
return a.ForEachState(func(run *Run) (ok bool, errs []error) {
|
||||
var any bool
|
||||
|
||||
mut := &sync.Mutex{}
|
||||
|
||||
err := a.ForEachState(func(run *Run) (ok bool, errs []error) {
|
||||
includeCRDs := !c.SkipCRDs()
|
||||
|
||||
prepErr := run.WithPreparedCharts("sync", state.ChartPrepareOptions{
|
||||
|
|
@ -500,7 +504,14 @@ func (a *App) Sync(c SyncConfigProvider) error {
|
|||
Validate: c.Validate(),
|
||||
Concurrency: c.Concurrency(),
|
||||
}, func() []error {
|
||||
ok, errs = a.SyncState(run, c)
|
||||
matched, updated, es := a.SyncState(run, c)
|
||||
|
||||
mut.Lock()
|
||||
any = any || updated
|
||||
mut.Unlock()
|
||||
|
||||
ok = matched
|
||||
errs = es
|
||||
return errs
|
||||
})
|
||||
|
||||
|
|
@ -510,6 +521,18 @@ func (a *App) Sync(c SyncConfigProvider) error {
|
|||
|
||||
return
|
||||
}, c.IncludeTransitiveNeeds())
|
||||
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if ec, ok := c.(interface{ DetailedExitcode() bool }); ok && ec.DetailedExitcode() && any {
|
||||
code := 2
|
||||
|
||||
return &Error{msg: "Identified at least one change", code: &code}
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func (a *App) Apply(c ApplyConfigProvider) error {
|
||||
|
|
@ -2210,16 +2233,16 @@ func (a *App) status(r *Run, c StatusesConfigProvider) (bool, []error) {
|
|||
return true, errs
|
||||
}
|
||||
|
||||
func (a *App) SyncState(r *Run, c SyncConfigProvider) (bool, []error) {
|
||||
func (a *App) SyncState(r *Run, c SyncConfigProvider) (bool, bool, []error) {
|
||||
st := r.state
|
||||
helm := r.helm
|
||||
|
||||
releasesWithNeeds, selectedAndNeededReleases, err := a.GetPlannedAndSelectedReleasesWithNeeds(r, c.SkipNeeds(), c.IncludeNeeds(), c.IncludeTransitiveNeeds())
|
||||
if err != nil {
|
||||
return false, []error{err}
|
||||
return false, false, []error{err}
|
||||
}
|
||||
if len(releasesWithNeeds) == 0 {
|
||||
return false, nil
|
||||
return false, false, nil
|
||||
}
|
||||
|
||||
// Do build deps and prepare only on selected releases so that we won't waste time
|
||||
|
|
@ -2228,7 +2251,7 @@ func (a *App) SyncState(r *Run, c SyncConfigProvider) (bool, []error) {
|
|||
|
||||
toDelete, err := st.DetectReleasesToBeDeletedForSync(helm, releasesWithNeeds)
|
||||
if err != nil {
|
||||
return false, []error{err}
|
||||
return false, false, []error{err}
|
||||
}
|
||||
|
||||
releasesToDelete := map[string]state.ReleaseSpec{}
|
||||
|
|
@ -2282,6 +2305,12 @@ func (a *App) SyncState(r *Run, c SyncConfigProvider) (bool, []error) {
|
|||
interactive := c.Interactive()
|
||||
|
||||
var infoMsg string
|
||||
var errs []error
|
||||
|
||||
r.helm.SetExtraArgs(GetArgs(c.Args(), r.state)...)
|
||||
|
||||
operationsAttempted := false
|
||||
|
||||
if interactive {
|
||||
if diffC, ok := c.(DiffConfigProvider); ok {
|
||||
detectedKubeVersion := a.detectKubeVersion(st)
|
||||
|
|
@ -2302,9 +2331,9 @@ func (a *App) SyncState(r *Run, c SyncConfigProvider) (bool, []error) {
|
|||
TakeOwnership: diffC.TakeOwnership(),
|
||||
DetectedKubeVersion: detectedKubeVersion,
|
||||
}
|
||||
infoMsgPtr, _, _, diffErrs := r.diff(false, false, diffC, diffOpts)
|
||||
infoMsgPtr, _, _, diffErrs := r.diff(false, diffC.DetailedExitcode(), diffC, diffOpts)
|
||||
if len(diffErrs) > 0 {
|
||||
return false, diffErrs
|
||||
return false, false, diffErrs
|
||||
}
|
||||
if infoMsgPtr != nil {
|
||||
infoMsg = *infoMsgPtr
|
||||
|
|
@ -2331,10 +2360,6 @@ Do you really want to sync?
|
|||
|
||||
`, infoMsg)
|
||||
|
||||
var errs []error
|
||||
|
||||
r.helm.SetExtraArgs(GetArgs(c.Args(), r.state)...)
|
||||
|
||||
// Traverse DAG of all the releases so that we don't suffer from false-positive missing dependencies
|
||||
st.Releases = selectedAndNeededReleases
|
||||
|
||||
|
|
@ -2342,6 +2367,7 @@ Do you really want to sync?
|
|||
|
||||
if !interactive || interactive && r.askForConfirmation(confMsg) {
|
||||
if len(releasesToDelete) > 0 {
|
||||
operationsAttempted = true
|
||||
_, deletionErrs := withDAG(st, helm, a.Logger, state.PlanOptions{Reverse: true, SelectedReleases: toDelete, SkipNeeds: true}, a.WrapWithoutSelector(func(subst *state.HelmState, helm helmexec.Interface) []error {
|
||||
var rs []state.ReleaseSpec
|
||||
|
||||
|
|
@ -2363,6 +2389,7 @@ Do you really want to sync?
|
|||
}
|
||||
|
||||
if len(releasesToUpdate) > 0 {
|
||||
operationsAttempted = true
|
||||
_, syncErrs := withDAG(st, helm, a.Logger, state.PlanOptions{SelectedReleases: toUpdate, SkipNeeds: true, IncludeTransitiveNeeds: c.IncludeTransitiveNeeds()}, a.WrapWithoutSelector(func(subst *state.HelmState, helm helmexec.Interface) []error {
|
||||
var rs []state.ReleaseSpec
|
||||
|
||||
|
|
@ -2415,7 +2442,9 @@ Do you really want to sync?
|
|||
}
|
||||
}
|
||||
|
||||
return true, errs
|
||||
changesApplied := operationsAttempted && len(errs) == 0
|
||||
|
||||
return true, changesApplied, errs
|
||||
}
|
||||
|
||||
func (a *App) template(r *Run, c TemplateConfigProvider) (bool, []error) {
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ func TestSyncInteractive(t *testing.T) {
|
|||
selectors []string
|
||||
lists map[exectest.ListKey]string
|
||||
diffs map[exectest.DiffKey]error
|
||||
wantDiffs int
|
||||
upgraded []exectest.Release
|
||||
deleted []exectest.Release
|
||||
}
|
||||
|
|
@ -73,12 +74,13 @@ func TestSyncInteractive(t *testing.T) {
|
|||
run.Ask = func(msg string) bool {
|
||||
return tc.confirm
|
||||
}
|
||||
return app.SyncState(run, applyConfig{
|
||||
ok, _, errs := app.SyncState(run, applyConfig{
|
||||
concurrency: 1,
|
||||
interactive: tc.interactive,
|
||||
skipNeeds: true,
|
||||
logger: logger,
|
||||
})
|
||||
return ok, errs
|
||||
}, false)
|
||||
|
||||
var gotErr string
|
||||
|
|
@ -100,11 +102,15 @@ func TestSyncInteractive(t *testing.T) {
|
|||
}
|
||||
for flagIdx := range wantUpgrades[relIdx].Flags {
|
||||
if wantUpgrades[relIdx].Flags[flagIdx] != helm.Releases[relIdx].Flags[flagIdx] {
|
||||
t.Errorf("releaes[%d].flags[%d]: got %v, want %v", relIdx, flagIdx, helm.Releases[relIdx].Flags[flagIdx], wantUpgrades[relIdx].Flags[flagIdx])
|
||||
t.Errorf("releases[%d].flags[%d]: got %v, want %v", relIdx, flagIdx, helm.Releases[relIdx].Flags[flagIdx], wantUpgrades[relIdx].Flags[flagIdx])
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if tc.wantDiffs > 0 && len(helm.Diffed) != tc.wantDiffs {
|
||||
t.Fatalf("unexpected number of diffs: got %d, want %d", len(helm.Diffed), tc.wantDiffs)
|
||||
}
|
||||
|
||||
if len(wantDeletes) > len(helm.Deleted) {
|
||||
t.Fatalf("insufficient number of deletes: got %d, want %d", len(helm.Deleted), len(wantDeletes))
|
||||
}
|
||||
|
|
@ -140,6 +146,7 @@ my-release 4 Fri Nov 1 08:40:07 2019 DEPLOYED raw-3.1.0 3.1.0 def
|
|||
check(t, testcase{
|
||||
interactive: true,
|
||||
confirm: true,
|
||||
wantDiffs: 1,
|
||||
files: map[string]string{
|
||||
"/path/to/helmfile.yaml": `
|
||||
releases:
|
||||
|
|
@ -166,6 +173,7 @@ my-release 4 Fri Nov 1 08:40:07 2019 DEPLOYED raw-3.1.0 3.1.0 def
|
|||
check(t, testcase{
|
||||
interactive: true,
|
||||
confirm: false,
|
||||
wantDiffs: 1,
|
||||
files: map[string]string{
|
||||
"/path/to/helmfile.yaml": `
|
||||
releases:
|
||||
|
|
@ -282,7 +290,7 @@ func TestSync(t *testing.T) {
|
|||
}
|
||||
for flagIdx := range wantUpgrades[relIdx].Flags {
|
||||
if wantUpgrades[relIdx].Flags[flagIdx] != helm.Releases[relIdx].Flags[flagIdx] {
|
||||
t.Errorf("releaes[%d].flags[%d]: got %v, want %v", relIdx, flagIdx, helm.Releases[relIdx].Flags[flagIdx], wantUpgrades[relIdx].Flags[flagIdx])
|
||||
t.Errorf("releases[%d].flags[%d]: got %v, want %v", relIdx, flagIdx, helm.Releases[relIdx].Flags[flagIdx], wantUpgrades[relIdx].Flags[flagIdx])
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue