Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport release-1.28] feat(helm): add option to disable helm upgrade force flag #5015

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions docs/helm-charts.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,16 @@ It is possible to customize the timeout by using the `timeout' field.

### Chart configuration

| Field | Default value | Description |
|-----------|---------------|----------------------------------------------------------------------|
| name | - | Release name |
| chartname | - | chartname in form "repository/chartname" or path to tgz file |
| version | - | version to install |
| timeout | - | timeout to wait for release install |
| values | - | yaml as a string, custom chart values |
| namespace | - | namespace to install chart into |
| order | 0 | order to apply manifest. For equal values, alphanum ordering is used |
| Field | Default value | Description |
|--------------|---------------|----------------------------------------------------------------------------------------|
| name | - | Release name |
| chartname | - | chartname in form "repository/chartname" or path to tgz file |
| version | - | version to install |
| timeout | - | timeout to wait for release install |
| values | - | yaml as a string, custom chart values |
| namespace | - | namespace to install chart into |
| forceUpgrade | true | when set to false, disables the use of the "--force" flag when upgrading the the chart |
| order | 0 | order to apply manifest. For equal values, alphanum ordering is used |

## Example

Expand Down
27 changes: 17 additions & 10 deletions inttest/addons/addons_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
"k8s.io/apimachinery/pkg/util/wait"
k8s "k8s.io/client-go/kubernetes"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/utils/pointer"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/yaml"
)
Expand Down Expand Up @@ -259,17 +260,19 @@ func (as *AddonsSuite) doTestAddonUpdate(addonName string, values map[string]int
Name: "testChartUpdate",
Template: chartCrdTemplate,
Data: struct {
Name string
ChartName string
Values string
Version string
TargetNS string
Name string
ChartName string
Values string
Version string
TargetNS string
ForceUpgrade *bool
}{
Name: "test-addon",
ChartName: "ealenn/echo-server",
Values: string(valuesBytes),
Version: "0.5.0",
TargetNS: "default",
Name: "test-addon",
ChartName: "ealenn/echo-server",
Values: string(valuesBytes),
Version: "0.5.0",
TargetNS: "default",
ForceUpgrade: pointer.Bool(false),
},
}
buf := bytes.NewBuffer([]byte{})
Expand Down Expand Up @@ -314,6 +317,7 @@ spec:
version: "0.0.1"
values: ""
namespace: kube-system
forceUpgrade: false
`

