Use background delete by default; register deletes of CRs correctly

pkg/await/await.go

@@ -67,10 +67,6 @@ func Creation(
 	var clientForResource dynamic.ResourceInterface
 	err := sleepingRetry(
 		func(i uint) error {
-			if i > 0 {
-				_ = host.LogStatus(ctx, diag.Info, urn, fmt.Sprintf("Creation failed, retrying (%d)", i))
-			}
-
 			// Recreate the client for resource, in case the client's cache of the server API was
 			// invalidated. For example, when a CRD is created, it will invalidate the client cache;
 			// this allows CRs that we tried (and failed) to create before to re-try with the new
@@ -83,6 +79,9 @@ func Creation(
 				}
 			}
 			outputs, err = clientForResource.Create(inputs)
+			if err != nil {
+				_ = host.LogStatus(ctx, diag.Info, urn, fmt.Sprintf("Retry #%d; creation failed: %v", i, err))
+			}
 			return err
 		}).
 		WithMaxRetries(5).
@@ -91,6 +90,7 @@ func Creation(
 	if err != nil {
 		return nil, err
 	}
+	_ = clearStatus(ctx, host, urn)
 
 	// Wait until create resolves as success or error. Note that the conditional is set up to log
 	// only if we don't have an entry for the resource type; in the event that we do, but the await
@@ -336,6 +336,16 @@ func Deletion(
 		// 1.6.x option. Background delete propagation is broken in k8s v1.6.
 		fg := metav1.DeletePropagationForeground
 		deleteOpts.PropagationPolicy = &fg
+	} else {
+		// > 1.7.x. Prior to 1.9.x, the default is to orphan children[1]. Our kubespy experiments
+		// with 1.9.11 show that the controller will actually _still_ mark these resources with the
+		// `orphan` finalizer, although it appears to actually do background delete correctly. We
+		// therefore set it to background manually, just to be safe.
+		//
+		// nolint
+		// [1] https://kubernetes.io/docs/concepts/workloads/controllers/garbage-collection/#setting-the-cascading-deletion-policy
+		bg := metav1.DeletePropagationBackground
+		deleteOpts.PropagationPolicy = &bg
 	}
 
 	// Obtain client for the resource being deleted.
@@ -353,7 +363,7 @@ func Deletion(
 	// Set up a watcher for the selected resource.
 	watcher, err := clientForResource.Watch(listOpts)
 	if err != nil {
-		return err
+		return nilIfGVKDeleted(err)
 	}
 
 	// Issue deletion request.
@@ -370,13 +380,12 @@ func Deletion(
 	if awaiter, exists := awaiters[id]; exists && awaiter.awaitDeletion != nil {
 		waitErr = awaiter.awaitDeletion(ctx, clientForResource, name)
 	} else {
-		_ = host.LogStatus(ctx, diag.Info, urn, fmt.Sprintf("Waiting for deletion of %s '%s'", id, name))
-
 		for {
 			select {
 			case event, ok := <-watcher.ResultChan():
 				if !ok {
 					if deleted, obj := checkIfResourceDeleted(name, clientForResource); deleted {
+						_ = clearStatus(ctx, host, urn)
 						return nil
 					} else {
 						return &timeoutError{
@@ -388,9 +397,11 @@ func Deletion(
 
 				switch event.Type {
 				case watch.Deleted:
+					_ = clearStatus(ctx, host, urn)
 					return nil
 				case watch.Error:
 					if deleted, obj := checkIfResourceDeleted(name, clientForResource); deleted {
+						_ = clearStatus(ctx, host, urn)
 						return nil
 					} else {
 						return &initializationError{
@@ -403,6 +414,7 @@ func Deletion(
 				watcher.Stop()
 				glog.V(3).Infof("Received error deleting object '%s': %#v", id, err)
 				if deleted, obj := checkIfResourceDeleted(name, clientForResource); deleted {
+					_ = clearStatus(ctx, host, urn)
 					return nil
 				} else {
 					return &cancellationError{
@@ -426,3 +438,8 @@ func checkIfResourceDeleted(name string, client dynamic.ResourceInterface) (bool
 
 	return false, obj
 }
+
+// clearStatus will clear the `Info` column of the CLI of all statuses and messages.
+func clearStatus(context context.Context, host *provider.HostClient, urn resource.URN) error {
+	return host.LogStatus(context, diag.Info, urn, "")
+}

pkg/await/retry.go

@@ -2,6 +2,8 @@ package await
 
 import (
 	"time"
+
+	"k8s.io/apimachinery/pkg/api/errors"
 )
 
 type retrier struct {
@@ -39,7 +41,7 @@ func (r *retrier) Do() error {
 	for r.tries <= r.maxRetries {
 		err = r.try(r.tries)
 		r.tries++
-		if err != nil {
+		if errors.IsNotFound(err) {
 			r.sleep(r.waitTime)
 		} else {
 			break

pkg/await/retry_test.go

@@ -1,11 +1,12 @@
 package await
 
 import (
-	"fmt"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func mockRetrier(try func(uint) error) *retrier {
@@ -19,6 +20,12 @@ func mockRetrier(try func(uint) error) *retrier {
 	}
 }
 
+func notFound(msg string) error {
+	return &errors.StatusError{ErrStatus: metav1.Status{
+		Reason:  metav1.StatusReasonNotFound,
+		Message: msg}}
+}
+
 func Test_Retrier(t *testing.T) {
 	tests := []struct {
 		description   string
@@ -44,7 +51,7 @@ func Test_Retrier(t *testing.T) {
 			retrier: mockRetrier(
 				func(i uint) error {
 					if i == 0 {
-						return fmt.Errorf("Operation failed")
+						return notFound("Operation failed")
 					}
 					return nil
 				}).
@@ -55,10 +62,10 @@ func Test_Retrier(t *testing.T) {
 		},
 		{
 			description: "Should fail if retry budget exceeded",
-			retrier: mockRetrier(func(uint) error { return fmt.Errorf("Operation failed") }).
+			retrier: mockRetrier(func(uint) error { return notFound("Operation failed") }).
 				WithMaxRetries(3).
 				WithBackoffFactor(2),
-			err:           fmt.Errorf("Operation failed"),
+			err:           notFound("Operation failed"),
 			tries:         4,
 			finalWaitTime: 16 * time.Second,
 		},

pkg/client/util.go

@@ -20,6 +20,7 @@ import (
 
 	"github.com/golang/glog"
 
+	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -115,5 +116,8 @@ func serverResourceForGVK(
 		}
 	}
 
-	return nil, fmt.Errorf("Server is unable to handle %s", gvk)
+	se := &errors.StatusError{ErrStatus: metav1.Status{
+		Reason:  metav1.StatusReasonNotFound,
+		Message: fmt.Sprintf("Server does not support kind: %s", gvk)}}
+	return nil, se
 }