From 1ed0c862128d7f7df9ed5ccc2ad94379b2e602a1 Mon Sep 17 00:00:00 2001 From: Luis Davim Date: Wed, 18 Nov 2020 10:59:09 +0000 Subject: [PATCH 1/4] fix: no special logic for rename is needed using helm v3 refactor: reduce nesting Signed-off-by: Luis Davim --- internal/app/cli.go | 2 ++ internal/app/decision_maker.go | 54 ++++++++++++++++++---------------- 2 files changed, 30 insertions(+), 26 deletions(-) diff --git a/internal/app/cli.go b/internal/app/cli.go index 2d12902c..ed355508 100644 --- a/internal/app/cli.go +++ b/internal/app/cli.go @@ -60,6 +60,7 @@ type cli struct { substSSMValues bool updateDeps bool forceUpgrades bool + renameReplace bool version bool noCleanup bool migrateContext bool @@ -107,6 +108,7 @@ func (c *cli) parse() { flag.BoolVar(&c.substSSMValues, "subst-ssm-values", false, "turn on SSM parameter substitution in values files.") flag.BoolVar(&c.updateDeps, "update-deps", false, "run 'helm dep up' for local chart") flag.BoolVar(&c.forceUpgrades, "force-upgrades", false, "use --force when upgrading helm releases. May cause resources to be recreated.") + flag.BoolVar(&c.renameReplace, "replace-on-rename", false, "Uninstall the existing release when a chart with a different name is used.") flag.BoolVar(&c.noCleanup, "no-cleanup", false, "keeps any credentials files that has been downloaded on the host where helmsman runs.") flag.BoolVar(&c.migrateContext, "migrate-context", false, "updates the context name for all apps defined in the DSF and applies Helmsman labels. Using this flag is required if you want to change context name after it has been set.") flag.BoolVar(&c.alwaysUpgrade, "always-upgrade", false, "upgrade release even if no changes are found") diff --git a/internal/app/decision_maker.go b/internal/app/decision_maker.go index 5e33efd1..d42c0060 100644 --- a/internal/app/decision_maker.go +++ b/internal/app/decision_maker.go @@ -296,32 +296,8 @@ func (cs *currentState) inspectUpgradeScenario(r *release, p *plan, c *chartInfo return } - if r.Namespace == rs.Namespace { - r.Version = c.Version - - if c.Name == rs.getChartName() && r.Version != rs.getChartVersion() { - // upgrade - r.diff() - r.upgrade(p) - p.addDecision("Release [ "+r.Name+" ] will be upgraded", r.Priority, change) - - } else if c.Name != rs.getChartName() { - r.reInstall(p) - p.addDecision("Release [ "+r.Name+" ] is desired to use a new chart [ "+r.Chart+ - " ]. Delete of the current release will be planned and new chart will be installed in namespace [ "+ - r.Namespace+" ]", r.Priority, change) - } else { - if flags.alwaysUpgrade { - r.upgrade(p) - p.addDecision("Release [ "+r.Name+" ] will be updated (forced)", r.Priority, change) - } else if diff := r.diff(); diff != "" { - r.upgrade(p) - p.addDecision("Release [ "+r.Name+" ] will be updated", r.Priority, change) - } else { - p.addDecision("Release [ "+r.Name+" ] installed and up-to-date", r.Priority, noop) - } - } - } else { + if r.Namespace != rs.Namespace { + // Namespace changed r.reInstall(p, rs.Namespace) p.addDecision("Release [ "+r.Name+" ] is desired to be enabled in a new namespace [ "+r.Namespace+ " ]. Uninstall of the current release from namespace [ "+rs.Namespace+" ] will be performed "+ @@ -329,5 +305,31 @@ func (cs *currentState) inspectUpgradeScenario(r *release, p *plan, c *chartInfo p.addDecision("WARNING: moving release [ "+r.Name+" ] from [ "+rs.Namespace+" ] to [ "+r.Namespace+ " ] might not correctly connect existing volumes. Check https://github.com/Praqma/helmsman/blob/master/docs/how_to/apps/moving_across_namespaces.md#note-on-persistent-volumes"+ " for details if this release uses PV and PVC.", r.Priority, change) + return + } + + r.Version = c.Version + + if c.Name != rs.getChartName() && flags.renameReplace { + // Chart changed + rs.uninstall(p) + r.install(p) + p.addDecision("Release [ "+r.Name+" ] is desired to use a new chart [ "+r.Chart+ + " ]. Delete of the current release will be planned and new chart will be installed in namespace [ "+ + r.Namespace+" ]", r.Priority, change) + return + } + + if flags.alwaysUpgrade || r.Version != rs.getChartVersion() || c.Name != rs.getChartName() { + // Version changed + r.upgrade(p) + p.addDecision("Release [ "+r.Name+" ] will be upgraded", r.Priority, change) + return + } + if diff := r.diff(); diff != "" { + r.upgrade(p) + p.addDecision("Release [ "+r.Name+" ] will be updated", r.Priority, change) + return } + p.addDecision("Release [ "+r.Name+" ] installed and up-to-date", r.Priority, noop) } From 25168601436a360db89a60091052fc6a3f3ffe51 Mon Sep 17 00:00:00 2001 From: Luis Davim Date: Thu, 19 Nov 2020 21:37:22 +0000 Subject: [PATCH 2/4] fix: NPE when checking versions Signed-off-by: Luis Davim --- internal/app/helm_helpers.go | 36 ++++++++++++++++++++----------- internal/app/helm_helpers_test.go | 6 ++++-- 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/internal/app/helm_helpers.go b/internal/app/helm_helpers.go index 13d06a63..0bdd68c4 100644 --- a/internal/app/helm_helpers.go +++ b/internal/app/helm_helpers.go @@ -4,19 +4,19 @@ import ( "encoding/json" "errors" "fmt" - "gopkg.in/yaml.v2" "net/url" "path/filepath" "strings" - "github.com/hashicorp/go-version" + "gopkg.in/yaml.v2" "github.com/Praqma/helmsman/internal/gcs" + "github.com/hashicorp/go-version" ) type helmRepo struct { Name string `json:"name"` - Url string `json:"url"` + URL string `json:"url"` } type chartInfo struct { @@ -46,7 +46,7 @@ func getChartInfo(chartName, chartVersion string) (*chartInfo, error) { maybeRepo := filepath.Base(filepath.Dir(chartName)) message := strings.TrimSpace(result.errors) - return nil, fmt.Errorf("Chart [ %s ] version [ %s ] can't be found. Inspection returned error: \"%s\" -- If this is not a local chart, add the repo [ %s ] in your helmRepos stanza.", chartName, chartVersion, message, maybeRepo) + return nil, fmt.Errorf("chart [ %s ] version [ %s ] can't be found. Inspection returned error: \"%s\" -- If this is not a local chart, add the repo [ %s ] in your helmRepos stanza", chartName, chartVersion, message, maybeRepo) } c := &chartInfo{} @@ -54,11 +54,17 @@ func getChartInfo(chartName, chartVersion string) (*chartInfo, error) { log.Fatal(fmt.Sprint(err)) } - constraint, _ := version.NewConstraint(chartVersion) - found, _ := version.NewVersion(c.Version) + constraint, err := version.NewConstraint(chartVersion) + if err != nil { + return nil, err + } + found, err := version.NewVersion(c.Version) + if err != nil { + return nil, err + } if !constraint.Check(found) { - return nil, fmt.Errorf("Chart [ %s ] with version [ %s ] was found with a mismatched version: %s", chartName, chartVersion, c.Version) + return nil, fmt.Errorf("chart [ %s ] with version [ %s ] was found with a mismatched version: %s", chartName, chartVersion, c.Version) } return c, nil @@ -82,12 +88,16 @@ func checkHelmVersion(constraint string) bool { if !strings.HasPrefix(helmVersion, "v") { extractedHelmVersion = strings.TrimSpace(strings.Split(helmVersion, ":")[1]) } - v, _ := version.NewVersion(extractedHelmVersion) - jsonConstraint, _ := version.NewConstraint(constraint) - if jsonConstraint.Check(v) { - return true + v, err := version.NewVersion(extractedHelmVersion) + if err != nil { + return false + } + + jsonConstraint, err := version.NewConstraint(constraint) + if err != nil { + return false } - return false + return jsonConstraint.Check(v) } // helmPluginExists returns true if the plugin is present in the environment and false otherwise. @@ -129,7 +139,7 @@ func addHelmRepos(repos map[string]string) error { } // create map of existing repositories for _, repo := range helmRepos { - existingRepos[repo.Name] = repo.Url + existingRepos[repo.Name] = repo.URL } } else { if !strings.Contains(reposResult.errors, "no repositories to show") { diff --git a/internal/app/helm_helpers_test.go b/internal/app/helm_helpers_test.go index f9ce826e..e08f03ee 100644 --- a/internal/app/helm_helpers_test.go +++ b/internal/app/helm_helpers_test.go @@ -70,8 +70,10 @@ func Test_getChartInfo(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - t.Log(tt.want) - got, _ := getChartInfo(tt.args.r.Chart, tt.args.r.Version) + got, err := getChartInfo(tt.args.r.Chart, tt.args.r.Version) + if err != nil && tt.want != nil { + t.Errorf("getChartInfo() = Unexpected error: %w", err) + } if !reflect.DeepEqual(got, tt.want) { t.Errorf("getChartInfo() = %v, want %v", got, tt.want) } From 6b0368f309a84771eabc6bca05ac0746e35421b7 Mon Sep 17 00:00:00 2001 From: Luis Davim Date: Thu, 19 Nov 2020 23:28:10 +0000 Subject: [PATCH 3/4] fix version constaint test --- internal/app/helm_helpers_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/app/helm_helpers_test.go b/internal/app/helm_helpers_test.go index e08f03ee..7a9a107d 100644 --- a/internal/app/helm_helpers_test.go +++ b/internal/app/helm_helpers_test.go @@ -34,7 +34,7 @@ func Test_getChartInfo(t *testing.T) { r: &release{ Name: "release1", Namespace: "namespace", - Version: "1.0.*", + Version: "~>1.0", Chart: "./../../tests/chart-test", Enabled: true, }, From 52723c1bce5b387d00ece389ba7d02e2fc619310 Mon Sep 17 00:00:00 2001 From: Luis Davim Date: Thu, 19 Nov 2020 23:36:04 +0000 Subject: [PATCH 4/4] feat: switch to the semver package since it is more flexible --- go.mod | 2 +- go.sum | 4 ++-- internal/app/helm_helpers.go | 10 +++++----- internal/app/helm_helpers_test.go | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/go.mod b/go.mod index daedaed8..29e4dada 100644 --- a/go.mod +++ b/go.mod @@ -7,10 +7,10 @@ require ( github.com/Azure/azure-pipeline-go v0.1.9 github.com/Azure/azure-storage-blob-go v0.0.0-20181022225951-5152f14ace1c github.com/BurntSushi/toml v0.3.1 + github.com/Masterminds/semver v1.5.0 github.com/apsdehal/go-logger v0.0.0-20190515211354-1abdf898e024 github.com/aws/aws-sdk-go v1.26.2 github.com/golang/groupcache v0.0.0-20191027212112-611e8accdfc9 // indirect - github.com/hashicorp/go-version v1.2.0 github.com/imdario/mergo v0.3.8 github.com/joho/godotenv v1.3.0 github.com/kr/pretty v0.1.0 // indirect diff --git a/go.sum b/go.sum index afb32cee..b1a39ca0 100644 --- a/go.sum +++ b/go.sum @@ -9,6 +9,8 @@ github.com/Azure/azure-storage-blob-go v0.0.0-20181022225951-5152f14ace1c h1:Y5u github.com/Azure/azure-storage-blob-go v0.0.0-20181022225951-5152f14ace1c/go.mod h1:oGfmITT1V6x//CswqY2gtAHND+xIP64/qL7a5QJix0Y= github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= +github.com/Masterminds/semver v1.5.0 h1:H65muMkzWKEuNDnfl9d70GUjFniHKHRbFPGBuZ3QEww= +github.com/Masterminds/semver v1.5.0/go.mod h1:MB6lktGJrhw8PrUyiEoblNEGEQ+RzHPF078ddwwvV3Y= github.com/apsdehal/go-logger v0.0.0-20190515211354-1abdf898e024 h1:dfZ6RF0UxHqt7xPz0r7h00apsaa6rIrFhT6Xly55Exk= github.com/apsdehal/go-logger v0.0.0-20190515211354-1abdf898e024/go.mod h1:U3/8D6R9+bVpX0ORZjV+3mU9pQ86m7h1lESgJbXNvXA= github.com/aws/aws-sdk-go v1.26.2 h1:MzYLmCeny4bMQcAbYcucIduVZKp0sEf1eRLvHpKI5Is= @@ -40,8 +42,6 @@ github.com/google/pprof v0.0.0-20181206194817-3ea8567a2e57/go.mod h1:zfwlbNMJ+OI github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+vpHVxEJEs9eg= github.com/googleapis/gax-go/v2 v2.0.5 h1:sjZBwGj9Jlw33ImPtvFviGYvseOtDM7hkSKB7+Tv3SM= github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5mhpdKc/us6bOk= -github.com/hashicorp/go-version v1.2.0 h1:3vNe/fWF5CBgRIguda1meWhsZHy3m8gCJ5wx+dIzX/E= -github.com/hashicorp/go-version v1.2.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= github.com/imdario/mergo v0.3.8 h1:CGgOkSJeqMRmt0D9XLWExdT4m4F1vd3FV3VPt+0VxkQ= diff --git a/internal/app/helm_helpers.go b/internal/app/helm_helpers.go index 0bdd68c4..0202306c 100644 --- a/internal/app/helm_helpers.go +++ b/internal/app/helm_helpers.go @@ -10,8 +10,8 @@ import ( "gopkg.in/yaml.v2" + "github.com/Masterminds/semver" "github.com/Praqma/helmsman/internal/gcs" - "github.com/hashicorp/go-version" ) type helmRepo struct { @@ -54,11 +54,11 @@ func getChartInfo(chartName, chartVersion string) (*chartInfo, error) { log.Fatal(fmt.Sprint(err)) } - constraint, err := version.NewConstraint(chartVersion) + constraint, err := semver.NewConstraint(chartVersion) if err != nil { return nil, err } - found, err := version.NewVersion(c.Version) + found, err := semver.NewVersion(c.Version) if err != nil { return nil, err } @@ -88,12 +88,12 @@ func checkHelmVersion(constraint string) bool { if !strings.HasPrefix(helmVersion, "v") { extractedHelmVersion = strings.TrimSpace(strings.Split(helmVersion, ":")[1]) } - v, err := version.NewVersion(extractedHelmVersion) + v, err := semver.NewVersion(extractedHelmVersion) if err != nil { return false } - jsonConstraint, err := version.NewConstraint(constraint) + jsonConstraint, err := semver.NewConstraint(constraint) if err != nil { return false } diff --git a/internal/app/helm_helpers_test.go b/internal/app/helm_helpers_test.go index 7a9a107d..e08f03ee 100644 --- a/internal/app/helm_helpers_test.go +++ b/internal/app/helm_helpers_test.go @@ -34,7 +34,7 @@ func Test_getChartInfo(t *testing.T) { r: &release{ Name: "release1", Namespace: "namespace", - Version: "~>1.0", + Version: "1.0.*", Chart: "./../../tests/chart-test", Enabled: true, },