// TODO: this actually duplicates logic from the controller code
Expand All @@ -333,4 +337,7 @@ spec:
{{ .Values | nindent 4 }}
version: {{ .Version }}
namespace: {{ .TargetNS }}
{{- if ne .ForceUpgrade nil }}
forceUpgrade: {{ .ForceUpgrade }}
{{- end }}
`
13 changes: 12 additions & 1 deletion pkg/apis/helm/v1beta1/chart_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@ type ChartSpec struct {
Version string `json:"version,omitempty"`
Namespace string `json:"namespace,omitempty"`
Timeout string `json:"timeout,omitempty"`
Order int `json:"order,omitempty"`
// ForceUpgrade when set to false, disables the use of the "--force" flag when upgrading the the chart (default: true).
// +kubebuilder:default=true
// +optional
ForceUpgrade *bool `json:"forceUpgrade,omitempty"`
Order int `json:"order,omitempty"`
}

// YamlValues returns values as map
Expand All @@ -54,6 +58,13 @@ func (cs ChartSpec) HashValues() string {
return fmt.Sprintf("%x", h.Sum(nil))
}

// ShouldForceUpgrade returns true if the chart should be force upgraded
func (cs ChartSpec) ShouldForceUpgrade() bool {
// This defaults to true when not explicitly set to false.
// Better have this the other way round in the next API version.
return cs.ForceUpgrade == nil || *cs.ForceUpgrade
}

// ChartStatus defines the observed state of Chart
type ChartStatus struct {
ReleaseName string `json:"releaseName,omitempty"`
Expand Down
7 changes: 6 additions & 1 deletion pkg/apis/helm/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion pkg/apis/k0s/v1beta1/extensions.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,11 @@ type Chart struct {
Values string `json:"values"`
TargetNS string `json:"namespace"`
Timeout time.Duration `json:"timeout"`
Order int `json:"order"`
// ForceUpgrade when set to false, disables the use of the "--force" flag when upgrading the the chart (default: true).
// +kubebuilder:default=true
// +optional
ForceUpgrade *bool `json:"forceUpgrade,omitempty"`
Order int `json:"order"`
}

// Validate performs validation
Expand Down
13 changes: 11 additions & 2 deletions pkg/apis/k0s/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

40 changes: 26 additions & 14 deletions pkg/component/controller/extensions_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,24 +177,13 @@ func (ec *ExtensionsController) reconcileHelmExtensions(helmSpec *k0sv1beta1.Hel
fileName := chartManifestFileName(&chart)
fileNamesToKeep = append(fileNamesToKeep, fileName)

tw := templatewriter.TemplateWriter{
Path: filepath.Join(ec.manifestsDir, fileName),
Name: "addon_crd_manifest",
Template: chartCrdTemplate,
Data: struct {
k0sv1beta1.Chart
Finalizer string
}{
Chart: chart,
Finalizer: finalizerName,
},
}
if err := tw.Write(); err != nil {
path, err := ec.writeChartManifestFile(chart, fileName)
if err != nil {
errs = append(errs, fmt.Errorf("can't write file for Helm chart manifest %q: %w", chart.ChartName, err))
continue
}

ec.L.Infof("Wrote Helm chart manifest file %q", tw.Path)
ec.L.Infof("Wrote Helm chart manifest file %q", path)
}

if err := filepath.WalkDir(ec.manifestsDir, func(path string, entry fs.DirEntry, err error) error {
Expand Down Expand Up @@ -223,6 +212,25 @@ func (ec *ExtensionsController) reconcileHelmExtensions(helmSpec *k0sv1beta1.Hel
return errors.Join(errs...)
}

func (ec *ExtensionsController) writeChartManifestFile(chart k0sv1beta1.Chart, fileName string) (string, error) {
tw := templatewriter.TemplateWriter{
Path: filepath.Join(ec.manifestsDir, fileName),
Name: "addon_crd_manifest",
Template: chartCrdTemplate,
Data: struct {
k0sv1beta1.Chart
Finalizer string
}{
Chart: chart,
Finalizer: finalizerName,
},
}
if err := tw.Write(); err != nil {
return "", err
}
return tw.Path, nil
}

// Determines the file name to use when storing a chart as a manifest on disk.
func chartManifestFileName(c *k0sv1beta1.Chart) string {
return fmt.Sprintf("%d_helm_extension_%s.yaml", c.Order, c.Name)
Expand Down Expand Up @@ -333,6 +341,7 @@ func (cr *ChartReconciler) updateOrInstallChart(ctx context.Context, chart helmv
chart.Status.Namespace,
chart.Spec.YamlValues(),
timeout,
chart.Spec.ShouldForceUpgrade(),
)
if err != nil {
return fmt.Errorf("can't reconcile upgrade for %q: %w", chart.GetName(), err)
Expand Down Expand Up @@ -405,6 +414,9 @@ spec:
{{ .Values | nindent 4 }}
version: {{ .Version }}
namespace: {{ .TargetNS }}
{{- if ne .ForceUpgrade nil }}
forceUpgrade: {{ .ForceUpgrade }}
{{- end }}
`

const finalizerName = "helm.k0sproject.io/uninstall-helm-release"
Expand Down
94 changes: 94 additions & 0 deletions pkg/component/controller/extensions_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,16 @@ limitations under the License.
package controller

import (
"os"
"strings"
"testing"
"time"

"github.com/k0sproject/k0s/pkg/apis/helm/v1beta1"
k0sv1beta1 "github.com/k0sproject/k0s/pkg/apis/k0s/v1beta1"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"k8s.io/utils/pointer"
)

