diff --git a/apis/core/v1alpha1/policies.go b/apis/core/v1alpha1/policies.go index 2f159c0..a85a65a 100644 --- a/apis/core/v1alpha1/policies.go +++ b/apis/core/v1alpha1/policies.go @@ -30,3 +30,17 @@ const ( // will be retained when the managed resource is deleted. ReclaimRetain ReclaimPolicy = "Retain" ) + +// A DeletionPolicy determines what should happen to the underlying external +// resource when a managed resource is deleted. +type DeletionPolicy string + +const ( + // DeletionOrphan means the external resource will orphaned when its managed + // resource is deleted. + DeletionOrphan DeletionPolicy = "Orphan" + + // DeletionDelete means both the external resource will be deleted when its + // managed resource is deleted. + DeletionDelete DeletionPolicy = "Delete" +) diff --git a/apis/core/v1alpha1/resource.go b/apis/core/v1alpha1/resource.go index dc9e2de..3c77c43 100644 --- a/apis/core/v1alpha1/resource.go +++ b/apis/core/v1alpha1/resource.go @@ -201,6 +201,14 @@ type ResourceSpec struct { // observe, update, and delete this managed resource. 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 + // no policy is specified. + // + // +optional + // +kubebuilder:validation:Enum=Orphan;Delete + DeletionPolicy DeletionPolicy `json:"deletionPolicy,omitempty"` + // ReclaimPolicy specifies what will happen to this managed resource when // its resource claim is deleted, and what will happen to the underlying // external resource when the managed resource is deleted. The "Delete" @@ -211,6 +219,10 @@ type ResourceSpec struct { // 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. + // + // Deprecated. DeletionPolicy takes precedence when both are set. + // See https://github.com/crossplane/crossplane-runtime/issues/179. + // // +optional // +kubebuilder:validation:Enum=Retain;Delete ReclaimPolicy ReclaimPolicy `json:"reclaimPolicy,omitempty"` diff --git a/pkg/reconciler/managed/reconciler.go b/pkg/reconciler/managed/reconciler.go index d5a6a28..7f83cca 100644 --- a/pkg/reconciler/managed/reconciler.go +++ b/pkg/reconciler/managed/reconciler.go @@ -545,7 +545,7 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error) if meta.WasDeleted(managed) { log = log.WithValues("deletion-timestamp", managed.GetDeletionTimestamp()) - if observation.ResourceExists && managed.GetReclaimPolicy() == v1alpha1.ReclaimDelete { + if observation.ResourceExists && shouldDelete(managed) { if err := external.Delete(externalCtx, managed); err != nil { // We'll hit this condition if we can't delete our external // resource, for example if our provider credentials don't have @@ -695,3 +695,16 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error) managed.SetConditions(v1alpha1.ReconcileSuccess()) return reconcile.Result{RequeueAfter: r.longWait}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) } + +// TODO(negz): ReclaimPolicy will be deprecated alongside resource claims; this +// should be inlined when claims (and thus reclaim policies) are removed. +func shouldDelete(mg resource.Managed) bool { + switch { + case mg.GetDeletionPolicy() == v1alpha1.DeletionDelete: + return true + case mg.GetDeletionPolicy() == v1alpha1.DeletionOrphan: + return false + } + + return mg.GetReclaimPolicy() == v1alpha1.ReclaimDelete +} diff --git a/pkg/reconciler/managed/reconciler_test.go b/pkg/reconciler/managed/reconciler_test.go index 537d78a..81deaf6 100644 --- a/pkg/reconciler/managed/reconciler_test.go +++ b/pkg/reconciler/managed/reconciler_test.go @@ -759,3 +759,58 @@ func TestReconciler(t *testing.T) { }) } } + +func TestShouldDelete(t *testing.T) { + cases := map[string]struct { + reason string + mg resource.Managed + want bool + }{ + "DeletionPolicyDelete": { + reason: "The delete deletion policy should take precedence over the reclaim policy.", + mg: &fake.Managed{ + Deletable: fake.Deletable{Policy: v1alpha1.DeletionDelete}, + Reclaimer: fake.Reclaimer{Policy: v1alpha1.ReclaimRetain}, + }, + want: true, + }, + "DeletionPolicyOrphan": { + reason: "The orphan deletion policy should take precedence over the reclaim policy.", + mg: &fake.Managed{ + Deletable: fake.Deletable{Policy: v1alpha1.DeletionOrphan}, + Reclaimer: fake.Reclaimer{Policy: v1alpha1.ReclaimDelete}, + }, + want: false, + }, + "ReclaimPolicyDelete": { + reason: "The delete reclaim policy should take effect when no deletion policy exists.", + mg: &fake.Managed{ + Reclaimer: fake.Reclaimer{Policy: v1alpha1.ReclaimDelete}, + }, + want: true, + }, + "ReclaimPolicyRetain": { + reason: "The retain reclaim policy should take effect when no deletion policy exists.", + mg: &fake.Managed{ + Reclaimer: fake.Reclaimer{Policy: v1alpha1.ReclaimRetain}, + }, + want: false, + }, + "NoPolicy": { + reason: "Resources should not be deleted when no deletion or reclaim policy is specified.", + mg: &fake.Managed{ + Reclaimer: fake.Reclaimer{}, + }, + want: false, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + got := shouldDelete(tc.mg) + if diff := cmp.Diff(tc.want, got); diff != "" { + t.Errorf("\nReason: %s\nshouldDelete(...): -want, +got:\n%s", tc.reason, diff) + } + }) + } +} diff --git a/pkg/resource/fake/mocks.go b/pkg/resource/fake/mocks.go index 0e8d9f7..dcd75b8 100644 --- a/pkg/resource/fake/mocks.go +++ b/pkg/resource/fake/mocks.go @@ -135,6 +135,15 @@ func (m *Reclaimer) SetReclaimPolicy(p v1alpha1.ReclaimPolicy) { m.Policy = p } // GetReclaimPolicy gets the ReclaimPolicy. func (m *Reclaimer) GetReclaimPolicy() v1alpha1.ReclaimPolicy { return m.Policy } +// Deletable implements the Deletable interface. +type Deletable struct{ Policy v1alpha1.DeletionPolicy } + +// SetDeletionPolicy sets the DeletionPolicy. +func (m *Deletable) SetDeletionPolicy(p v1alpha1.DeletionPolicy) { m.Policy = p } + +// GetDeletionPolicy gets the DeletionPolicy. +func (m *Deletable) GetDeletionPolicy() v1alpha1.DeletionPolicy { return m.Policy } + // CredentialsSecretReferencer is a mock that satisfies CredentialsSecretReferencer // interface. type CredentialsSecretReferencer struct{ Ref v1alpha1.SecretKeySelector } @@ -272,6 +281,7 @@ type Managed struct { ClaimReferencer ProviderReferencer ConnectionSecretWriterTo + Deletable Reclaimer v1alpha1.ConditionedStatus v1alpha1.BindingStatus diff --git a/pkg/resource/interfaces.go b/pkg/resource/interfaces.go index afc54fc..caa4a36 100644 --- a/pkg/resource/interfaces.go +++ b/pkg/resource/interfaces.go @@ -84,6 +84,12 @@ type Reclaimer interface { GetReclaimPolicy() v1alpha1.ReclaimPolicy } +// A Deletable may specify a DeletionPolicy. +type Deletable interface { + SetDeletionPolicy(p v1alpha1.DeletionPolicy) + GetDeletionPolicy() v1alpha1.DeletionPolicy +} + // A CredentialsSecretReferencer may refer to a credential secret in an arbitrary // namespace. type CredentialsSecretReferencer interface { @@ -187,6 +193,7 @@ type Managed interface { ClaimReferencer ProviderReferencer ConnectionSecretWriterTo + Deletable Reclaimer Conditioned