Skip to content

🌱 Simplify KCP matchesKubeadmConfig #12813

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
k8s-ci-robot merged 1 commit into from
Oct 2, 2025
Merged

🌱 Simplify KCP matchesKubeadmConfig #12813

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
134 changes: 42 additions & 92 deletions controlplane/kubeadm/internal/filters.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,109 +163,88 @@ func matchesKubeadmConfig(kubeadmConfigs map[string]*bootstrapv1.KubeadmConfig,
return "", true, nil
}

// Check if KCP and machine ClusterConfiguration matches, if not return
match, diff, err := matchClusterConfiguration(currentKubeadmConfig, kcp)
// takes the KubeadmConfigSpec from KCP and applies the transformations required
// to allow a comparison with the KubeadmConfig referenced from the machine.
desiredKubeadmConfig := getAdjustedKcpConfig(kcp, currentKubeadmConfig)
Copy link
Member

@fabriziopandini fabriziopandini Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about moving getAdjustedKcpConfig into PrepareKubeadmConfigsForDiff, so we have everything in one func?

Copy link
Member Author

@sbueringer sbueringer Oct 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would keep it here for now.

We don't need it in the upcoming case where we are going to use PrepareKubeadmConfigsForDiff (right in front of CanUpdateMachine) and I'm going to delete getAdjustedKcpConfig entirely as it will be part of ComputeDesiredKubeadmConfig in the next PR


desiredKubeadmConfigForDiff, currentKubeadmConfigForDiff, err := PrepareKubeadmConfigsForDiff(desiredKubeadmConfig, currentKubeadmConfig, kcp)
if err != nil {
return "", false, errors.Wrapf(err, "failed to match KubeadmConfig")
}
if !match {
return fmt.Sprintf("Machine KubeadmConfig ClusterConfiguration is outdated: diff: %s", diff), false, nil
}

// Check if KCP and machine InitConfiguration or JoinConfiguration matches
// Check if current and desired KubeadmConfigs match.
// NOTE: only one between init configuration and join configuration is set on a machine, depending
// on the fact that the machine was the initial control plane node or a joining control plane node.
match, diff, err = matchInitOrJoinConfiguration(currentKubeadmConfig, kcp)
match, diff, err := compare.Diff(&currentKubeadmConfigForDiff.Spec, &desiredKubeadmConfigForDiff.Spec)
if err != nil {
return "", false, errors.Wrapf(err, "failed to match KubeadmConfig")
}
if !match {
return fmt.Sprintf("Machine KubeadmConfig InitConfiguration or JoinConfiguration are outdated: diff: %s", diff), false, nil
return fmt.Sprintf("Machine KubeadmConfig is outdated: diff: %s", diff), false, nil
}

return "", true, nil
}

