federation: Support cascading deletion #33612

nikhiljindal · 2016-09-27T22:31:32Z

Problem: There are 2 related problems:

When user deletes a federated object, corresponding objects created in underlying objects are not deleted. For ex: if user creates a federated service, we create a corresponding service in each cluster. When the federated service is deleted, these services in underlying clusters does not get deleted automatically.
When a cluster is removed from federation, objects that the federation control plane created in that cluster are not deleted. For ex: if user creates a federated service, we create a corresponding service in each cluster. When a cluster is removed from federation, the service corresponding to the federated service in that cluster does not get deleted automatically.

Now that we have cascading deletion in kubernetes, we should support cascading deletion in federation as well. As in kubernetes, we will give users the option to disable it.

cc @kubernetes/sig-cluster-federation

nikhiljindal · 2016-09-28T00:10:49Z

We have 2 options:

Use kubernetes Garbage Collector model; Or
Let each controller delete the corresponding dependent objects

Garbage collector model

How it works in kubernetes

There is a separate Garbage Collector controller whose job is to ensure that all dependent objects are deleted when the "owner" object is deleted.
It watches for all resources and if it finds an object for which all objects referred by its ObjectMeta.OwnerReference do not exist, then it deletes that object.

Work required to extend it for federation:

Garbage collector today assumes that a single apiserver knows about all resources. We will need to make it aware of cross cluster dependencies and how to fetch resources from multiple clusters.
Will need to extend OwnerReference to represent cross cluster references.
To prevent conflict with the kubernetes garbage collector, will need to limit the k8s garbage collector to objects with OwnerReferences in the same cluster, and limit federation garbage collector to objects with OwnerReferences in federation control plane.

Pros:

Centralized cascading deletion logic, rather than spread across controllers.

Cons

Requires an update to k8s garbage collector. federation garbage collector will conflict with cluster gc, if its not the latest.

Let each controller delete the corresponding dependent objects

How will this work

When a controller (for ex: the federation replica set controller) observes a new federated object (federated replicaset), it first adds a finalizer to it before creating any dependent objects (replicasets in underlying clusters).
When a federated object is deleted, federation-apiserver will just set the deletion timestamp instead of deleting it if the object has a finalizer.
When the controller sees an object with deletion timestamp set, it will delete all dependent objects it had created for that federated object and then delete the federated object after removing the finalizer.

Pros:

The controller that is creating the dependent objects is responsible for deleting the same objects.
These federation controllers are already cross-cluster resources aware.
No conflict with k8s garbage collector.

Cons:

Deletion logic is spread across controllers.

nikhiljindal · 2016-09-28T00:12:10Z

Based on discussion with @caesarxuchao and @lavalamp, the second model seems better.

The second model is similar to how namespace deletion works in kubernetes.

lavalamp · 2016-09-28T00:33:52Z

I think the second model is workable.

The first model seems to have dependencies going the wrong way at first
glance.

On Tue, Sep 27, 2016 at 5:12 PM, Nikhil Jindal notifications@github.com
wrote:

Based on discussion with @caesarxuchao https://github.com/caesarxuchao
and @lavalamp https://github.com/lavalamp, the second model seems
better.

The second model is similar to how namespace deletion works in kubernetes.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#33612 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAngliHHOTFDBX0pZ16k_lpB-8NmFUwmks5qubDkgaJpZM4KIOjF
.

ghost · 2016-09-28T00:39:08Z

Yes, agree. The second model seems much better/simpler/more reliable/less
prone to terrible failures.

On Tue, Sep 27, 2016 at 5:34 PM, Daniel Smith notifications@github.com
wrote:

I think the second model is workable.

The first model seems to have dependencies going the wrong way at first
glance.

On Tue, Sep 27, 2016 at 5:12 PM, Nikhil Jindal notifications@github.com
wrote:

Based on discussion with @caesarxuchao https://github.com/caesarxuchao
and @lavalamp https://github.com/lavalamp, the second model seems
better.

