Skip to content

Commit

Permalink
feat: options to configure chart upgrade strategy
Browse files Browse the repository at this point in the history
Signed-off-by: Thuan Vo <[email protected]>
  • Loading branch information
tthvo committed May 26, 2024
1 parent 65fa127 commit f2997ca
Show file tree
Hide file tree
Showing 16 changed files with 164 additions and 18 deletions.
3 changes: 3 additions & 0 deletions ct/cmd/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions doc/ct_install.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
```

Expand Down
2 changes: 2 additions & 0 deletions doc/ct_lint-and-install.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion pkg/chart/chart.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
3 changes: 2 additions & 1 deletion pkg/chart/chart_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,14 +235,15 @@ 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",
}
for _, chart := range actual {
assert.Contains(t, expected, chart)
}
assert.Len(t, actual, 8)
assert.Len(t, actual, 9)
assert.Nil(t, err)
}

Expand Down
28 changes: 25 additions & 3 deletions pkg/chart/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}
Expand Down Expand Up @@ -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
Expand All @@ -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")
Expand Down Expand Up @@ -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)
Expand All @@ -165,6 +186,7 @@ func TestUpgradeChart(t *testing.T) {
}
})
}

}

func mustNewChart(chartPath string) *Chart {
Expand Down
5 changes: 5 additions & 0 deletions pkg/chart/test_charts/simple-deployment-v2/Chart.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
apiVersion: v1
appVersion: "2.0"
description: A Helm chart for Kubernetes
name: nginx
version: 0.2.0
4 changes: 4 additions & 0 deletions pkg/chart/test_charts/simple-deployment-v2/README.md
Original file line number Diff line number Diff line change
@@ -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.
32 changes: 32 additions & 0 deletions pkg/chart/test_charts/simple-deployment-v2/templates/_helpers.tpl
Original file line number Diff line number Diff line change
@@ -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 -}}
Original file line number Diff line number Diff line change
@@ -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 }}
15 changes: 15 additions & 0 deletions pkg/chart/test_charts/simple-deployment-v2/values.yaml
Original file line number Diff line number Diff line change
@@ -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: ""
18 changes: 16 additions & 2 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 3 additions & 1 deletion pkg/config/test_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"

}
1 change: 1 addition & 0 deletions pkg/config/test_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,4 @@ exclude-deprecated: true
kubectl-timeout: 120s
skip-clean-up: true
use-helmignore: true
upgrade-strategy: reset-then-reuse-values
29 changes: 19 additions & 10 deletions pkg/tool/helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit f2997ca

Please sign in to comment.