hooks: implement for helm and kustomize deployers

docs/content/en/docs/pipeline-stages/lifecycle-hooks.md

@@ -64,7 +64,7 @@ build:
 ```
 This config snippet defines that `hook.sh` (for `darwin` or `linux` OS) or `hook.bat` (for `windows` OS) will be executed `before` and `after` each file sync operation for artifact `hooks-example`.
 
-### `before-deploy` and `after-deploy` (only for `kubectl` deployer)
+### `before-deploy` and `after-deploy`
 
 Example: _skaffold.yaml_ snippet
 ```yaml
@@ -127,7 +127,7 @@ build:
 ```
 This config snippet defines a command to run inside the container corresponding to the artifact `hooks-example` image, `before` and `after` each file sync operation.
 
-### `before-deploy` and `after-deploy` (only for `kubectl` deployer)
+### `before-deploy` and `after-deploy`
 
 Example: _skaffold.yaml_ snippet
 ```yaml

docs/content/en/schemas/v2beta22.json

@@ -1703,6 +1703,11 @@
           "description": "additional option flags that are passed on the command line to `helm`.",
           "x-intellij-html-description": "additional option flags that are passed on the command line to <code>helm</code>."
         },
+        "hooks": {
+          "$ref": "#/definitions/DeployHooks",
+          "description": "describes a set of lifecycle hooks that are executed before and after every deploy.",
+          "x-intellij-html-description": "describes a set of lifecycle hooks that are executed before and after every deploy."
+        },
         "releases": {
           "items": {
             "$ref": "#/definitions/HelmRelease"
@@ -1714,7 +1719,8 @@
       },
       "preferredOrder": [
         "releases",
-        "flags"
+        "flags",
+        "hooks"
       ],
       "additionalProperties": false,
       "type": "object",
@@ -2680,6 +2686,11 @@
           "description": "additional flags passed to `kubectl`.",
           "x-intellij-html-description": "additional flags passed to <code>kubectl</code>."
         },
