Enforce good selectors on MachineDeployments

[killianmuldoon]

As documented in #5957 it is possible to configure MachineDeployment Selectors in Cluster API minimally e.g. using the cluster name only, so that they are created correctly but are not unique enough to ensure correct adoption, for example in the event of a backup and restore. The default selector, set when no selector is set on the MachineDeployment, contains both the Cluster name and the MachineDeployment name and is sufficient to ensure adoption occurs correctly.

This configuration can cause unexpected rollouts to occur when attempting to adopt a cluster by a restored management cluster. It can also cause Machines to be adopted by the wrong MachineSet, putting the cluster in a working but invalid state.

Right now the correct form of selector is opaque to users and it only causes an issue on Day N when MachineDeployment and MachineSet adoption unexpectedly shows errors.

To solve this we could (from strongest to weakest):

1. Add the default selectors to the MachineDeployment on creation, even if another selector is defined.
2. Block the creation of MachineDeployments with insufficiently specific selectors?
3. Create a webhook warning (requires Support warnings in webhook utils controller-runtime#1788) to advise users on creating MachineDeployments
4. Document the requirement for MachineDeployment selectors.

I'd prefer to got with the stronger version of this to preclude any future occurrence of this issue. I'm not sure if there's workflows that would be blocked by making strong assumptions about the MachineDeployment / MachineSet selector.

Is there any reason we shouldn't default the selector to always include both the Cluster name and the MachineDeployment name?

[fabriziopandini]

/kind feature
/milestone v1.2

I think the current implementation derives from Kubernetes Deployments, and AFAIK also there it is up to the users to provide not overlapping selectors, but at the same time it allows the users to choose their own criteria for uniqueness.

I'm also worried by the effect of enforcing all the selectors on existing MD (it is probably a breaking change, and as far as I remember it will trigger rollouts on upgrade).

given that my first reaction is to go for option 3, but probably this requires some more thinking/different opinions

[sbueringer]

Same here. I would prefer option 3. I think it should be enough to nudge users a bit.

[k8s-triage-robot]

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]

/lifecycle frozen

[fabriziopandini]

/milestone Next