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.

[fabriziopandini]

@ykakarap do you mind adding #7293 to the task given that this work is complementary to #7088 for the autoscaler support in cluster class?

[ykakarap]

@ykakarap do you mind adding #7293 to the task given that this work is complementary to #7088 for the autoscaler support in cluster class?

done

[ykakarap]

/milestone v1.4

[k8s-ci-robot]

@ykakarap: You must be a member of the kubernetes-sigs/cluster-api-maintainers GitHub team to set the milestone. If you believe you should be able to issue the /milestone command, please contact your Cluster API Maintainers and have them propose you as an additional delegate for this responsibility.

In response to this:

/milestone v1.4

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.

[sbueringer]

@ykakarap Thx for updating the issue, gives a great overview!

[enxebre]

@sbueringer @fabriziopandini @ykakarap e2e In-place propagation feature is breaking and potentially disruptive for existing clusters workflows. E.g automation relying on labels to trigger rolling upgrade.
How are we planning to contain disruption for existing workflows? Is the plan just to document changes/impact clearly as part of the minor release?

[sbueringer]

We are planning to implement RolloutAfter on MachineDeployments, adjust clusterctl accordingly + update the documentation (a corresponding note in the release notes also makes sense I think)

[ykakarap]

+1 to what Stefan said and implementing rolloutAfter support for MachineDeployments is tracked in this umbrella issue and will be done as part of the v1.4 release.

[ykakarap]

@sbueringer @fabriziopandini Can I move all the follow-up items into a separate issues and close this?

[fabriziopandini]

I'm +1 to close/migrate pending items to a follow up issue as soon as the documentation PR is out

[ykakarap]

All the follow-up items are moved into separate issues. This issue can now be closed.

/close

[k8s-ci-robot]

@ykakarap: Closing this issue.

In response to this:

All the follow-up items are moved into separate issues. This issue can now be closed.

/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.