Fix `no RESOURCE with the name NAME found`

This is the fix for only one particular, but important case.

The case when a new resource has been added to the chart and
there is an error in the chart, which leads to release failure.
In this case after first failed release upgrade new resource will be
created in the cluster. On the next release upgrade there will be the error:
`no RESOURCE with the name NAME found` for this newly created resource
from the previous release upgrade.

The root of this problem is in the side effect of the first release process,
Release invariant says: if resouce exists in the kubernetes cluster, then
it should exist in the release storage. But this invariant has been broken
by helm itself -- because helm created new resources as side effect and not
adopted them into release storage.

To maintain release invariant for such case during release upgrade operation
all newly *successfully* created resources will be deleted in the case
of an error in the subsequent resources update.

This behaviour will be enabled only when `--cleanup-on-fail` option used
for `helm upgrade` or `helm rollback`.
Signed-off-by: Timofey Kirillov <timofey.kirillov@flant.com>

_proto/hapi/rudder/rudder.proto

@@ -92,6 +92,7 @@ message UpgradeReleaseRequest{
         bool Wait = 4;
         bool Recreate = 5;
         bool Force = 6;
+        bool CleanupOnFail = 7;
 }
 message UpgradeReleaseResponse{
 	hapi.release.Release release = 1;
@@ -105,6 +106,7 @@ message RollbackReleaseRequest{
         bool Wait = 4;
         bool Recreate = 5;
         bool Force = 6;
+        bool CleanupOnFail = 7;
 }
 message RollbackReleaseResponse{
 	hapi.release.Release release = 1;

_proto/hapi/services/tiller.proto

@@ -212,8 +212,10 @@ message UpdateReleaseRequest {
 	bool force = 11;
 	// Description, if set, will set the description for the updated release
 	string description = 12;
-        // Render subchart notes if enabled
+	// Render subchart notes if enabled
 	bool subNotes = 13;
+	// Allow deletion of new resources created in this update when update failed
+	bool cleanup_on_fail = 14;
 }
 
 // UpdateReleaseResponse is the response to an update request.
@@ -241,6 +243,8 @@ message RollbackReleaseRequest {
 	bool force = 8;
 	// Description, if set, will set the description for the rollback
 	string description = 9;
+	// Allow deletion of new resources created in this rollback when rollback failed
+	bool cleanup_on_fail = 10;
 }
 
 // RollbackReleaseResponse is the response to an update request.
@@ -283,8 +287,8 @@ message InstallReleaseRequest {
 
 	// Description, if set, will set the description for the installed release
 	string description = 11;
-  
-        bool subNotes = 12;
+
+	bool subNotes = 12;
 
 }
 

cmd/helm/rollback.go

@@ -36,17 +36,18 @@ second is a revision (version) number. To see revision numbers, run
 `
 
 type rollbackCmd struct {
-	name         string
-	revision     int32
-	dryRun       bool
-	recreate     bool
-	force        bool
-	disableHooks bool
-	out          io.Writer
-	client       helm.Interface
-	timeout      int64
-	wait         bool
-	description  string
+	name          string
+	revision      int32
+	dryRun        bool
+	recreate      bool
+	force         bool
+	disableHooks  bool
+	out           io.Writer
+	client        helm.Interface
+	timeout       int64
+	wait          bool
+	description   string
+	cleanupOnFail bool
 }
 
 func newRollbackCmd(c helm.Interface, out io.Writer) *cobra.Command {
@@ -87,6 +88,7 @@ func newRollbackCmd(c helm.Interface, out io.Writer) *cobra.Command {
 	f.Int64Var(&rollback.timeout, "timeout", 300, "time in seconds to wait for any individual Kubernetes operation (like Jobs for hooks)")
 	f.BoolVar(&rollback.wait, "wait", false, "if set, will wait until all Pods, PVCs, Services, and minimum number of Pods of a Deployment are in a ready state before marking the release as successful. It will wait for as long as --timeout")
 	f.StringVar(&rollback.description, "description", "", "specify a description for the release")
+	f.BoolVar(&rollback.cleanupOnFail, "cleanup-on-fail", false, "allow deletion of new resources created in this rollback when rollback failed")
 
 	// set defaults from environment
 	settings.InitTLS(f)
@@ -104,7 +106,8 @@ func (r *rollbackCmd) run() error {
 		helm.RollbackVersion(r.revision),
 		helm.RollbackTimeout(r.timeout),
 		helm.RollbackWait(r.wait),
-		helm.RollbackDescription(r.description))
+		helm.RollbackDescription(r.description),
+		helm.RollbackCleanupOnFail(r.cleanupOnFail))
 	if err != nil {
 		return prettyError(err)
 	}

cmd/helm/upgrade.go

@@ -84,34 +84,35 @@ which results in "pwd: 3jk$o2z=f\30with'quote".
 `
 
 type upgradeCmd struct {
-	release      string
-	chart        string
-	out          io.Writer
-	client       helm.Interface
-	dryRun       bool
-	recreate     bool
-	force        bool
-	disableHooks bool
-	valueFiles   valueFiles
-	values       []string
-	stringValues []string
-	fileValues   []string
-	verify       bool
-	keyring      string
-	install      bool
-	namespace    string
-	version      string
-	timeout      int64
-	resetValues  bool
-	reuseValues  bool
-	wait         bool
-	atomic       bool
-	repoURL      string
-	username     string
-	password     string
-	devel        bool
-	subNotes     bool
-	description  string
+	release       string
+	chart         string
+	out           io.Writer
+	client        helm.Interface
+	dryRun        bool
+	recreate      bool
+	force         bool
+	disableHooks  bool
+	valueFiles    valueFiles
+	values        []string
+	stringValues  []string
+	fileValues    []string
+	verify        bool
+	keyring       string
+	install       bool
+	namespace     string
+	version       string
+	timeout       int64
+	resetValues   bool
+	reuseValues   bool
+	wait          bool
+	atomic        bool
+	repoURL       string
+	username      string
+	password      string
+	devel         bool
+	subNotes      bool
+	description   string
+	cleanupOnFail bool
 
 	certFile string
 	keyFile  string
@@ -179,6 +180,7 @@ func newUpgradeCmd(client helm.Interface, out io.Writer) *cobra.Command {
 	f.BoolVar(&upgrade.devel, "devel", false, "use development versions, too. Equivalent to version '>0.0.0-0'. If --version is set, this is ignored.")
 	f.BoolVar(&upgrade.subNotes, "render-subchart-notes", false, "render subchart notes along with parent")
 	f.StringVar(&upgrade.description, "description", "", "specify the description to use for the upgrade, rather than the default")
+	f.BoolVar(&upgrade.cleanupOnFail, "cleanup-on-fail", false, "allow deletion of new resources created in this upgrade when upgrade failed")
 
 	f.MarkDeprecated("disable-hooks", "use --no-hooks instead")
 
@@ -273,7 +275,8 @@ func (u *upgradeCmd) run() error {
 		helm.ReuseValues(u.reuseValues),
 		helm.UpgradeSubNotes(u.subNotes),
 		helm.UpgradeWait(u.wait),
-		helm.UpgradeDescription(u.description))
+		helm.UpgradeDescription(u.description),
+		helm.UpgradeCleanupOnFail(u.cleanupOnFail))
 	if err != nil {
 		fmt.Fprintf(u.out, "UPGRADE FAILED\nROLLING BACK\nError: %v\n", prettyError(err))
 		if u.atomic {

cmd/rudder/rudder.go

@@ -131,7 +131,13 @@ func (r *ReleaseModuleServiceServer) RollbackRelease(ctx context.Context, in *ru
 	grpclog.Print("rollback")
 	c := bytes.NewBufferString(in.Current.Manifest)
 	t := bytes.NewBufferString(in.Target.Manifest)
-	err := kubeClient.Update(in.Target.Namespace, c, t, in.Force, in.Recreate, in.Timeout, in.Wait)
+	err := kubeClient.UpdateWithOptions(in.Target.Namespace, c, t, kube.UpdateOptions{
+		Force:         in.Force,
+		Recreate:      in.Recreate,
+		Timeout:       in.Timeout,
+		ShouldWait:    in.Wait,
+		CleanupOnFail: in.CleanupOnFail,
+	})
 	return &rudderAPI.RollbackReleaseResponse{}, err
 }
 
@@ -140,7 +146,13 @@ func (r *ReleaseModuleServiceServer) UpgradeRelease(ctx context.Context, in *rud
 	grpclog.Print("upgrade")
 	c := bytes.NewBufferString(in.Current.Manifest)
 	t := bytes.NewBufferString(in.Target.Manifest)
-	err := kubeClient.Update(in.Target.Namespace, c, t, in.Force, in.Recreate, in.Timeout, in.Wait)
+	err := kubeClient.UpdateWithOptions(in.Target.Namespace, c, t, kube.UpdateOptions{
+		Force:         in.Force,
+		Recreate:      in.Recreate,
+		Timeout:       in.Timeout,
+		ShouldWait:    in.Wait,
+		CleanupOnFail: in.CleanupOnFail,
+	})
 	// upgrade response object should be changed to include status
 	return &rudderAPI.UpgradeReleaseResponse{}, err
 }

docs/helm/helm_rollback.md

@@ -20,6 +20,7 @@ helm rollback [flags] [RELEASE] [REVISION]
 ### Options
 
 ```
+      --cleanup-on-fail       allow deletion of new resources created in this rollback when rollback failed
       --description string    specify a description for the release
       --dry-run               simulate a rollback
       --force                 force resource update through delete/recreate if needed
@@ -52,4 +53,4 @@ helm rollback [flags] [RELEASE] [REVISION]
 
 * [helm](helm.md)	 - The Helm package manager for Kubernetes.
 
-###### Auto generated by spf13/cobra on 29-Jan-2019
+###### Auto generated by spf13/cobra on 5-Feb-2019

docs/helm/helm_upgrade.md

@@ -68,6 +68,7 @@ helm upgrade [RELEASE] [CHART] [flags]
       --atomic                   if set, upgrade process rolls back changes made in case of failed upgrade, also sets --wait flag
       --ca-file string           verify certificates of HTTPS-enabled servers using this CA bundle
       --cert-file string         identify HTTPS client using this SSL certificate file
+      --cleanup-on-fail          allow deletion of new resources created in this upgrade when upgrade failed
       --description string       specify the description to use for the upgrade, rather than the default
       --devel                    use development versions, too. Equivalent to version '>0.0.0-0'. If --version is set, this is ignored.
       --dry-run                  simulate an upgrade
@@ -117,4 +118,4 @@ helm upgrade [RELEASE] [CHART] [flags]
 
 * [helm](helm.md)	 - The Helm package manager for Kubernetes.
 
-###### Auto generated by spf13/cobra on 28-Jan-2019
+###### Auto generated by spf13/cobra on 5-Feb-2019

pkg/helm/option.go

@@ -297,6 +297,20 @@ func DeleteDescription(description string) DeleteOption {
 	}
 }
 
+// UpgradeCleanupOnFail allows deletion of new resources created in this upgrade when upgrade failed
+func UpgradeCleanupOnFail(cleanupOnFail bool) UpdateOption {
+	return func(opts *options) {
+		opts.updateReq.CleanupOnFail = cleanupOnFail
+	}
+}
+
+// RollbackCleanupOnFail allows deletion of new resources created in this rollback when rollback failed
+func RollbackCleanupOnFail(cleanupOnFail bool) RollbackOption {
+	return func(opts *options) {
+		opts.rollbackReq.CleanupOnFail = cleanupOnFail
+	}
+}
+
 // DeleteDisableHooks will disable hooks for a deletion operation.
 func DeleteDisableHooks(disable bool) DeleteOption {
 	return func(opts *options) {

pkg/kube/client.go

@@ -290,13 +290,33 @@ func (c *Client) Get(namespace string, reader io.Reader) (string, error) {
 	return buf.String(), nil
 }
 
-// Update reads in the current configuration and a target configuration from io.reader
+// Deprecated; use UpdateWithOptions instead
+func (c *Client) Update(namespace string, originalReader, targetReader io.Reader, force bool, recreate bool, timeout int64, shouldWait bool) error {
+	return c.UpdateWithOptions(namespace, originalReader, targetReader, UpdateOptions{
+		Force:      force,
+		Recreate:   recreate,
+		Timeout:    timeout,
+		ShouldWait: shouldWait,
+	})
+}
+
+// UpdateOptions provides options to control update behavior
+type UpdateOptions struct {
+	Force      bool
+	Recreate   bool
+	Timeout    int64
+	ShouldWait bool
+	// Allow deletion of new resources created in this update when update failed
+	CleanupOnFail bool
+}
+
+// UpdateWithOptions reads in the current configuration and a target configuration from io.reader
 // and creates resources that don't already exists, updates resources that have been modified
 // in the target configuration and deletes resources from the current configuration that are
 // not present in the target configuration.
 //
 // Namespace will set the namespaces.
-func (c *Client) Update(namespace string, originalReader, targetReader io.Reader, force bool, recreate bool, timeout int64, shouldWait bool) error {
+func (c *Client) UpdateWithOptions(namespace string, originalReader, targetReader io.Reader, opts UpdateOptions) error {
 	original, err := c.BuildUnstructured(namespace, originalReader)
 	if err != nil {
 		return fmt.Errorf("failed decoding reader into objects: %s", err)
@@ -308,6 +328,7 @@ func (c *Client) Update(namespace string, originalReader, targetReader io.Reader
 		return fmt.Errorf("failed decoding reader into objects: %s", err)
 	}
 
+	newlyCreatedResources := []*resource.Info{}
 	updateErrors := []string{}
 
 	c.Log("checking %d resources for changes", len(target))
@@ -326,6 +347,7 @@ func (c *Client) Update(namespace string, originalReader, targetReader io.Reader
 			if err := createResource(info); err != nil {
 				return fmt.Errorf("failed to create resource: %s", err)
 			}
+			newlyCreatedResources = append(newlyCreatedResources, info)
 
 			kind := info.Mapping.GroupVersionKind.Kind
 			c.Log("Created a new %s called %q\n", kind, info.Name)
@@ -338,7 +360,7 @@ func (c *Client) Update(namespace string, originalReader, targetReader io.Reader
 			return fmt.Errorf("no %s with the name %q found", kind, info.Name)
 		}
 
-		if err := updateResource(c, info, originalInfo.Object, force, recreate); err != nil {
+		if err := updateResource(c, info, originalInfo.Object, opts.Force, opts.Recreate); err != nil {
 			c.Log("error updating the resource %q:\n\t %v", info.Name, err)
 			updateErrors = append(updateErrors, err.Error())
 		}
@@ -346,11 +368,27 @@ func (c *Client) Update(namespace string, originalReader, targetReader io.Reader
 		return nil
 	})
 
+	cleanupErrors := []string{}
+
+	if opts.CleanupOnFail {
+		if err != nil || len(updateErrors) != 0 {
+			for _, info := range newlyCreatedResources {
+				kind := info.Mapping.GroupVersionKind.Kind
+
+				c.Log("Deleting newly created %s with the name %q in %s...", kind, info.Name, info.Namespace)
+				if err := deleteResource(info); err != nil {
+					c.Log("Error deleting newly created %s with the name %q in %s: %s", kind, info.Name, info.Namespace, err)
+					cleanupErrors = append(cleanupErrors, err.Error())
+				}
+			}
+		}
+	}
+
 	switch {
 	case err != nil:
-		return err
+		return fmt.Errorf(strings.Join(append([]string{err.Error()}, cleanupErrors...), " && "))
 	case len(updateErrors) != 0:
-		return fmt.Errorf(strings.Join(updateErrors, " && "))
+		return fmt.Errorf(strings.Join(append(updateErrors, cleanupErrors...), " && "))
 	}
 
 	for _, info := range original.Difference(target) {
@@ -373,8 +411,8 @@ func (c *Client) Update(namespace string, originalReader, targetReader io.Reader
 			c.Log("Failed to delete %q, err: %s", info.Name, err)
 		}
 	}
-	if shouldWait {
-		return c.waitForResources(time.Duration(timeout)*time.Second, target)
+	if opts.ShouldWait {
+		return c.waitForResources(time.Duration(opts.Timeout)*time.Second, target)
 	}
 	return nil
 }

pkg/tiller/environment/environment.go

@@ -126,14 +126,17 @@ type KubeClient interface {
 	// error.
 	WatchUntilReady(namespace string, reader io.Reader, timeout int64, shouldWait bool) error
 
-	// Update updates one or more resources or creates the resource
+	// Deprecated; use UpdateWithOptions instead
+	Update(namespace string, originalReader, modifiedReader io.Reader, force bool, recreate bool, timeout int64, shouldWait bool) error
+
+	// UpdateWithOptions updates one or more resources or creates the resource
 	// if it doesn't exist.
 	//
 	// namespace must contain a valid existing namespace.
 	//
 	// reader must contain a YAML stream (one or more YAML documents separated
 	// by "\n---\n").
-	Update(namespace string, originalReader, modifiedReader io.Reader, force bool, recreate bool, timeout int64, shouldWait bool) error
+	UpdateWithOptions(namespace string, originalReader, modifiedReader io.Reader, opts kube.UpdateOptions) error
 
 	Build(namespace string, reader io.Reader) (kube.Result, error)
 	BuildUnstructured(namespace string, reader io.Reader) (kube.Result, error)
@@ -177,6 +180,16 @@ func (p *PrintingKubeClient) WatchUntilReady(ns string, r io.Reader, timeout int
 
 // Update implements KubeClient Update.
 func (p *PrintingKubeClient) Update(ns string, currentReader, modifiedReader io.Reader, force bool, recreate bool, timeout int64, shouldWait bool) error {
+	return p.UpdateWithOptions(ns, currentReader, modifiedReader, kube.UpdateOptions{
+		Force:      force,
+		Recreate:   recreate,
+		Timeout:    timeout,
+		ShouldWait: shouldWait,
+	})
+}
+
+// UpdateWithOptions implements KubeClient UpdateWithOptions.
+func (p *PrintingKubeClient) UpdateWithOptions(ns string, currentReader, modifiedReader io.Reader, opts kube.UpdateOptions) error {
 	_, err := io.Copy(p.Out, modifiedReader)
 	return err
 }

pkg/tiller/environment/environment_test.go

@@ -52,6 +52,9 @@ func (k *mockKubeClient) Delete(ns string, r io.Reader) error {
 func (k *mockKubeClient) Update(ns string, currentReader, modifiedReader io.Reader, force bool, recreate bool, timeout int64, shouldWait bool) error {
 	return nil
 }
+func (k *mockKubeClient) UpdateWithOptions(ns string, currentReader, modifiedReader io.Reader, opts kube.UpdateOptions) error {
+	return nil
+}
 func (k *mockKubeClient) WatchUntilReady(ns string, r io.Reader, timeout int64, shouldWait bool) error {
 	return nil
 }

pkg/tiller/release_modules.go

@@ -58,14 +58,26 @@ func (m *LocalReleaseModule) Create(r *release.Release, req *services.InstallRel
 func (m *LocalReleaseModule) Update(current, target *release.Release, req *services.UpdateReleaseRequest, env *environment.Environment) error {
 	c := bytes.NewBufferString(current.Manifest)
 	t := bytes.NewBufferString(target.Manifest)
-	return env.KubeClient.Update(target.Namespace, c, t, req.Force, req.Recreate, req.Timeout, req.Wait)
+	return env.KubeClient.UpdateWithOptions(target.Namespace, c, t, kube.UpdateOptions{
+		Force:         req.Force,
+		Recreate:      req.Recreate,
+		Timeout:       req.Timeout,
+		ShouldWait:    req.Wait,
+		CleanupOnFail: req.CleanupOnFail,
+	})
 }
 
 // Rollback performs a rollback from current to target release
 func (m *LocalReleaseModule) Rollback(current, target *release.Release, req *services.RollbackReleaseRequest, env *environment.Environment) error {
 	c := bytes.NewBufferString(current.Manifest)
 	t := bytes.NewBufferString(target.Manifest)
-	return env.KubeClient.Update(target.Namespace, c, t, req.Force, req.Recreate, req.Timeout, req.Wait)
+	return env.KubeClient.UpdateWithOptions(target.Namespace, c, t, kube.UpdateOptions{
+		Force:         req.Force,
+		Recreate:      req.Recreate,
+		Timeout:       req.Timeout,
+		ShouldWait:    req.Wait,
+		CleanupOnFail: req.CleanupOnFail,
+	})
 }
 
 // Status returns kubectl-like formatted status of release objects

pkg/tiller/release_server_test.go

@@ -500,6 +500,15 @@ type updateFailingKubeClient struct {
 }
 
 func (u *updateFailingKubeClient) Update(namespace string, originalReader, modifiedReader io.Reader, force bool, recreate bool, timeout int64, shouldWait bool) error {
+	return u.UpdateWithOptions(namespace, originalReader, modifiedReader, kube.UpdateOptions{
+		Force:      force,
+		Recreate:   recreate,
+		Timeout:    timeout,
+		ShouldWait: shouldWait,
+	})
+}
+
+func (u *updateFailingKubeClient) UpdateWithOptions(namespace string, originalReader, modifiedReader io.Reader, opts kube.UpdateOptions) error {
 	return errors.New("Failed update in kube client")
 }
 
@@ -632,6 +641,9 @@ func (kc *mockHooksKubeClient) WatchUntilReady(ns string, r io.Reader, timeout i
 func (kc *mockHooksKubeClient) Update(ns string, currentReader, modifiedReader io.Reader, force bool, recreate bool, timeout int64, shouldWait bool) error {
 	return nil
 }
+func (kc *mockHooksKubeClient) UpdateWithOptions(ns string, currentReader, modifiedReader io.Reader, opts kube.UpdateOptions) error {
+	return nil
+}
 func (kc *mockHooksKubeClient) Build(ns string, reader io.Reader) (kube.Result, error) {
 	return []*resource.Info{}, nil
 }