diff --git a/ct/cmd/install.go b/ct/cmd/install.go index 734ea079..c5c78195 100644 --- a/ct/cmd/install.go +++ b/ct/cmd/install.go @@ -79,6 +79,9 @@ func addInstallFlags(flags *flag.FlagSet) { (e.g. "--set=name=value"`)) flags.Bool("skip-clean-up", false, heredoc.Doc(` Skip resources clean-up. Used if need to continue other flows or keep it around.`)) + flags.String("upgrade-strategy", "reuse-values", heredoc.Doc(` + When --upgrade is used, this flag configures the upgrade strategy for chart values. + Available options: reset-values, reuse-values, reset-then-reuse-values`)) } func install(cmd *cobra.Command, _ []string) error { diff --git a/doc/ct_install.md b/doc/ct_install.md index fe1e72d3..fda2881c 100644 --- a/doc/ct_install.md +++ b/doc/ct_install.md @@ -77,6 +77,8 @@ ct install [flags] --target-branch string The name of the target branch used to identify changed charts (default "main") --upgrade Whether to test an in-place upgrade of each chart from its previous revision if the current version should not introduce a breaking change according to the SemVer spec + --upgrade-strategy string When --upgrade is used, this flag configures the upgrade strategy for chart values. + Available options: reset-values, reuse-values, reset-then-reuse-values (default "reuse-values") --use-helmignore Use .helmignore when identifying changed charts ``` diff --git a/doc/ct_lint-and-install.md b/doc/ct_lint-and-install.md index 215a70fe..ee6a0981 100644 --- a/doc/ct_lint-and-install.md +++ b/doc/ct_lint-and-install.md @@ -72,6 +72,8 @@ ct lint-and-install [flags] --target-branch string The name of the target branch used to identify changed charts (default "main") --upgrade Whether to test an in-place upgrade of each chart from its previous revision if the current version should not introduce a breaking change according to the SemVer spec + --upgrade-strategy string When --upgrade is used, this flag configures the upgrade strategy for chart values. + Available options: reset-values, reuse-values, reset-then-reuse-values (default "reuse-values") --use-helmignore Use .helmignore when identifying changed charts --validate-chart-schema Enable schema validation of 'Chart.yaml' using Yamale (default true) --validate-maintainers Enable validation of maintainer account names in chart.yml. diff --git a/pkg/chart/chart.go b/pkg/chart/chart.go index 4790b825..78fdf487 100644 --- a/pkg/chart/chart.go +++ b/pkg/chart/chart.go @@ -267,7 +267,7 @@ func NewTesting(config config.Configuration, extraSetArgs string) (Testing, erro testing := Testing{ config: config, - helm: tool.NewHelm(procExec, helmExtraArgs, helmLintExtraArgs, strings.Fields(extraSetArgs)), + helm: tool.NewHelm(procExec, helmExtraArgs, helmLintExtraArgs, strings.Fields(extraSetArgs), config.UpgradeStrategy), git: tool.NewGit(procExec), kubectl: tool.NewKubectl(procExec, config.KubectlTimeout), linter: tool.NewLinter(procExec), diff --git a/pkg/chart/chart_test.go b/pkg/chart/chart_test.go index 678bb801..0072c588 100644 --- a/pkg/chart/chart_test.go +++ b/pkg/chart/chart_test.go @@ -235,6 +235,7 @@ func TestReadAllChartDirectories(t *testing.T) { "test_charts/must-pass-upgrade-install", "test_charts/mutating-deployment-selector", "test_charts/simple-deployment", + "test_charts/simple-deployment-v2", "test_charts/simple-deployment-different-selector", "test_charts/mutating-sfs-volumeclaim", "test_chart_at_root", @@ -242,7 +243,7 @@ func TestReadAllChartDirectories(t *testing.T) { for _, chart := range actual { assert.Contains(t, expected, chart) } - assert.Len(t, actual, 8) + assert.Len(t, actual, 9) assert.Nil(t, err) } diff --git a/pkg/chart/integration_test.go b/pkg/chart/integration_test.go index c6b5915a..3b0daf9a 100644 --- a/pkg/chart/integration_test.go +++ b/pkg/chart/integration_test.go @@ -43,7 +43,7 @@ func newTestingHelmIntegration(cfg config.Configuration, extraSetArgs string) Te utils: util.Utils{}, accountValidator: fakeAccountValidator{}, linter: fakeMockLinter, - helm: tool.NewHelm(procExec, extraArgs, extraLintArgs, strings.Fields(extraSetArgs)), + helm: tool.NewHelm(procExec, extraArgs, extraLintArgs, strings.Fields(extraSetArgs), cfg.UpgradeStrategy), kubectl: tool.NewKubectl(procExec, 30*time.Second), } } @@ -111,6 +111,12 @@ func TestInstallChart(t *testing.T) { } func TestUpgradeChart(t *testing.T) { + runUpgradeChartTest(t, tool.ResetValues) + runUpgradeChartTest(t, tool.ReuseValues) + runUpgradeChartTest(t, tool.ResetThenReuseValues) +} + +func runUpgradeChartTest(t *testing.T, upgradeStrategy string) { type testCase struct { name string old string @@ -119,8 +125,9 @@ func TestUpgradeChart(t *testing.T) { } cfg := config.Configuration{ - Debug: true, - Upgrade: true, + Debug: true, + Upgrade: true, + UpgradeStrategy: upgradeStrategy, } ct := newTestingHelmIntegration(cfg, "") processError := fmt.Errorf("failed waiting for process: exit status 1") @@ -152,6 +159,20 @@ func TestUpgradeChart(t *testing.T) { }, } + v2UpgradeCase := testCase{ + "upgrade nginx to v2 with breaking changes", + "test_charts/simple-deployment", + "test_charts/simple-deployment-v2", + nil, + } + // Expect chart upgrade with breaking changes to fail + // if values are reused + if upgradeStrategy == tool.ReuseValues { + v2UpgradeCase.err = processError + } + + cases = append(cases, v2UpgradeCase) + for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { err := ct.doUpgrade(mustNewChart(tc.old), mustNewChart(tc.new), true) @@ -165,6 +186,7 @@ func TestUpgradeChart(t *testing.T) { } }) } + } func mustNewChart(chartPath string) *Chart { diff --git a/pkg/chart/test_charts/simple-deployment-v2/Chart.yaml b/pkg/chart/test_charts/simple-deployment-v2/Chart.yaml new file mode 100644 index 00000000..8adc6e1b --- /dev/null +++ b/pkg/chart/test_charts/simple-deployment-v2/Chart.yaml @@ -0,0 +1,5 @@ +apiVersion: v1 +appVersion: "2.0" +description: A Helm chart for Kubernetes +name: nginx +version: 0.2.0 diff --git a/pkg/chart/test_charts/simple-deployment-v2/README.md b/pkg/chart/test_charts/simple-deployment-v2/README.md new file mode 100644 index 00000000..4fc7c826 --- /dev/null +++ b/pkg/chart/test_charts/simple-deployment-v2/README.md @@ -0,0 +1,4 @@ +Simple chart with a Deployment (v2) with breaking changes to values file. + +The integration test will install first simple-deployment and then try to upgrade +to simple-deployment-v2, which is expected to pass. diff --git a/pkg/chart/test_charts/simple-deployment-v2/templates/_helpers.tpl b/pkg/chart/test_charts/simple-deployment-v2/templates/_helpers.tpl new file mode 100644 index 00000000..165d01e7 --- /dev/null +++ b/pkg/chart/test_charts/simple-deployment-v2/templates/_helpers.tpl @@ -0,0 +1,32 @@ +{{/* vim: set filetype=mustache: */}} +{{/* +Expand the name of the chart. +*/}} +{{- define "nginx.name" -}} +{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" -}} +{{- end -}} + +{{/* +Create a default fully qualified app name. +We truncate at 63 chars because some Kubernetes name fields are limited to this (by the DNS naming spec). +If release name contains chart name it will be used as a full name. +*/}} +{{- define "nginx.fullname" -}} +{{- if .Values.fullnameOverride -}} +{{- .Values.fullnameOverride | trunc 63 | trimSuffix "-" -}} +{{- else -}} +{{- $name := default .Chart.Name .Values.nameOverride -}} +{{- if contains $name .Release.Name -}} +{{- .Release.Name | trunc 63 | trimSuffix "-" -}} +{{- else -}} +{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" -}} +{{- end -}} +{{- end -}} +{{- end -}} + +{{/* +Create chart name and version as used by the chart label. +*/}} +{{- define "nginx.chart" -}} +{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 | trimSuffix "-" -}} +{{- end -}} diff --git a/pkg/chart/test_charts/simple-deployment-v2/templates/deployment.yaml b/pkg/chart/test_charts/simple-deployment-v2/templates/deployment.yaml new file mode 100644 index 00000000..59609444 --- /dev/null +++ b/pkg/chart/test_charts/simple-deployment-v2/templates/deployment.yaml @@ -0,0 +1,33 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: {{ include "nginx.fullname" . }} + labels: + app.kubernetes.io/name: {{ include "nginx.name" . }} + helm.sh/chart: {{ include "nginx.chart" . }} + app.kubernetes.io/instance: {{ .Release.Name }} + app.kubernetes.io/managed-by: {{ .Release.Service }} +spec: + replicas: 1 + selector: + matchLabels: + app.kubernetes.io/name: {{ include "nginx.name" . }} + app.kubernetes.io/instance: {{ .Release.Name }} + template: + metadata: + labels: + app.kubernetes.io/name: {{ include "nginx.name" . }} + app.kubernetes.io/instance: {{ .Release.Name }} + spec: + securityContext: + {{- toYaml .Values.podSecurityContext | nindent 8 }} + containers: + - name: {{ .Chart.Name }} + image: "{{ .Values.core.image.repository }}:{{ .Values.core.image.tag }}" + imagePullPolicy: {{ .Values.core.image.pullPolicy }} + ports: + - name: http + containerPort: 80 + protocol: TCP + securityContext: + {{- toYaml .Values.core.securityContext | nindent 12 }} diff --git a/pkg/chart/test_charts/simple-deployment-v2/values.yaml b/pkg/chart/test_charts/simple-deployment-v2/values.yaml new file mode 100644 index 00000000..eac66ba3 --- /dev/null +++ b/pkg/chart/test_charts/simple-deployment-v2/values.yaml @@ -0,0 +1,15 @@ +# Default values for nginx. +# This is a YAML-formatted file. +# Declare variables to be passed into your templates. + +core: + image: + repository: nginx + tag: stable + pullPolicy: IfNotPresent + securityContext: {} + +podSecurityContext: {} + +nameOverride: "" +fullnameOverride: "" diff --git a/pkg/config/config.go b/pkg/config/config.go index 989c467b..4a6cfbd3 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -25,6 +25,7 @@ import ( "github.com/mitchellh/go-homedir" + "github.com/helm/chart-testing/v3/pkg/tool" "github.com/helm/chart-testing/v3/pkg/util" "github.com/spf13/cobra" flag "github.com/spf13/pflag" @@ -74,6 +75,7 @@ type Configuration struct { PrintLogs bool `mapstructure:"print-logs"` GithubGroups bool `mapstructure:"github-groups"` UseHelmignore bool `mapstructure:"use-helmignore"` + UpgradeStrategy string `mapstructure:"upgrade-strategy"` } func LoadConfiguration(cfgFile string, cmd *cobra.Command, printConfig bool) (*Configuration, error) { @@ -139,8 +141,20 @@ func LoadConfiguration(cfgFile string, cmd *cobra.Command, printConfig bool) (*C // Disable upgrade (this does some expensive dependency building on previous revisions) // when neither "install" nor "lint-and-install" have not been specified. cfg.Upgrade = isInstall && cfg.Upgrade - if (cfg.TargetBranch == "" || cfg.Remote == "") && cfg.Upgrade { - return nil, errors.New("specifying '--upgrade=true' without '--target-branch' or '--remote', is not allowed") + + if cfg.Upgrade { + if cfg.TargetBranch == "" || cfg.Remote == "" { + return nil, errors.New("specifying '--upgrade=true' without '--target-branch' or '--remote', is not allowed") + } + + // Ensure upgrade strategy is valid + switch cfg.UpgradeStrategy { + case tool.ResetValues: + case tool.ReuseValues: + case tool.ResetThenReuseValues: + default: + return nil, fmt.Errorf("invalid upgrade strategy %s specified", cfg.UpgradeStrategy) + } } chartYamlSchemaPath := cfg.ChartYamlSchema diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index f07d912e..42d6a728 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -62,6 +62,7 @@ func loadAndAssertConfigFromFile(t *testing.T, configFile string) { require.Equal(t, 120*time.Second, cfg.KubectlTimeout) require.Equal(t, true, cfg.SkipCleanUp) require.Equal(t, true, cfg.UseHelmignore) + require.Equal(t, "reset-then-reuse-values", cfg.UpgradeStrategy) } func Test_findConfigFile(t *testing.T) { diff --git a/pkg/config/test_config.json b/pkg/config/test_config.json index 73cd4574..2a20e741 100644 --- a/pkg/config/test_config.json +++ b/pkg/config/test_config.json @@ -33,5 +33,7 @@ "exclude-deprecated": true, "kubectl-timeout": "120s", "skip-clean-up": true, - "use-helmignore": true + "use-helmignore": true, + "upgrade-strategy": "reset-then-reuse-values" + } diff --git a/pkg/config/test_config.yaml b/pkg/config/test_config.yaml index 0d1c7c02..cf413f79 100644 --- a/pkg/config/test_config.yaml +++ b/pkg/config/test_config.yaml @@ -29,3 +29,4 @@ exclude-deprecated: true kubectl-timeout: 120s skip-clean-up: true use-helmignore: true +upgrade-strategy: reset-then-reuse-values diff --git a/pkg/tool/helm.go b/pkg/tool/helm.go index 42724b04..bbeee3ec 100644 --- a/pkg/tool/helm.go +++ b/pkg/tool/helm.go @@ -22,18 +22,26 @@ import ( ) type Helm struct { - exec exec.ProcessExecutor - extraArgs []string - lintExtraArgs []string - extraSetArgs []string + exec exec.ProcessExecutor + extraArgs []string + lintExtraArgs []string + extraSetArgs []string + upgradeStrategy string } -func NewHelm(exec exec.ProcessExecutor, extraArgs, lintExtraArgs, extraSetArgs []string) Helm { +const ( + ResetValues string = "reset-values" + ReuseValues string = "reuse-values" + ResetThenReuseValues string = "reset-then-reuse-values" +) + +func NewHelm(exec exec.ProcessExecutor, extraArgs, lintExtraArgs, extraSetArgs []string, upgradeStrategy string) Helm { return Helm{ - exec: exec, - extraArgs: extraArgs, - lintExtraArgs: lintExtraArgs, - extraSetArgs: extraSetArgs, + exec: exec, + extraArgs: extraArgs, + lintExtraArgs: lintExtraArgs, + extraSetArgs: extraSetArgs, + upgradeStrategy: upgradeStrategy, } } @@ -76,8 +84,9 @@ func (h Helm) InstallWithValues(chart string, valuesFile string, namespace strin } func (h Helm) Upgrade(chart string, namespace string, release string) error { + strategyFlag := fmt.Sprintf("--%s", h.upgradeStrategy) return h.exec.RunProcess("helm", "upgrade", release, chart, "--namespace", namespace, - "--reuse-values", "--wait", h.extraArgs, h.extraSetArgs) + strategyFlag, "--wait", h.extraArgs, h.extraSetArgs) } func (h Helm) Test(namespace string, release string) error {