public_git
/
helmfile
mirror of https://github.com/helmfile/helmfile.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

fix: Fix --selector to not break `needs` (#934)

This commit is contained in:
KUOKA Yusuke 2019-11-06 17:05:25 +09:00 committed by GitHub
parent dad8e72bc5
commit f41fe86452
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 429 additions and 104 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

218
pkg/app/app.go
View File

@ -98,13 +98,13 @@ func Init(app *App) *App {
}
func (a *App) Deps(c DepsConfigProvider) error {
return a.ForEachState(func(run *Run) []error {
return a.ForEachStateFiltered(func(run *Run) []error {
return run.Deps(c)
})
}
func (a *App) Repos(c ReposConfigProvider) error {
return a.ForEachState(func(run *Run) []error {
return a.ForEachStateFiltered(func(run *Run) []error {
return run.Repos(c)
})
}
@ -116,68 +116,68 @@ func (a *App) reverse() *App {
}
func (a *App) DeprecatedSyncCharts(c DeprecatedChartsConfigProvider) error {
return a.ForEachState(func(run *Run) []error {
return a.ForEachStateFiltered(func(run *Run) []error {
return run.DeprecatedSyncCharts(c)
})
}
func (a *App) Diff(c DiffConfigProvider) error {
return a.ForEachState(func(run *Run) []error {
return a.ForEachStateFiltered(func(run *Run) []error {
return run.Diff(c)
})
}
func (a *App) Template(c TemplateConfigProvider) error {
return a.ForEachState(func(run *Run) []error {
return a.ForEachStateFiltered(func(run *Run) []error {
return run.Template(c)
})
}
func (a *App) Lint(c LintConfigProvider) error {
return a.ForEachState(func(run *Run) []error {
return a.ForEachStateFiltered(func(run *Run) []error {
return run.Lint(c)
})
}
func (a *App) Sync(c SyncConfigProvider) error {
return a.ForEachState(func(run *Run) []error {
return a.ForEachStateFiltered(func(run *Run) []error {
return run.Sync(c)
})
}
func (a *App) Apply(c ApplyConfigProvider) error {
return a.ForEachState(func(run *Run) []error {
return a.ForEachState(func(run *Run) (bool, []error) {
return a.apply(run, c)
})
}
func (a *App) Status(c StatusesConfigProvider) error {
return a.ForEachState(func(run *Run) []error {
return a.ForEachStateFiltered(func(run *Run) []error {
return run.Status(c)
})
}
func (a *App) Delete(c DeleteConfigProvider) error {
return a.reverse().ForEachState(func(run *Run) []error {
return a.reverse().ForEachStateFiltered(func(run *Run) []error {
return run.Delete(c)
}, true)
}
func (a *App) Destroy(c DestroyConfigProvider) error {
return a.reverse().ForEachState(func(run *Run) []error {
return a.reverse().ForEachStateFiltered(func(run *Run) []error {
return run.Destroy(c)
}, true)
}
func (a *App) Test(c TestConfigProvider) error {
return a.ForEachState(func(run *Run) []error {
return a.ForEachStateFiltered(func(run *Run) []error {
return run.Test(c)
})
}
func (a *App) PrintState(c StateConfigProvider) error {
return a.ForEachState(func(run *Run) []error {
return a.ForEachStateFiltered(func(run *Run) []error {
state, err := run.state.ToYaml()
if err != nil {
return []error{err}
@ -191,7 +191,7 @@ func (a *App) ListReleases(c StateConfigProvider) error {
table := uitable.New()
table.AddRow("NAME", "NAMESPACE", "INSTALLED", "LABELS")
err := a.ForEachState(func(run *Run) []error {
err := a.ForEachStateFiltered(func(run *Run) []error {
//var releases m
for _, r := range run.state.Releases {
labels := ""
@ -399,7 +399,7 @@ func (a *App) visitStates(fileOrDir string, defOpts LoadOpts, converge func(*sta
return nil
}
func (a *App) ForEachState(do func(*Run) []error, dagEnabled ...bool) error {
func (a *App) ForEachStateFiltered(do func(*Run) []error, dagEnabled ...bool) error {
ctx := NewContext()
err := a.VisitDesiredStatesWithReleasesFiltered(a.FileOrDir, func(st *state.HelmState, helm helmexec.Interface) []error {
run := NewRun(st, helm, ctx)
@ -414,6 +414,20 @@ func (a *App) ForEachState(do func(*Run) []error, dagEnabled ...bool) error {
return err
}
func (a *App) ForEachState(do func(*Run) (bool, []error)) error {
ctx := NewContext()
err := a.visitStatesWithSelectorsAndRemoteSupport(a.FileOrDir, func(st *state.HelmState, helm helmexec.Interface) (bool, []error) {
run := NewRun(st, helm, ctx)
return do(run)
})
if err != nil && a.ErrorHandler != nil {
return a.ErrorHandler(err)
}
return err
}
func printBatches(batches [][]state.Release) string {
buf := &bytes.Buffer{}
@ -644,7 +658,7 @@ func (a *App) findDesiredStateFiles(specifiedPath string) ([]string, error) {
return files, nil
}
func (a *App) apply(r *Run, c ApplyConfigProvider) []error {
func (a *App) apply(r *Run, c ApplyConfigProvider) (bool, []error) {
st := r.state
helm := r.helm
ctx := r.ctx
@ -652,14 +666,14 @@ func (a *App) apply(r *Run, c ApplyConfigProvider) []error {
affectedReleases := state.AffectedReleases{}
if !c.SkipDeps() {
if errs := ctx.SyncReposOnce(st, helm); errs != nil && len(errs) > 0 {
return errs
return false, errs
}
if errs := st.BuildDeps(helm); errs != nil && len(errs) > 0 {
return errs
return false, errs
}
}
if errs := st.PrepareReleases(helm, "apply"); errs != nil && len(errs) > 0 {
return errs
return false, errs
}
// helm must be 2.11+ and helm-diff should be provided `--detailed-exitcode` in order for `helmfile apply` to work properly
@ -671,29 +685,53 @@ func (a *App) apply(r *Run, c ApplyConfigProvider) []error {
Set: c.Set(),
}
changedReleases, errs := st.DiffReleases(helm, c.Values(), c.Concurrency(), detailedExitCode, c.SuppressSecrets(), false, diffOpts)
allReleases := st.Releases
deletingReleases, err := st.DetectReleasesToBeDeleted(helm, st.Releases)
if err != nil {
errs = append(errs, err)
var changedReleases []state.ReleaseSpec
var deletingReleases []state.ReleaseSpec
var planningErrs []error
// TODO Better way to detect diff on only filtered releases
{
if len(st.Selectors) > 0 {
releases, err := st.GetFilteredReleases()
if err != nil {
return false, []error{err}
}
if len(releases) == 0 {
return false, nil
}
st.Releases = releases
}
changedReleases, planningErrs = st.DiffReleases(helm, c.Values(), c.Concurrency(), detailedExitCode, c.SuppressSecrets(), false, diffOpts)
var err error
deletingReleases, err = st.DetectReleasesToBeDeleted(helm, st.Releases)
if err != nil {
planningErrs = append(planningErrs, err)
}
}
st.Releases = allReleases
fatalErrs := []error{}
noError := true
for _, e := range errs {
for _, e := range planningErrs {
switch err := e.(type) {
case *state.ReleaseError:
if err.Code != 2 {
noError = false
fatalErrs = append(fatalErrs, e)
}
default:
noError = false
fatalErrs = append(fatalErrs, e)
}
}
if len(fatalErrs) > 0 {
return false, fatalErrs
}
releasesToBeDeleted := map[string]state.ReleaseSpec{}
for _, r := range deletingReleases {
id := state.ReleaseToID(&r)
@ -711,90 +749,90 @@ func (a *App) apply(r *Run, c ApplyConfigProvider) []error {
}
// sync only when there are changes
if noError {
if len(releasesToBeUpdated) == 0 && len(releasesToBeDeleted) == 0 {
// TODO better way to get the logger
logger := c.Logger()
logger.Infof("")
logger.Infof("No affected releases")
} else {
names := []string{}
for _, r := range releasesToBeUpdated {
names = append(names, fmt.Sprintf(" %s (%s) UPDATED", r.Name, r.Chart))
}
for _, r := range releasesToBeDeleted {
names = append(names, fmt.Sprintf(" %s (%s) DELETED", r.Name, r.Chart))
}
// Make the output deterministic for testing purpose
sort.Strings(names)
if len(releasesToBeUpdated) == 0 && len(releasesToBeDeleted) == 0 {
// TODO better way to get the logger
logger := c.Logger()
logger.Infof("")
logger.Infof("No affected releases")
return true, nil
}
infoMsg := fmt.Sprintf(`Affected releases are:
names := []string{}
for _, r := range releasesToBeUpdated {
names = append(names, fmt.Sprintf(" %s (%s) UPDATED", r.Name, r.Chart))
}
for _, r := range releasesToBeDeleted {
names = append(names, fmt.Sprintf(" %s (%s) DELETED", r.Name, r.Chart))
}
// Make the output deterministic for testing purpose
sort.Strings(names)
infoMsg := fmt.Sprintf(`Affected releases are:
%s
`, strings.Join(names, "\n"))
confMsg := fmt.Sprintf(`%s
confMsg := fmt.Sprintf(`%s
Do you really want to apply?
Helmfile will apply all your changes, as shown above.
`, infoMsg)
interactive := c.Interactive()
if !interactive {
a.Logger.Debug(infoMsg)
interactive := c.Interactive()
if !interactive {
a.Logger.Debug(infoMsg)
}
syncErrs := []error{}
if !interactive || interactive && r.askForConfirmation(confMsg) {
r.helm.SetExtraArgs(argparser.GetArgs(c.Args(), r.state)...)
// We deleted releases by traversing the DAG in reverse order
if len(releasesToBeDeleted) > 0 {
_, deletionErrs := withDAG(st, helm, a.Logger, true, a.Wrap(func(subst *state.HelmState, helm helmexec.Interface) []error {
var rs []state.ReleaseSpec
for _, r := range subst.Releases {
if _, ok := releasesToBeDeleted[state.ReleaseToID(&r)]; ok {
rs = append(rs, r)
}
}
subst.Releases = rs
return subst.DeleteReleasesForSync(&affectedReleases, helm, c.Concurrency())
}))
if deletionErrs != nil && len(deletionErrs) > 0 {
syncErrs = append(syncErrs, deletionErrs...)
}
if !interactive || interactive && r.askForConfirmation(confMsg) {
r.helm.SetExtraArgs(argparser.GetArgs(c.Args(), r.state)...)
}
// We deleted releases by traversing the DAG in reverse order
if len(releasesToBeDeleted) > 0 {
_, errs := withDAG(st, helm, a.Logger, true, a.Wrap(func(subst *state.HelmState, helm helmexec.Interface) []error {
var rs []state.ReleaseSpec
// We upgrade releases by traversing the DAG
if len(releasesToBeUpdated) > 0 {
_, updateErrs := withDAG(st, helm, a.Logger, false, a.Wrap(func(subst *state.HelmState, helm helmexec.Interface) []error {
var rs []state.ReleaseSpec
for _, r := range subst.Releases {
if _, ok := releasesToBeDeleted[state.ReleaseToID(&r)]; ok {
rs = append(rs, r)
}
}
subst.Releases = rs
return subst.DeleteReleasesForSync(&affectedReleases, helm, c.Concurrency())
}))
if errs != nil && len(errs) > 0 {
return errs
for _, r := range subst.Releases {
if _, ok := releasesToBeUpdated[state.ReleaseToID(&r)]; ok {
rs = append(rs, r)
}
}
// We upgrade releases by traversing the DAG
if len(releasesToBeUpdated) > 0 {
_, errs := withDAG(st, helm, a.Logger, false, a.Wrap(func(subst *state.HelmState, helm helmexec.Interface) []error {
var rs []state.ReleaseSpec
subst.Releases = rs
for _, r := range subst.Releases {
if _, ok := releasesToBeUpdated[state.ReleaseToID(&r)]; ok {
rs = append(rs, r)
}
}
subst.Releases = rs
syncOpts := state.SyncOpts{
Set: c.Set(),
}
return subst.SyncReleases(&affectedReleases, helm, c.Values(), c.Concurrency(), &syncOpts)
}))
if errs != nil && len(errs) > 0 {
return errs
}
syncOpts := state.SyncOpts{
Set: c.Set(),
}
return subst.SyncReleases(&affectedReleases, helm, c.Values(), c.Concurrency(), &syncOpts)
}))
return nil
if updateErrs != nil && len(updateErrs) > 0 {
syncErrs = append(syncErrs, updateErrs...)
}
}
}
affectedReleases.DisplayAffectedReleases(c.Logger())
return fatalErrs
return true, syncErrs
}
func fileExistsAt(path string) bool {

279
pkg/app/app_test.go
View File

@ -2061,6 +2061,7 @@ func TestApply(t *testing.T) {
concurrency int
error string
files map[string]string
selectors []string
lists map[exectest.ListKey]string
diffs map[exectest.DiffKey]error
upgraded []exectest.Release
@ -2361,6 +2362,22 @@ worker 1/1 finished
worker 1/1 started
getting deployed release version failed:unexpected list key: {^frontend-v3$ --kube-contextdefault}
worker 1/1 finished
UPDATED RELEASES:
NAME CHART VERSION
front-proxy stable/envoy
logging charts/fluent-bit
database charts/mysql
servicemesh charts/istio
anotherbackend charts/anotherbackend
backend-v2 charts/backend
frontend-v3 charts/frontend
DELETED RELEASES:
NAME
frontend-v1
backend-v1
`,
},
//
@ -2487,6 +2504,13 @@ worker 1/1 finished
worker 1/1 started
getting deployed release version failed:unexpected list key: {^foo$ --kube-contextdefault}
worker 1/1 finished
UPDATED RELEASES:
NAME CHART VERSION
bar mychart2
baz mychart3
foo mychart1
`,
},
//
@ -2756,6 +2780,12 @@ worker 1/1 finished
worker 1/1 started
getting deployed release version failed:unexpected list key: {^bar$ --tiller-namespacetns2--kube-contextdefault}
worker 1/1 finished
UPDATED RELEASES:
NAME CHART VERSION
foo mychart1
bar mychart2
`,
},
//
@ -2967,6 +2997,251 @@ bar 4 Fri Nov 1 08:40:07 2019 DEPLOYED mychart2-3.1.0 3.1.0 defau
},
},
//
// upgrades with selector
//
{
// see https://github.com/roboll/helmfile/issues/919#issuecomment-549831747
name: "upgrades with good selector",
loc: location(),
files: map[string]string{
"/path/to/helmfile.yaml": `
{{ $mark := "a" }}
releases:
- name: kubernetes-external-secrets
chart: incubator/raw
namespace: kube-system
- name: external-secrets
chart: incubator/raw
namespace: default
labels:
app: test
needs:
- kube-system/kubernetes-external-secrets
- name: my-release
chart: incubator/raw
namespace: default
labels:
app: test
needs:
- default/external-secrets
`,
},
selectors: []string{"app=test"},
diffs: map[exectest.DiffKey]error{
exectest.DiffKey{Name: "external-secrets", Chart: "incubator/raw", Flags: "--kube-contextdefault--namespacedefault--detailed-exitcode"}: helmexec.ExitError{Code: 2},
exectest.DiffKey{Name: "my-release", Chart: "incubator/raw", Flags: "--kube-contextdefault--namespacedefault--detailed-exitcode"}: helmexec.ExitError{Code: 2},
},
upgraded: []exectest.Release{
{Name: "external-secrets", Flags: []string{"--kube-context", "default", "--namespace", "default"}},
{Name: "my-release", Flags: []string{"--kube-context", "default", "--namespace", "default"}},
},
// as we check for log output, set concurrency to 1 to avoid non-deterministic test result
concurrency: 1,
log: `processing file "helmfile.yaml" in directory "."
first-pass rendering starting for "helmfile.yaml.part.0": inherited=&{default map[] map[]}, overrode=<nil>
first-pass uses: &{default map[] map[]}
first-pass rendering output of "helmfile.yaml.part.0":
0:
1:
2:
3: releases:
4: - name: kubernetes-external-secrets
5: chart: incubator/raw
6: namespace: kube-system
7:
8: - name: external-secrets
9: chart: incubator/raw
10: namespace: default
11: labels:
12: app: test
13: needs:
14: - kube-system/kubernetes-external-secrets
15:
16: - name: my-release
17: chart: incubator/raw
18: namespace: default
19: labels:
20: app: test
21: needs:
22: - default/external-secrets
23:
first-pass produced: &{default map[] map[]}
first-pass rendering result of "helmfile.yaml.part.0": {default map[] map[]}
vals:
map[]
defaultVals:[]
second-pass rendering result of "helmfile.yaml.part.0":
0:
1:
2:
3: releases:
4: - name: kubernetes-external-secrets
5: chart: incubator/raw
6: namespace: kube-system
7:
8: - name: external-secrets
9: chart: incubator/raw
10: namespace: default
11: labels:
12: app: test
13: needs:
14: - kube-system/kubernetes-external-secrets
15:
16: - name: my-release
17: chart: incubator/raw
18: namespace: default
19: labels:
20: app: test
21: needs:
22: - default/external-secrets
23:
merged environment: &{default map[] map[]}
worker 1/1 started
worker 1/1 finished
worker 1/1 started
worker 1/1 finished
Affected releases are:
external-secrets (incubator/raw) UPDATED
my-release (incubator/raw) UPDATED
processing 3 groups of releases in this order:
GROUP RELEASES
1 kube-system/kubernetes-external-secrets
2 default/external-secrets
3 default/my-release
processing releases in group 1/3: kube-system/kubernetes-external-secrets
0 release(s) matching app=test found in helmfile.yaml
processing releases in group 2/3: default/external-secrets
1 release(s) matching app=test found in helmfile.yaml
worker 1/1 started
worker 1/1 finished
worker 1/1 started
getting deployed release version failed:unexpected list key: {^external-secrets$ --kube-contextdefault}
worker 1/1 finished
processing releases in group 3/3: default/my-release
1 release(s) matching app=test found in helmfile.yaml
worker 1/1 started
worker 1/1 finished
worker 1/1 started
getting deployed release version failed:unexpected list key: {^my-release$ --kube-contextdefault}
worker 1/1 finished
UPDATED RELEASES:
NAME CHART VERSION
external-secrets incubator/raw
my-release incubator/raw
`,
},
{
// see https://github.com/roboll/helmfile/issues/919#issuecomment-549831747
name: "upgrades with bad selector",
loc: location(),
files: map[string]string{
"/path/to/helmfile.yaml": `
{{ $mark := "a" }}
releases:
- name: kubernetes-external-secrets
chart: incubator/raw
namespace: kube-system
- name: external-secrets
chart: incubator/raw
namespace: default
labels:
app: test
needs:
- kube-system/kubernetes-external-secrets
- name: my-release
chart: incubator/raw
namespace: default
labels:
app: test
needs:
- default/external-secrets
`,
},
selectors: []string{"app=test_non_existent"},
diffs: map[exectest.DiffKey]error{},
upgraded: []exectest.Release{},
error: "err: no releases found that matches specified selector(app=test_non_existent) and environment(default), in any helmfile",
// as we check for log output, set concurrency to 1 to avoid non-deterministic test result
concurrency: 1,
log: `processing file "helmfile.yaml" in directory "."
first-pass rendering starting for "helmfile.yaml.part.0": inherited=&{default map[] map[]}, overrode=<nil>
first-pass uses: &{default map[] map[]}
first-pass rendering output of "helmfile.yaml.part.0":
0:
1:
2:
3: releases:
4: - name: kubernetes-external-secrets
5: chart: incubator/raw
6: namespace: kube-system
7:
8: - name: external-secrets
9: chart: incubator/raw
10: namespace: default
11: labels:
12: app: test
13: needs:
14: - kube-system/kubernetes-external-secrets
15:
16: - name: my-release
17: chart: incubator/raw
18: namespace: default
19: labels:
20: app: test
21: needs:
22: - default/external-secrets
23:
first-pass produced: &{default map[] map[]}
first-pass rendering result of "helmfile.yaml.part.0": {default map[] map[]}
vals:
map[]
defaultVals:[]
second-pass rendering result of "helmfile.yaml.part.0":
0:
1:
2:
3: releases:
4: - name: kubernetes-external-secrets
5: chart: incubator/raw
6: namespace: kube-system
7:
8: - name: external-secrets
9: chart: incubator/raw
10: namespace: default
11: labels:
12: app: test
13: needs:
14: - kube-system/kubernetes-external-secrets
15:
16: - name: my-release
17: chart: incubator/raw
18: namespace: default
19: labels:
20: app: test
21: needs:
22: - default/external-secrets
23:
merged environment: &{default map[] map[]}
`,
},
//
// error cases
//
{
@ -3101,6 +3376,10 @@ err: "foo" has dependency to inexistent release "bar"
app.Namespace = tc.ns
}
if tc.selectors != nil {
app.Selectors = tc.selectors
}
applyErr := app.Apply(applyConfig{
// if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic.
concurrency: tc.concurrency,

36
pkg/state/state.go
View File

@ -1258,11 +1258,10 @@ func MarkFilteredReleases(releases []ReleaseSpec, selectors []string) ([]Release
return filteredReleases, nil
}
// FilterReleases allows for the execution of helm commands against a subset of the releases in the helmfile.
func (st *HelmState) FilterReleases() error {
func (st *HelmState) GetFilteredReleases() ([]ReleaseSpec, error) {
filteredReleases, err := MarkFilteredReleases(st.Releases, st.Selectors)
if err != nil {
return err
return nil, err
}
var releases []ReleaseSpec
for _, r := range filteredReleases {
@ -1270,8 +1269,17 @@ func (st *HelmState) FilterReleases() error {
releases = append(releases, r.ReleaseSpec)
}
}
return releases, nil
}
// FilterReleases allows for the execution of helm commands against a subset of the releases in the helmfile.
func (st *HelmState) FilterReleases() error {
releases, err := st.GetFilteredReleases()
if err != nil {
return err
}
st.Releases = releases
st.logger.Debugf("%d release(s) matching %s found in %s\n", len(filteredReleases), strings.Join(st.Selectors, ","), st.FilePath)
st.logger.Debugf("%d release(s) matching %s found in %s\n", len(releases), strings.Join(st.Selectors, ","), st.FilePath)
return nil
}
@ -1852,9 +1860,9 @@ func renderValsSecrets(e vals.Evaluator, input ...string) ([]string, error) {
// DisplayAffectedReleases logs the upgraded, deleted and in error releases
func (ar *AffectedReleases) DisplayAffectedReleases(logger *zap.SugaredLogger) {
if ar.Upgraded != nil {
logger.Info("\nList of updated releases :")
tbl, _ := prettytable.NewTable(prettytable.Column{Header: "RELEASE"},
if ar.Upgraded != nil && len(ar.Upgraded) > 0 {
logger.Info("\nUPDATED RELEASES:")
tbl, _ := prettytable.NewTable(prettytable.Column{Header: "NAME"},
prettytable.Column{Header: "CHART", MinWidth: 6},
prettytable.Column{Header: "VERSION", AlignRight: true},
)
@ -1862,18 +1870,18 @@ func (ar *AffectedReleases) DisplayAffectedReleases(logger *zap.SugaredLogger) {
for _, release := range ar.Upgraded {
tbl.AddRow(release.Name, release.Chart, release.installedVersion)
}
tbl.Print()
logger.Info(tbl.String())
}
if ar.Deleted != nil {
logger.Info("\nList of deleted releases :")
logger.Info("RELEASE")
if ar.Deleted != nil && len(ar.Deleted) > 0 {
logger.Info("\nDELETED RELEASES:")
logger.Info("NAME")
for _, release := range ar.Deleted {
logger.Info(release.Name)
}
}
if ar.Failed != nil {
logger.Info("\nList of releases in error :")
logger.Info("RELEASE")
if ar.Failed != nil && len(ar.Failed) > 0 {
logger.Info("\nFAILED RELEASES:")
logger.Info("NAME")
for _, release := range ar.Failed {
logger.Info(release.Name)
}