Unable to delete master machine

[michaelgugino]

/kind bug

What steps did you take and what happened:
Deleting a master machine where the machine-controller running is prohibited in code:

cluster-api/pkg/controller/machine/machine_controller.go

Lines 167 to 168 in bf950ad

	if !r.isDeleteAllowed(m) {
	klog.Infof("Deleting machine hosting this controller is not allowed. Skipping reconciliation of machine %q", name)

What did you expect to happen:
We should delete this machine, the machine-controller can get rescheduled.

Anything else you would like to add:
The primary motivation for calling this a bug is if the machine-controller gets rescheduled for some reason onto another host, the master will be abruptly deleted later. There is no way to un-delete the machine object, so this is essentially a ticking time-bomb.

At this point, the machine-controller is re-entrant, an interruption of it's operation should be transparent to it's operation.

[vsamidurai]

@timothysc Can I take this up if it's not already taken?

[ncdc]

@vsamidurai yes, definitely. No one is currently assigned and it's not marked active.

[detiber]

@vsamidurai One thing to keep in mind for this issue: The current check was put in place to avoid orphaning cloud resources.

[ncdc]

@detiber could you elaborate on that scenario please?

[detiber]

Sure thing, the scenario is:

• a self-hosted cluster (cluster-api running on a cluster it is managing)
• where the cluster-api controller manager(s) are running on the single control plane Node
• A user issues a delete for the Machine backing the control plane Node

Today, the existing check would prevent the Machine from being deleted prior to pivoting the cluster-api resources to a different cluster (such as with clusterctl alpha phases pivot).

This is a common scenario that is hit during testing today, such as when doing clusterctl create followed by a clusterctl delete.

That said, the current check does not prevent other potentially dangerous conditions from happening and causing the management cluster from becoming unavailable.

[michaelgugino]

One way to solve this problem (IMO) is to use leader election, always deploy at least two machine-controllers, and use pod disruption budget of 1.

Otherwise, maybe just use an annotation to prevent delete that you can remove in CI after the pivot, or you could attempt to count to ensure masters > 1 (or >= 3?)

[detiber]

It could potentially require a combination of those approaches.

In general today clusterctl based-deployments will have the controller managers running on the "first" control plane node, since it is the only Machine that is deployed prior to pivoting the cluster-api resources. However, there wouldn't be anything that would prevent the other two control plane Machines from being deleted and causing the cluster to become inaccessible today. Longer term, this will likely be solved through control plane management, but we don't have that today.

[ncdc]

This logic has been removed from master/v1alpha2
/close

[k8s-ci-robot]

@ncdc: Closing this issue.

In response to this:

This logic has been removed from master/v1alpha2
/close

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.