Can't use Kubernetes delete cascade orphan

[dacofr]

Hi 👋

With the current implementation of Fiaas, we cannot use :
kubectl delete fa bla-bla --cascade=orphan

Some objects like PDB, RoleBindings, SA are well orphaned after deletion (owner reference is removed and objects still remain clusters), but that's not the case for Deployment, Service, Ingress because there is an explicit deletion that is done here :

    def delete(self, app_spec: AppSpec):
        self._ingress_deployer.delete(app_spec)
        self._autoscaler_deployer.delete(app_spec)
        self._service_deployer.delete(app_spec)
        self._deployment_deployer.delete(app_spec)

https://github.com/fiaas/fiaas-deploy-daemon/blob/master/fiaas_deploy_daemon/deployer/kubernetes/adapter.py#L69

This made sense in the past, but now that each objects has an owner reference, we can let Kubernetes do the garbage collection for us.

I will send a PR to remove this explicit deletion, WDYT ?

Thanks

[perj]

Hi @dacofr!

I think that makes a lot of sense to support all three deletion modes.

If I'm reading the docs (also this one) correctly the controller (fiaas-deploy-daemon) should still delete the children in foreground mode.
Thus the code should check the finalizers of the application and still run the above code if it's set to foreground, but not run if it's unset. In orphan mode, it still needs to update the children to remove the ownerReference, I belive, I don't see anything in the docs about that being automatic. But you seem to imply it's working as expected so perhaps it's just not well documented.

When using foreground of orphan mode, it seems the controller is expected to update the Application object and remove those finalizers. This part also need to be implemented for it to work fully. But I think if the children still has the ownerReference when that happens, they'll still be automatically deleted. Do test that part though.

[perj]

Found some more info here that seems to verify what I said above.

[oyvindio]

As I understand the garbage collector documentation, as well as observed behavior is that garbage collection of resources where all owner references reference objects that no longer exist, is automatic. I understand the different cascade modes to work like

• foreground cascading deletion marks the resource as deleted by setting metadata.deletionTimestamp, then deletes any dependent resources, and then deletes the resource. This enables clients to wait for a resource with dependents to be properly deleted if needed.
• background cascading deletion deletes the resource, and any dependent resources will be deleted asynchronously/in the background.
• orphan deletion deletes the resource, and leaves any dependent resources in place.

Finalizers can be set on a resource by controllers (such as fiaas-deploy-daemon) to control its deletion, for example by blocking deletion of a resource until some external state is deleted, at which point the controller which set the finalizer initially should remove the finalizer to indicate to the garbage collector that the resource can safely be deleted. fiaas-deploy-daemon doesn't set any finalizers, so I don't think we need to consider those in this context.

It seems to me like both foreground and background cascading deletion for Application resources works like it should already.
Deleting Application resources with orphan deletion results in some dependent / managed resources being deleted by fiaas-deploy-daemon itself, as you say @dacofr. To support orphan deletion for Application resources properly, based on the above I think fiaas-deploy-daemon should leave these resources in place and rely on the garbage collector to delete them (in the case of cascading deletion) via their owner references.

There is a PR (#101) that attempted to make this change earlier which never landed, but as the PR is quite old and stale it may be difficult to continue from directly.

[perj]

Alright, I actually tested it now. Both foreground and orphan seems like it's working with simply commenting out the delete function. I guess I must be misunderstanding the docs somehow 🤷‍♂️

[dacofr]

Hello

@perj and @oyvindio, thanks both for your feedback 👍
Yes during my tests, simply commenting out the four lines of explicit deletion was enough to have all three modes of deletion to work as expected.

Didn't find this existing PR.
But @oyvindio, If you want, I can start a new fresh PR and do the same kind of stuff.

[dacofr]

In case of #262 is ready 😃