Fix UpToDateResult struct and variable names

Signed-off-by: Stefan Büringer [email protected]

Author: sbueringer

controlplane/kubeadm/internal/control_plane.go

@@ -51,12 +51,12 @@ type ControlPlane struct {
 	machinesPatchHelpers map[string]*patch.Helper
 
 	// MachinesNotUpToDate is the source of truth for Machines that are not up-to-date.
-	// It should be used to check if a Machine is up-to-date (not machinesNotUpToDateResults).
+	// It should be used to check if a Machine is up-to-date (not machinesUpToDateResults).
 	MachinesNotUpToDate collections.Machines
-	// machinesNotUpToDateResults is used to store the result of the UpToDate call for all Machines
+	// machinesUpToDateResults is used to store the result of the UpToDate call for all Machines
 	// (even for Machines that are up-to-date).
 	// MachinesNotUpToDate should always be used instead to check if a Machine is up-to-date.
-	machinesNotUpToDateResults map[string]NotUpToDateResult
+	machinesUpToDateResults map[string]UpToDateResult
 
 	// reconciliationTime is the time of the current reconciliation, and should be used for all "now" calculations
 	reconciliationTime metav1.Time
@@ -122,9 +122,9 @@ 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))
-	machinesNotUpToDateResults := map[string]NotUpToDateResult{}
+	machinesUpToDateResults := map[string]UpToDateResult{}
 	for _, m := range ownedMachines {
-		upToDate, notUpToDateResult, err := UpToDate(ctx, client, cluster, m, kcp, &reconciliationTime, infraMachines, kubeadmConfigs)
+		upToDate, upToDateResult, err := UpToDate(ctx, client, cluster, m, kcp, &reconciliationTime, infraMachines, kubeadmConfigs)
 		if err != nil {
 			return nil, err
 		}
@@ -133,20 +133,20 @@ func NewControlPlane(ctx context.Context, managementCluster ManagementCluster, c
 		}
 		// Set this even if machine is UpToDate. This is needed to complete triggering in-place updates
 		// MachinesNotUpToDate should always be used instead to check if a Machine is up-to-date.
-		machinesNotUpToDateResults[m.Name] = *notUpToDateResult
+		machinesUpToDateResults[m.Name] = *upToDateResult
 	}
 
 	return &ControlPlane{
-		KCP:                        kcp,
-		Cluster:                    cluster,
-		Machines:                   ownedMachines,
-		machinesPatchHelpers:       patchHelpers,
-		MachinesNotUpToDate:        machinesNotUptoDate,
-		machinesNotUpToDateResults: machinesNotUpToDateResults,
-		KubeadmConfigs:             kubeadmConfigs,
-		InfraResources:             infraMachines,
-		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
 }
 
