How to surfacing runtime hook blocking cluster deletion in condition

kubernetes-sigs / cluster-api

Home for Cluster API, a subproject of sig-cluster-lifecycle

https://cluster-api.sigs.k8s.io

Apache License 2.0

3.58k stars 1.31k forks source link

How to surfacing runtime hook blocking cluster deletion in condition #6807

Open ykakarap opened 2 years ago

ykakarap commented 2 years ago

Details:

This issue is to discuss how to surface that a cluster deletion is blocked when the BeforeClusterDelete hook blocks the deletion of a process with a blocking response.

We have 2 options: 1) Use TopologyReconciled condition - Since only managed clusters are affected by BeforeClusterDelete hook we can use the TopologyReconciled condition to surface if a cluster deletion is blocked. Reason will be LifecycleHookBlocking. This will be consistent with how the other lifecycle hook blocking cases are surfaced.

2) Use a separate condition - Since TopologyReconciled condition reflects if the underlying objects reflect the topology spec of the cluster using it for delete might be deviating from it purpose.

Some additional concerns:

How should the condition be updated after deletion is unblocked and the cluster is in the process of deleting the objects?
Which controller owns this condition?

/area runtime-sdk

k8s-ci-robot commented 2 years ago

@ykakarap: The label(s) kind/runtime-sdk cannot be applied, because the repository doesn't have them.

In response to [this](https://github.com/kubernetes-sigs/cluster-api/issues/6807): >**Details:** > >This issue is to discuss how to surface that a cluster deletion is blocked when the `BeforeClusterDelete` hook blocks the deletion of a process with a blocking response. > >We have 2 options: >1) Use `TopologyReconciled` condition - Since only managed clusters are affected by `BeforeClusterDelete` hook we can use the `TopologyReconciled` condition to surface if a cluster deletion is blocked. Reason will be `LifecycleHookBlocking`. This will be consistent with how the other lifecycle hook blocking cases are surfaced. > >2) Use a separate condition - Since `TopologyReconciled` condition reflects if the underlying objects reflect the topology spec of the cluster using it for delete might be deviating from it purpose. > >Some additional concerns: >* How should the condition be updated after deletion is unblocked and the cluster is in the process of deleting the objects? >* Which controller owns this condition? > > >/kind runtime-sdk Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.

ykakarap commented 2 years ago

/area runtime-sdk

ykakarap commented 2 years ago

/cc @fabriziopandini @sbueringer

sbueringer commented 2 years ago

I think we should also discuss how we surface the various "blocking states" of reconcileDelete of the regular controller (deleting MDs/ControlPlane/InfraCluster/ ...) so that all our "delete" condition(s) fit nicely together

I'm not sure if we need an owner for a condition. If the condition is written by different controllers at different times I wonder if that's good enough (topology controller until the BeforeClusterDelete hook is successfully called, cluster controller afterwards).

One comment, considering that reconcileDelete of the regular controller is deleting MD/ControlPlane/InfraCluster this is essentially "delete" reconciliation of the objects of the managed topology. Even if we implemented that before ClusterClass and it works with non-classy clusters as well.

fabriziopandini commented 2 years ago

/triage accepted

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle stale
Mark this issue or PR as rotten with /lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

fabriziopandini commented 2 years ago

/lifecycle frozen

k8s-triage-robot commented 9 months ago

This issue has not been updated in over 1 year, and should be re-triaged.

You can:

Confirm that this issue is still relevant with /triage accepted (org members only)
Close this issue with /close

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

sbueringer commented 9 months ago

/triage accepted

fabriziopandini commented 7 months ago

/kind cleanup /priority important-longterm