Merge 597eb278e8 into 8602af9e92

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")

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",

cmd/helm/testdata/output/rollback-use-source-hooks.txt

@@ -0,0 +1 @@
+Rollback was a success! Happy Helming!

pkg/action/rollback.go

@@ -35,16 +35,17 @@ import (
 type Rollback struct {
 	cfg *Configuration
 
-	Version       int
+	Version        int
-	Timeout       time.Duration
+	Timeout        time.Duration
-	Wait          bool
+	Wait           bool
-	WaitForJobs   bool
+	WaitForJobs    bool
-	DisableHooks  bool
+	DisableHooks   bool
-	DryRun        bool
+	UseSourceHooks bool
-	Recreate      bool // will (if true) recreate pods after a rollback.
+	DryRun         bool
-	Force         bool // will (if true) force resource upgrade through uninstall/recreate if needed
+	Recreate       bool // will (if true) recreate pods after a rollback.
-	CleanupOnFail bool
+	Force          bool // will (if true) force resource upgrade through uninstall/recreate if needed
-	MaxHistory    int // MaxHistory limits the maximum number of revisions saved per release
+	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
 		}
 	}

pkg/action/rollback_test.go

@@ -0,0 +1,153 @@
+/*
+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())
+	}
+}