@@ -240,15 +240,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]NotUpToDateResult) {
+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.machinesNotUpToDateResults
+	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]NotUpToDateResult) {
-	return c.MachinesNotUpToDate, c.machinesNotUpToDateResults
+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

controlplane/kubeadm/internal/control_plane_test.go

@@ -122,19 +122,19 @@ func TestControlPlane(t *testing.T) {
 
 		g.Expect(controlPlane.Machines).To(HaveLen(5))
 
-		machinesNotUptoDate, machinesNotUpToDateResults := controlPlane.NotUpToDateMachines()
+		machinesNotUptoDate, machinesUpToDateResults := controlPlane.NotUpToDateMachines()
 		g.Expect(machinesNotUptoDate.Names()).To(ConsistOf("m2", "m3"))
-		// machinesNotUpToDateResults contains results for all Machines (including up-to-date Machines).
-		g.Expect(machinesNotUpToDateResults).To(HaveLen(5))
-		g.Expect(machinesNotUpToDateResults["m2"].ConditionMessages).To(Equal([]string{"Version v1.29.0, v1.31.0 required"}))
-		g.Expect(machinesNotUpToDateResults["m3"].ConditionMessages).To(Equal([]string{"Version v1.29.3, v1.31.0 required"}))
+		// machinesUpToDateResults contains results for all Machines (including up-to-date Machines).
+		g.Expect(machinesUpToDateResults).To(HaveLen(5))
+		g.Expect(machinesUpToDateResults["m2"].ConditionMessages).To(Equal([]string{"Version v1.29.0, v1.31.0 required"}))
+		g.Expect(machinesUpToDateResults["m3"].ConditionMessages).To(Equal([]string{"Version v1.29.3, v1.31.0 required"}))
 
-		machinesNeedingRollout, machinesNotUpToDateResults := controlPlane.MachinesNeedingRollout()
+		machinesNeedingRollout, machinesUpToDateResults := controlPlane.MachinesNeedingRollout()
 		g.Expect(machinesNeedingRollout.Names()).To(ConsistOf("m2"))
-		// machinesNotUpToDateResults contains results for all Machines (including up-to-date Machines).
-		g.Expect(machinesNotUpToDateResults).To(HaveLen(5))
-		g.Expect(machinesNotUpToDateResults["m2"].LogMessages).To(Equal([]string{"Machine version \"v1.29.0\" is not equal to KCP version \"v1.31.0\""}))
-		g.Expect(machinesNotUpToDateResults["m3"].LogMessages).To(Equal([]string{"Machine version \"v1.29.3\" is not equal to KCP version \"v1.31.0\""}))
+		// machinesUpToDateResults contains results for all Machines (including up-to-date Machines).
+		g.Expect(machinesUpToDateResults).To(HaveLen(5))
+		g.Expect(machinesUpToDateResults["m2"].LogMessages).To(Equal([]string{"Machine version \"v1.29.0\" is not equal to KCP version \"v1.31.0\""}))
+		g.Expect(machinesUpToDateResults["m3"].LogMessages).To(Equal([]string{"Machine version \"v1.29.3\" is not equal to KCP version \"v1.31.0\""}))
 
 		upToDateMachines := controlPlane.UpToDateMachines()
 		g.Expect(upToDateMachines).To(HaveLen(3))

controlplane/kubeadm/internal/controllers/controller.go

@@ -97,7 +97,7 @@ type KubeadmControlPlaneReconciler struct {
 	ssaCache                  ssa.Cache
 
 	// Only used for testing
-	overrideTryInPlaceUpdateFunc      func(ctx context.Context, controlPlane *internal.ControlPlane, machineToInPlaceUpdate *clusterv1.Machine, machinesNeedingRolloutResult internal.NotUpToDateResult) (bool, ctrl.Result, error)
+	overrideTryInPlaceUpdateFunc      func(ctx context.Context, controlPlane *internal.ControlPlane, machineToInPlaceUpdate *clusterv1.Machine, machineUpToDateResult internal.UpToDateResult) (bool, ctrl.Result, error)
 	overrideScaleUpControlPlaneFunc   func(ctx context.Context, controlPlane *internal.ControlPlane) (ctrl.Result, error)
 	overrideScaleDownControlPlaneFunc func(ctx context.Context, controlPlane *internal.ControlPlane, machineToDelete *clusterv1.Machine) (ctrl.Result, error)
 }
@@ -472,16 +472,16 @@ 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, machinesNeedingRolloutResults := controlPlane.MachinesNeedingRollout()
+	machinesNeedingRollout, machinesUpToDateResults := controlPlane.MachinesNeedingRollout()
 	switch {
 	case len(machinesNeedingRollout) > 0:
 		var allMessages []string
-		for machine, machinesNeedingRolloutResult := range machinesNeedingRolloutResults {
-			allMessages = append(allMessages, fmt.Sprintf("Machine %s needs rollout: %s", machine, strings.Join(machinesNeedingRolloutResult.LogMessages, ",")))
+		for machine, machineUpToDateResult := range machinesUpToDateResults {
+			allMessages = append(allMessages, fmt.Sprintf("Machine %s needs rollout: %s", machine, strings.Join(machineUpToDateResult.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))
-		return r.updateControlPlane(ctx, controlPlane, machinesNeedingRollout, machinesNeedingRolloutResults)
+		return r.updateControlPlane(ctx, controlPlane, machinesNeedingRollout, machinesUpToDateResults)
 	default:
 		// make sure last upgrade operation is marked as completed.
 		// NOTE: we are checking the condition already exists in order to avoid to set this condition at the first
@@ -985,16 +985,16 @@ func (r *KubeadmControlPlaneReconciler) reconcileControlPlaneAndMachinesConditio
 }
 
 func reconcileMachineUpToDateCondition(_ context.Context, controlPlane *internal.ControlPlane) {
-	machinesNotUptoDate, machinesNotUpToDateResults := 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 machinesNotUpToDateResult, ok := machinesNotUpToDateResults[machine.Name]; ok && len(machinesNotUpToDateResult.ConditionMessages) > 0 {
+			if machineUpToDateResult, ok := machinesUpToDateResults[machine.Name]; ok && len(machineUpToDateResult.ConditionMessages) > 0 {
 				var reasons []string
-				for _, conditionMessage := range machinesNotUpToDateResult.ConditionMessages {
+				for _, conditionMessage := range machineUpToDateResult.ConditionMessages {
 					reasons = append(reasons, fmt.Sprintf("* %s", conditionMessage))
 				}
 				message = strings.Join(reasons, "\n")

controlplane/kubeadm/internal/controllers/inplace.go

@@ -29,10 +29,10 @@ func (r *KubeadmControlPlaneReconciler) tryInPlaceUpdate(
 	ctx context.Context,
 	controlPlane *internal.ControlPlane,
 	machineToInPlaceUpdate *clusterv1.Machine,
-	machinesNeedingRolloutResult internal.NotUpToDateResult,
+	machineUpToDateResult internal.UpToDateResult,
 ) (fallbackToScaleDown bool, _ ctrl.Result, _ error) {
 	if r.overrideTryInPlaceUpdateFunc != nil {
-		return r.overrideTryInPlaceUpdateFunc(ctx, controlPlane, machineToInPlaceUpdate, machinesNeedingRolloutResult)
+		return r.overrideTryInPlaceUpdateFunc(ctx, controlPlane, machineToInPlaceUpdate, machineUpToDateResult)
 	}
 
 	// Always fallback to scale down until in-place is implemented.

controlplane/kubeadm/internal/controllers/upgrade.go

@@ -35,7 +35,7 @@ func (r *KubeadmControlPlaneReconciler) updateControlPlane(
 	ctx context.Context,
 	controlPlane *internal.ControlPlane,
 	machinesNeedingRollout collections.Machines,
-	machinesNeedingRolloutResults map[string]internal.NotUpToDateResult,
+	machinesUpToDateResults map[string]internal.UpToDateResult,
 ) (ctrl.Result, error) {
 	log := ctrl.LoggerFrom(ctx)
 
@@ -89,7 +89,7 @@ func (r *KubeadmControlPlaneReconciler) updateControlPlane(
 	switch controlPlane.KCP.Spec.Rollout.Strategy.Type {
 	case controlplanev1.RollingUpdateStrategyType:
 		// RolloutStrategy is currently defaulted and validated to be RollingUpdate
-		res, err := r.rollingUpdate(ctx, controlPlane, machinesNeedingRollout, machinesNeedingRolloutResults)
+		res, err := r.rollingUpdate(ctx, controlPlane, machinesNeedingRollout, machinesUpToDateResults)
 		if err != nil {
 			return ctrl.Result{}, errors.Wrapf(err, "failed to update control plane")
 		}
@@ -104,7 +104,7 @@ func (r *KubeadmControlPlaneReconciler) rollingUpdate(
 	ctx context.Context,
 	controlPlane *internal.ControlPlane,
 	machinesNeedingRollout collections.Machines,
-	machinesNeedingRolloutResults map[string]internal.NotUpToDateResult,
+	machinesUpToDateResults map[string]internal.UpToDateResult,
 ) (ctrl.Result, error) {
 	currentReplicas := int32(controlPlane.Machines.Len())
 	currentUpToDateReplicas := int32(controlPlane.UpToDateMachines().Len())
@@ -129,19 +129,19 @@ func (r *KubeadmControlPlaneReconciler) rollingUpdate(
 	if err != nil {
 		return ctrl.Result{}, errors.Wrap(err, "failed to select next Machine for rollout")
 	}
-	machinesNeedingRolloutResult, ok := machinesNeedingRolloutResults[machineToInPlaceUpdateOrScaleDown.Name]
+	machineUpToDateResult, ok := machinesUpToDateResults[machineToInPlaceUpdateOrScaleDown.Name]
 	if !ok {
-		// Note: This should never happen as we store results for all Machines in machinesNeedingRolloutResults.
+		// Note: This should never happen as we store results for all Machines in machinesUpToDateResults.
 		return ctrl.Result{}, errors.Errorf("failed to check if Machine %s is UpToDate", machineToInPlaceUpdateOrScaleDown.Name)
 	}
 
 	// If the selected Machine is eligible for in-place update and we don't already have enough up-to-date replicas, try in-place update.
 	// Note: To be safe we only try an in-place update when we would otherwise delete a Machine. This ensures we could
 	// afford if the in-place update fails and the Machine becomes unavailable (and eventually MHC kicks in and the Machine is recreated).
 	if feature.Gates.Enabled(feature.InPlaceUpdates) &&
-		machinesNeedingRolloutResult.EligibleForInPlaceUpdate &&
+		machineUpToDateResult.EligibleForInPlaceUpdate &&
 		currentUpToDateReplicas < desiredReplicas {
-		fallbackToScaleDown, res, err := r.tryInPlaceUpdate(ctx, controlPlane, machineToInPlaceUpdateOrScaleDown, machinesNeedingRolloutResult)
+		fallbackToScaleDown, res, err := r.tryInPlaceUpdate(ctx, controlPlane, machineToInPlaceUpdateOrScaleDown, machineUpToDateResult)
 		if err != nil {
 			return ctrl.Result{}, err
 		}