Updating the dns imageTag causes KCP rolling update

[Levi080513]

Detailed Description

Currently we can update the coredns version by updating KCP.Spec.KubeadmControlPlaneSpec.kubeadmConfigSpec.clusterConfiguration.DNS.imageTag, but this will cause a rolling update of the control plane nodes.

KCP use the following method to update the coredns version, this does not require rolling updates of control plane nodes, or can we skip checking this field when checking MachinesNeedingRollout?

cluster-api/controlplane/kubeadm/internal/controllers/controller.go

Lines 466 to 469 in 9d36ddc

	// Update CoreDNS deployment.
	if err := workloadCluster.UpdateCoreDNS(ctx, controlPlane.KCP, parsedVersion); err != nil {
	return ctrl.Result{}, errors.Wrap(err, "failed to update CoreDNS deployment")
	}

Anything else you would like to add?

No response

Label(s) to be applied

/area provider/control-plane-kubeadm

[sbueringer]

Sounds good to me.

/triage accepted

cc @fabriziopandini in case I'm missing something

[sbueringer]

If I see correctly we can simply set the fields of both to nil before running reflect.DeepEqual:

cluster-api/controlplane/kubeadm/internal/filters.go

Line 198 in 359f92e

return reflect.DeepEqual(machineClusterConfig, kcpLocalClusterConfiguration)

[Levi080513]

Perhaps it is necessary to synchronize kubeadmConfiguration to avoid configuration differences?

[Levi080513]

/assign Levi080513

[sbueringer]

Perhaps it is necessary to synchronize kubeadmConfiguration to avoid configuration differences?

Not as far as I'm aware. The configs won't be identical but the one on the Machine will be still accurately the one the Machine was created with.

[Levi080513]

It is my fault I didn't make it clear. I mean do we need to synchronize CP Machinde kubeadmConfig CR and Machine controlplane.cluster.x-k8s.io/kubeadm-cluster-configuration annotation, but now it seems that is not necessary.

[sbueringer]

I think we're fine. But I take a closer look when reviewing the PR

[sbueringer]

/close

#9857 was merged (no idea why GitHub didn't close the issue automatically, PR description looks correct)

[k8s-ci-robot]

@sbueringer: Closing this issue.

In response to this:

/close

#9857 was merged (no idea why GitHub didn't close the issue automatically, PR description looks correct)

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.