+        "hooks": {
+          "$ref": "#/definitions/DeployHooks",
+          "description": "describes a set of lifecycle hooks that are executed before and after every deploy.",
+          "x-intellij-html-description": "describes a set of lifecycle hooks that are executed before and after every deploy."
+        },
         "paths": {
           "items": {
             "type": "string"
@@ -2694,7 +2705,8 @@
         "paths",
         "flags",
         "buildArgs",
-        "defaultNamespace"
+        "defaultNamespace",
+        "hooks"
       ],
       "additionalProperties": false,
       "type": "object",

pkg/skaffold/deploy/helm/deploy.go

@@ -45,6 +45,7 @@ import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/types"
 	deployutil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/util"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/graph"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/hooks"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/instrumentation"
 	pkgkubectl "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubectl"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes"
@@ -93,6 +94,7 @@ type Deployer struct {
 	logger        log.Logger
 	statusMonitor status.Monitor
 	syncer        sync.Syncer
+	hookRunner    hooks.Runner
 
 	podSelector    *kubernetes.ImageList
 	originalImages []graph.Artifact // the set of images defined in ArtifactOverrides
@@ -162,6 +164,7 @@ func NewDeployer(cfg Config, labeller *label.DefaultLabeller, h *latestV1.HelmDe
 		logger:         logger,
 		statusMonitor:  component.NewMonitor(cfg, cfg.GetKubeContext(), labeller, &namespaces),
 		syncer:         component.NewSyncer(kubectl, &namespaces, logger.GetFormatter()),
+		hookRunner:     hooks.NewDeployRunner(kubectl, h.LifecycleHooks, &namespaces, logger.GetFormatter(), hooks.NewDeployEnvOpts(labeller.GetRunID(), kubectl.KubeContext, namespaces)),
 		originalImages: originalImages,
 		kubeContext:    cfg.GetKubeContext(),
 		kubeConfig:     cfg.GetKubeConfig(),
@@ -433,6 +436,26 @@ func (h *Deployer) Render(ctx context.Context, out io.Writer, builds []graph.Art
 	return manifest.Write(renderedManifests.String(), filepath, out)
 }
 
+func (h *Deployer) PreDeployHooks(ctx context.Context, out io.Writer) error {
+	childCtx, endTrace := instrumentation.StartTrace(ctx, "Deploy_PreHooks")
+	if err := h.hookRunner.RunPreHooks(childCtx, out); err != nil {
+		endTrace(instrumentation.TraceEndError(err))
+		return err
+	}
+	endTrace()
+	return nil
+}
+
+func (h *Deployer) PostDeployHooks(ctx context.Context, out io.Writer) error {
+	childCtx, endTrace := instrumentation.StartTrace(ctx, "Deploy_PostHooks")
+	if err := h.hookRunner.RunPostHooks(childCtx, out); err != nil {
+		endTrace(instrumentation.TraceEndError(err))
+		return err
+	}
+	endTrace()
+	return nil
+}
+
 // deployRelease deploys a single release
 func (h *Deployer) deployRelease(ctx context.Context, out io.Writer, releaseName string, r latestV1.HelmRelease, builds []graph.Artifact, valuesSet map[string]bool, helmVersion semver.Version, chartVersion string) ([]types.Artifact, error) {
 	var err error

pkg/skaffold/deploy/helm/helm_test.go

@@ -19,18 +19,23 @@ package helm
 import (
 	"context"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"path/filepath"
 	"strings"
 	"testing"
 
 	"github.com/mitchellh/go-homedir"
+	"github.com/pkg/errors"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kubectl"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/label"
 	deployutil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/util"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/graph"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/hooks"
+	ctl "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubectl"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/client"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/logger"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
 	latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
 	schemautil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
@@ -1601,6 +1606,53 @@ func TestGenerateSkaffoldDebugFilter(t *testing.T) {
 	}
 }
 
+func TestHelmHooks(t *testing.T) {
+	tests := []struct {
+		description string
+		runner      hooks.Runner
+		shouldErr   bool
+	}{
+		{
+			description: "hooks run successfully",
+			runner: hooks.MockRunner{
+				PreHooks: func(context.Context, io.Writer) error {
+					return nil
+				},
+				PostHooks: func(context.Context, io.Writer) error {
+					return nil
+				},
+			},
+		},
+		{
+			description: "hooks fails",
+			runner: hooks.MockRunner{
+				PreHooks: func(context.Context, io.Writer) error {
+					return errors.New("failed to execute hooks")
+				},
+				PostHooks: func(context.Context, io.Writer) error {
+					return errors.New("failed to execute hooks")
+				},
+			},
+			shouldErr: true,
+		},
+	}
+	for _, test := range tests {
+		testutil.Run(t, test.description, func(t *testutil.T) {
+			t.Override(&util.DefaultExecCommand, testutil.CmdRunWithOutput("helm version --client", version31))
+			t.Override(&hooks.NewDeployRunner, func(*ctl.CLI, latestV1.DeployHooks, *[]string, logger.Formatter, hooks.DeployEnvOpts) hooks.Runner {
+				return test.runner
+			})
+
+			k, err := NewDeployer(&helmConfig{}, &label.DefaultLabeller{}, &testDeployConfig)
+			t.RequireNoError(err)
+			err = k.PreDeployHooks(context.Background(), ioutil.Discard)
+			t.CheckError(test.shouldErr, err)
+			err = k.PostDeployHooks(context.Background(), ioutil.Discard)
+			t.CheckError(test.shouldErr, err)
+		})
+	}
+}
+
 type helmConfig struct {
 	runcontext.RunContext // Embedded to provide the default values.
 	namespace             string

pkg/skaffold/deploy/kubectl/kubectl.go

@@ -109,7 +109,7 @@ func NewDeployer(cfg Config, labeller *label.DefaultLabeller, d *latestV1.Kubect
 		logger:             logger,
 		statusMonitor:      component.NewMonitor(cfg, cfg.GetKubeContext(), labeller, &namespaces),
 		syncer:             component.NewSyncer(kubectl.CLI, &namespaces, logger.GetFormatter()),
-		hookRunner:         hooks.NewDeployRunner(kubectl.CLI, d.LifecycleHooks, namespaces, logger.GetFormatter(), hooks.NewDeployEnvOpts(labeller.GetRunID(), kubectl.KubeContext, namespaces)),
+		hookRunner:         hooks.NewDeployRunner(kubectl.CLI, d.LifecycleHooks, &namespaces, logger.GetFormatter(), hooks.NewDeployEnvOpts(labeller.GetRunID(), kubectl.KubeContext, namespaces)),
 		workingDir:         cfg.GetWorkingDir(),
 		globalConfig:       cfg.GlobalConfig(),
 		defaultRepo:        cfg.DefaultRepo(),

pkg/skaffold/deploy/kustomize/kustomize.go

@@ -37,6 +37,7 @@ import (
 	deployutil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/util"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/event"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/graph"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/hooks"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/instrumentation"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/manifest"
@@ -111,6 +112,7 @@ type Deployer struct {
 	debugger      debug.Debugger
 	statusMonitor status.Monitor
 	syncer        sync.Syncer
+	hookRunner    hooks.Runner
 
 	podSelector    *kubernetes.ImageList
 	originalImages []graph.Artifact // the set of images parsed from the Deployer's manifest set
@@ -151,6 +153,7 @@ func NewDeployer(cfg kubectl.Config, labeller *label.DefaultLabeller, d *latestV
 		namespaces:          &namespaces,
 		accessor:            component.NewAccessor(cfg, cfg.GetKubeContext(), kubectl.CLI, podSelector, labeller, &namespaces),
 		debugger:            component.NewDebugger(cfg.Mode(), podSelector, &namespaces, cfg.GetKubeContext()),
+		hookRunner:          hooks.NewDeployRunner(kubectl.CLI, d.LifecycleHooks, &namespaces, logger.GetFormatter(), hooks.NewDeployEnvOpts(labeller.GetRunID(), kubectl.KubeContext, namespaces)),
 		imageLoader:         component.NewImageLoader(cfg, kubectl.CLI),
 		logger:              logger,
 		statusMonitor:       component.NewMonitor(cfg, cfg.GetKubeContext(), labeller, &namespaces),
@@ -203,6 +206,26 @@ func kustomizeBinaryExists() bool {
 	return err == nil
 }
 
+func (k *Deployer) PreDeployHooks(ctx context.Context, out io.Writer) error {
+	childCtx, endTrace := instrumentation.StartTrace(ctx, "Deploy_PreHooks")
+	if err := k.hookRunner.RunPreHooks(childCtx, out); err != nil {
+		endTrace(instrumentation.TraceEndError(err))
+		return err
+	}
+	endTrace()
+	return nil
+}
+
+func (k *Deployer) PostDeployHooks(ctx context.Context, out io.Writer) error {
+	childCtx, endTrace := instrumentation.StartTrace(ctx, "Deploy_PostHooks")
+	if err := k.hookRunner.RunPostHooks(childCtx, out); err != nil {
+		endTrace(instrumentation.TraceEndError(err))
+		return err
+	}
+	endTrace()
+	return nil
+}
+
 // Check that kubectl version is valid to use kubectl kustomize
 func kubectlVersionCheck(kubectl kubectl.CLI) bool {
 	gt, err := kubectl.CompareVersionTo(context.Background(), 1, 14)

pkg/skaffold/deploy/kustomize/kustomize_test.go

@@ -20,6 +20,7 @@ import (
 	"bytes"
 	"context"
 	"errors"
+	"io"
 	"io/ioutil"
 	"path/filepath"
 	"testing"
@@ -30,7 +31,10 @@ import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/label"
 	deployutil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/util"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/graph"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/hooks"
+	ctl "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubectl"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/client"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/logger"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
 	latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
@@ -265,6 +269,57 @@ func TestKustomizeCleanup(t *testing.T) {
 	}
 }
 
+func TestKustomizeHooks(t *testing.T) {
+	tests := []struct {
+		description string
+		runner      hooks.Runner
+		shouldErr   bool
+	}{
+		{
+			description: "hooks run successfully",
+			runner: hooks.MockRunner{
+				PreHooks: func(context.Context, io.Writer) error {
+					return nil
+				},
+				PostHooks: func(context.Context, io.Writer) error {
+					return nil
+				},
+			},
+		},
+		{
+			description: "hooks fails",
+			runner: hooks.MockRunner{
+				PreHooks: func(context.Context, io.Writer) error {
+					return errors.New("failed to execute hooks")
+				},
+				PostHooks: func(context.Context, io.Writer) error {
+					return errors.New("failed to execute hooks")
+				},
+			},
+			shouldErr: true,
+		},
+	}
+	for _, test := range tests {
+		testutil.Run(t, test.description, func(t *testutil.T) {
+			t.Override(&KustomizeBinaryCheck, func() bool { return true })
+			t.Override(&hooks.NewDeployRunner, func(*ctl.CLI, latestV1.DeployHooks, *[]string, logger.Formatter, hooks.DeployEnvOpts) hooks.Runner {
+				return test.runner
+			})
+
+			k, err := NewDeployer(&kustomizeConfig{
+				workingDir: ".",
+				RunContext: runcontext.RunContext{Opts: config.SkaffoldOptions{
+					Namespace: kubectl.TestNamespace}},
+			}, &label.DefaultLabeller{}, &latestV1.KustomizeDeploy{})
+			t.RequireNoError(err)
+			err = k.PreDeployHooks(context.Background(), ioutil.Discard)
+			t.CheckError(test.shouldErr, err)
+			err = k.PostDeployHooks(context.Background(), ioutil.Discard)
+			t.CheckError(test.shouldErr, err)
+		})
+	}
+}
+
 func TestDependenciesForKustomization(t *testing.T) {
 	tests := []struct {
 		description    string

pkg/skaffold/hooks/deploy.go

@@ -31,7 +31,12 @@ import (
 	v1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
 )
 
-func NewDeployRunner(cli *kubectl.CLI, d v1.DeployHooks, namespaces []string, formatter logger.Formatter, opts DeployEnvOpts) Runner {
+// for testing
+var (
+	NewDeployRunner = newDeployRunner
+)
+
+func newDeployRunner(cli *kubectl.CLI, d v1.DeployHooks, namespaces *[]string, formatter logger.Formatter, opts DeployEnvOpts) Runner {
 	return deployRunner{d, cli, namespaces, formatter, opts, new(sync.Map)}
 }
 
@@ -46,7 +51,7 @@ func NewDeployEnvOpts(runID string, kubeContext string, namespaces []string) Dep
 type deployRunner struct {
 	v1.DeployHooks
 	cli         *kubectl.CLI
-	namespaces  []string
+	namespaces  *[]string
 	formatter   logger.Formatter
 	opts        DeployEnvOpts
 	visitedPods *sync.Map // maintain a list of previous iteration pods, so that they can be skipped
@@ -82,7 +87,7 @@ func (r deployRunner) run(ctx context.Context, out io.Writer, hooks []v1.DeployH
 				cfg:        v1.ContainerHook{Command: h.ContainerHook.Command},
 				cli:        r.cli,
 				selector:   filterPodsSelector(r.visitedPods, phase, namePatternSelector(h.ContainerHook.PodName, h.ContainerHook.ContainerName)),
-				namespaces: r.namespaces,
+				namespaces: *r.namespaces,
 				formatter:  r.formatter,
 			}
 			if err := hook.run(ctx, out); err != nil {

pkg/skaffold/hooks/deploy_test.go

@@ -94,7 +94,7 @@ func TestDeployHooks(t *testing.T) {
 		kubeContext := "context1"
 		opts := NewDeployEnvOpts("run_id", kubeContext, namespaces)
 		formatter := func(corev1.Pod, corev1.ContainerStatus, func() bool) log.Formatter { return mockLogFormatter{} }
-		runner := NewDeployRunner(&kubectl.CLI{KubeContext: kubeContext}, deployer.LifecycleHooks, namespaces, formatter, opts)
+		runner := NewDeployRunner(&kubectl.CLI{KubeContext: kubeContext}, deployer.LifecycleHooks, &namespaces, formatter, opts)
 
 		t.Override(&util.DefaultExecCommand,
 			testutil.CmdRunWithOutput("kubectl --context context1 exec pod1 --namespace np1 -c container1 -- foo pre-hook", preContainerHookOut).

pkg/skaffold/hooks/types.go

@@ -46,3 +46,23 @@ var phases = struct {
 	PreDeploy:  "pre-deploy",
 	PostDeploy: "post-deploy",
 }
+
+// MockRunner implements the Runner interface, to be used in unit tests
+type MockRunner struct {
+	PreHooks  func(ctx context.Context, out io.Writer) error
+	PostHooks func(ctx context.Context, out io.Writer) error
+}
+
+func (m MockRunner) RunPreHooks(ctx context.Context, out io.Writer) error {
+	if m.PreHooks != nil {
+		return m.PreHooks(ctx, out)
+	}
+	return nil
+}
+
+func (m MockRunner) RunPostHooks(ctx context.Context, out io.Writer) error {
+	if m.PostHooks != nil {
+		return m.PostHooks(ctx, out)
+	}
+	return nil
+}