Fix the logic of helmfile deps and add tests. (#1588)

This commit is contained in:
Wi1dcard
2020-11-19 09:29:59 +09:00
committed by GitHub
parent b0e042988c
commit 4e485219d7
9 changed files with 298 additions and 94 deletions
+22 -14
View File
@@ -4,11 +4,12 @@ import (
"bytes"
"errors"
"fmt"
"github.com/roboll/helmfile/pkg/helmexec"
"github.com/roboll/helmfile/pkg/remote"
"io"
"os"
"github.com/roboll/helmfile/pkg/helmexec"
"github.com/roboll/helmfile/pkg/remote"
"github.com/imdario/mergo"
"github.com/roboll/helmfile/pkg/environment"
"github.com/roboll/helmfile/pkg/maputil"
@@ -39,12 +40,15 @@ func (e *UndefinedEnvError) Error() string {
}
type StateCreator struct {
logger *zap.SugaredLogger
readFile func(string) ([]byte, error)
fileExists func(string) (bool, error)
abs func(string) (string, error)
glob func(string) ([]string, error)
DeleteFile func(string) error
logger *zap.SugaredLogger
readFile func(string) ([]byte, error)
fileExists func(string) (bool, error)
abs func(string) (string, error)
glob func(string) ([]string, error)
DeleteFile func(string) error
directoryExistsAt func(string) bool
valsRuntime vals.Evaluator
Strict bool
@@ -58,13 +62,16 @@ type StateCreator struct {
remote *remote.Remote
}
func NewCreator(logger *zap.SugaredLogger, readFile func(string) ([]byte, error), fileExists func(string) (bool, error), abs func(string) (string, error), glob func(string) ([]string, error), valsRuntime vals.Evaluator, getHelm func(*HelmState) helmexec.Interface, overrideHelmBinary string, remote *remote.Remote) *StateCreator {
func NewCreator(logger *zap.SugaredLogger, readFile func(string) ([]byte, error), fileExists func(string) (bool, error), abs func(string) (string, error), glob func(string) ([]string, error), directoryExistsAt func(string) bool, valsRuntime vals.Evaluator, getHelm func(*HelmState) helmexec.Interface, overrideHelmBinary string, remote *remote.Remote) *StateCreator {
return &StateCreator{
logger: logger,
readFile: readFile,
fileExists: fileExists,
abs: abs,
glob: glob,
logger: logger,
readFile: readFile,
fileExists: fileExists,
abs: abs,
glob: glob,
directoryExistsAt: directoryExistsAt,
Strict: true,
valsRuntime: valsRuntime,
getHelm: getHelm,
@@ -131,6 +138,7 @@ func (c *StateCreator) Parse(content []byte, baseDir, file string) (*HelmState,
state.removeFile = os.Remove
state.fileExists = c.fileExists
state.glob = c.glob
state.directoryExistsAt = c.directoryExistsAt
state.valsRuntime = c.valsRuntime
return &state, nil
+5 -4
View File
@@ -1,13 +1,14 @@
package state
import (
"github.com/roboll/helmfile/pkg/remote"
"io/ioutil"
"os"
"path/filepath"
"reflect"
"testing"
"github.com/roboll/helmfile/pkg/remote"
"github.com/roboll/helmfile/pkg/testhelper"
"go.uber.org/zap"
@@ -83,7 +84,7 @@ func (testEnv stateTestEnv) MustLoadState(t *testing.T, file, envName string) *H
}
r := remote.NewRemote(logger, testFs.Cwd, testFs.ReadFile, testFs.DirectoryExistsAt, testFs.FileExistsAt)
state, err := NewCreator(logger, testFs.ReadFile, testFs.FileExists, testFs.Abs, testFs.Glob, nil, nil, "", r).
state, err := NewCreator(logger, testFs.ReadFile, testFs.FileExists, testFs.Abs, testFs.Glob, testFs.DirectoryExistsAt, nil, nil, "", r).
ParseAndLoad([]byte(yamlContent), filepath.Dir(file), file, envName, true, nil)
if err != nil {
t.Fatalf("unexpected error: %v", err)
@@ -148,7 +149,7 @@ releaseNamespace: mynamespace
testFs.Cwd = "/example/path/to"
r := remote.NewRemote(logger, testFs.Cwd, testFs.ReadFile, testFs.DirectoryExistsAt, testFs.FileExistsAt)
state, err := NewCreator(logger, testFs.ReadFile, testFs.FileExists, testFs.Abs, testFs.Glob, nil, nil, "", r).
state, err := NewCreator(logger, testFs.ReadFile, testFs.FileExists, testFs.Abs, testFs.Glob, testFs.DirectoryExistsAt, nil, nil, "", r).
ParseAndLoad(yamlContent, filepath.Dir(yamlFile), yamlFile, "production", true, nil)
if err != nil {
t.Fatalf("unexpected error: %v", err)
@@ -235,7 +236,7 @@ overrideNamespace: myns
testFs.Cwd = "/example/path/to"
r := remote.NewRemote(logger, testFs.Cwd, testFs.ReadFile, testFs.DirectoryExistsAt, testFs.FileExistsAt)
state, err := NewCreator(logger, testFs.ReadFile, testFs.FileExists, testFs.Abs, testFs.Glob, nil, nil, "", r).
state, err := NewCreator(logger, testFs.ReadFile, testFs.FileExists, testFs.Abs, testFs.Glob, testFs.DirectoryExistsAt, nil, nil, "", r).
ParseAndLoad(yamlContent, filepath.Dir(yamlFile), yamlFile, "production", true, nil)
if err != nil {
t.Fatalf("unexpected error: %v", err)
+15 -17
View File
@@ -82,12 +82,12 @@ type HelmState struct {
logger *zap.SugaredLogger
readFile func(string) ([]byte, error)
removeFile func(string) error
fileExists func(string) (bool, error)
glob func(string) ([]string, error)
tempDir func(string, string) (string, error)
readFile func(string) ([]byte, error)
removeFile func(string) error
fileExists func(string) (bool, error)
glob func(string) ([]string, error)
tempDir func(string, string) (string, error)
directoryExistsAt func(string) bool
runner helmexec.Runner
valsRuntime vals.Evaluator
@@ -833,6 +833,7 @@ func releasesNeedCharts(releases []ReleaseSpec) []ReleaseSpec {
type ChartPrepareOptions struct {
ForceDownload bool
SkipRepos bool
SkipDeps bool
SkipResolve bool
}
@@ -923,7 +924,7 @@ func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurre
chartName := release.Chart
isLocal := pathExists(normalizeChart(st.basePath, chartName))
isLocal := st.directoryExistsAt(normalizeChart(st.basePath, chartName))
chartPath, err := st.goGetterChart(chartName, release.Directory, release.ForceGoGetter)
if err != nil {
@@ -942,7 +943,7 @@ func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurre
var buildDeps bool
skipDepsGlobal := opts.SkipRepos
skipDepsGlobal := opts.SkipDeps
skipDepsRelease := release.SkipDeps != nil && *release.SkipDeps
skipDepsDefault := release.SkipDeps == nil && st.HelmDefaults.SkipDeps
skipDeps := !isLocal || skipDepsGlobal || skipDepsRelease || skipDepsDefault
@@ -970,7 +971,7 @@ func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurre
// Skip `helm dep build` and `helm dep up` altogether when the chart is from remote or the dep is
// explicitly skipped.
buildDeps = !skipDeps
} else if pathExists(normalizeChart(st.basePath, chartPath)) {
} else if normalizedChart := normalizeChart(st.basePath, chartPath); st.directoryExistsAt(normalizedChart) {
// At this point, we are sure that chartPath is a local directory containing either:
// - A remote chart fetched by go-getter or
// - A local chart
@@ -994,7 +995,7 @@ func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurre
// Given that, we always run `helm dep build` on the chart here, but tolerate any error caused by it
// for a remote chart, so that the user can notice/fix the issue in a local chart while
// a broken remote chart won't completely block their job.
chartPath = normalizeChart(st.basePath, chartPath)
chartPath = normalizedChart
buildDeps = !skipDeps
} else if !opts.ForceDownload {
@@ -1929,10 +1930,12 @@ func (st *HelmState) UpdateDeps(helm helmexec.Interface) []error {
var errs []error
for _, release := range st.Releases {
if isLocalChart(release.Chart) {
if err := helm.UpdateDeps(normalizeChart(st.basePath, release.Chart)); err != nil {
if st.directoryExistsAt(release.Chart) {
if err := helm.UpdateDeps(release.Chart); err != nil {
errs = append(errs, err)
}
} else {
st.logger.Debugf("skipped updating dependencies for remote chart %s", release.Chart)
}
}
@@ -1953,11 +1956,6 @@ func (st *HelmState) UpdateDeps(helm helmexec.Interface) []error {
return nil
}
func pathExists(chart string) bool {
_, err := os.Stat(chart)
return err == nil
}
func chartNameWithoutRepository(chart string) string {
chartSplit := strings.Split(chart, "/")
return chartSplit[len(chartSplit)-1]
+42 -23
View File
@@ -22,6 +22,7 @@ func injectFs(st *HelmState, fs *testhelper.TestFs) *HelmState {
st.glob = fs.Glob
st.readFile = fs.ReadFile
st.fileExists = fs.FileExists
st.directoryExistsAt = fs.DirectoryExistsAt
return st
}
@@ -786,15 +787,23 @@ func Test_normalizeChart(t *testing.T) {
{
name: "construct local chart path",
args: args{
basePath: "/Users/jane/code/deploy/charts",
basePath: "/src",
chart: "./app",
},
want: "/Users/jane/code/deploy/charts/app",
want: "/src/app",
},
{
name: "construct local chart path, without leading dot",
args: args{
basePath: "/src",
chart: "published",
},
want: "/src/published",
},
{
name: "repo path",
args: args{
basePath: "/Users/jane/code/deploy/charts",
basePath: "/src",
chart: "remote/app",
},
want: "remote/app",
@@ -802,18 +811,26 @@ func Test_normalizeChart(t *testing.T) {
{
name: "chartcenter repo path",
args: args{
basePath: "/Users/jane/code/deploy/charts",
basePath: "/src",
chart: "center/stable/myapp",
},
want: "center/stable/myapp",
},
{
name: "construct local chart path, parent dir",
name: "construct local chart path, sibling dir",
args: args{
basePath: "/Users/jane/code/deploy/charts",
basePath: "/src",
chart: "../app",
},
want: "/Users/jane/code/deploy/app",
want: "/app",
},
{
name: "construct local chart path, parent dir",
args: args{
basePath: "/src",
chart: "./..",
},
want: "/",
},
{
name: "too much parent levels",
@@ -1752,22 +1769,17 @@ generated: 2019-05-16T15:42:45.50486+09:00
}
logger := helmexec.NewLogger(os.Stderr, "debug")
basePath := "/src"
state := &HelmState{
basePath: "/src",
basePath: basePath,
FilePath: "/src/helmfile.yaml",
ReleaseSetSpec: ReleaseSetSpec{
Releases: []ReleaseSpec{
{
Chart: "./..",
Chart: "/example",
},
{
Chart: "../examples",
},
{
Chart: "../../helmfile",
},
{
Chart: "published",
Chart: "./example",
},
{
Chart: "published/deeper",
@@ -1792,25 +1804,32 @@ generated: 2019-05-16T15:42:45.50486+09:00
logger: logger,
}
fs := testhelper.NewTestFs(map[string]string{
"/example/Chart.yaml": `foo: FOO`,
"/src/example/Chart.yaml": `foo: FOO`,
})
fs.Cwd = basePath
state = injectFs(state, fs)
errs := state.UpdateDeps(helm)
want := []string{"/", "/examples", "/helmfile", "/src/published", generatedDir}
want := []string{"/example", "./example", generatedDir}
if !reflect.DeepEqual(helm.Charts, want) {
t.Errorf("HelmState.UpdateDeps() = %v, want %v", helm.Charts, want)
}
if len(errs) != 0 {
t.Errorf("HelmState.UpdateDeps() - no errors, but got %d: %v", len(errs), errs)
t.Errorf("HelmState.UpdateDeps() - unexpected %d errors: %v", len(errs), errs)
}
resolved, err := state.ResolveDeps()
if err != nil {
t.Errorf("unexpected error: %v", err)
t.Errorf("HelmState.ResolveDeps() - unexpected error: %v", err)
}
if resolved.Releases[5].Version != "1.5.0" {
t.Errorf("unexpected version number: expected=1.5.0, got=%s", resolved.Releases[5].Version)
if resolved.Releases[3].Version != "1.5.0" {
t.Errorf("HelmState.ResolveDeps() - unexpected version number: expected=1.5.0, got=%s", resolved.Releases[5].Version)
}
if resolved.Releases[6].Version != "1.4.0" {
t.Errorf("unexpected version number: expected=1.4.0, got=%s", resolved.Releases[6].Version)
if resolved.Releases[4].Version != "1.4.0" {
t.Errorf("HelmState.ResolveDeps() - unexpected version number: expected=1.4.0, got=%s", resolved.Releases[6].Version)
}
}