From 3564df49fa8919133ab090be7f99de22fc1247fa Mon Sep 17 00:00:00 2001 From: Jared Stehr Rivera Date: Wed, 31 Jan 2024 23:56:21 +0800 Subject: [PATCH 1/3] feat(helm): add --use-source-hooks flag to helm 'rollback' Signed-off-by: Jared Stehr Rivera --- cmd/helm/rollback.go | 1 + cmd/helm/rollback_test.go | 5 +++ .../output/rollback-use-source-hooks.txt | 1 + pkg/action/rollback.go | 32 ++++++++++++------- 4 files changed, 27 insertions(+), 12 deletions(-) create mode 100644 cmd/helm/testdata/output/rollback-use-source-hooks.txt diff --git a/cmd/helm/rollback.go b/cmd/helm/rollback.go index 7de98e404..bb7e3b802 100644 --- a/cmd/helm/rollback.go +++ b/cmd/helm/rollback.go @@ -80,6 +80,7 @@ func newRollbackCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f.BoolVar(&client.Recreate, "recreate-pods", false, "performs pods restart for the resource if applicable") f.BoolVar(&client.Force, "force", false, "force resource update through delete/recreate if needed") f.BoolVar(&client.DisableHooks, "no-hooks", false, "prevent hooks from running during rollback") + f.BoolVar(&client.UseSourceHooks, "use-source-hooks", false, "run hooks from current release during rollback") f.DurationVar(&client.Timeout, "timeout", 300*time.Second, "time to wait for any individual Kubernetes operation (like Jobs for hooks)") f.BoolVar(&client.Wait, "wait", false, "if set, will wait until all Pods, PVCs, Services, and minimum number of Pods of a Deployment, StatefulSet, or ReplicaSet are in a ready state before marking the release as successful. It will wait for as long as --timeout") f.BoolVar(&client.WaitForJobs, "wait-for-jobs", false, "if set and --wait enabled, will wait until all Jobs have been completed before marking the release as successful. It will wait for as long as --timeout") diff --git a/cmd/helm/rollback_test.go b/cmd/helm/rollback_test.go index b58e4c162..d166a0516 100644 --- a/cmd/helm/rollback_test.go +++ b/cmd/helm/rollback_test.go @@ -61,6 +61,11 @@ func TestRollbackCmd(t *testing.T) { cmd: "rollback funny-honey 1 --wait --wait-for-jobs", golden: "output/rollback-wait-for-jobs.txt", rels: rels, + }, { + name: "rollback a release with source hooks", + cmd: "rollback funny-honey 1 --use-source-hooks", + golden: "output/rollback-use-source-hooks.txt", + rels: rels, }, { name: "rollback a release without revision", cmd: "rollback funny-honey", diff --git a/cmd/helm/testdata/output/rollback-use-source-hooks.txt b/cmd/helm/testdata/output/rollback-use-source-hooks.txt new file mode 100644 index 000000000..ae3c6f1c4 --- /dev/null +++ b/cmd/helm/testdata/output/rollback-use-source-hooks.txt @@ -0,0 +1 @@ +Rollback was a success! Happy Helming! diff --git a/pkg/action/rollback.go b/pkg/action/rollback.go index b0be17d13..9a33ec7f0 100644 --- a/pkg/action/rollback.go +++ b/pkg/action/rollback.go @@ -35,16 +35,17 @@ import ( type Rollback struct { cfg *Configuration - Version int - Timeout time.Duration - Wait bool - WaitForJobs bool - DisableHooks bool - DryRun bool - Recreate bool // will (if true) recreate pods after a rollback. - Force bool // will (if true) force resource upgrade through uninstall/recreate if needed - CleanupOnFail bool - MaxHistory int // MaxHistory limits the maximum number of revisions saved per release + Version int + Timeout time.Duration + Wait bool + WaitForJobs bool + DisableHooks bool + UseSourceHooks bool + DryRun bool + Recreate bool // will (if true) recreate pods after a rollback. + Force bool // will (if true) force resource upgrade through uninstall/recreate if needed + CleanupOnFail bool + MaxHistory int // MaxHistory limits the maximum number of revisions saved per release } // NewRollback creates a new Rollback object with the given configuration. @@ -174,9 +175,16 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas return targetRelease, errors.Wrap(err, "unable to build kubernetes objects from new release manifest") } + // release whose hooks will be executed during rollback + releaseForHooks := targetRelease + if r.UseSourceHooks { + r.cfg.Log("Using hooks from current release") + releaseForHooks = currentRelease + } + // pre-rollback hooks if !r.DisableHooks { - if err := r.cfg.execHook(targetRelease, release.HookPreRollback, r.Timeout); err != nil { + if err := r.cfg.execHook(releaseForHooks, release.HookPreRollback, r.Timeout); err != nil { return targetRelease, err } } else { @@ -243,7 +251,7 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas // post-rollback hooks if !r.DisableHooks { - if err := r.cfg.execHook(targetRelease, release.HookPostRollback, r.Timeout); err != nil { + if err := r.cfg.execHook(releaseForHooks, release.HookPostRollback, r.Timeout); err != nil { return targetRelease, err } } From fdd6698e4d634dfdc758ebef59d4b274cd9f8f88 Mon Sep 17 00:00:00 2001 From: Jared Stehr Rivera Date: Sun, 4 Feb 2024 14:28:55 +0800 Subject: [PATCH 2/3] add tests for hook execution during rollback Signed-off-by: Jared Stehr Rivera --- pkg/action/rollback_test.go | 152 ++++++++++++++++++++++++++++++++++++ 1 file changed, 152 insertions(+) create mode 100644 pkg/action/rollback_test.go diff --git a/pkg/action/rollback_test.go b/pkg/action/rollback_test.go new file mode 100644 index 000000000..7faf64cc4 --- /dev/null +++ b/pkg/action/rollback_test.go @@ -0,0 +1,152 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package action + +import ( + "bytes" + "context" + "strings" + "testing" + "text/template" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "helm.sh/helm/v3/pkg/chart" + "helm.sh/helm/v3/pkg/release" +) + +func rollbackTestActions(t *testing.T) (*Install, *Upgrade, *Rollback) { + config := actionConfigFixture(t) + + instAction := NewInstall(config) + instAction.Namespace = "spaced" + instAction.ReleaseName = "rollback-test" + + upAction := NewUpgrade(config) + upAction.Namespace = "spaced" + + rollbackAction := NewRollback(config) + rollbackAction.Wait = true + + return instAction, upAction, rollbackAction +} + +func buildChartWithRollbackHooks(hookNamePrefix string) *chart.Chart { + hookTmpl, _ := template.New("hook-template").Parse(`kind: ConfigMap +metadata: + name: {{.Prefix}}-{{.Type}} + annotations: + "helm.sh/hook": {{.Type}} +data: + name: value`) + + type HookTmplArgs struct { + Prefix string + Type release.HookEvent + } + + var preRollbackManifest, postRollbackManifest bytes.Buffer + hookTmpl.Execute(&preRollbackManifest, HookTmplArgs{Prefix: hookNamePrefix, Type: release.HookPreRollback}) + hookTmpl.Execute(&postRollbackManifest, HookTmplArgs{Prefix: hookNamePrefix, Type: release.HookPostRollback}) + + return buildChart(func(opts *chartOptions) { + opts.Metadata.Name = "rollback-test" + opts.Templates = []*chart.File{ + {Name: "templates/pre-rollback", Data: preRollbackManifest.Bytes()}, + {Name: "templates/post-rollback", Data: postRollbackManifest.Bytes()}, + } + }) +} + +func TestRollbackRelease_useTargetHooks(t *testing.T) { + is := assert.New(t) + req := require.New(t) + + instAction, upAction, rollbackAction := rollbackTestActions(t) + + vals := map[string]interface{}{} + + // create release with chart A (hooks prefixed with "chart-a") + ctx, done := context.WithCancel(context.Background()) + relV1, err := instAction.RunWithContext(ctx, buildChartWithRollbackHooks("chart-a"), vals) + done() + req.NoError(err) + + // upgrade release with chart B (hooks prefixed with "chart-b") + ctx, done = context.WithCancel(context.Background()) + relV2, err := upAction.RunWithContext(ctx, relV1.Name, buildChartWithRollbackHooks("chart-b"), vals) + done() + req.NoError(err) + + // rollback release to chart A (B -> A) + err = rollbackAction.Run(relV2.Name) + req.NoError(err) + relV3, err := rollbackAction.cfg.Releases.Last(relV2.Name) + req.NoError(err) + + // check hooks WERE NOT RUN from source release (chart-b) + for _, hook := range relV2.Hooks { + is.True(strings.HasPrefix(hook.Name, "chart-b")) + is.True(hook.LastRun.StartedAt.IsZero()) + } + + // check hooks WERE RUN from target release (chart-a) + for _, hook := range relV3.Hooks { + is.True(strings.HasPrefix(hook.Name, "chart-a")) + is.False(hook.LastRun.StartedAt.IsZero()) + } +} + +func TestRollbackRelease_useSourceHooks(t *testing.T) { + is := assert.New(t) + req := require.New(t) + + instAction, upAction, rollbackAction := rollbackTestActions(t) + + vals := map[string]interface{}{} + + // create release with chart A (hooks prefixed with "chart-a") + ctx, done := context.WithCancel(context.Background()) + relV1, err := instAction.RunWithContext(ctx, buildChartWithRollbackHooks("chart-a"), vals) + done() + req.NoError(err) + + // upgrade release with chart B (hooks prefixed with "chart-b") + ctx, done = context.WithCancel(context.Background()) + relV2, err := upAction.RunWithContext(ctx, relV1.Name, buildChartWithRollbackHooks("chart-b"), vals) + done() + req.NoError(err) + + // rollback release to chart A (B -> A) BUT run hooks of current release + rollbackAction.UseSourceHooks = true + err = rollbackAction.Run(relV2.Name) + req.NoError(err) + relV3, err := rollbackAction.cfg.Releases.Last(relV2.Name) + req.NoError(err) + + // check hooks WERE RUN from source release (chart-b) + for _, hook := range relV2.Hooks { + is.True(strings.HasPrefix(hook.Name, "chart-b")) + is.False(hook.LastRun.StartedAt.IsZero()) + } + + // check hooks WERE NOT RUN from target release (chart-a) + for _, hook := range relV3.Hooks { + is.True(strings.HasPrefix(hook.Name, "chart-a")) + is.True(hook.LastRun.StartedAt.IsZero()) + } +} From 597eb278e8d5e30802316860512aba30d3d05e1d Mon Sep 17 00:00:00 2001 From: Jared Stehr Rivera Date: Sat, 10 Feb 2024 18:11:20 +0800 Subject: [PATCH 3/3] fix linting issues Signed-off-by: Jared Stehr Rivera --- pkg/action/rollback_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/action/rollback_test.go b/pkg/action/rollback_test.go index 7faf64cc4..90fe402cc 100644 --- a/pkg/action/rollback_test.go +++ b/pkg/action/rollback_test.go @@ -25,6 +25,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/release" )