provider-kubernetes icon indicating copy to clipboard operation
provider-kubernetes copied to clipboard

Add ownerReference for dependsOn references

Open bobh66 opened this issue 2 years ago • 3 comments

Signed-off-by: Bob Haddleton [email protected]

Description of your changes

Add an ownerReference along with the finalizer for dependent resources

Fixes #46

I have:

  • [X] Read and followed Crossplane's contribution process.
  • [X] Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

Tested in a local cluster, using examples/references/depends-on-resource.yaml as a test sample.

This file creates a ConfigMap called "bar" and an Object called "foo" which creates another ConfigMap, also called "foo", with a dependsOn reference to "bar".

I confirmed that:

  • provider-kubernetes adds a finalizer to "bar":
finalizers:
  - kubernetes.crossplane.io/referred-by-object-d74a5845-2645-4931-9f7a-868814c1edb8
  • provider-kubernetes adds an ownerReference to the Object which points at "bar":
  ownerReferences:
  - apiVersion: v1
    blockOwnerDeletion: true
    kind: ConfigMap
    name: bar
    uid: aa019dcd-3045-44ea-9835-bf9643ce9d49
  • When "bar" is deleted using foreground cascading deletion, a foregroundDeletion finalizer is added to "bar":
  deletionGracePeriodSeconds: 0
  deletionTimestamp: "2022-09-25T02:33:42Z"
  finalizers:
  - kubernetes.crossplane.io/referred-by-object-d74a5845-2645-4931-9f7a-868814c1edb8
  - foregroundDeletion
  • Deleting "bar" with foreground cascading deletion will first delete the Object "foo", since it is "owned" by "bar", which deletes ConfigMap "foo" and removes the finalizer from "bar", and then "bar" is deleted.

So if we call "bar" the Database Server and "foo" the Database, we can make the Database dependOn the Database Server and ensure that the Database gets deleted before the Database Server.

bobh66 avatar Aug 09 '22 17:08 bobh66

This needs rework after further discussion on #46

bobh66 avatar Aug 09 '22 19:08 bobh66

@bobh66 IIUC, that only works when Object and dependencies are co-located in the same cluster. How would that be handled when it's a "remote" dependency? cc: @turkenh

morningspace avatar Sep 25 '22 08:09 morningspace

@morningspace If I'm reading this code correctly - https://github.com/crossplane-contrib/provider-kubernetes/blob/main/internal/controller/object/object.go#L393 - the provider only supports references for local resources - it always uses the local kubernetes client to resolve the referenced resource.

Support for remote references would be interesting - patchesFrom references would probably work using the specified providerConfig, and it would also be possible to put a finalizer on a remote resource, For a remote dependsOn reference the ownerReference would have to be placed on the remote kubernetes object specified in the manifest, which we could also do for local dependsOn references if that makes sense. Maybe in the local case the manifest object should always be "owned by" the Object which is then "owned by" the dependent resource. In the remote case the remote manifest object would be owned by the remote dependsOn reference. It would also be theoretically possibly to add a providerConfig to each reference, so you could pull patches from several remote clusters and the local cluster, and have dependencies in different clusters.

I don't know how we would determine local vs remote if we wanted to treat them differently.

I think this change is independent of that case as it doesn't appear to work for remote dependencies in the existing implementation.

Thanks!

bobh66 avatar Sep 25 '22 17:09 bobh66

@bobh66 is this PR still relevant? or should we close it?

turkenh avatar Aug 18 '23 07:08 turkenh

Closing in favor of the Usage resource in https://github.com/crossplane/crossplane/pull/4444

bobh66 avatar Aug 19 '23 19:08 bobh66