From 8d450bbf7dada573ec740c554284e77b16439a66 Mon Sep 17 00:00:00 2001 From: Colin Hoglund Date: Wed, 19 May 2021 13:16:43 -0400 Subject: [PATCH 1/4] DEVOPS-2496 add max history setting (#12) * add history_max setting * update docs * use suite for env test --- docs/parameter_reference.md | 1 + internal/env/config.go | 8 +++ internal/env/config_test.go | 9 +++ internal/env/testing.go | 15 +++++ internal/run/upgrade.go | 6 ++ internal/run/upgrade_test.go | 114 ++++++++++++++++++----------------- 6 files changed, 98 insertions(+), 55 deletions(-) create mode 100644 internal/env/testing.go diff --git a/docs/parameter_reference.md b/docs/parameter_reference.md index 80fd14d..c7a9c1d 100644 --- a/docs/parameter_reference.md +++ b/docs/parameter_reference.md @@ -44,6 +44,7 @@ Installations are triggered when the `mode` setting is "upgrade." They can also | force_upgrade | boolean | | force | Pass `--force` to `helm upgrade`. | | atomic_upgrade | boolean | | | Pass `--atomic` to `helm upgrade`. | | cleanup_failed_upgrade | boolean | | | Pass `--cleanup-on-fail` to `helm upgrade`. | +| history_max | int | | | Pass `--history-max` to `helm upgrade`. | | values | list\ | | | Chart values to use as the `--set` argument to `helm upgrade`. | | string_values | list\ | | | Chart values to use as the `--set-string` argument to `helm upgrade`. | | values_files | list\ | | | Values to use as `--values` arguments to `helm upgrade`. | diff --git a/internal/env/config.go b/internal/env/config.go index d80c697..635377d 100644 --- a/internal/env/config.go +++ b/internal/env/config.go @@ -10,6 +10,10 @@ import ( "github.com/kelseyhightower/envconfig" ) +const ( + DefaultHistoryMax = 10 +) + var ( justNumbers = regexp.MustCompile(`^\d+$`) deprecatedVars = []string{"PURGE", "RECREATE_PODS", "TILLER_NS", "UPGRADE", "CANARY_IMAGE", "CLIENT_ONLY", "STABLE_REPO_URL"} @@ -45,6 +49,7 @@ type Config struct { Wait bool `envconfig:"wait_for_upgrade"` // Pass --wait to applicable helm commands ReuseValues bool `split_words:"true"` // Pass --reuse-values to `helm upgrade` KeepHistory bool `split_words:"true"` // Pass --keep-history to `helm uninstall` + HistoryMax int `split_words:"true"` // Pass --history-max option Timeout string `` // Argument to pass to --timeout in applicable helm commands Chart string `` // Chart argument to use in applicable helm commands Release string `` // Release argument to use in applicable helm commands @@ -79,6 +84,9 @@ func NewConfig(stdout, stderr io.Writer) (*Config, error) { KubeToken: aliases.KubeToken, Certificate: aliases.Certificate, + // set to same default as helm CLI + HistoryMax: DefaultHistoryMax, + Stdout: stdout, Stderr: stderr, } diff --git a/internal/env/config_test.go b/internal/env/config_test.go index 10f87ff..6eae629 100644 --- a/internal/env/config_test.go +++ b/internal/env/config_test.go @@ -217,6 +217,15 @@ func (suite *ConfigTestSuite) TestValuesSecretsWithDebugLogging() { suite.Contains(stderr.String(), `$SECRET_WATER not present in environment, replaced with ""`) } +func (suite *ConfigTestSuite) TestHistoryMax() { + conf := NewTestConfig(suite.T()) + suite.Assert().Equal(10, conf.HistoryMax) + + suite.setenv("PLUGIN_HISTORY_MAX", "0") + conf = NewTestConfig(suite.T()) + suite.Assert().Equal(0, conf.HistoryMax) +} + func (suite *ConfigTestSuite) setenv(key, val string) { orig, ok := os.LookupEnv(key) if ok { diff --git a/internal/env/testing.go b/internal/env/testing.go new file mode 100644 index 0000000..743f528 --- /dev/null +++ b/internal/env/testing.go @@ -0,0 +1,15 @@ +package env + +import ( + "os" + "testing" + + "github.com/stretchr/testify/require" +) + +func NewTestConfig(t *testing.T) *Config { + conf, err := NewConfig(os.Stdout, os.Stderr) + require.NoError(t, err) + + return conf +} diff --git a/internal/run/upgrade.go b/internal/run/upgrade.go index 718d173..1c07d2f 100644 --- a/internal/run/upgrade.go +++ b/internal/run/upgrade.go @@ -2,6 +2,7 @@ package run import ( "fmt" + "github.com/pelotech/drone-helm3/internal/env" ) @@ -22,6 +23,7 @@ type Upgrade struct { force bool atomic bool cleanupOnFail bool + historyMax int certs *repoCerts createNamespace bool skipCrds bool @@ -46,6 +48,7 @@ func NewUpgrade(cfg env.Config) *Upgrade { force: cfg.Force, atomic: cfg.AtomicUpgrade, cleanupOnFail: cfg.CleanupOnFail, + historyMax: cfg.HistoryMax, certs: newRepoCerts(cfg), createNamespace: cfg.CreateNamespace, skipCrds: cfg.SkipCrds, @@ -110,6 +113,9 @@ func (u *Upgrade) Prepare() error { } args = append(args, u.certs.flags()...) + // always set --history-max since it defaults to non-zero value + args = append(args, fmt.Sprintf("--history-max=%d", u.historyMax)) + args = append(args, u.release, u.chart) u.cmd = command(helmBin, args...) u.cmd.Stdout(u.stdout) diff --git a/internal/run/upgrade_test.go b/internal/run/upgrade_test.go index 0d60d2c..e7240a5 100644 --- a/internal/run/upgrade_test.go +++ b/internal/run/upgrade_test.go @@ -33,23 +33,23 @@ func TestUpgradeTestSuite(t *testing.T) { } func (suite *UpgradeTestSuite) TestNewUpgrade() { - cfg := env.Config{ - ChartVersion: "seventeen", - DryRun: true, - Wait: true, - Values: "steadfastness,forthrightness", - StringValues: "tensile_strength,flexibility", - ValuesFiles: []string{"/root/price_inventory.yml"}, - ReuseValues: true, - Timeout: "go sit in the corner", - Chart: "billboard_top_100", - Release: "post_malone_circles", - Force: true, - AtomicUpgrade: true, - CleanupOnFail: true, - } + cfg := env.NewTestConfig(suite.T()) + cfg.ChartVersion = "seventeen" + cfg.DryRun = true + cfg.Wait = true + cfg.Values = "steadfastness,forthrightness" + cfg.StringValues = "tensile_strength,flexibility" + cfg.ValuesFiles = []string{"/root/price_inventory.yml"} + cfg.ReuseValues = true + cfg.Timeout = "go sit in the corner" + cfg.Chart = "billboard_top_100" + cfg.Release = "post_malone_circles" + cfg.Force = true + cfg.AtomicUpgrade = true + cfg.CleanupOnFail = true + + up := NewUpgrade(*cfg) - up := NewUpgrade(cfg) suite.Equal(cfg.Chart, up.chart) suite.Equal(cfg.Release, up.release) suite.Equal(cfg.ChartVersion, up.chartVersion) @@ -70,15 +70,15 @@ func (suite *UpgradeTestSuite) TestNewUpgrade() { func (suite *UpgradeTestSuite) TestPrepareAndExecute() { defer suite.ctrl.Finish() - cfg := env.Config{ - Chart: "at40", - Release: "jonas_brothers_only_human", - } - u := NewUpgrade(cfg) + cfg := env.NewTestConfig(suite.T()) + cfg.Chart = "at40" + cfg.Release = "jonas_brothers_only_human" + + u := NewUpgrade(*cfg) command = func(path string, args ...string) cmd { suite.Equal(helmBin, path) - suite.Equal([]string{"upgrade", "--install", "jonas_brothers_only_human", "at40"}, args) + suite.Equal([]string{"upgrade", "--install", "--history-max=10", "jonas_brothers_only_human", "at40"}, args) return suite.mockCmd } @@ -99,16 +99,16 @@ func (suite *UpgradeTestSuite) TestPrepareAndExecute() { func (suite *UpgradeTestSuite) TestPrepareNamespaceFlag() { defer suite.ctrl.Finish() - cfg := env.Config{ - Namespace: "melt", - Chart: "at40", - Release: "shaed_trampoline", - } - u := NewUpgrade(cfg) + cfg := env.NewTestConfig(suite.T()) + cfg.Namespace = "melt" + cfg.Chart = "at40" + cfg.Release = "shaed_trampoline" + + u := NewUpgrade(*cfg) command = func(path string, args ...string) cmd { suite.Equal(helmBin, path) - suite.Equal([]string{"--namespace", "melt", "upgrade", "--install", "shaed_trampoline", "at40"}, args) + suite.Equal([]string{"--namespace", "melt", "upgrade", "--install", "--history-max=10", "shaed_trampoline", "at40"}, args) return suite.mockCmd } @@ -123,22 +123,22 @@ func (suite *UpgradeTestSuite) TestPrepareNamespaceFlag() { func (suite *UpgradeTestSuite) TestPrepareWithUpgradeFlags() { defer suite.ctrl.Finish() - cfg := env.Config{ - Chart: "hot_ac", - Release: "maroon_5_memories", - ChartVersion: "radio_edit", - DryRun: true, - Wait: true, - Values: "age=35", - StringValues: "height=5ft10in", - ValuesFiles: []string{"/usr/local/stats", "/usr/local/grades"}, - ReuseValues: true, - Timeout: "sit_in_the_corner", - Force: true, - AtomicUpgrade: true, - CleanupOnFail: true, - } - u := NewUpgrade(cfg) + cfg := env.NewTestConfig(suite.T()) + cfg.Chart = "hot_ac" + cfg.Release = "maroon_5_memories" + cfg.ChartVersion = "radio_edit" + cfg.DryRun = true + cfg.Wait = true + cfg.Values = "age=35" + cfg.StringValues = "height=5ft10in" + cfg.ValuesFiles = []string{"/usr/local/stats", "/usr/local/grades"} + cfg.ReuseValues = true + cfg.Timeout = "sit_in_the_corner" + cfg.Force = true + cfg.AtomicUpgrade = true + cfg.CleanupOnFail = true + + u := NewUpgrade(*cfg) // inject a ca cert filename so repoCerts won't create any files that we'd have to clean up u.certs.caCertFilename = "local_ca.cert" @@ -158,6 +158,7 @@ func (suite *UpgradeTestSuite) TestPrepareWithUpgradeFlags() { "--values", "/usr/local/stats", "--values", "/usr/local/grades", "--ca-file", "local_ca.cert", + "--history-max=10", "maroon_5_memories", "hot_ac"}, args) return suite.mockCmd @@ -191,14 +192,15 @@ func (suite *UpgradeTestSuite) TestRequiresChartAndRelease() { func (suite *UpgradeTestSuite) TestPrepareDebugFlag() { stdout := strings.Builder{} stderr := strings.Builder{} - cfg := env.Config{ - Chart: "at40", - Release: "lewis_capaldi_someone_you_loved", - Debug: true, - Stdout: &stdout, - Stderr: &stderr, - } - u := NewUpgrade(cfg) + + cfg := env.NewTestConfig(suite.T()) + cfg.Chart = "at40" + cfg.Release = "lewis_capaldi_someone_you_loved" + cfg.Debug = true + cfg.Stdout = &stdout + cfg.Stderr = &stderr + + u := NewUpgrade(*cfg) command = func(path string, args ...string) cmd { suite.mockCmd.EXPECT(). @@ -215,8 +217,10 @@ func (suite *UpgradeTestSuite) TestPrepareDebugFlag() { u.Prepare() - want := fmt.Sprintf("Generated command: '%s --debug upgrade "+ - "--install lewis_capaldi_someone_you_loved at40'\n", helmBin) + want := fmt.Sprintf( + "Generated command: '%s --debug upgrade --install --history-max=10 lewis_capaldi_someone_you_loved at40'\n", + helmBin, + ) suite.Equal(want, stderr.String()) suite.Equal("", stdout.String()) } From eeccfdd143aebeb28bc720692798cdc42a268b4f Mon Sep 17 00:00:00 2001 From: Colin Hoglund Date: Tue, 16 Aug 2022 11:42:02 -0700 Subject: [PATCH 2/4] fix test --- internal/run/upgrade_test.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/internal/run/upgrade_test.go b/internal/run/upgrade_test.go index e7240a5..3313eec 100644 --- a/internal/run/upgrade_test.go +++ b/internal/run/upgrade_test.go @@ -2,11 +2,12 @@ package run import ( "fmt" + "strings" + "testing" + "github.com/golang/mock/gomock" "github.com/pelotech/drone-helm3/internal/env" "github.com/stretchr/testify/suite" - "strings" - "testing" ) type UpgradeTestSuite struct { @@ -228,16 +229,16 @@ func (suite *UpgradeTestSuite) TestPrepareDebugFlag() { func (suite *UpgradeTestSuite) TestPrepareSkipCrdsFlag() { defer suite.ctrl.Finish() - cfg := env.Config{ - Chart: "at40", - Release: "cabbages_smell_great", - SkipCrds: true, - } - u := NewUpgrade(cfg) + cfg := env.NewTestConfig(suite.T()) + cfg.Chart = "at40" + cfg.Release = "cabbages_smell_great" + cfg.SkipCrds = true + + u := NewUpgrade(*cfg) command = func(path string, args ...string) cmd { suite.Equal(helmBin, path) - suite.Equal([]string{"upgrade", "--install", "--skip-crds", "cabbages_smell_great", "at40"}, args) + suite.Equal([]string{"upgrade", "--install", "--skip-crds", "--history-max=10", "cabbages_smell_great", "at40"}, args) return suite.mockCmd } From c5a7551da83942562672f35cb7f5b225b27bb515 Mon Sep 17 00:00:00 2001 From: Colin Hoglund Date: Tue, 16 Aug 2022 11:43:37 -0700 Subject: [PATCH 3/4] unexport --- internal/env/config.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/env/config.go b/internal/env/config.go index 635377d..0317eb8 100644 --- a/internal/env/config.go +++ b/internal/env/config.go @@ -11,7 +11,7 @@ import ( ) const ( - DefaultHistoryMax = 10 + defaultHistoryMax = 10 ) var ( @@ -85,7 +85,7 @@ func NewConfig(stdout, stderr io.Writer) (*Config, error) { Certificate: aliases.Certificate, // set to same default as helm CLI - HistoryMax: DefaultHistoryMax, + HistoryMax: defaultHistoryMax, Stdout: stdout, Stderr: stderr, From 64045fadfbb79e664520a8f927abf4119b7136b6 Mon Sep 17 00:00:00 2001 From: Colin Hoglund Date: Tue, 16 Aug 2022 11:44:57 -0700 Subject: [PATCH 4/4] add comment --- internal/env/testing.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/env/testing.go b/internal/env/testing.go index 743f528..99bca38 100644 --- a/internal/env/testing.go +++ b/internal/env/testing.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/require" ) +// NewTestConfig is a helper for setting up a Config object and error checking func NewTestConfig(t *testing.T) *Config { conf, err := NewConfig(os.Stdout, os.Stderr) require.NoError(t, err)