Cleanup KCP code: variable/func renames, func order, UpToDate result

struct

Author: sbueringer

api/bootstrap/kubeadm/v1beta2/kubeadm_types.go

@@ -174,16 +174,7 @@ type ClusterConfiguration struct {
 	CertificatesDir string `json:"certificatesDir,omitempty"`
 
 	// imageRepository sets the container registry to pull images from.
-	// * If not set, the default registry of kubeadm will be used, i.e.
-	//   * registry.k8s.io (new registry): >= v1.22.17, >= v1.23.15, >= v1.24.9, >= v1.25.0
-	//   * k8s.gcr.io (old registry): all older versions
-	//   Please note that when imageRepository is not set we don't allow upgrades to
-	//   versions >= v1.22.0 which use the old registry (k8s.gcr.io). Please use
-	//   a newer patch version with the new registry instead (i.e. >= v1.22.17,
-	//   >= v1.23.15, >= v1.24.9, >= v1.25.0).
-	// * If the version is a CI build (kubernetes version starts with `ci/` or `ci-cross/`)
-	//  `gcr.io/k8s-staging-ci-images` will be used as a default for control plane components
-	//   and for kube-proxy, while `registry.k8s.io` will be used for all the other images.
+	// If not set, the default registry of kubeadm will be used (registry.k8s.io).
 	// +optional
 	// +kubebuilder:validation:MinLength=1
 	// +kubebuilder:validation:MaxLength=512

api/controlplane/kubeadm/v1beta2/kubeadm_control_plane_types.go

@@ -420,11 +420,6 @@ type KubeadmControlPlaneSpec struct {
 	Replicas *int32 `json:"replicas,omitempty"`
 
 	// version defines the desired Kubernetes version.
-	// Please note that if kubeadmConfigSpec.ClusterConfiguration.imageRepository is not set
-	// we don't allow upgrades to versions >= v1.22.0 for which kubeadm uses the old registry (k8s.gcr.io).
-	// Please use a newer patch version with the new registry instead. The default registries of kubeadm are:
-	//   * registry.k8s.io (new registry): >= v1.22.17, >= v1.23.15, >= v1.24.9, >= v1.25.0
-	//   * k8s.gcr.io (old registry): all older versions
 	// +required
 	// +kubebuilder:validation:MinLength=1
 	// +kubebuilder:validation:MaxLength=256

bootstrap/kubeadm/config/crd/bases/bootstrap.cluster.x-k8s.io_kubeadmconfigs.yaml

@@ -50,9 +50,8 @@ type ControlPlane struct {
 	Machines             collections.Machines
 	machinesPatchHelpers map[string]*patch.Helper
 
-	machinesNotUptoDate                  collections.Machines
-	machinesNotUptoDateLogMessages       map[string][]string
-	machinesNotUptoDateConditionMessages map[string][]string
+	machinesNotUptoDate     collections.Machines
+	machinesUpToDateResults map[string]UpToDateResult
 
 	// reconciliationTime is the time of the current reconciliation, and should be used for all "now" calculations
 	reconciliationTime metav1.Time
@@ -84,6 +83,11 @@ type ControlPlane struct {
 	DeletingMessage string
 }
 
+type UpToDateResult struct {
+	LogMessages       []string
+	ConditionMessages []string
+}
+
 // PreflightCheckResults contains description about pre flight check results blocking machines creation or deletion.
 type PreflightCheckResults struct {
 	// HasDeletingMachine reports true if preflight check detected a deleting machine.
@@ -98,7 +102,7 @@ type PreflightCheckResults struct {
 
 // NewControlPlane returns an instantiated ControlPlane.
 func NewControlPlane(ctx context.Context, managementCluster ManagementCluster, client client.Client, cluster *clusterv1.Cluster, kcp *controlplanev1.KubeadmControlPlane, ownedMachines collections.Machines) (*ControlPlane, error) {
-	infraObjects, err := getInfraResources(ctx, client, ownedMachines)
+	infraMachines, err := getInfraMachines(ctx, client, ownedMachines)
 	if err != nil {
 		return nil, err
 	}
@@ -118,32 +122,29 @@ func NewControlPlane(ctx context.Context, managementCluster ManagementCluster, c
 	// Select machines that should be rolled out because of an outdated configuration or because rolloutAfter/Before expired.
 	reconciliationTime := metav1.Now()
 	machinesNotUptoDate := make(collections.Machines, len(ownedMachines))
-	machinesNotUptoDateLogMessages := map[string][]string{}
-	machinesNotUptoDateConditionMessages := map[string][]string{}
+	machinesUpToDateResults := map[string]UpToDateResult{}
 	for _, m := range ownedMachines {
-		upToDate, logMessages, conditionMessages, err := UpToDate(m, kcp, &reconciliationTime, infraObjects, kubeadmConfigs)
+		upToDate, upToDateResult, err := UpToDate(m, kcp, &reconciliationTime, infraMachines, kubeadmConfigs)
 		if err != nil {
 			return nil, err
 		}
 		if !upToDate {
 			machinesNotUptoDate.Insert(m)
-			machinesNotUptoDateLogMessages[m.Name] = logMessages
-			machinesNotUptoDateConditionMessages[m.Name] = conditionMessages
+			machinesUpToDateResults[m.Name] = *upToDateResult
 		}
 	}
 
 	return &ControlPlane{
-		KCP:                                  kcp,
-		Cluster:                              cluster,
-		Machines:                             ownedMachines,
-		machinesPatchHelpers:                 patchHelpers,
-		machinesNotUptoDate:                  machinesNotUptoDate,
-		machinesNotUptoDateLogMessages:       machinesNotUptoDateLogMessages,
-		machinesNotUptoDateConditionMessages: machinesNotUptoDateConditionMessages,
-		KubeadmConfigs:                       kubeadmConfigs,
-		InfraResources:                       infraObjects,
-		reconciliationTime:                   reconciliationTime,
-		managementCluster:                    managementCluster,
+		KCP:                     kcp,
+		Cluster:                 cluster,
+		Machines:                ownedMachines,
+		machinesPatchHelpers:    patchHelpers,
+		machinesNotUptoDate:     machinesNotUptoDate,
+		machinesUpToDateResults: machinesUpToDateResults,
+		KubeadmConfigs:          kubeadmConfigs,
+		InfraResources:          infraMachines,
+		reconciliationTime:      reconciliationTime,
+		managementCluster:       managementCluster,
 	}, nil
 }
 
@@ -256,15 +257,15 @@ func (c *ControlPlane) GetKubeadmConfig(machineName string) (*bootstrapv1.Kubead
 }
 
 // MachinesNeedingRollout return a list of machines that need to be rolled out.
-func (c *ControlPlane) MachinesNeedingRollout() (collections.Machines, map[string][]string) {
+func (c *ControlPlane) MachinesNeedingRollout() (collections.Machines, map[string]UpToDateResult) {
 	// Note: Machines already deleted are dropped because they will be replaced by new machines after deletion completes.
-	return c.machinesNotUptoDate.Filter(collections.Not(collections.HasDeletionTimestamp)), c.machinesNotUptoDateLogMessages
+	return c.machinesNotUptoDate.Filter(collections.Not(collections.HasDeletionTimestamp)), c.machinesUpToDateResults
 }
 
 // NotUpToDateMachines return a list of machines that are not up to date with the control
 // plane's configuration.
-func (c *ControlPlane) NotUpToDateMachines() (collections.Machines, map[string][]string) {
-	return c.machinesNotUptoDate, c.machinesNotUptoDateConditionMessages
+func (c *ControlPlane) NotUpToDateMachines() (collections.Machines, map[string]UpToDateResult) {
+	return c.machinesNotUptoDate, c.machinesUpToDateResults
 }
 
 // UpToDateMachines returns the machines that are up to date with the control
@@ -273,18 +274,18 @@ func (c *ControlPlane) UpToDateMachines() collections.Machines {
 	return c.Machines.Difference(c.machinesNotUptoDate)
 }
 
-// getInfraResources fetches the external infrastructure resource for each machine in the collection and returns a map of machine.Name -> infraResource.
-func getInfraResources(ctx context.Context, cl client.Client, machines collections.Machines) (map[string]*unstructured.Unstructured, error) {
+// getInfraMachines fetches the InfraMachine for each machine in the collection and returns a map of machine.Name -> InfraMachine.
+func getInfraMachines(ctx context.Context, cl client.Client, machines collections.Machines) (map[string]*unstructured.Unstructured, error) {
 	result := map[string]*unstructured.Unstructured{}
 	for _, m := range machines {
-		infraObj, err := external.GetObjectFromContractVersionedRef(ctx, cl, m.Spec.InfrastructureRef, m.Namespace)
+		infraMachine, err := external.GetObjectFromContractVersionedRef(ctx, cl, m.Spec.InfrastructureRef, m.Namespace)
 		if err != nil {
 			if apierrors.IsNotFound(errors.Cause(err)) {
 				continue
 			}
-			return nil, errors.Wrapf(err, "failed to retrieve infra obj for machine %q", m.Name)
+			return nil, errors.Wrapf(err, "failed to retrieve InfraMachine for Machine %s", m.Name)
 		}
-		result[m.Name] = infraObj
+		result[m.Name] = infraMachine
 	}
 	return result, nil
 }
@@ -297,14 +298,14 @@ func getKubeadmConfigs(ctx context.Context, cl client.Client, machines collectio
 		if !bootstrapRef.IsDefined() {
 			continue
 		}
-		machineConfig := &bootstrapv1.KubeadmConfig{}
-		if err := cl.Get(ctx, client.ObjectKey{Name: bootstrapRef.Name, Namespace: m.Namespace}, machineConfig); err != nil {
+		kubeadmConfig := &bootstrapv1.KubeadmConfig{}
+		if err := cl.Get(ctx, client.ObjectKey{Name: bootstrapRef.Name, Namespace: m.Namespace}, kubeadmConfig); err != nil {
 			if apierrors.IsNotFound(errors.Cause(err)) {
 				continue
 			}
-			return nil, errors.Wrapf(err, "failed to retrieve bootstrap config for machine %q", m.Name)
+			return nil, errors.Wrapf(err, "failed to retrieve KubeadmConfig for Machine %s", m.Name)
 		}
-		result[m.Name] = machineConfig
+		result[m.Name] = kubeadmConfig
 	}
 	return result, nil
 }

bootstrap/kubeadm/config/crd/bases/bootstrap.cluster.x-k8s.io_kubeadmconfigtemplates.yaml

@@ -467,12 +467,12 @@ func (r *KubeadmControlPlaneReconciler) reconcile(ctx context.Context, controlPl
 	}
 
 	// Control plane machines rollout due to configuration changes (e.g. upgrades) takes precedence over other operations.
-	machinesNeedingRollout, machinesNeedingRolloutLogMessages := controlPlane.MachinesNeedingRollout()
+	machinesNeedingRollout, machinesUpToDateResults := controlPlane.MachinesNeedingRollout()
 	switch {
 	case len(machinesNeedingRollout) > 0:
 		var allMessages []string
-		for machine, messages := range machinesNeedingRolloutLogMessages {
-			allMessages = append(allMessages, fmt.Sprintf("Machine %s needs rollout: %s", machine, strings.Join(messages, ",")))
+		for machine, upToDateResult := range machinesUpToDateResults {
+			allMessages = append(allMessages, fmt.Sprintf("Machine %s needs rollout: %s", machine, strings.Join(upToDateResult.LogMessages, ",")))
 		}
 		log.Info(fmt.Sprintf("Rolling out Control Plane machines: %s", strings.Join(allMessages, ",")), "machinesNeedingRollout", machinesNeedingRollout.Names())
 		v1beta1conditions.MarkFalse(controlPlane.KCP, controlplanev1.MachinesSpecUpToDateV1Beta1Condition, controlplanev1.RollingUpdateInProgressV1Beta1Reason, clusterv1.ConditionSeverityWarning, "Rolling %d replicas with outdated spec (%d replicas up to date)", len(machinesNeedingRollout), len(controlPlane.Machines)-len(machinesNeedingRollout))
@@ -840,7 +840,7 @@ func (r *KubeadmControlPlaneReconciler) syncMachines(ctx context.Context, contro
 				return errors.Wrapf(err, "failed to clean up managedFields of InfrastructureMachine %s", klog.KObj(infraMachine))
 			}
 			// Update in-place mutating fields on InfrastructureMachine.
-			if err := r.updateExternalObject(ctx, infraMachine, controlPlane.KCP, controlPlane.Cluster); err != nil {
+			if err := r.updateExternalObject(ctx, infraMachine, infraMachine.GroupVersionKind(), controlPlane.KCP, controlPlane.Cluster); err != nil {
 				return errors.Wrapf(err, "failed to update InfrastructureMachine %s", klog.KObj(infraMachine))
 			}
 		}
@@ -849,8 +849,6 @@ func (r *KubeadmControlPlaneReconciler) syncMachines(ctx context.Context, contro
 		// Only update the KubeadmConfig if it is already found, otherwise just skip it.
 		// This could happen e.g. if the cache is not up-to-date yet.
 		if kubeadmConfigFound {
-			// Note: Set the GroupVersionKind because updateExternalObject depends on it.
-			kubeadmConfig.SetGroupVersionKind(bootstrapv1.GroupVersion.WithKind("KubeadmConfig"))
 			// Cleanup managed fields of all KubeadmConfigs to drop ownership of labels and annotations
 			// from "manager". We do this so that KubeadmConfigs that are created using the Create method
 			// can also work with SSA. Otherwise, labels and annotations would be co-owned by our "old" "manager"
@@ -859,7 +857,7 @@ func (r *KubeadmControlPlaneReconciler) syncMachines(ctx context.Context, contro
 				return errors.Wrapf(err, "failed to clean up managedFields of KubeadmConfig %s", klog.KObj(kubeadmConfig))
 			}
 			// Update in-place mutating fields on BootstrapConfig.
-			if err := r.updateExternalObject(ctx, kubeadmConfig, controlPlane.KCP, controlPlane.Cluster); err != nil {
+			if err := r.updateExternalObject(ctx, kubeadmConfig, bootstrapv1.GroupVersion.WithKind("KubeadmConfig"), controlPlane.KCP, controlPlane.Cluster); err != nil {
 				return errors.Wrapf(err, "failed to update KubeadmConfig %s", klog.KObj(kubeadmConfig))
 			}
 		}
@@ -980,16 +978,17 @@ func (r *KubeadmControlPlaneReconciler) reconcileControlPlaneAndMachinesConditio
 }
 
 func reconcileMachineUpToDateCondition(_ context.Context, controlPlane *internal.ControlPlane) {
-	machinesNotUptoDate, machinesNotUptoDateConditionMessages := controlPlane.NotUpToDateMachines()
+	machinesNotUptoDate, machinesUpToDateResults := controlPlane.NotUpToDateMachines()
 	machinesNotUptoDateNames := sets.New(machinesNotUptoDate.Names()...)
 
 	for _, machine := range controlPlane.Machines {
 		if machinesNotUptoDateNames.Has(machine.Name) {
 			// Note: the code computing the message for KCP's RolloutOut condition is making assumptions on the format/content of this message.
 			message := ""
-			if reasons, ok := machinesNotUptoDateConditionMessages[machine.Name]; ok {
-				for i := range reasons {
-					reasons[i] = fmt.Sprintf("* %s", reasons[i])
+			if upToDateResults, ok := machinesUpToDateResults[machine.Name]; ok && len(upToDateResults.ConditionMessages) > 0 {
+				var reasons []string
+				for _, conditionMessage := range upToDateResults.ConditionMessages {
+					reasons = append(reasons, fmt.Sprintf("* %s", conditionMessage))
 				}
 				message = strings.Join(reasons, "\n")
 			}