From de8644a5044feedade98ffa6691976347f230925 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Sat, 1 May 2021 15:06:07 +0900 Subject: [PATCH] Fix --selector results to correctly deduplicate releases (#1822) Fixes #1818 --- pkg/app/app.go | 24 +++--------------------- pkg/app/app_test.go | 4 ++-- pkg/app/diff_nokubectx_test.go | 4 ++-- pkg/app/diff_test.go | 4 ++-- pkg/state/state_run.go | 15 --------------- 5 files changed, 9 insertions(+), 42 deletions(-) diff --git a/pkg/app/app.go b/pkg/app/app.go index d305f627..232a67c0 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -1120,8 +1120,6 @@ func (a *App) apply(r *Run, c ApplyConfigProvider) (bool, bool, []error) { st := r.state helm := r.helm - allReleases := st.GetReleasesWithOverrides() - toApply, err := a.getSelectedReleases(r) if err != nil { return false, false, []error{err} @@ -1214,9 +1212,6 @@ Do you really want to apply? affectedReleases := state.AffectedReleases{} - // Traverse DAG of all the releases so that we don't suffer from false-positive missing dependencies - st.Releases = allReleases - if !interactive || interactive && r.askForConfirmation(confMsg) { r.helm.SetExtraArgs(argparser.GetArgs(c.Args(), r.state)...) @@ -1360,8 +1355,6 @@ Do you really want to delete? func (a *App) diff(r *Run, c DiffConfigProvider) (*string, bool, bool, []error) { st := r.state - allReleases := st.GetReleasesWithOverrides() - toDiff, err := a.getSelectedReleases(r) if err != nil { return nil, false, false, []error{err} @@ -1371,10 +1364,6 @@ func (a *App) diff(r *Run, c DiffConfigProvider) (*string, bool, bool, []error) return nil, false, false, nil } - // Do build deps and prepare only on selected releases so that we won't waste time - // on running various helm commands on unnecessary releases - st.Releases = toDiff - r.helm.SetExtraArgs(argparser.GetArgs(c.Args(), r.state)...) opts := &state.DiffOpts{ @@ -1384,9 +1373,6 @@ func (a *App) diff(r *Run, c DiffConfigProvider) (*string, bool, bool, []error) Set: c.Set(), } - // Validate all releases for missing `needs` targets - st.Releases = allReleases - plan, err := st.PlanReleases(state.PlanOptions{Reverse: false, SelectedReleases: toDiff, SkipNeeds: c.SkipNeeds(), IncludeNeeds: c.IncludeNeeds()}) if err != nil { return nil, false, false, []error{err} @@ -1549,8 +1535,6 @@ func (a *App) sync(r *Run, c SyncConfigProvider) (bool, []error) { st := r.state helm := r.helm - allReleases := st.GetReleasesWithOverrides() - toSync, err := a.getSelectedReleases(r) if err != nil { return false, []error{err} @@ -1574,7 +1558,7 @@ func (a *App) sync(r *Run, c SyncConfigProvider) (bool, []error) { // Do build deps and prepare only on selected releases so that we won't waste time // on running various helm commands on unnecessary releases - st.Releases = toSync + st.Releases = toSyncWithNeeds toDelete, err := st.DetectReleasesToBeDeletedForSync(helm, toSyncWithNeeds) if err != nil { @@ -1644,7 +1628,7 @@ func (a *App) sync(r *Run, c SyncConfigProvider) (bool, []error) { r.helm.SetExtraArgs(argparser.GetArgs(c.Args(), r.state)...) // Traverse DAG of all the releases so that we don't suffer from false-positive missing dependencies - st.Releases = allReleases + st.Releases = toSyncWithNeeds affectedReleases := state.AffectedReleases{} @@ -1701,8 +1685,6 @@ func (a *App) template(r *Run, c TemplateConfigProvider) (bool, []error) { st := r.state helm := r.helm - allReleases := st.GetReleasesWithOverrides() - selectedReleases, err := a.getSelectedReleases(r) if err != nil { return false, []error{err} @@ -1739,7 +1721,7 @@ func (a *App) template(r *Run, c TemplateConfigProvider) (bool, []error) { var errs []error // Traverse DAG of all the releases so that we don't suffer from false-positive missing dependencies - st.Releases = allReleases + st.Releases = selectedReleasesWithNeeds args := argparser.GetArgs(c.Args(), st) diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index 42242319..b5e4e37f 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -4201,7 +4201,7 @@ releases: upgraded: []exectest.Release{}, deleted: []exectest.Release{}, concurrency: 1, - error: `in ./helmfile.yaml: "default/foo" depends on nonexistent release "default/bar"`, + error: `in ./helmfile.yaml: release "default/foo" depends on "default/bar" which does not match the selectors. Please add a selector like "--selector name=bar", or indicate whether to skip (--skip-needs) or include (--include-needs) these dependencies`, log: `processing file "helmfile.yaml" in directory "." first-pass rendering starting for "helmfile.yaml.part.0": inherited=&{default map[] map[]}, overrode= first-pass uses: &{default map[] map[]} @@ -4237,7 +4237,7 @@ second-pass rendering result of "helmfile.yaml.part.0": merged environment: &{default map[] map[]} 2 release(s) found in helmfile.yaml -err: "default/foo" depends on nonexistent release "default/bar" +err: release "default/foo" depends on "default/bar" which does not match the selectors. Please add a selector like "--selector name=bar", or indicate whether to skip (--skip-needs) or include (--include-needs) these dependencies `, }, } diff --git a/pkg/app/diff_nokubectx_test.go b/pkg/app/diff_nokubectx_test.go index cd64dcda..254d43c7 100644 --- a/pkg/app/diff_nokubectx_test.go +++ b/pkg/app/diff_nokubectx_test.go @@ -1159,7 +1159,7 @@ releases: upgraded: []exectest.Release{}, deleted: []exectest.Release{}, concurrency: 1, - error: `in ./helmfile.yaml: "foo" depends on nonexistent release "bar"`, + error: `in ./helmfile.yaml: release "foo" depends on "bar" which does not match the selectors. Please add a selector like "--selector name=bar", or indicate whether to skip (--skip-needs) or include (--include-needs) these dependencies`, log: `processing file "helmfile.yaml" in directory "." first-pass rendering starting for "helmfile.yaml.part.0": inherited=&{default map[] map[]}, overrode= first-pass uses: &{default map[] map[]} @@ -1195,7 +1195,7 @@ second-pass rendering result of "helmfile.yaml.part.0": merged environment: &{default map[] map[]} 2 release(s) found in helmfile.yaml -err: "foo" depends on nonexistent release "bar" +err: release "foo" depends on "bar" which does not match the selectors. Please add a selector like "--selector name=bar", or indicate whether to skip (--skip-needs) or include (--include-needs) these dependencies `, }, } diff --git a/pkg/app/diff_test.go b/pkg/app/diff_test.go index 049b7873..7dc6cfa0 100644 --- a/pkg/app/diff_test.go +++ b/pkg/app/diff_test.go @@ -1263,7 +1263,7 @@ releases: upgraded: []exectest.Release{}, deleted: []exectest.Release{}, concurrency: 1, - error: `in ./helmfile.yaml: "default/foo" depends on nonexistent release "default/bar"`, + error: `in ./helmfile.yaml: release "default/foo" depends on "default/bar" which does not match the selectors. Please add a selector like "--selector name=bar", or indicate whether to skip (--skip-needs) or include (--include-needs) these dependencies`, log: `processing file "helmfile.yaml" in directory "." first-pass rendering starting for "helmfile.yaml.part.0": inherited=&{default map[] map[]}, overrode= first-pass uses: &{default map[] map[]} @@ -1299,7 +1299,7 @@ second-pass rendering result of "helmfile.yaml.part.0": merged environment: &{default map[] map[]} 2 release(s) found in helmfile.yaml -err: "default/foo" depends on nonexistent release "default/bar" +err: release "default/foo" depends on "default/bar" which does not match the selectors. Please add a selector like "--selector name=bar", or indicate whether to skip (--skip-needs) or include (--include-needs) these dependencies `, }, } diff --git a/pkg/state/state_run.go b/pkg/state/state_run.go index 37456b98..39a5ac52 100644 --- a/pkg/state/state_run.go +++ b/pkg/state/state_run.go @@ -165,21 +165,6 @@ func GroupReleasesByDependency(releases []Release, opts PlanOptions) ([][]Releas } } - for _, r := range releases { - if !r.Filtered { - for _, n := range r.Needs { - // To map n into idToReleases correctly, prepend KubeContext to n. - if r.KubeContext != "" { - n = r.KubeContext + "/" + n - } - if _, ok := idToReleases[n]; !ok { - id := ReleaseToID(&r.ReleaseSpec) - return nil, fmt.Errorf("%q depends on nonexistent release %q", id, n) - } - } - } - } - var selectedReleaseIDs []string for _, r := range opts.SelectedReleases {