🐛Requeue while cluster's control plane object is being deleted

[vincepri]

Signed-off-by: Vince Prignano [email protected]

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #2507

/assign @sedefsavas @ncdc
/milestone v0.3.0-rc.3

[k8s-ci-robot]

@vincepri: GitHub didn't allow me to assign the following users: sedefsavas.

Note that only kubernetes-sigs members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

Signed-off-by: Vince Prignano [email protected]

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #2507

/assign @sedefsavas @ncdc
/milestone v0.3.0-rc.3

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vincepri

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [vincepri]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ncdc]

I don't think we can do this here - we have code in the KCP controller that defers deletion of the control plane machines if there are any non-control plane machines still around. Making this change here would cause a deadlock, I think.

[vincepri]

I forget why the ControlPlaneRef is handled first, shouldn't that be the last object to be deleted?

[ncdc]

I'm sure we must have had a reason for doing it first...

[vincepri]

Original PR #2094, trying to dig through the comments, I might just test it moving it down though

[vincepri]

/hold

[vincepri]

The latest code change behaves as we want, the main changes are:

• Avoid adding a Cluster OwnerReference when there is a controller OwnerRef, we already do this for MachineSets, seems fair to do the same here.
• Only count ControlPlaneMachines when listing the Cluster's descendants if there is no control plane provider.

Deleting a cluster with $ k delete cluster test1 results in the Machines to be deleted first:

$ k get machines -A
NAMESPACE   NAME                         PROVIDERID                    PHASE
default     test1-control-plane-828k8    aws:////i-00710d0d692170b68   Running
default     test1-control-plane-ktbmh    aws:////i-0e531be42f37cb0a9   Running
default     test1-control-plane-lnnx8    aws:////i-0092f77ca43ce27d0   Running
default     test1-md-0-7bdfd9f44-k79d7   aws:////i-03d60eff678a27f0b   Deleting

Then after all the workers are gone, the control plane machines in KCP gets a deletion timestamp:

$ k get kubeadmcontrolplanes.controlplane.cluster.x-k8s.io -o json | jq .items[0].metadata.deletionTimestamp
"2020-03-04T02:40:00Z"

And machines associated with it get deleted:

default     test1-control-plane-828k8   aws:////i-00710d0d692170b68   Deleting
default     test1-control-plane-ktbmh   aws:////i-0e531be42f37cb0a9   Deleting
default     test1-control-plane-lnnx8   aws:////i-0092f77ca43ce27d0   Deleting

After a little, the cluster is gone without errors:

$ k get cluster
No resources found in default namespace.

[vincepri]

/hold cancel

[ncdc]

/lgtm