Default to deleting, not retaining, external resources.

Signed-off-by: Nic Cope <negz@rk0n.org>

apis/core/v1alpha1/resource.go

@@ -202,8 +202,9 @@ type ResourceSpec struct {
 	ProviderReference Reference `json:"providerRef"`
 
 	// DeletionPolicy specifies what will happen to the underlying external
-	// when this managed resource is deleted. The "Orphan" policy is used when
+	// when this managed resource is deleted - either "Delete" or "Orphan" the
-	// no policy is specified.
+	// external resource. The "Delete" policy is the default when no policy is
+	// specified.
 	//
 	// +optional
 	// +kubebuilder:validation:Enum=Orphan;Delete
@@ -217,7 +218,7 @@ type ResourceSpec struct {
 	// when its managed resource is deleted. The "Retain" policy causes the
 	// managed resource to be retained, in binding phase "Released", when its
 	// resource claim is deleted, and in turn causes the external resource to be
-	// retained when its managed resource is deleted. The "Retain" policy is
+	// retained when its managed resource is deleted. The "Delete" policy is
 	// used when no policy is specified.
 	//
 	// Deprecated. DeletionPolicy takes precedence when both are set.
@@ -257,8 +258,7 @@ type ClassSpecTemplate struct {
 	// policy causes the managed resource to be retained, in binding phase
 	// "Released", when its resource claim is deleted, and in turn causes the
 	// external resource to be retained when its managed resource is deleted.
-	// The "Retain" policy is used when no policy is specified, however the
+	// The "Delete" policy is used when no policy is specified.
-	// "Delete" policy is set at dynamic provisioning time if no policy is set.
 	// +optional
 	// +kubebuilder:validation:Enum=Retain;Delete
 	ReclaimPolicy ReclaimPolicy `json:"reclaimPolicy,omitempty"`

pkg/reconciler/managed/reconciler.go

@@ -700,11 +700,18 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
 // should be inlined when claims (and thus reclaim policies) are removed.
 func shouldDelete(mg resource.Managed) bool {
 	switch {
-	case mg.GetDeletionPolicy() == v1alpha1.DeletionDelete:
+	// The deletion policy should take precedence over the reclaim policy.
-		return true
 	case mg.GetDeletionPolicy() == v1alpha1.DeletionOrphan:
 		return false
+	case mg.GetDeletionPolicy() == v1alpha1.DeletionDelete:
+		return true
+
+	case mg.GetReclaimPolicy() == v1alpha1.ReclaimRetain:
+		return false
+	case mg.GetReclaimPolicy() == v1alpha1.ReclaimDelete:
+		return true
 	}
 
-	return mg.GetReclaimPolicy() == v1alpha1.ReclaimDelete
+	// If no policy is set, we default to deleting the resource.
+	return true
 }

pkg/reconciler/managed/reconciler_test.go

@@ -797,11 +797,11 @@ func TestShouldDelete(t *testing.T) {
 			want: false,
 		},
 		"NoPolicy": {
-			reason: "Resources should not be deleted when no deletion or reclaim policy is specified.",
+			reason: "Resources should be deleted when no deletion or reclaim policy is specified.",
 			mg: &fake.Managed{
 				Reclaimer: fake.Reclaimer{},
 			},
-			want: false,
+			want: true,
 		},
 	}