feat(helm): add --no-hooks to 'helm delete'

This also adds several tests, cleanups to the API, and removal of dead
code.

Relates to #932

cmd/helm/delete.go

@@ -18,6 +18,7 @@ package main
 
 import (
 	"errors"
+	"io"
 
 	"github.com/spf13/cobra"
 
@@ -32,32 +33,49 @@ Use the '--dry-run' flag to see which releases will be deleted without actually
 deleting them.
 `
 
-var deleteDryRun bool
+type deleteCmd struct {
+	name         string
+	dryRun       bool
+	disableHooks bool
 
-var deleteCommand = &cobra.Command{
+	out    io.Writer
+	client helm.Interface
+}
+
+func newDeleteCmd(c helm.Interface, out io.Writer) *cobra.Command {
+	del := &deleteCmd{
+		out:    out,
+		client: c,
+	}
+
+	cmd := &cobra.Command{
 		Use:               "delete [flags] RELEASE_NAME",
 		Aliases:           []string{"del"},
 		SuggestFor:        []string{"remove", "rm"},
 		Short:             "given a release name, delete the release from Kubernetes",
 		Long:              deleteDesc,
-	RunE:              delRelease,
 		PersistentPreRunE: setupConnection,
-}
+		RunE: func(cmd *cobra.Command, args []string) error {
-
-func init() {
-	RootCommand.AddCommand(deleteCommand)
-	deleteCommand.Flags().BoolVar(&deleteDryRun, "dry-run", false, "Simulate action, but don't actually do it.")
-}
-
-func delRelease(cmd *cobra.Command, args []string) error {
 			if len(args) == 0 {
 				return errors.New("command 'delete' requires a release name")
 			}
+			del.name = args[0]
+			del.client = ensureHelmClient(del.client)
+			return del.run()
+		},
+	}
+	f := cmd.Flags()
+	f.BoolVar(&del.dryRun, "dry-run", false, "simulate a delete")
+	f.BoolVar(&del.disableHooks, "no-hooks", false, "prevent hooks from running during deletion")
 
-	_, err := helm.UninstallRelease(args[0], deleteDryRun)
+	return cmd
-	if err != nil {
-		return prettyError(err)
 }
 
-	return nil
+func (d *deleteCmd) run() error {
+	opts := []helm.DeleteOption{
+		helm.DeleteDryRun(d.dryRun),
+		helm.DeleteDisableHooks(d.disableHooks),
+	}
+	_, err := d.client.DeleteRelease(d.name, opts...)
+	return prettyError(err)
 }

cmd/helm/delete_test.go

@@ -0,0 +1,52 @@
+/*
+Copyright 2016 The Kubernetes Authors All rights reserved.
+
+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 main
+
+import (
+	"io"
+	"testing"
+
+	"github.com/spf13/cobra"
+)
+
+func TestDelete(t *testing.T) {
+
+	tests := []releaseCase{
+		{
+			name:     "basic delete",
+			args:     []string{"aeneas"},
+			flags:    []string{},
+			expected: "", // Output of a delete is an empty string and exit 0.
+			resp:     releaseMock("aeneas"),
+		},
+		{
+			name:     "delete without hooks",
+			args:     []string{"aeneas"},
+			flags:    []string{"--no-hooks"},
+			expected: "",
+			resp:     releaseMock("aeneas"),
+		},
+		{
+			name: "delete without release",
+			args: []string{},
+			err:  true,
+		},
+	}
+	runReleaseCases(t, tests, func(c *fakeReleaseClient, out io.Writer) *cobra.Command {
+		return newDeleteCmd(c, out)
+	})
+}

cmd/helm/get.go

@@ -66,7 +66,7 @@ func newGetCmd(client helm.Interface, out io.Writer) *cobra.Command {
 			}
 			get.release = args[0]
 			if get.client == nil {
-				get.client = helm.NewClient(helm.Host(helm.Config.ServAddr))
+				get.client = helm.NewClient(helm.Host(tillerHost))
 			}
 			return get.run()
 		},
@@ -126,9 +126,9 @@ func tpl(t string, vals map[string]interface{}, out io.Writer) error {
 	return tt.Execute(out, vals)
 }
 
-func ensureHelmClient(h helm.OptionalInterface) helm.OptionalInterface {
+func ensureHelmClient(h helm.Interface) helm.Interface {
 	if h != nil {
 		return h
 	}
-	return helm.NewClient(helm.Host(helm.Config.ServAddr))
+	return helm.NewClient(helm.Host(tillerHost))
 }

cmd/helm/get_hooks.go

@@ -34,10 +34,10 @@ Hooks are formatted in YAML and separated by the YAML '---\n' separator.
 type getHooksCmd struct {
 	release string
 	out     io.Writer
-	client  helm.OptionalInterface
+	client  helm.Interface
 }
 
-func newGetHooksCmd(client helm.OptionalInterface, out io.Writer) *cobra.Command {
+func newGetHooksCmd(client helm.Interface, out io.Writer) *cobra.Command {
 	ghc := &getHooksCmd{
 		out:    out,
 		client: client,

cmd/helm/get_manifest.go

@@ -54,7 +54,7 @@ func newGetManifestCmd(client helm.Interface, out io.Writer) *cobra.Command {
 			}
 			get.release = args[0]
 			if get.client == nil {
-				get.client = helm.NewClient(helm.Host(helm.Config.ServAddr))
+				get.client = helm.NewClient(helm.Host(tillerHost))
 			}
 			return get.run()
 		},

cmd/helm/get_values.go

@@ -34,10 +34,10 @@ type getValuesCmd struct {
 	release   string
 	allValues bool
 	out       io.Writer
-	client    helm.OptionalInterface
+	client    helm.Interface
 }
 
-func newGetValuesCmd(client helm.OptionalInterface, out io.Writer) *cobra.Command {
+func newGetValuesCmd(client helm.Interface, out io.Writer) *cobra.Command {
 	get := &getValuesCmd{
 		out:    out,
 		client: client,

cmd/helm/helm.go

@@ -25,8 +25,6 @@ import (
 
 	"github.com/spf13/cobra"
 	"google.golang.org/grpc"
-
-	"k8s.io/helm/pkg/helm"
 )
 
 const (
@@ -91,6 +89,7 @@ func newRootCmd(out io.Writer) *cobra.Command {
 		newListCmd(nil, out),
 		newStatusCmd(nil, out),
 		newInstallCmd(nil, out),
+		newDeleteCmd(nil, out),
 	)
 	return cmd
 }
@@ -119,9 +118,8 @@ func setupConnection(c *cobra.Command, args []string) error {
 	}
 
 	// Set up the gRPC config.
-	helm.Config.ServAddr = tillerHost
 	if flagDebug {
-		fmt.Printf("Server: %q\n", helm.Config.ServAddr)
+		fmt.Printf("Server: %q\n", tillerHost)
 	}
 	return nil
 }
@@ -154,6 +152,9 @@ func requireInit(cmd *cobra.Command, args []string) error {
 
 // prettyError unwraps or rewrites certain errors to make them more user-friendly.
 func prettyError(err error) error {
+	if err == nil {
+		return nil
+	}
 	// This is ridiculous. Why is 'grpc.rpcError' not exported? The least they
 	// could do is throw an interface on the lib that would let us get back
 	// the desc. Instead, we have to pass ALL errors through this.

cmd/helm/helm_test.go

@@ -118,7 +118,7 @@ func (c *fakeReleaseClient) ReleaseContent(rlsName string, opts ...helm.ContentO
 	return resp, c.err
 }
 
-func (c *fakeReleaseClient) Option(opt ...helm.Option) helm.OptionalInterface {
+func (c *fakeReleaseClient) Option(opt ...helm.Option) helm.Interface {
 	return c
 }
 
@@ -134,10 +134,9 @@ func runReleaseCases(t *testing.T, tests []releaseCase, rcmd releaseCmd) {
 		}
 		cmd := rcmd(c, &buf)
 		cmd.ParseFlags(tt.flags)
-		println("parsed flags")
 		err := cmd.RunE(cmd, tt.args)
 		if (err != nil) != tt.err {
-			t.Errorf("%q. expected error: %v, got '%v'", tt.name, tt.err, err)
+			t.Errorf("%q. expected error, got '%v'", tt.name, err)
 		}
 		re := regexp.MustCompile(tt.expected)
 		if !re.Match(buf.Bytes()) {

cmd/helm/install.go

@@ -39,18 +39,6 @@ path to a chart directory or the name of a
 chart in the current working directory.
 `
 
-// install flags & args
-var (
-	// installDryRun performs a dry-run install
-	installDryRun bool
-	// installValues is the filename of supplied values.
-	installValues string
-	// installRelName is the user-supplied release name.
-	installRelName string
-	// installNoHooks is the flag for controlling hook execution.
-	installNoHooks bool
-)
-
 type installCmd struct {
 	name         string
 	valuesFile   string
@@ -59,10 +47,10 @@ type installCmd struct {
 	disableHooks bool
 
 	out    io.Writer
-	client helm.OptionalInterface
+	client helm.Interface
 }
 
-func newInstallCmd(c helm.OptionalInterface, out io.Writer) *cobra.Command {
+func newInstallCmd(c helm.Interface, out io.Writer) *cobra.Command {
 	inst := &installCmd{
 		out:    out,
 		client: c,
@@ -74,7 +62,6 @@ func newInstallCmd(c helm.OptionalInterface, out io.Writer) *cobra.Command {
 		Long:              installDesc,
 		PersistentPreRunE: setupConnection,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			fmt.Printf("%v", args)
 			if err := checkArgsLength(1, len(args), "chart name"); err != nil {
 				return err
 			}
@@ -107,14 +94,7 @@ func (i *installCmd) run() error {
 		return err
 	}
 
-	if i.dryRun {
+	res, err := i.client.InstallRelease(i.chartPath, helm.ValueOverrides(rawVals), helm.ReleaseName(i.name), helm.InstallDryRun(i.dryRun), helm.InstallDisableHooks(i.disableHooks))
-		i.client.Option(helm.DryRun())
-	}
-	if i.disableHooks {
-		i.client.Option(helm.DisableHooks())
-	}
-
-	res, err := i.client.InstallRelease(i.chartPath, helm.ValueOverrides(rawVals), helm.ReleaseName(i.name))
 	if err != nil {
 		return prettyError(err)
 	}

cmd/helm/list.go

@@ -80,7 +80,7 @@ func newListCmd(client helm.Interface, out io.Writer) *cobra.Command {
 				list.filter = strings.Join(args, " ")
 			}
 			if list.client == nil {
-				list.client = helm.NewClient(helm.Host(helm.Config.ServAddr))
+				list.client = helm.NewClient(helm.Host(tillerHost))
 			}
 			return list.run()
 		},

cmd/helm/status.go

@@ -52,7 +52,7 @@ func newStatusCmd(client helm.Interface, out io.Writer) *cobra.Command {
 			}
 			status.release = args[0]
 			if status.client == nil {
-				status.client = helm.NewClient(helm.Host(helm.Config.ServAddr))
+				status.client = helm.NewClient(helm.Host(tillerHost))
 			}
 			return status.run()
 		},

cmd/tiller/release_server_test.go

@@ -31,7 +31,6 @@ import (
 	"k8s.io/helm/pkg/proto/hapi/release"
 	"k8s.io/helm/pkg/proto/hapi/services"
 	"k8s.io/helm/pkg/storage"
-	//"k8s.io/helm/pkg/timeconv"
 )
 
 var manifestWithHook = `apiVersion: v1

pkg/helm/client.go

@@ -44,11 +44,11 @@ type Client struct {
 
 // NewClient creates a new client.
 func NewClient(opts ...Option) *Client {
-	return new(Client).Init().Option(opts...).(*Client)
+	return new(Client).Init().Option(opts...)
 }
 
 // Option configures the helm client with the provided options
-func (h *Client) Option(opts ...Option) OptionalInterface {
+func (h *Client) Option(opts ...Option) *Client {
 	for _, opt := range opts {
 		opt(&h.opts)
 	}
@@ -58,7 +58,7 @@ func (h *Client) Option(opts ...Option) OptionalInterface {
 // Init initializes the helm client with default options
 func (h *Client) Init() *Client {
 	return h.Option(Host(DefaultHelmHost)).
-		Option(Home(os.ExpandEnv(DefaultHelmHome))).(*Client)
+		Option(Home(os.ExpandEnv(DefaultHelmHome)))
 }
 
 // ListReleases lists the current releases.

pkg/helm/compat.go

@@ -1,85 +0,0 @@
-/*
-Copyright 2016 The Kubernetes Authors All rights reserved.
-
-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 helm
-
-import (
-	rls "k8s.io/helm/pkg/proto/hapi/services"
-)
-
-// These APIs are a temporary abstraction layer that captures the interaction between the current cmd/helm and old
-// pkg/helm implementations. Post refactor the cmd/helm package will use the APIs exposed on helm.Client directly.
-
-// Config is the base configuration
-var Config struct {
-	ServAddr string
-}
-
-// ListReleases lists releases. DEPRECATED.
-//
-// Soon to be deprecated helm ListReleases API.
-func ListReleases(limit int, offset string, sort rls.ListSort_SortBy, order rls.ListSort_SortOrder, filter string) (*rls.ListReleasesResponse, error) {
-	opts := []ReleaseListOption{
-		ReleaseListLimit(limit),
-		ReleaseListOffset(offset),
-		ReleaseListFilter(filter),
-		ReleaseListSort(int32(sort)),
-		ReleaseListOrder(int32(order)),
-	}
-	return NewClient(Host(Config.ServAddr)).ListReleases(opts...)
-}
-
-// GetReleaseStatus gets a release status. DEPRECATED
-//
-// Soon to be deprecated helm GetReleaseStatus API.
-func GetReleaseStatus(rlsName string) (*rls.GetReleaseStatusResponse, error) {
-	return NewClient(Host(Config.ServAddr)).ReleaseStatus(rlsName)
-}
-
-// GetReleaseContent gets the content of a release.
-// Soon to be deprecated helm GetReleaseContent API.
-func GetReleaseContent(rlsName string) (*rls.GetReleaseContentResponse, error) {
-	return NewClient(Host(Config.ServAddr)).ReleaseContent(rlsName)
-}
-
-// UpdateRelease updates a release.
-// Soon to be deprecated helm UpdateRelease API.
-func UpdateRelease(rlsName string) (*rls.UpdateReleaseResponse, error) {
-	return NewClient(Host(Config.ServAddr)).UpdateRelease(rlsName)
-}
-
-// InstallRelease runs an install for a release.
-// Soon to be deprecated helm InstallRelease API.
-func InstallRelease(vals []byte, rlsName, chStr string, dryRun, skipHooks bool) (*rls.InstallReleaseResponse, error) {
-	client := NewClient(Host(Config.ServAddr))
-	if dryRun {
-		client.Option(DryRun())
-	}
-	if skipHooks {
-		client.Option(DisableHooks())
-	}
-	return client.InstallRelease(chStr, ValueOverrides(vals), ReleaseName(rlsName))
-}
-
-// UninstallRelease destroys an existing release.
-// Soon to be deprecated helm UninstallRelease API.
-func UninstallRelease(rlsName string, dryRun bool) (*rls.UninstallReleaseResponse, error) {
-	client := NewClient(Host(Config.ServAddr))
-	if dryRun {
-		client.Option(DryRun())
-	}
-	return client.DeleteRelease(rlsName)
-}

pkg/helm/interface.go

@@ -29,9 +29,3 @@ type Interface interface {
 	UpdateRelease(rlsName string, opts ...UpdateOption) (*rls.UpdateReleaseResponse, error)
 	ReleaseContent(rlsName string, opts ...ContentOption) (*rls.GetReleaseContentResponse, error)
 }
-
-// OptionalInterface is an Interface that also supports Option().
-type OptionalInterface interface {
-	Interface
-	Option(opts ...Option) OptionalInterface
-}

pkg/helm/option.go

@@ -46,19 +46,6 @@ type options struct {
 	instReq rls.InstallReleaseRequest
 }
 
-// DryRun returns an Option which instructs the helm client to dry-run tiller rpcs.
-func DryRun() Option {
-	return func(opts *options) {
-		opts.dryRun = true
-	}
-}
-
-func DisableHooks() Option {
-	return func(opts *options) {
-		opts.disableHooks = true
-	}
-}
-
 // Home specifies the location of helm home, (default = "$HOME/.helm").
 func Home(home string) Option {
 	return func(opts *options) {
@@ -132,6 +119,34 @@ func ReleaseName(name string) InstallOption {
 	}
 }
 
+// DeleteDisableHooks will disable hooks for a deletion operation.
+func DeleteDisableHooks(disable bool) DeleteOption {
+	return func(opts *options) {
+		opts.disableHooks = disable
+	}
+}
+
+// DeleteDryRun will (if true) execute a deletion as a dry run.
+func DeleteDryRun(dry bool) DeleteOption {
+	return func(opts *options) {
+		opts.dryRun = dry
+	}
+}
+
+// InstallDisableHooks disables hooks during installation.
+func InstallDisableHooks(disable bool) InstallOption {
+	return func(opts *options) {
+		opts.disableHooks = disable
+	}
+}
+
+// InstallDryRun will (if true) execute an installation as a dry run.
+func InstallDryRun(dry bool) InstallOption {
+	return func(opts *options) {
+		opts.dryRun = dry
+	}
+}
+
 // ContentOption -- TODO
 type ContentOption func(*options)
 
@@ -178,6 +193,9 @@ func (o *options) rpcInstallRelease(chr *cpb.Chart, rlc rls.ReleaseServiceClient
 
 // Executes tiller.UninstallRelease RPC.
 func (o *options) rpcDeleteRelease(rlsName string, rlc rls.ReleaseServiceClient, opts ...DeleteOption) (*rls.UninstallReleaseResponse, error) {
+	for _, opt := range opts {
+		opt(o)
+	}
 	if o.dryRun {
 		// In the dry run case, just see if the release exists
 		r, err := o.rpcGetReleaseContent(rlsName, rlc)
@@ -186,8 +204,7 @@ func (o *options) rpcDeleteRelease(rlsName string, rlc rls.ReleaseServiceClient,
 		}
 		return &rls.UninstallReleaseResponse{Release: r.Release}, nil
 	}
-
+	return rlc.UninstallRelease(context.TODO(), &rls.UninstallReleaseRequest{Name: rlsName, DisableHooks: o.disableHooks})
-	return rlc.UninstallRelease(context.TODO(), &rls.UninstallReleaseRequest{Name: rlsName})
 }
 
 // Executes tiller.UpdateRelease RPC.