move shared function

internal/controllers/topology/cluster/scope/state.go

@@ -18,16 +18,14 @@ package scope
 
 import (
 	"context"
-	"fmt"
 
 	"github.com/pkg/errors"
-	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
+	"sigs.k8s.io/cluster-api/internal/topology/check"
 )
 
 // ClusterState holds all the objects representing the state of a managed Cluster topology.
@@ -102,31 +100,7 @@ type MachineDeploymentState struct {
 // A machine deployment is considered upgrading if at least one of the Machines of this
 // MachineDeployment has a different version.
 func (md *MachineDeploymentState) IsUpgrading(ctx context.Context, c client.Reader) (bool, error) {
-	// If the MachineDeployment has no version there is no definitive way to check if it is upgrading. Therefore, return false.
-	// Note: This case should not happen.
-	if md.Object.Spec.Template.Spec.Version == nil {
-		return false, nil
-	}
-	selectorMap, err := metav1.LabelSelectorAsMap(&md.Object.Spec.Selector)
-	if err != nil {
-		return false, errors.Wrapf(err, "failed to check if MachineDeployment %s is upgrading: failed to convert label selector to map", md.Object.Name)
-	}
-	machines := &clusterv1.MachineList{}
-	if err := c.List(ctx, machines, client.InNamespace(md.Object.Namespace), client.MatchingLabels(selectorMap)); err != nil {
-		return false, errors.Wrapf(err, "failed to check if MachineDeployment %s is upgrading: failed to list Machines", md.Object.Name)
-	}
-	mdVersion := *md.Object.Spec.Template.Spec.Version
-	// Check if the versions of the all the Machines match the MachineDeployment version.
-	for i := range machines.Items {
-		machine := machines.Items[i]
-		if machine.Spec.Version == nil {
-			return false, fmt.Errorf("failed to check if MachineDeployment %s is upgrading: Machine %s has no version", md.Object.Name, machine.Name)
-		}
-		if *machine.Spec.Version != mdVersion {
-			return true, nil
-		}
-	}
-	return false, nil
+	return check.IsMachineDeploymentUpgrading(ctx, c, md.Object)
 }
 
 // MachinePoolsStateMap holds a collection of MachinePool states.
@@ -164,21 +138,5 @@ type MachinePoolState struct {
 // A machine pool is considered upgrading if at least one of the Machines of this
 // MachinePool has a different version.
 func (mp *MachinePoolState) IsUpgrading(ctx context.Context, c client.Reader) (bool, error) {
-	// If the MachinePool has no version there is no definitive way to check if it is upgrading. Therefore, return false.
-	// Note: This case should not happen.
-	if mp.Object.Spec.Template.Spec.Version == nil {
-		return false, nil
-	}
-	mpVersion := *mp.Object.Spec.Template.Spec.Version
-	// Check if the kubelet versions of the MachinePool noderefs match the MachinePool version.
-	for _, nodeRef := range mp.Object.Status.NodeRefs {
-		node := &corev1.Node{}
-		if err := c.Get(ctx, client.ObjectKey{Name: nodeRef.Name}, node); err != nil {
-			return false, fmt.Errorf("failed to check if MachinePool %s is upgrading: failed to get Node %s", mp.Object.Name, nodeRef.Name)
-		}
-		if mpVersion != node.Status.NodeInfo.KubeletVersion {
-			return true, nil
-		}
-	}
-	return false, nil
+	return check.IsMachinePoolUpgrading(ctx, c, mp.Object)
 }

internal/controllers/topology/cluster/scope/state_test.go

@@ -29,92 +29,6 @@ import (
 	"sigs.k8s.io/cluster-api/internal/test/builder"
 )
 
-func TestIsUpgrading(t *testing.T) {
-	g := NewWithT(t)
-	scheme := runtime.NewScheme()
-	g.Expect(clusterv1.AddToScheme(scheme)).To(Succeed())
-
-	tests := []struct {
-		name     string
-		md       *clusterv1.MachineDeployment
-		machines []*clusterv1.Machine
-		want     bool
-		wantErr  bool
-	}{
-		{
-			name: "should return false if all the machines of MachineDeployment have the same version as the MachineDeployment",
-			md: builder.MachineDeployment("ns", "md1").
-				WithClusterName("cluster1").
-				WithVersion("v1.2.3").
-				Build(),
-			machines: []*clusterv1.Machine{
-				builder.Machine("ns", "machine1").
-					WithClusterName("cluster1").
-					WithVersion("v1.2.3").
-					Build(),
-				builder.Machine("ns", "machine2").
-					WithClusterName("cluster1").
-					WithVersion("v1.2.3").
-					Build(),
-			},
-			want:    false,
-			wantErr: false,
-		},
-		{
-			name: "should return true if at least one of the machines of MachineDeployment has a different version",
-			md: builder.MachineDeployment("ns", "md1").
-				WithClusterName("cluster1").
-				WithVersion("v1.2.3").
-				Build(),
-			machines: []*clusterv1.Machine{
-				builder.Machine("ns", "machine1").
-					WithClusterName("cluster1").
-					WithVersion("v1.2.3").
-					Build(),
-				builder.Machine("ns", "machine2").
-					WithClusterName("cluster1").
-					WithVersion("v1.2.2").
-					Build(),
-			},
-			want:    true,
-			wantErr: false,
-		},
-		{
-			name: "should return false if the MachineDeployment has no machines (creation phase)",
-			md: builder.MachineDeployment("ns", "md1").
-				WithClusterName("cluster1").
-				WithVersion("v1.2.3").
-				Build(),
-			machines: []*clusterv1.Machine{},
-			want:     false,
-			wantErr:  false,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			g := NewWithT(t)
-			ctx := context.Background()
-			objs := []client.Object{}
-			objs = append(objs, tt.md)
-			for _, m := range tt.machines {
-				objs = append(objs, m)
-			}
-			fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build()
-			mdState := &MachineDeploymentState{
-				Object: tt.md,
-			}
-			got, err := mdState.IsUpgrading(ctx, fakeClient)
-			if tt.wantErr {
-				g.Expect(err).To(HaveOccurred())
-			} else {
-				g.Expect(err).ToNot(HaveOccurred())
-				g.Expect(got).To(Equal(tt.want))
-			}
-		})
-	}
-}
-
 func TestUpgrading(t *testing.T) {
 	g := NewWithT(t)
 	scheme := runtime.NewScheme()

internal/webhooks/cluster.go

@@ -41,7 +41,6 @@ import (
 	expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
 	"sigs.k8s.io/cluster-api/feature"
 	"sigs.k8s.io/cluster-api/internal/contract"
-	"sigs.k8s.io/cluster-api/internal/controllers/topology/cluster/scope"
 	"sigs.k8s.io/cluster-api/internal/topology/check"
 	"sigs.k8s.io/cluster-api/internal/topology/variables"
 	"sigs.k8s.io/cluster-api/util/conditions"
@@ -525,7 +524,7 @@ func validateTopologyMachineDeploymentVersions(ctx context.Context, fldPath *fie
 	for i := range mds.Items {
 		md := &mds.Items[i]
 
-		upgrading, err := (&scope.MachineDeploymentState{Object: md}).IsUpgrading(ctx, ctrlClient)
+		upgrading, err := check.IsMachineDeploymentUpgrading(ctx, ctrlClient, md)
 		if err != nil {
 			return errors.Wrap(err, "failed to list upgrading MachinePools")
 		}
@@ -571,7 +570,8 @@ func validateTopologyMachinePoolVersions(ctx context.Context, fldPath *field.Pat
 
 	for i := range mps.Items {
 		mp := &mps.Items[i]
-		upgrading, err := (&scope.MachinePoolState{Object: mp}).IsUpgrading(ctx, wlClient)
+
+		upgrading, err := check.IsMachinePoolUpgrading(ctx, wlClient, mp)
 		if err != nil {
 			return errors.Wrap(err, "failed to list upgrading MachinePools")
 		}