The second model is similar to how namespace deletion works in
kubernetes.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<https://github.com/kubernetes/kubernetes/issues/
33612#issuecomment-250035528>,
or mute the thread
<https://github.com/notifications/unsubscribe-
auth/AAngliHHOTFDBX0pZ16k_lpB-8NmFUwmks5qubDkgaJpZM4KIOjF>
.

—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub
#33612 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AJ6NASmz24nm1v3_lxVBaLslz0Mfbvlxks5qubYigaJpZM4KIOjF
.

caesarxuchao · 2016-09-28T02:28:58Z

Some notes from our offline discussion: the k8s garbage collector is designed to handle dependencies among arbitrary resources. In the current federation use cases, the dependency is between an federation object and its k8s counterpart, so a generic garbage collector is an overkill. The second model is much easier, and the deletion logic spread in each controller is manageable.

@caesarxuchao

Automatic merge from submit-queue Adding cascading deletion support to federated namespaces Ref #33612 With this change, whenever a federated namespace is deleted with `DeleteOptions.OrphanDependents = false`, then federation namespace controller first deletes the corresponding namespaces from all underlying clusters before deleting the federated namespace. cc @kubernetes/sig-cluster-federation @caesarxuchao ```release-note Adding support for DeleteOptions.OrphanDependents for federated namespaces. Setting it to false while deleting a federated namespace also deletes the corresponding namespace from all registered clusters. ```

nikhiljindal · 2016-11-03T18:15:34Z

Code for cascading deletion is now merged for namespaces. And now that we have the generic DeletionHelper class it should be relatively easy to extend it for all other federation resources which is what I am going to do next.
I am also refactoring namespaced resources deletion code to share the same common code in federation and kube namespace controllers: #35950
That fixes the first problem.

Cascading deletion for clusters (second problem in the original description of this issue) is a bit different. This involves deleting all resources from a kube cluster that were created by federation controllers when the cluster is removed from federation. Its different because now we are deleting different kind of resources than the one that was deleted by the user. The way I am planning to implement this is by cluster controller adding a finalizer to the cluster whenever it sees a new cluster. When user deletes a cluster, apiserver sets the deletion timestamp and adds the OrphanFinalizer depending on DeleteOptions. Cluster controller can then do the following for all clusters that have deletion timespace set:

It checks if it has Orphan finalizer. If yes then it removes both the finalizers and deletes the cluster. Job done.
If not, then cluster controller needs to ensure that all resources created by federation controllers are deleted from that cluster before removing its finalizer and deleting the cluster.

To track all resources created by federation controllers, I will update federation controllers to add a specific annotation when they create resources in underlying clusters. Cluster controller can use this annotation to list and delete these resources.
Will also update GetReadyClusters() to not return a cluster if it has deletion timestamp set, so that controllers dont create new resources in a cluster while it is being deleted.

nikhiljindal · 2016-11-04T20:14:12Z

Documenting discussion with @quinton-hoole

There should be a clear way for users to find out what all will be deleted if they set orphanDependents = false.

The API documentation for that field is "Should the dependent objects be orphaned" without defining "dependents". For kubernetes its all resources with OwnerReferece pointing to this resource and for federation it is all resources of the same type and name in underlying clusters. This can clearly cause user confusion.

In future, we might want another option that enables deleting dependents in federation control plane, but not in underlying clusters.

None of our resources have a dependent in federation control plane today. But good to keep in mind for future.

@caesarxuchao

Automatic merge from submit-queue Adding more e2e tests for federated namespace cascading deletion and fixing bugs Ref #33612 Adding more e2e tests for testing cascading deletion of federated namespace. New tests are now verifying that cascading deletion happen when DeletionOptions.OrphanDependents=false and it does not happen when DeleteOptions.OrphanDependents=true. Also updated deletion helper to always add OrphanFinalizer. generic registry will remove it if DeleteOptions.OrphanDependents=false. Also updated namespace registry to do the same. We need to add the orphan finalizer to keep the orphan by default behavior. We assume that its dependents are going to be orphaned and hence add that finalizer. If user does not want the orphan behavior, he can do so using DeleteOptions and then the registry will remove that finalizer. cc @kubernetes/sig-cluster-federation @caesarxuchao @derekwaynecarr