// matchClusterConfiguration verifies if KCP and machine ClusterConfiguration matches.
func matchClusterConfiguration(machineConfig *bootstrapv1.KubeadmConfig, kcp *controlplanev1.KubeadmControlPlane) (bool, string, error) {
if machineConfig == nil {
// Return true here because failing to get KubeadmConfig should not be considered as unmatching.
// This is a safety precaution to avoid rolling out machines if the client or the api-server is misbehaving.
return true, "", nil
}
machineConfig = machineConfig.DeepCopy()

kcpLocalKubeadmConfig := kcp.Spec.KubeadmConfigSpec.DeepCopy()
if kcpLocalKubeadmConfig == nil {
kcpLocalKubeadmConfig = &bootstrapv1.KubeadmConfigSpec{}
}
// PrepareKubeadmConfigsForDiff cleans up all fields that are not relevant for the comparison.
func PrepareKubeadmConfigsForDiff(desiredKubeadmConfig, currentKubeadmConfig *bootstrapv1.KubeadmConfig, kcp *controlplanev1.KubeadmControlPlane) (desired *bootstrapv1.KubeadmConfig, current *bootstrapv1.KubeadmConfig, _ error) {
// DeepCopy to ensure the passed in KubeadmConfigs are not modified.
// This has to be done because we eventually want to be able to apply the desiredKubeadmConfig
// (without the modifications that we make here).
desiredKubeadmConfig = desiredKubeadmConfig.DeepCopy()
currentKubeadmConfig = currentKubeadmConfig.DeepCopy()

// Default feature gates like in initializeControlPlane / scaleUpControlPlane.
// Note: Changes in feature gates can still trigger rollouts.
// TODO(in-place) refactor this area so the desired KubeadmConfig is not computed in multiple places independently.
parsedVersion, err := semver.ParseTolerant(kcp.Spec.Version)
if err != nil {
return false, "", errors.Wrapf(err, "failed to parse Kubernetes version %q", kcp.Spec.Version)
return nil, nil, errors.Wrapf(err, "failed to parse Kubernetes version %q", kcp.Spec.Version)
}
DefaultFeatureGates(kcpLocalKubeadmConfig, parsedVersion)
DefaultFeatureGates(&desiredKubeadmConfig.Spec, parsedVersion)

// Ignore ControlPlaneEndpoint which is added on the Machine KubeadmConfig by CABPK.
// Note: ControlPlaneEndpoint should also never change for a Cluster, so no reason to trigger a rollout because of that.
machineConfig.Spec.ClusterConfiguration.ControlPlaneEndpoint = kcpLocalKubeadmConfig.ClusterConfiguration.ControlPlaneEndpoint
currentKubeadmConfig.Spec.ClusterConfiguration.ControlPlaneEndpoint = desiredKubeadmConfig.Spec.ClusterConfiguration.ControlPlaneEndpoint

// Skip checking DNS fields because we can update the configuration of the working cluster in place.
machineConfig.Spec.ClusterConfiguration.DNS = kcpLocalKubeadmConfig.ClusterConfiguration.DNS

// Drop differences that do not lead to changes to Machines, but that might exist due
// to changes in how we serialize objects or how webhooks work.
dropOmittableFields(kcpLocalKubeadmConfig)
dropOmittableFields(&machineConfig.Spec)

// Compare and return.
match, diff, err := compare.Diff(machineConfig.Spec.ClusterConfiguration, kcpLocalKubeadmConfig.ClusterConfiguration)
if err != nil {
return false, "", errors.Wrapf(err, "failed to match ClusterConfiguration")
}
return match, diff, nil
}

// matchInitOrJoinConfiguration verifies if KCP and machine InitConfiguration or JoinConfiguration matches.
// NOTE: By extension this method takes care of detecting changes in other fields of the KubeadmConfig configuration (e.g. Files, Mounts etc.)
func matchInitOrJoinConfiguration(machineConfig *bootstrapv1.KubeadmConfig, kcp *controlplanev1.KubeadmControlPlane) (bool, string, error) {
if machineConfig == nil {
// Return true here because failing to get KubeadmConfig should not be considered as unmatching.
// This is a safety precaution to avoid rolling out machines if the client or the api-server is misbehaving.
return true, "", nil
}
machineConfig = machineConfig.DeepCopy()

// takes the KubeadmConfigSpec from KCP and applies the transformations required
// to allow a comparison with the KubeadmConfig referenced from the machine.
kcpConfig := getAdjustedKcpConfig(kcp, machineConfig)
currentKubeadmConfig.Spec.ClusterConfiguration.DNS = desiredKubeadmConfig.Spec.ClusterConfiguration.DNS

// Default both KubeadmConfigSpecs before comparison.
// *Note* This assumes that newly added default values never
// introduce a semantic difference to the unset value.
// But that is something that is ensured by our API guarantees.
defaulting.ApplyPreviousKubeadmConfigDefaults(kcpConfig)
defaulting.ApplyPreviousKubeadmConfigDefaults(&machineConfig.Spec)
defaulting.ApplyPreviousKubeadmConfigDefaults(&desiredKubeadmConfig.Spec)
defaulting.ApplyPreviousKubeadmConfigDefaults(&currentKubeadmConfig.Spec)

// Cleanup all fields that are not relevant for the comparison.
cleanupConfigFields(kcpConfig, machineConfig)
// Cleanup JoinConfiguration.Discovery from desiredKubeadmConfig and currentKubeadmConfig, because those info are relevant only for
// the join process and not for comparing the configuration of the machine.
// Note: Changes to Discovery will apply for the next join, but they will not lead to a rollout.
// Note: We should also not send Discovery.BootstrapToken.Token to a RuntimeExtension for security reasons.
desiredKubeadmConfig.Spec.JoinConfiguration.Discovery = bootstrapv1.Discovery{}
currentKubeadmConfig.Spec.JoinConfiguration.Discovery = bootstrapv1.Discovery{}

// Compare and return.
match, diff, err := compare.Diff(&machineConfig.Spec, kcpConfig)
if err != nil {
return false, "", errors.Wrapf(err, "failed to match InitConfiguration or JoinConfiguration")
// If KCP JoinConfiguration.ControlPlane is nil and the Machine JoinConfiguration.ControlPlane is empty,
// set Machine JoinConfiguration.ControlPlane to nil.
// NOTE: This is required because CABPK applies an empty JoinConfiguration.ControlPlane in case it is nil.
if desiredKubeadmConfig.Spec.JoinConfiguration.ControlPlane == nil &&
reflect.DeepEqual(currentKubeadmConfig.Spec.JoinConfiguration.ControlPlane, &bootstrapv1.JoinControlPlane{}) {
currentKubeadmConfig.Spec.JoinConfiguration.ControlPlane = nil
}
return match, diff, nil

// Drop differences that do not lead to changes to Machines, but that might exist due
// to changes in how we serialize objects or how webhooks work.
dropOmittableFields(&desiredKubeadmConfig.Spec)
dropOmittableFields(&currentKubeadmConfig.Spec)

return desiredKubeadmConfig, currentKubeadmConfig, nil
}

// getAdjustedKcpConfig takes the KubeadmConfigSpec from KCP and applies the transformations required
// to allow a comparison with the KubeadmConfig referenced from the machine.
// NOTE: The KCP controller applies a set of transformations when creating a KubeadmConfig referenced from the machine;
// those transformations are implemented in ControlPlane.InitialControlPlaneConfig() and ControlPlane.JoinControlPlaneConfig().
func getAdjustedKcpConfig(kcp *controlplanev1.KubeadmControlPlane, machineConfig *bootstrapv1.KubeadmConfig) *bootstrapv1.KubeadmConfigSpec {
func getAdjustedKcpConfig(kcp *controlplanev1.KubeadmControlPlane, machineConfig *bootstrapv1.KubeadmConfig) *bootstrapv1.KubeadmConfig {
kcpConfig := kcp.Spec.KubeadmConfigSpec.DeepCopy()

// if Machine's JoinConfiguration is set, this is a joining control plane machine, so empty out the InitConfiguration;
Expand All @@ -278,36 +257,7 @@ func getAdjustedKcpConfig(kcp *controlplanev1.KubeadmControlPlane, machineConfig
kcpConfig.JoinConfiguration = bootstrapv1.JoinConfiguration{}
}

return kcpConfig
}

// cleanupConfigFields cleanups all the fields that are not relevant for the comparison.
// Note: This function assumes that old defaults have been applied to kcpConfig and machineConfig
// as a consequence we can assume JoinConfiguration and JoinConfiguration.NodeRegistration are always defined.
func cleanupConfigFields(kcpConfig *bootstrapv1.KubeadmConfigSpec, machineConfig *bootstrapv1.KubeadmConfig) {
// KCP ClusterConfiguration will be compared in `matchClusterConfiguration` so we are cleaning it up here
// so it doesn't lead to a duplicate diff in `matchInitOrJoinConfiguration`.
kcpConfig.ClusterConfiguration = bootstrapv1.ClusterConfiguration{}
machineConfig.Spec.ClusterConfiguration = bootstrapv1.ClusterConfiguration{}

// Cleanup JoinConfiguration.Discovery from kcpConfig and machineConfig, because those info are relevant only for
// the join process and not for comparing the configuration of the machine.
// Note: Changes to Discovery will apply for the next join, but they will not lead to a rollout.
kcpConfig.JoinConfiguration.Discovery = bootstrapv1.Discovery{}
machineConfig.Spec.JoinConfiguration.Discovery = bootstrapv1.Discovery{}

// If KCP JoinConfiguration.ControlPlane is nil and the Machine JoinConfiguration.ControlPlane is empty,
// set Machine JoinConfiguration.ControlPlane to nil.
// NOTE: This is required because CABPK applies an empty JoinConfiguration.ControlPlane in case it is nil.
if kcpConfig.JoinConfiguration.ControlPlane == nil &&
reflect.DeepEqual(machineConfig.Spec.JoinConfiguration.ControlPlane, &bootstrapv1.JoinControlPlane{}) {
machineConfig.Spec.JoinConfiguration.ControlPlane = nil
}

// Drop differences that do not lead to changes to Machines, but that might exist due
// to changes in how we serialize objects or how webhooks work.
dropOmittableFields(kcpConfig)
dropOmittableFields(&machineConfig.Spec)
return &bootstrapv1.KubeadmConfig{Spec: *kcpConfig}
}

// dropOmittableFields makes the comparison tolerant to omittable fields being set in the go struct. It applies to:
Expand Down
Loading