diff --git a/internal/controllers/topology/cluster/scope/state.go b/internal/controllers/topology/cluster/scope/state.go index b2a20c7f1da2..00ebc04bbbb5 100644 --- a/internal/controllers/topology/cluster/scope/state.go +++ b/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) } diff --git a/internal/controllers/topology/cluster/scope/state_test.go b/internal/controllers/topology/cluster/scope/state_test.go index 40a7a9d97a6c..b76e0d21ef51 100644 --- a/internal/controllers/topology/cluster/scope/state_test.go +++ b/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() diff --git a/internal/topology/check/upgrade.go b/internal/topology/check/upgrade.go new file mode 100644 index 000000000000..affb359cbf11 --- /dev/null +++ b/internal/topology/check/upgrade.go @@ -0,0 +1,84 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package check + +import ( + "context" + "fmt" + + "github.com/pkg/errors" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1" +) + +// IsMachineDeploymentUpgrading determines if the MachineDeployment is upgrading. +// A machine deployment is considered upgrading if at least one of the Machines of this +// MachineDeployment has a different version. +func IsMachineDeploymentUpgrading(ctx context.Context, c client.Reader, md *clusterv1.MachineDeployment) (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.Spec.Template.Spec.Version == nil { + return false, nil + } + selectorMap, err := metav1.LabelSelectorAsMap(&md.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.Name) + } + machines := &clusterv1.MachineList{} + if err := c.List(ctx, machines, client.InNamespace(md.Namespace), client.MatchingLabels(selectorMap)); err != nil { + return false, errors.Wrapf(err, "failed to check if MachineDeployment %s is upgrading: failed to list Machines", md.Name) + } + mdVersion := *md.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.Name, machine.Name) + } + if *machine.Spec.Version != mdVersion { + return true, nil + } + } + return false, nil +} + +// IsMachinePoolUpgrading determines if the MachinePool is upgrading. +// A machine pool is considered upgrading if at least one of the Machines of this +// MachinePool has a different version. +func IsMachinePoolUpgrading(ctx context.Context, c client.Reader, mp *expv1.MachinePool) (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.Spec.Template.Spec.Version == nil { + return false, nil + } + mpVersion := *mp.Spec.Template.Spec.Version + // Check if the kubelet versions of the MachinePool noderefs match the MachinePool version. + for _, nodeRef := range mp.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.Name, nodeRef.Name) + } + if mpVersion != node.Status.NodeInfo.KubeletVersion { + return true, nil + } + } + return false, nil +} diff --git a/internal/topology/check/upgrade_test.go b/internal/topology/check/upgrade_test.go new file mode 100644 index 000000000000..601343ef9c07 --- /dev/null +++ b/internal/topology/check/upgrade_test.go @@ -0,0 +1,222 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package check + +import ( + "context" + "testing" + + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1" + "sigs.k8s.io/cluster-api/internal/test/builder" +) + +func TestIsMachineDeploymentUpgrading(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() + + got, err := IsMachineDeploymentUpgrading(ctx, fakeClient, tt.md) + if tt.wantErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).To(Equal(tt.want)) + } + }) + } +} + +func TestIsMachinePoolUpgrading(t *testing.T) { + g := NewWithT(t) + scheme := runtime.NewScheme() + g.Expect(corev1.AddToScheme(scheme)).To(Succeed()) + + tests := []struct { + name string + mp *expv1.MachinePool + nodes []*corev1.Node + want bool + wantErr bool + }{ + { + name: "should return false if all the nodes of MachinePool have the same version as the MachinePool", + mp: builder.MachinePool("ns", "mp1"). + WithClusterName("cluster1"). + WithVersion("v1.2.3"). + WithStatus(expv1.MachinePoolStatus{ + NodeRefs: []corev1.ObjectReference{ + {Name: "node1"}, + {Name: "node2"}, + }}). + Build(), + nodes: []*corev1.Node{ + { + ObjectMeta: metav1.ObjectMeta{Name: "node1"}, + Status: corev1.NodeStatus{ + NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.2.3"}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "node2"}, + Status: corev1.NodeStatus{ + NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.2.3"}, + }, + }, + }, + want: false, + wantErr: false, + }, + { + name: "should return true if at least one of the nodes of MachinePool has a different version", + mp: builder.MachinePool("ns", "mp1"). + WithClusterName("cluster1"). + WithVersion("v1.2.3"). + WithStatus(expv1.MachinePoolStatus{ + NodeRefs: []corev1.ObjectReference{ + {Name: "node1"}, + {Name: "node2"}, + }}). + Build(), + nodes: []*corev1.Node{ + { + ObjectMeta: metav1.ObjectMeta{Name: "node1"}, + Status: corev1.NodeStatus{ + NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.2.3"}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "node2"}, + Status: corev1.NodeStatus{ + NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.2.2"}, + }, + }, + }, + want: true, + wantErr: false, + }, + { + name: "should return false if the MachinePool has no nodes (creation phase)", + mp: builder.MachinePool("ns", "mp1"). + WithClusterName("cluster1"). + WithVersion("v1.2.3"). + WithStatus(expv1.MachinePoolStatus{ + NodeRefs: []corev1.ObjectReference{}}). + Build(), + nodes: []*corev1.Node{}, + 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{} + for _, m := range tt.nodes { + objs = append(objs, m) + } + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build() + + got, err := IsMachinePoolUpgrading(ctx, fakeClient, tt.mp) + if tt.wantErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).To(Equal(tt.want)) + } + }) + } +} diff --git a/internal/webhooks/cluster.go b/internal/webhooks/cluster.go index c2a265968de6..bc2f3a4088a8 100644 --- a/internal/webhooks/cluster.go +++ b/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") }