@caesarxuchao

Automatic merge from submit-queue Adding cadcading deletion support for federated secrets Ref #33612 Adding cascading deletion support for federated secrets. The code is same as that for namespaces. Just ensuring that DeletionHelper functions are called at right places in secret_controller. Also added e2e tests. cc @kubernetes/sig-cluster-federation @caesarxuchao ```release-note federation: Adding support for DeleteOptions.OrphanDependents for federated secrets. Setting it to false while deleting a federated secret also deletes the corresponding secrets from all registered clusters. ```

@caesarxuchao

Automatic merge from submit-queue Adding cascading deletion support to more federation controllers Ref #33612 Adding cascading deletion support for federated daemonsets and ingress. The code is same as that for namespaces. Just ensuring that DeletionHelper functions are called at right places in these controllers. e2e tests coming up in another PR. cc @kubernetes/sig-cluster-federation @caesarxuchao @madhusudancs @mwielgus ```release-note federation: Adding support for DeleteOptions.OrphanDependents for federated daemonsets and ingresses. Setting it to false while deleting a federated daemonset or ingress also deletes the corresponding resource from all registered clusters. ```

Automatic merge from submit-queue Adding cascading deletion support to federation replicaset and deployments Forked from #36330 Ref #33612 Adding cascading deletion support for federated replicasets and deployments. ```release-note federation: Adding support for DeleteOptions.OrphanDependents for federated replicasets and deployments. Setting it to false while deleting a federated replicaset or deployment also deletes the corresponding resource from all registered clusters. ```

@caesarxuchao

Automatic merge from submit-queue Adding e2e tests for validating cascading deletion of federation resources Ref #33612 Adding e2e tests for code in #36330 and #36476. Adding test cases to ensure that cascading deletion happens as expected. Also adding code to ensure that all resources are cleaned up in AfterEach. cc @kubernetes/sig-cluster-federation @caesarxuchao @madhusudancs

nikhiljindal · 2016-11-24T08:53:36Z

Status update for 1.5:
We support cascading deletion in Ingresses, Namespaces, Replica Sets, Secrets, Deployments and Daemon Sets. Config maps and services do not support this.
For services we were deleting all underlying resources by default, which was a bug. Fixing it in 1.5: #36799.

Next step is to support it in these 2 resources as well.

nikhiljindal · 2016-12-16T20:21:34Z

We also need to ensure that cascading deletion for federation resources works consistently with kubectl. Filed #38897 for that

@mwielgus

Automatic merge from submit-queue (batch tested with PRs 38212, 38792, 39641, 36390, 39005) Updating federated service controller to support cascading deletion Ref #33612 Service controller is special than other federation controllers because it does not use federatedinformer and updater to sync services (it was written before we had those frameworks). Updating service controller code to instantiate these frameworks and then use deletion helper to perform cascading deletion. Note that, I havent changed the queuing logic in this PR so we still dont use federated informer to manage the queue. Will do that in the next PR. cc @kubernetes/sig-federation-misc @mwielgus @quinton-hoole ```release-note federation: Adding support for DeleteOptions.OrphanDependents for federated services. Setting it to false while deleting a federated service also deletes the corresponding services from all registered clusters. ```

marun · 2017-02-06T01:35:17Z

@nikhiljindal I see that cascade deletion was implemented for services.

I'm working on #40989, and as part of that work can enable cascade deletion for config maps.

nikhiljindal · 2017-02-06T08:02:37Z

@csbell has #40464 for adding cascading deletion support to configmaps.

marun · 2017-02-06T15:10:59Z

Ok, I won't work on refactoring the configmap controller until that merges.

@caesarxuchao

