diff --git a/pkg/app/app.go b/pkg/app/app.go index 38242b37..e3a6d228 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -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) { diff --git a/pkg/app/app_sync_test.go b/pkg/app/app_sync_test.go index 6b8238a7..9573d42b 100644 --- a/pkg/app/app_sync_test.go +++ b/pkg/app/app_sync_test.go @@ -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]) } } }