Umbrella issue for `In place propagation of changes affecting Kubernetes objects only` proposal implementation

[ykakarap]

This is an umbrella issue for the implementation of the In place propagation of changes affecting Kubernetes objects only proposal. As the proposal is merged the implementation can now be done.

The work is broken down into:

• Propagate labels from Managed Clusters and ClusterClass to the underlying KCP and MachineDeployment objects.

  • PR: ✨ Managed Cluster should set labels and annotations on ControlPlane and MDs #7917
  • Other Issues/PRs that depend/affect this work:
    • MachineDeployment.spec.replicas defaulting should take into account autoscaler min/max size if defined #7293

• Labels and Annotations should be always synced. Not only at create.

  • Cluster/ClusterClass to KCP
  • Cluster/ClusterClass to MachineDeployments
  • KCP to Machines, InfraMachines and KubeadmConfigs ⚠️ in-place propagation support for KCP #8057
  • MachineDeployments to MachineSets ⚠️ in-place propagation from MD to MS #8110
  • MachineSets to Machines ⚠️ in-place propagation from MS to Machines #8111
  • MachineSets to InfraMachines and BootstrapConfigs ⚠️ in-place propagation from MS to InfraMachine and BootstrapConfig #8060

• In-place propagation

  Fields like labels, annotations, MinReadySeconds, NodeDrainTimeout, NodeVolumeDetachTimeout, NodeDeletionTimeout , things that only affect the Kubernetes objects and not the underlying node should not trigger a rollout and be propagated in-place.

  • MachineDeployment to MachineSet
    • PR: ⚠️ in-place propagation from MD to MS #8110
  • MachineSets to Machines
    • PR: ⚠️ in-place propagation from MS to Machines #8111
  • KCP to Machines
    • PR: ⚠️ in-place propagation support for KCP #8057

Follow-up:

• End-to-end testing 🌱 test/e2e: Add ClusterClass rollout test, extend MD rollout test #8171
• Add support for rolloutAfter in MachineDeployments. ✨ MachineDeployment rolloutAfter support #8216
  • clusterctl alpha rollout restart md/md-0 should switch to using rolloutAfter
• Documentation Updates
  • PR: 📖 update book on in-place propagation #8319

Follow-up (can do after v1.4):

• Revision history support for in-place mutating changes in MDs.
  • Issue: MachineDeployment history should also track in-place propagating changes #8392
• Drop disableInPlacePropagation from KCP reconciler and migrate the affected tests to envtest.
  • Issue: Drop disableInPlacePropagation from KCP reconciler and migrate the affected tests to envtest. #8393
• Clean up FindOldMachineSets to drop the first return parameter. It is never used.
  • Issue: Clean up FindOldMachineSets to drop the first return parameter #8394
  • Ref: [WIP] ✨ in-place propagation from MD to MS to Machines #7921 (comment)
• Replace all occurrences of helperOptions with ssa.FilterObjectInput.
  • Issue: Replace HelperOptions with ssa.FilterObjectInput #8395
  • ssa.FilterObjectInput is introduced in ⚠️ in-place propagation from MS to InfraMachine and BootstrapConfig #8060 and can replace helperOptions.
  • Ref: ⚠️ in-place propagation from MS to InfraMachine and BootstrapConfig #8060 (comment)
• Rename ssa util functions:
  • Issue: Rename internal/util/ssa util functions for better naming consistency  #8396
  • IsAllowedPath -> IsPathAllowed (Ref)
  • IsIgnorePath -> IsPathIgnored (Ref)
  • Evaluate semantics of IsNotAllowed (Ref)

---

[ykakarap]

/assign

[fabriziopandini]

/triage accepted

[sbueringer]

Q: (not 100% sure). As of today rolloutAfter is not implement for MachineDeployments. Because of that we recommend folks to "make an arbitrary change to its spec.template" or use clusterctl alpha rollout restart machinedeployment/my-md-0 (link). clusterctl alpha rollout restart machinedeployment/my-md-0 depends on that an annotation change triggers a rollout which actually replaces machines.

Does this mean to avoid breaking this behavior part of this effort will be to implement rolloutAfter for MachineDeployments + modifying clusterctl alpha rollout restart machinedeployment/my-md-0 accordingly? I'm not sure if after in-place mutation there are other fields left that we could modify instead.

xrefs:

• currently open PR to implement MD rolloutAfter: ✨ Implement MachineDeployment rolloutAfter support #7053

[sbueringer]

Q: Do we have to track the following as well?

In order to prevent this we are modifying the MachineDeployment controller in order to pick the "semantically equal" MachineSet with more Machines, thus avoiding or minimizing turbulence in the Cluster.
https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/proposals/20221003-In-place-propagation-of-Kubernetes-objects-only-changes.md#machinedeployment-rollouts

[fabriziopandini]

Another thing that must be considered is if to apply in-place propagation to the new/current machine set only or to old machine sets as well

[sbueringer]

Another thing that must be considered is if to apply in-place propagation to the new/current machine set only or to old machine sets as well

That's a very good point. I think if we apply it to old MachineSets as well it would essentially break the history/rollback functionality?

[ykakarap]

Q: (not 100% sure). As of today rolloutAfter is not implement for MachineDeployments. Because of that we recommend folks to "make an arbitrary change to its spec.template" or use clusterctl alpha rollout restart machinedeployment/my-md-0 (link). clusterctl alpha rollout restart machinedeployment/my-md-0 depends on that an annotation change triggers a rollout which actually replaces machines.

Does this mean to avoid breaking this behavior part of this effort will be to implement rolloutAfter for MachineDeployments + modifying clusterctl alpha rollout restart machinedeployment/my-md-0 accordingly? I'm not sure if after in-place mutation there are other fields left that we could modify instead.

xrefs:

• currently open PR to implement MD rolloutAfter: ✨ Implement MachineDeployment rolloutAfter support #7053

Do not have an answer for this yet but had a similar question in the kcp rollout PR (Ref: #6858 (review)).

One thing to consider while using RolloutAfter for triggering manual rollouts is that if a user has already set a RolloutAfter value to a later time we would be overriding that value and the original information will be lost.
We should take a step back and look at how we want to trigger manual rollouts for both KCP and MDs.

Also, how about tracking this discussion in a dedicated issue?

[sbueringer]

One thing to consider while using RolloutAfter for triggering manual rollouts is that if a user has already set a RolloutAfter value to a later time we would be overriding that value and the original information will be lost.

That's a question for the clusterctl PR about the UX of the clusterctl command right?

Feel free to move it to a separate issue just wondering how the rolloutAfter topic might impact the current issue.