From 1ff5499be754283343732ce49d0ae388da0e4608 Mon Sep 17 00:00:00 2001 From: Matt Butcher Date: Mon, 18 Jul 2016 15:13:25 -0600 Subject: [PATCH] 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 | 60 ++++++++++++++-------- cmd/helm/delete_test.go | 52 +++++++++++++++++++ cmd/helm/get.go | 6 +-- cmd/helm/get_hooks.go | 4 +- cmd/helm/get_manifest.go | 2 +- cmd/helm/get_values.go | 4 +- cmd/helm/helm.go | 9 ++-- cmd/helm/helm_test.go | 5 +- cmd/helm/install.go | 26 ++-------- cmd/helm/list.go | 2 +- cmd/helm/status.go | 2 +- cmd/tiller/release_server_test.go | 1 - pkg/helm/client.go | 6 +-- pkg/helm/compat.go | 85 ------------------------------- pkg/helm/interface.go | 6 --- pkg/helm/option.go | 47 +++++++++++------ 16 files changed, 146 insertions(+), 171 deletions(-) create mode 100644 cmd/helm/delete_test.go delete mode 100644 pkg/helm/compat.go diff --git a/cmd/helm/delete.go b/cmd/helm/delete.go index d28a04dd6..0d42032c1 100644 --- a/cmd/helm/delete.go +++ b/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{ - 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, + out io.Writer + client helm.Interface } -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") +func newDeleteCmd(c helm.Interface, out io.Writer) *cobra.Command { + del := &deleteCmd{ + out: out, + client: c, } - _, err := helm.UninstallRelease(args[0], deleteDryRun) - if err != nil { - return prettyError(err) + 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, + PersistentPreRunE: setupConnection, + RunE: func(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") - return nil + return cmd +} + +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) } diff --git a/cmd/helm/delete_test.go b/cmd/helm/delete_test.go new file mode 100644 index 000000000..b67afe579 --- /dev/null +++ b/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) + }) +} diff --git a/cmd/helm/get.go b/cmd/helm/get.go index c873f891e..c98975148 100644 --- a/cmd/helm/get.go +++ b/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)) } diff --git a/cmd/helm/get_hooks.go b/cmd/helm/get_hooks.go index 13547f034..f880bf789 100644 --- a/cmd/helm/get_hooks.go +++ b/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, diff --git a/cmd/helm/get_manifest.go b/cmd/helm/get_manifest.go index f3b9679bd..f37bafbe4 100644 --- a/cmd/helm/get_manifest.go +++ b/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() }, diff --git a/cmd/helm/get_values.go b/cmd/helm/get_values.go index ea0350e15..bd06e7fb2 100644 --- a/cmd/helm/get_values.go +++ b/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, diff --git a/cmd/helm/helm.go b/cmd/helm/helm.go index ba188ee29..3a089ced9 100644 --- a/cmd/helm/helm.go +++ b/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. diff --git a/cmd/helm/helm_test.go b/cmd/helm/helm_test.go index 34c686192..a07f43e7a 100644 --- a/cmd/helm/helm_test.go +++ b/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()) { diff --git a/cmd/helm/install.go b/cmd/helm/install.go index c5b51b17f..4de7a7c80 100644 --- a/cmd/helm/install.go +++ b/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 { - 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)) + res, err := i.client.InstallRelease(i.chartPath, helm.ValueOverrides(rawVals), helm.ReleaseName(i.name), helm.InstallDryRun(i.dryRun), helm.InstallDisableHooks(i.disableHooks)) if err != nil { return prettyError(err) } diff --git a/cmd/helm/list.go b/cmd/helm/list.go index 7314a4a22..058932766 100644 --- a/cmd/helm/list.go +++ b/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() }, diff --git a/cmd/helm/status.go b/cmd/helm/status.go index b60748dea..3b522644f 100644 --- a/cmd/helm/status.go +++ b/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() }, diff --git a/cmd/tiller/release_server_test.go b/cmd/tiller/release_server_test.go index 71e5a8a20..c80b8eb88 100644 --- a/cmd/tiller/release_server_test.go +++ b/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 diff --git a/pkg/helm/client.go b/pkg/helm/client.go index be011a928..b1efcac2d 100644 --- a/pkg/helm/client.go +++ b/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. diff --git a/pkg/helm/compat.go b/pkg/helm/compat.go deleted file mode 100644 index d539cf787..000000000 --- a/pkg/helm/compat.go +++ /dev/null @@ -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) -} diff --git a/pkg/helm/interface.go b/pkg/helm/interface.go index e6aba2a34..4cba4db43 100644 --- a/pkg/helm/interface.go +++ b/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 -} diff --git a/pkg/helm/option.go b/pkg/helm/option.go index ebc6dfab5..fd34cc4d9 100644 --- a/pkg/helm/option.go +++ b/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}) + return rlc.UninstallRelease(context.TODO(), &rls.UninstallReleaseRequest{Name: rlsName, DisableHooks: o.disableHooks}) } // Executes tiller.UpdateRelease RPC.