Automatic merge from submit-queue federation: Refactoring namespaced resources deletion code from kube ns controller and sharing it with fed ns controller Ref #33612 Refactoring code in kube namespace controller to delete all resources in a namespace when the namespace is deleted. Refactored this code into a separate NamespacedResourcesDeleter class and calling it from federation namespace controller. This is required for enabling cascading deletion of namespaced resources in federation apiserver. Before this PR, we were directly deleting the namespaced resources and assuming that they go away immediately. With cascading deletion, we will have to wait for the corresponding controllers to first delete the resources from underlying clusters and then delete the resource from federation control plane. NamespacedResourcesDeleter has this waiting logic. cc @kubernetes/sig-federation-misc @caesarxuchao @derekwaynecarr @mwielgus

@caesarxuchao

Automatic merge from submit-queue Adding cascading deletion support to federated namespaces Ref kubernetes/kubernetes#33612 With this change, whenever a federated namespace is deleted with `DeleteOptions.OrphanDependents = false`, then federation namespace controller first deletes the corresponding namespaces from all underlying clusters before deleting the federated namespace. cc @kubernetes/sig-cluster-federation @caesarxuchao ```release-note Adding support for DeleteOptions.OrphanDependents for federated namespaces. Setting it to false while deleting a federated namespace also deletes the corresponding namespace from all registered clusters. ```

@caesarxuchao

Automatic merge from submit-queue Adding more e2e tests for federated namespace cascading deletion and fixing bugs Ref kubernetes/kubernetes#33612 Adding more e2e tests for testing cascading deletion of federated namespace. New tests are now verifying that cascading deletion happen when DeletionOptions.OrphanDependents=false and it does not happen when DeleteOptions.OrphanDependents=true. Also updated deletion helper to always add OrphanFinalizer. generic registry will remove it if DeleteOptions.OrphanDependents=false. Also updated namespace registry to do the same. We need to add the orphan finalizer to keep the orphan by default behavior. We assume that its dependents are going to be orphaned and hence add that finalizer. If user does not want the orphan behavior, he can do so using DeleteOptions and then the registry will remove that finalizer. cc @kubernetes/sig-cluster-federation @caesarxuchao @derekwaynecarr

@caesarxuchao

Automatic merge from submit-queue Adding cadcading deletion support for federated secrets Ref kubernetes/kubernetes#33612 Adding cascading deletion support for federated secrets. The code is same as that for namespaces. Just ensuring that DeletionHelper functions are called at right places in secret_controller. Also added e2e tests. cc @kubernetes/sig-cluster-federation @caesarxuchao ```release-note federation: Adding support for DeleteOptions.OrphanDependents for federated secrets. Setting it to false while deleting a federated secret also deletes the corresponding secrets from all registered clusters. ```

@mwielgus

Automatic merge from submit-queue (batch tested with PRs 38212, 38792, 39641, 36390, 39005) Updating federated service controller to support cascading deletion Ref kubernetes/kubernetes#33612 Service controller is special than other federation controllers because it does not use federatedinformer and updater to sync services (it was written before we had those frameworks). Updating service controller code to instantiate these frameworks and then use deletion helper to perform cascading deletion. Note that, I havent changed the queuing logic in this PR so we still dont use federated informer to manage the queue. Will do that in the next PR. cc @kubernetes/sig-federation-misc @mwielgus @quinton-hoole ```release-note federation: Adding support for DeleteOptions.OrphanDependents for federated services. Setting it to false while deleting a federated service also deletes the corresponding services from all registered clusters. ```

ghost · 2017-09-08T02:52:35Z

@nikhiljindal it doesn't look like we've addressed the second part of this issue, namely what happens when clusters are removed from the federation. Can you confirm? Are you still working on this? If not, could you provide current status, and unassign yourself, so that we can find someone else to finish the required work please. Thanks.

ghost · 2017-09-08T02:53:04Z

Required for GA of federation.

irfanurrehman · 2017-10-30T18:04:36Z

This issue was labelled only for sig/multicluster and is thus moved over to kubernetes-retired/federation#75.
If this does not seem to be right, please reopen this and notify us @kubernetes/sig-multicluster-misc.
/close

