Skip to content

Commit

Permalink
Improve handling of releases being newly installed by helmfile-apply (#…
Browse files Browse the repository at this point in the history 
…1618)

This improves helmfile-apply with two things:

- Some users had timing-out issues or annoyed by huge output from helm-diff run as part of helmfile-apply on first install. `--skip-diff-on-install` skips running helm-diff for releases being newly installed, so that you can avoid those issues.
- Some users had difficultly or found it not straight-forward to install CRDs and custom resources from separate charts in one helmfile-apply (#1353). The new helmfile.yaml release field `disableValidationOnInstall: true` adds `--disable-validation` to helm-diff only for releases being newly released, which should mostly resolve the issue.

Resolves #1353
  • Loading branch information
@mumoshu
mumoshu committed Dec 11, 2020
1 parent b910591 commit 1ef9b29
Show file tree
Hide file tree
Showing 5 changed files with 301 additions and 27 deletions.
8 changes: 8 additions & 0 deletions main.go
View file
Expand Up @@ -397,6 +397,10 @@ func main() {
Name: "skip-cleanup",
Usage: "Stop cleaning up temporary values generated by helmfile and helm-secrets. Useful for debugging. Don't use in production for security",
},
cli.BoolFlag{
Name: "skip-diff-on-install",
Usage: "Skips running helm-diff on releases being newly installed on this apply. Useful when the release manifests are too huge to be reviewed, or it's too time-consuming to diff at all",
},
cli.BoolFlag{
Name: "include-tests",
Usage: "enable the diffing of the helm test hooks",
Expand Down Expand Up @@ -738,6 +742,10 @@ func (c configImpl) SkipCleanup() bool {
return c.c.Bool("skip-cleanup")
}

func (c configImpl) SkipDiffOnInstall() bool {
return c.c.Bool("skip-diff-on-install")
}

func (c configImpl) EmbedValues() bool {
return c.c.Bool("embed-values")
}
Expand Down
9 changes: 5 additions & 4 deletions pkg/app/app.go
View file
Expand Up @@ -1084,10 +1084,11 @@ func (a *App) apply(r *Run, c ApplyConfigProvider) (bool, bool, []error) {
detailedExitCode := true

diffOpts := &state.DiffOpts{
NoColor: c.NoColor(),
Context: c.Context(),
Set: c.Set(),
SkipCleanup: c.RetainValuesFiles() || c.SkipCleanup(),
NoColor: c.NoColor(),
Context: c.Context(),
Set: c.Set(),
SkipCleanup: c.RetainValuesFiles() || c.SkipCleanup(),
SkipDiffOnInstall: c.SkipDiffOnInstall(),
}

infoMsg, releasesToBeUpdated, releasesToBeDeleted, errs := r.diff(false, detailedExitCode, c, diffOpts)
Expand Down
251 changes: 237 additions & 14 deletions pkg/app/app_test.go
View file
Expand Up @@ -2313,6 +2313,7 @@ type applyConfig struct {
concurrency int
detailedExitcode bool
interactive bool
skipDiffOnInstall bool
logger *zap.SugaredLogger
}

Expand Down Expand Up @@ -2380,6 +2381,10 @@ func (a applyConfig) RetainValuesFiles() bool {
return a.retainValuesFiles
}

func (a applyConfig) SkipDiffOnInstall() bool {
return a.skipDiffOnInstall
}

type depsConfig struct {
skipRepos bool
}
Expand Down Expand Up @@ -2651,18 +2656,19 @@ releases:

func TestApply(t *testing.T) {
testcases := []struct {
name string
loc string
ns string
concurrency int
error string
files map[string]string
selectors []string
lists map[exectest.ListKey]string
diffs map[exectest.DiffKey]error
upgraded []exectest.Release
deleted []exectest.Release
log string
name string
loc string
ns string
concurrency int
skipDiffOnInstall bool
error string
files map[string]string
selectors []string
lists map[exectest.ListKey]string
diffs map[exectest.DiffKey]error
upgraded []exectest.Release
deleted []exectest.Release
log string
}{
//
// complex test cases for smoke testing
Expand Down Expand Up @@ -3071,6 +3077,222 @@ baz stable/mychart3
bar stable/mychart2
foo stable/mychart1
`,
},
//
// install with upgrade
//
{
name: "install-with-upgrade-with-validation-control",
loc: location(),
files: map[string]string{
"/path/to/helmfile.yaml": `
releases:
- name: baz
chart: stable/mychart3
disableValidationOnInstall: true
- name: foo
chart: stable/mychart1
disableValidationOnInstall: true
needs:
- bar
- name: bar
chart: stable/mychart2
disableValidation: true
`,
},
diffs: map[exectest.DiffKey]error{
exectest.DiffKey{Name: "baz", Chart: "stable/mychart3", Flags: "--kube-contextdefault--detailed-exitcode"}: helmexec.ExitError{Code: 2},
exectest.DiffKey{Name: "foo", Chart: "stable/mychart1", Flags: "--disable-validation--kube-contextdefault--detailed-exitcode"}: helmexec.ExitError{Code: 2},
exectest.DiffKey{Name: "bar", Chart: "stable/mychart2", Flags: "--disable-validation--kube-contextdefault--detailed-exitcode"}: helmexec.ExitError{Code: 2},
},
lists: map[exectest.ListKey]string{
exectest.ListKey{Filter: "^foo$", Flags: "--kube-contextdefault--deployed--failed--pending"}: ``,
exectest.ListKey{Filter: "^bar$", Flags: "--kube-contextdefault--deployed--failed--pending"}: `NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE
bar 4 Fri Nov 1 08:40:07 2019 DEPLOYED mychart2-3.1.0 3.1.0 default
`,
exectest.ListKey{Filter: "^baz$", Flags: "--kube-contextdefault--deployed--failed--pending"}: `NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE
baz 4 Fri Nov 1 08:40:07 2019 DEPLOYED mychart3-3.1.0 3.1.0 default
`,
},
upgraded: []exectest.Release{
{Name: "baz", Flags: []string{"--kube-context", "default"}},
{Name: "bar", Flags: []string{"--kube-context", "default"}},
{Name: "foo", Flags: []string{"--kube-context", "default"}},
},
deleted: []exectest.Release{},
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: releases:
2: - name: baz
3: chart: stable/mychart3
4: disableValidationOnInstall: true
5: - name: foo
6: chart: stable/mychart1
7: disableValidationOnInstall: true
8: needs:
9: - bar
10: - name: bar
11: chart: stable/mychart2
12: disableValidation: true
13:
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: releases:
2: - name: baz
3: chart: stable/mychart3
4: disableValidationOnInstall: true
5: - name: foo
6: chart: stable/mychart1
7: disableValidationOnInstall: true
8: needs:
9: - bar
10: - name: bar
11: chart: stable/mychart2
12: disableValidation: true
13:
merged environment: &{default map[] map[]}
3 release(s) found in helmfile.yaml
Affected releases are:
bar (stable/mychart2) UPDATED
baz (stable/mychart3) UPDATED
foo (stable/mychart1) UPDATED
processing 2 groups of releases in this order:
GROUP RELEASES
1 baz, bar
2 foo
processing releases in group 1/2: baz, bar
processing releases in group 2/2: foo
getting deployed release version failed:Failed to get the version for:mychart1
UPDATED RELEASES:
NAME CHART VERSION
baz stable/mychart3 3.1.0
bar stable/mychart2 3.1.0
foo stable/mychart1
`,
},
//
// install with upgrade and --skip-diff-on-install
//
{
name: "install-with-upgrade-with-skip-diff-on-install",
loc: location(),
skipDiffOnInstall: true,
files: map[string]string{
"/path/to/helmfile.yaml": `
releases:
- name: baz
chart: stable/mychart3
disableValidationOnInstall: true
- name: foo
chart: stable/mychart1
disableValidationOnInstall: true
needs:
- bar
- name: bar
chart: stable/mychart2
disableValidation: true
`,
},
diffs: map[exectest.DiffKey]error{
exectest.DiffKey{Name: "baz", Chart: "stable/mychart3", Flags: "--kube-contextdefault--detailed-exitcode"}: helmexec.ExitError{Code: 2},
exectest.DiffKey{Name: "bar", Chart: "stable/mychart2", Flags: "--disable-validation--kube-contextdefault--detailed-exitcode"}: helmexec.ExitError{Code: 2},
},
lists: map[exectest.ListKey]string{
exectest.ListKey{Filter: "^foo$", Flags: "--kube-contextdefault--deployed--failed--pending"}: ``,
exectest.ListKey{Filter: "^bar$", Flags: "--kube-contextdefault--deployed--failed--pending"}: `NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE
bar 4 Fri Nov 1 08:40:07 2019 DEPLOYED mychart2-3.1.0 3.1.0 default
`,
exectest.ListKey{Filter: "^baz$", Flags: "--kube-contextdefault--deployed--failed--pending"}: `NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE
baz 4 Fri Nov 1 08:40:07 2019 DEPLOYED mychart3-3.1.0 3.1.0 default
`,
},
upgraded: []exectest.Release{
{Name: "baz", Flags: []string{"--kube-context", "default"}},
{Name: "bar", Flags: []string{"--kube-context", "default"}},
{Name: "foo", Flags: []string{"--kube-context", "default"}},
},
deleted: []exectest.Release{},
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: releases:
2: - name: baz
3: chart: stable/mychart3
4: disableValidationOnInstall: true
5: - name: foo
6: chart: stable/mychart1
7: disableValidationOnInstall: true
8: needs:
9: - bar
10: - name: bar
11: chart: stable/mychart2
12: disableValidation: true
13:
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: releases:
2: - name: baz
3: chart: stable/mychart3
4: disableValidationOnInstall: true
5: - name: foo
6: chart: stable/mychart1
7: disableValidationOnInstall: true
8: needs:
9: - bar
10: - name: bar
11: chart: stable/mychart2
12: disableValidation: true
13:
merged environment: &{default map[] map[]}
3 release(s) found in helmfile.yaml
Affected releases are:
bar (stable/mychart2) UPDATED
baz (stable/mychart3) UPDATED
foo (stable/mychart1) UPDATED
processing 2 groups of releases in this order:
GROUP RELEASES
1 baz, bar
2 foo
processing releases in group 1/2: baz, bar
processing releases in group 2/2: foo
getting deployed release version failed:Failed to get the version for:mychart1
UPDATED RELEASES:
NAME CHART VERSION
baz stable/mychart3 3.1.0
bar stable/mychart2 3.1.0
foo stable/mychart1
`,
},
//
Expand Down Expand Up @@ -3919,8 +4141,9 @@ err: "foo" depends on nonexistent release "bar"

applyErr := app.Apply(applyConfig{
// if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic.
concurrency: tc.concurrency,
logger: logger,
concurrency: tc.concurrency,
logger: logger,
skipDiffOnInstall: tc.skipDiffOnInstall,
})
if tc.error == "" && applyErr != nil {
t.Fatalf("unexpected error for data defined at %s: %v", tc.loc, applyErr)
Expand Down
1 change: 1 addition & 0 deletions pkg/app/config.go
View file
Expand Up @@ -52,6 +52,7 @@ type ApplyConfigProvider interface {

RetainValuesFiles() bool
SkipCleanup() bool
SkipDiffOnInstall() bool

concurrencyConfig
interactive
Expand Down

0 comments on commit 1ef9b29

Please sign in to comment.