🐛 MachineSet should allow scale down operations to proceed when templates don't exist

[vincepri]

We're currently always trying to check that the controller can access and set owner references on MachineSets, even if no operations are ongoing. If a MachineSet is scaling down, we should not block operations if the external reference isn't present anymore (was deleted) and we should let the controller proceed.

In most gitops scenarios, it's preferable that any change being rolled out it's done by setting new template references to new resources. For example, let's say we have a MachineDeployment/MachineSet with a boostrap and infrastructure reference set. To apply a rollout and change to all Machines under management, new templates need to be created, and the references changed to the new templates, which will cause a rollout.

With gitops flows, usually new resources replacing old ones have the side effect that old resources are cleaned up almost immediately; this causes the MachineSet controller to stop scale down (and any rollout) operations due to the references returning a not found error.

What this PR does / why we need it:

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

/area machineset

[sbueringer]

One nit

[sbueringer]

/lgtm

/assign @fabriziopandini

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: ba4848aad06fd02e84885eb39ad07e6e54103205

[fabriziopandini]

/test ?

[k8s-ci-robot]

@fabriziopandini: The following commands are available to trigger required jobs:

• /test pull-cluster-api-build-main
• /test pull-cluster-api-e2e-blocking-main
• /test pull-cluster-api-e2e-conformance-ci-latest-main
• /test pull-cluster-api-e2e-conformance-main
• /test pull-cluster-api-e2e-main
• /test pull-cluster-api-e2e-mink8s-main
• /test pull-cluster-api-e2e-upgrade-1-30-1-31-main
• /test pull-cluster-api-test-main
• /test pull-cluster-api-test-mink8s-main
• /test pull-cluster-api-verify-main

The following commands are available to trigger optional jobs:

• /test pull-cluster-api-apidiff-main

Use /test all to run the following jobs that were automatically triggered:

• pull-cluster-api-apidiff-main
• pull-cluster-api-build-main
• pull-cluster-api-e2e-blocking-main
• pull-cluster-api-test-main
• pull-cluster-api-verify-main

In response to this:

/test ?

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-sigs/prow repository.

[vincepri]

/test pull-cluster-api-e2e-main

[sbueringer]

Thx!

/lgtm

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: f446ddb2df6d4e960060392a243b15f087bef92d

[sbueringer]

/test pull-cluster-api-e2e-main

[fabriziopandini]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fabriziopandini

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 [fabriziopandini]

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

[sbueringer]

/hold

I think second time in a row this test case is failing. As far as I know it's not (this) flaky on main

[sbueringer]

/test pull-cluster-api-e2e-main

[sbueringer]

Also saw it on other PRs I think. Let's check periodics and then merge (just trying to get tests stable so we should be careful to not introduce new flakes)

[sbueringer]

Okay clearly a flake that also happens on main since a few days:

• https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_cluster-api/10913/pull-cluster-api-e2e-main/1815789359991885824
• https://storage.googleapis.com/k8s-triage/index.html?job=.*cluster-api.*main&xjob=.*-provider-.*

(cc @chrischdi, just fyi, let's see who gets around to it first)

/hold cancel

[vincepri]

/cherry-pick release-1.7

[k8s-infra-cherrypick-robot]

@vincepri: #10913 failed to apply on top of branch "release-1.7":

Applying: 🐛 MachineSet should allow scale down operations to proceed when templates don't exist
Using index info to reconstruct a base tree...
M	internal/controllers/machineset/machineset_controller.go
M	internal/controllers/machineset/machineset_controller_test.go
Falling back to patching base and 3-way merge...
Auto-merging internal/controllers/machineset/machineset_controller_test.go
Auto-merging internal/controllers/machineset/machineset_controller.go
CONFLICT (content): Merge conflict in internal/controllers/machineset/machineset_controller.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 🐛 MachineSet should allow scale down operations to proceed when templates don't exist
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-1.7

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-sigs/prow repository.