nikhiljindal added the area/cluster-federation label Sep 27, 2016

k8s-github-robot added area/apiserver team/control-plane labels Sep 27, 2016

nikhiljindal self-assigned this Oct 3, 2016

This was referenced Oct 12, 2016

Adding cascading deletion support to federated namespaces #34648

Merged

Federation: Support generalized deletions, including cascading and finalizers kubernetes/enhancements#99

Closed

This was referenced Oct 27, 2016

Updating namespace controller utils to use versioned clientset #35738

Closed

federation: Refactoring namespaced resources deletion code from kube ns controller and sharing it with fed ns controller #35950

Merged

This was referenced Nov 5, 2016

Adding more e2e tests for federated namespace cascading deletion and fixing bugs #36283

Merged

Adding cadcading deletion support for federated secrets #36296

Merged

Adding cascading deletion support to more federation controllers #36330

Merged

nikhiljindal mentioned this issue Nov 7, 2016

Updating federated service controller to support cascading deletion #36390

Merged

nikhiljindal mentioned this issue Nov 9, 2016

Adding cascading deletion support to federation replicaset and deployments #36476

Merged

nikhiljindal mentioned this issue Nov 11, 2016

Adding e2e tests for validating cascading deletion of federation resources #36630

Merged

nikhiljindal mentioned this issue Dec 16, 2016

federation: kubectl --cascade should be false by default for federation #38897

Closed

nikhiljindal removed the team/control-plane (deprecated - do not use) label Mar 5, 2017

ash2k mentioned this issue May 7, 2017

Handle Bundle deletion atlassian/smith#55

Closed

ghost added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Sep 8, 2017

csbell added area/federation sig/multicluster Categorizes an issue or PR as relevant to SIG Multicluster. and removed sig/federation labels Oct 10, 2017

irfanurrehman mentioned this issue Oct 30, 2017

federation: Support cascading deletion kubernetes-retired/federation#75

Closed

k8s-ci-robot assigned irfanurrehman Oct 30, 2017

k8s-ci-robot closed this as completed Oct 30, 2017

irfanurrehman mentioned this issue Oct 31, 2017

federation: kubectl --cascade should be false by default for federation kubernetes-retired/federation#89

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

federation: Support cascading deletion #33612

federation: Support cascading deletion #33612

nikhiljindal commented Sep 27, 2016

nikhiljindal commented Sep 28, 2016 •

edited

nikhiljindal commented Sep 28, 2016

lavalamp commented Sep 28, 2016

ghost commented Sep 28, 2016

caesarxuchao commented Sep 28, 2016

nikhiljindal commented Nov 3, 2016

nikhiljindal commented Nov 4, 2016

nikhiljindal commented Nov 24, 2016

nikhiljindal commented Dec 16, 2016

marun commented Feb 6, 2017

nikhiljindal commented Feb 6, 2017

marun commented Feb 6, 2017

ghost commented Sep 8, 2017

ghost commented Sep 8, 2017

irfanurrehman commented Oct 30, 2017

federation: Support cascading deletion #33612

federation: Support cascading deletion #33612

Comments

nikhiljindal commented Sep 27, 2016

nikhiljindal commented Sep 28, 2016 • edited

Garbage collector model

How it works in kubernetes

Work required to extend it for federation:

Pros:

Cons

Let each controller delete the corresponding dependent objects

How will this work

Pros:

Cons:

nikhiljindal commented Sep 28, 2016

lavalamp commented Sep 28, 2016

ghost commented Sep 28, 2016

caesarxuchao commented Sep 28, 2016

nikhiljindal commented Nov 3, 2016

nikhiljindal commented Nov 4, 2016

nikhiljindal commented Nov 24, 2016

nikhiljindal commented Dec 16, 2016

marun commented Feb 6, 2017

nikhiljindal commented Feb 6, 2017

marun commented Feb 6, 2017

ghost commented Sep 8, 2017

ghost commented Sep 8, 2017

irfanurrehman commented Oct 30, 2017

nikhiljindal commented Sep 28, 2016 •

edited