func TestChartNeedsUpgrade(t *testing.T) {
Expand Down Expand Up @@ -162,3 +167,92 @@ func TestChartManifestFileName(t *testing.T) {
assert.Equal(t, chartManifestFileName(&chart2), "2_helm_extension_release.yaml")
assert.True(t, isChartManifestFileName("0_helm_extension_release.yaml"))
}

func TestExtensionsController_writeChartManifestFile(t *testing.T) {
type args struct {
chart k0sv1beta1.Chart
fileName string
}
tests := []struct {
name string
args args
want string
}{
{
name: "forceUpgrade is nil should omit from manifest",
args: args{
chart: k0sv1beta1.Chart{
Name: "release",
ChartName: "k0s/chart",
Version: "0.0.1",
Values: "values",
TargetNS: "default",
Timeout: 5 * time.Minute,
},
fileName: "0_helm_extension_release.yaml",
},
want: `apiVersion: helm.k0sproject.io/v1beta1
kind: Chart
metadata:
name: k0s-addon-chart-release
namespace: "kube-system"
finalizers:
- helm.k0sproject.io/uninstall-helm-release
spec:
chartName: k0s/chart
releaseName: release
timeout: 5m0s
values: |

values
version: 0.0.1
namespace: default
`,
},
{
name: "forceUpgrade is false should be included in manifest",
args: args{
chart: k0sv1beta1.Chart{
Name: "release",
ChartName: "k0s/chart",
Version: "0.0.1",
Values: "values",
TargetNS: "default",
Timeout: 5 * time.Minute,
ForceUpgrade: pointer.Bool(false),
},
fileName: "0_helm_extension_release.yaml",
},
want: `apiVersion: helm.k0sproject.io/v1beta1
kind: Chart
metadata:
name: k0s-addon-chart-release
namespace: "kube-system"
finalizers:
- helm.k0sproject.io/uninstall-helm-release
spec:
chartName: k0s/chart
releaseName: release
timeout: 5m0s
values: |

values
version: 0.0.1
namespace: default
forceUpgrade: false
`,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ec := &ExtensionsController{
manifestsDir: t.TempDir(),
}
path, err := ec.writeChartManifestFile(tt.args.chart, tt.args.fileName)
require.NoError(t, err)
contents, err := os.ReadFile(path)
require.NoError(t, err)
assert.Equal(t, strings.TrimSpace(tt.want), strings.TrimSpace(string(contents)))
})
}
}
4 changes: 2 additions & 2 deletions pkg/helm/helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ func (hc *Commands) InstallChart(ctx context.Context, chartName string, version

// UpgradeChart upgrades a helm chart.
// InstallChart, UpgradeChart and UninstallRelease(releaseName are *NOT* thread-safe
func (hc *Commands) UpgradeChart(ctx context.Context, chartName string, version string, releaseName string, namespace string, values map[string]interface{}, timeout time.Duration) (*release.Release, error) {
func (hc *Commands) UpgradeChart(ctx context.Context, chartName string, version string, releaseName string, namespace string, values map[string]interface{}, timeout time.Duration, force bool) (*release.Release, error) {
cfg, err := hc.getActionCfg(namespace)
if err != nil {
return nil, fmt.Errorf("can't create action configuration: %v", err)
Expand All @@ -280,7 +280,7 @@ func (hc *Commands) UpgradeChart(ctx context.Context, chartName string, version
upgrade.Wait = true
upgrade.WaitForJobs = true
upgrade.Install = true
upgrade.Force = true
upgrade.Force = force
upgrade.Atomic = true
upgrade.Timeout = timeout
chartDir, err := hc.locateChart(chartName, version)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ spec:
properties:
chartName:
type: string
forceUpgrade:
default: true
description: 'ForceUpgrade when set to false, disables the use of
the "--force" flag when upgrading the the chart (default: true).'
type: boolean
namespace:
type: string
order:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,12 @@ spec:
properties:
chartname:
type: string
forceUpgrade:
default: true
description: 'ForceUpgrade when set to false, disables
the use of the "--force" flag when upgrading the the
chart (default: true).'
type: boolean
name:
type: string
namespace:
Expand Down
Loading