diff --git a/internal/webhooks/cluster.go b/internal/webhooks/cluster.go index f94a24925fcb..bbb759be3713 100644 --- a/internal/webhooks/cluster.go +++ b/internal/webhooks/cluster.go @@ -359,7 +359,9 @@ func (webhook *Cluster) validateTopology(ctx context.Context, oldCluster, newClu if _, ok := newCluster.GetAnnotations()[clusterv1.ClusterTopologyUnsafeUpdateVersionAnnotation]; ok { log := ctrl.LoggerFrom(ctx) - log.Info(fmt.Sprintf("Skipping version validation for object because annotation %q is set", clusterv1.ClusterTopologyUnsafeUpdateVersionAnnotation)) + warningMsg := fmt.Sprintf("Skipping version validation for Cluster because annotation %q is exists.", clusterv1.ClusterTopologyUnsafeUpdateVersionAnnotation) + log.Info(warningMsg) + allWarnings = append(allWarnings, warningMsg) } else { if errs := webhook.validateTopologyVersion(ctx, fldPath.Child("version"), newCluster.Spec.Topology.Version, inVersion, oldVersion, oldCluster); len(errs) > 0 { allErrs = append(allErrs, errs...) @@ -465,36 +467,37 @@ func validateTopologyControlPlaneVersion(ctx context.Context, fldPath *field.Pat if err != nil { return field.InternalError(fldPath, err) } - provisioning, err := contract.ControlPlane().IsProvisioning(cp) + + cpVersionString, err := contract.ControlPlane().Version().Get(cp) if err != nil { return field.InternalError(fldPath, err) } - if provisioning { - return errors.New("ControlPlane is currently provisioning") + cpVersion, err := semver.ParseTolerant(*cpVersionString) + if err != nil { + // NOTE: this should never happen. Nevertheless, handling this for extra caution. + return field.InternalError(fldPath, errors.New("unable to parse version of ControlPlane")) + } + if cpVersion.NE(oldVersion) { + return fmt.Errorf("ControlPlane version %q does not match the current version %q", cpVersion, oldVersion) } - upgrading, err := contract.ControlPlane().IsUpgrading(cp) + provisioning, err := contract.ControlPlane().IsProvisioning(cp) if err != nil { return field.InternalError(fldPath, err) } - if upgrading { - return errors.New("ControlPlane is currently upgrading") + if provisioning { + return errors.New("ControlPlane is currently provisioning") } - cpVersionString, err := contract.ControlPlane().Version().Get(cp) + upgrading, err := contract.ControlPlane().IsUpgrading(cp) if err != nil { return field.InternalError(fldPath, err) } - cpVersion, err := semver.ParseTolerant(*cpVersionString) - if err != nil { - // NOTE: this should never happen. Nevertheless, handling this for extra caution. - return field.InternalError(fldPath, errors.New("unable to parse version of ControlPlane")) - } - if cpVersion.NE(oldVersion) { - return fmt.Errorf("ControlPlane version %q does not match the current version %q", cpVersion, oldVersion) + if upgrading { + return errors.New("ControlPlane is currently upgrading") } return nil diff --git a/internal/webhooks/cluster_test.go b/internal/webhooks/cluster_test.go index db0b4283e1c8..f3dbef3182c0 100644 --- a/internal/webhooks/cluster_test.go +++ b/internal/webhooks/cluster_test.go @@ -20,6 +20,7 @@ import ( "context" "testing" + "github.com/blang/semver/v4" . "github.com/onsi/gomega" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" @@ -28,6 +29,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation/field" utilfeature "k8s.io/component-base/featuregate/testing" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" @@ -1262,14 +1264,6 @@ func TestClusterTopologyValidation(t *testing.T) { WithTopology(builder.ClusterTopology(). WithClass("foo"). WithVersion("v1.19.1"). - WithMachineDeployment( - builder.MachineDeploymentTopology("workers1"). - WithClass("aa"). - Build()). - WithMachinePool( - builder.MachinePoolTopology("pool1"). - WithClass("aa"). - Build()). Build()). Build(), in: builder.Cluster("fooboo", "cluster1"). @@ -1282,16 +1276,6 @@ func TestClusterTopologyValidation(t *testing.T) { builder.ControlPlane("fooboo", "cluster1-cp").WithVersion("v1.19.1"). WithStatusFields(map[string]interface{}{"status.version": "v1.19.1"}). Build(), - builder.MachineDeployment("fooboo", "cluster1-workers1").WithLabels(map[string]string{ - clusterv1.ClusterNameLabel: "cluster1", - clusterv1.ClusterTopologyOwnedLabel: "", - clusterv1.ClusterTopologyMachineDeploymentNameLabel: "workers1", - }).WithVersion("v1.19.1").Build(), - builder.MachinePool("fooboo", "cluster1-pool1").WithLabels(map[string]string{ - clusterv1.ClusterNameLabel: "cluster1", - clusterv1.ClusterTopologyOwnedLabel: "", - clusterv1.ClusterTopologyMachinePoolNameLabel: "pool1", - }).WithVersion("v1.19.1").Build(), }, }, { @@ -1335,28 +1319,6 @@ func TestClusterTopologyValidation(t *testing.T) { builder.ControlPlane("fooboo", "cluster1-cp").WithVersion("v1.18.1").Build(), }, }, - { - name: "should block update if cluster kcp is not yet upgraded", - expectErr: true, - old: builder.Cluster("fooboo", "cluster1"). - WithControlPlane(builder.ControlPlane("fooboo", "cluster1-cp").Build()). - WithTopology(builder.ClusterTopology(). - WithClass("foo"). - WithVersion("v1.19.1"). - Build()). - Build(), - in: builder.Cluster("fooboo", "cluster1"). - WithTopology(builder.ClusterTopology(). - WithClass("foo"). - WithVersion("v1.20.2"). - Build()). - Build(), - additionalObjects: []client.Object{ - builder.ControlPlane("fooboo", "cluster1-cp").WithVersion("v1.18.1"). - WithStatusFields(map[string]interface{}{"status.version": "v1.18.1"}). - Build(), - }, - }, { name: "should block update if md is not yet upgraded", expectErr: true, @@ -1388,43 +1350,6 @@ func TestClusterTopologyValidation(t *testing.T) { }).WithVersion("v1.18.1").Build(), }, }, - { - name: "should block update if machine of md is not yet upgraded", - expectErr: true, - old: builder.Cluster("fooboo", "cluster1"). - WithControlPlane(builder.ControlPlane("fooboo", "cluster1-cp").Build()). - WithTopology(builder.ClusterTopology(). - WithClass("foo"). - WithVersion("v1.19.1"). - WithMachineDeployment( - builder.MachineDeploymentTopology("workers1"). - WithClass("aa"). - Build()). - Build()). - Build(), - in: builder.Cluster("fooboo", "cluster1"). - WithTopology(builder.ClusterTopology(). - WithClass("foo"). - WithVersion("v1.20.2"). - Build()). - Build(), - additionalObjects: []client.Object{ - builder.ControlPlane("fooboo", "cluster1-cp").WithVersion("v1.19.1"). - WithStatusFields(map[string]interface{}{"status.version": "v1.19.1"}). - Build(), - builder.MachineDeployment("fooboo", "cluster1-workers1").WithLabels(map[string]string{ - clusterv1.ClusterNameLabel: "cluster1", - clusterv1.ClusterTopologyOwnedLabel: "", - clusterv1.ClusterTopologyMachineDeploymentNameLabel: "workers1", - }).WithVersion("v1.19.1").WithSelector(*metav1.SetAsLabelSelector(labels.Set{clusterv1.ClusterTopologyMachineDeploymentNameLabel: "workers1"})).Build(), - - builder.Machine("fooboo", "cluster1-workers1-1").WithLabels(map[string]string{ - clusterv1.ClusterNameLabel: "cluster1", - // clusterv1.ClusterTopologyOwnedLabel: "", - clusterv1.ClusterTopologyMachineDeploymentNameLabel: "workers1", - }).WithVersion("v1.18.1").Build(), - }, - }, { name: "should block update if mp is not yet upgraded", expectErr: true, @@ -1456,41 +1381,6 @@ func TestClusterTopologyValidation(t *testing.T) { }).WithVersion("v1.18.1").Build(), }, }, - { - name: "should block update if node of mp is not yet upgraded", - expectErr: true, - old: builder.Cluster("fooboo", "cluster1"). - WithControlPlane(builder.ControlPlane("fooboo", "cluster1-cp").Build()). - WithTopology(builder.ClusterTopology(). - WithClass("foo"). - WithVersion("v1.19.1"). - WithMachinePool( - builder.MachinePoolTopology("pool1"). - WithClass("aa"). - Build()). - Build()). - Build(), - in: builder.Cluster("fooboo", "cluster1"). - WithTopology(builder.ClusterTopology(). - WithClass("foo"). - WithVersion("v1.20.2"). - Build()). - Build(), - additionalObjects: []client.Object{ - builder.ControlPlane("fooboo", "cluster1-cp").WithVersion("v1.19.1"). - WithStatusFields(map[string]interface{}{"status.version": "v1.19.1"}). - Build(), - builder.MachinePool("fooboo", "cluster1-pool1").WithLabels(map[string]string{ - clusterv1.ClusterNameLabel: "cluster1", - clusterv1.ClusterTopologyOwnedLabel: "", - clusterv1.ClusterTopologyMachinePoolNameLabel: "pool1", - }).WithVersion("v1.19.1").WithStatus(expv1.MachinePoolStatus{NodeRefs: []corev1.ObjectReference{{Name: "mp-node-1"}}}).Build(), - }, - clusterCacheTrackerReader: &fakeClusterCacheTracker{client: fake.NewFakeClient(&corev1.Node{ - ObjectMeta: metav1.ObjectMeta{Name: "mp-node-1"}, - Status: corev1.NodeStatus{NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.18.1"}}, - })}, - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -2571,6 +2461,355 @@ func TestClusterClassPollingErrors(t *testing.T) { } } +func Test_validateTopologyControlPlaneVersion(t *testing.T) { + tests := []struct { + name string + expectErr bool + old *clusterv1.Cluster + additionalObjects []client.Object + }{ + { + name: "should update if kcp is fully upgraded and up to date", + expectErr: false, + old: builder.Cluster("fooboo", "cluster1"). + WithControlPlane(builder.ControlPlane("fooboo", "cluster1-cp").Build()). + WithTopology(builder.ClusterTopology(). + WithClass("foo"). + WithVersion("v1.19.1"). + Build()). + Build(), + additionalObjects: []client.Object{ + builder.ControlPlane("fooboo", "cluster1-cp").WithVersion("v1.19.1"). + WithStatusFields(map[string]interface{}{"status.version": "v1.19.1"}). + Build(), + }, + }, + { + name: "should block update if kcp is provisioning", + expectErr: true, + old: builder.Cluster("fooboo", "cluster1"). + WithControlPlane(builder.ControlPlane("fooboo", "cluster1-cp").Build()). + WithTopology(builder.ClusterTopology(). + WithClass("foo"). + WithVersion("v1.19.1"). + Build()). + Build(), + additionalObjects: []client.Object{ + builder.ControlPlane("fooboo", "cluster1-cp").WithVersion("v1.19.1"). + Build(), + }, + }, + { + name: "should block update if kcp is upgrading", + expectErr: true, + old: builder.Cluster("fooboo", "cluster1"). + WithControlPlane(builder.ControlPlane("fooboo", "cluster1-cp").Build()). + WithTopology(builder.ClusterTopology(). + WithClass("foo"). + WithVersion("v1.19.1"). + Build()). + Build(), + additionalObjects: []client.Object{ + builder.ControlPlane("fooboo", "cluster1-cp").WithVersion("v1.18.1"). + WithStatusFields(map[string]interface{}{"status.version": "v1.17.1"}). + Build(), + }, + }, + { + name: "should block update if kcp is not yet upgraded", + expectErr: true, + old: builder.Cluster("fooboo", "cluster1"). + WithControlPlane(builder.ControlPlane("fooboo", "cluster1-cp").Build()). + WithTopology(builder.ClusterTopology(). + WithClass("foo"). + WithVersion("v1.19.1"). + Build()). + Build(), + additionalObjects: []client.Object{ + builder.ControlPlane("fooboo", "cluster1-cp").WithVersion("v1.18.1"). + WithStatusFields(map[string]interface{}{"status.version": "v1.18.1"}). + Build(), + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + fldPath := field.NewPath("spec", "topology", "version") + fakeClient := fake.NewClientBuilder(). + WithObjects(tt.additionalObjects...). + WithScheme(fakeScheme). + Build() + + oldVersion, err := semver.ParseTolerant(tt.old.Spec.Topology.Version) + g.Expect(err).ToNot(HaveOccurred()) + + err = validateTopologyControlPlaneVersion(ctx, fldPath, fakeClient, tt.old, oldVersion) + if tt.expectErr { + g.Expect(err).To(HaveOccurred()) + return + } + g.Expect(err).ToNot(HaveOccurred()) + }) + } +} + +func Test_validateTopologyMachineDeploymentVersions(t *testing.T) { + tests := []struct { + name string + expectErr bool + old *clusterv1.Cluster + additionalObjects []client.Object + }{ + { + name: "should update if no machine deployment is exists", + expectErr: false, + old: builder.Cluster("fooboo", "cluster1"). + WithTopology(builder.ClusterTopology(). + WithClass("foo"). + WithVersion("v1.19.1"). + Build()). + Build(), + additionalObjects: []client.Object{}, + }, + { + name: "should update if machine deployments are fully upgraded and up to date", + expectErr: false, + old: builder.Cluster("fooboo", "cluster1"). + WithTopology(builder.ClusterTopology(). + WithClass("foo"). + WithVersion("v1.19.1"). + WithMachineDeployment( + builder.MachineDeploymentTopology("workers1"). + WithClass("aa"). + Build()). + Build()). + Build(), + additionalObjects: []client.Object{ + builder.MachineDeployment("fooboo", "cluster1-workers1").WithLabels(map[string]string{ + clusterv1.ClusterNameLabel: "cluster1", + clusterv1.ClusterTopologyOwnedLabel: "", + clusterv1.ClusterTopologyMachineDeploymentNameLabel: "workers1", + }).WithVersion("v1.19.1").Build(), + }, + }, + { + name: "should block update if machine deployment is not yet upgraded", + expectErr: true, + old: builder.Cluster("fooboo", "cluster1"). + WithTopology(builder.ClusterTopology(). + WithClass("foo"). + WithVersion("v1.19.1"). + WithMachineDeployment( + builder.MachineDeploymentTopology("workers1"). + WithClass("aa"). + Build()). + Build()). + Build(), + additionalObjects: []client.Object{ + builder.MachineDeployment("fooboo", "cluster1-workers1").WithLabels(map[string]string{ + clusterv1.ClusterNameLabel: "cluster1", + clusterv1.ClusterTopologyOwnedLabel: "", + clusterv1.ClusterTopologyMachineDeploymentNameLabel: "workers1", + }).WithVersion("v1.18.1").Build(), + }, + }, + { + name: "should block update if machine deployment is upgrading", + expectErr: true, + old: builder.Cluster("fooboo", "cluster1"). + WithTopology(builder.ClusterTopology(). + WithClass("foo"). + WithVersion("v1.19.1"). + WithMachineDeployment( + builder.MachineDeploymentTopology("workers1"). + WithClass("aa"). + Build()). + Build()). + Build(), + additionalObjects: []client.Object{ + builder.MachineDeployment("fooboo", "cluster1-workers1").WithLabels(map[string]string{ + clusterv1.ClusterNameLabel: "cluster1", + clusterv1.ClusterTopologyOwnedLabel: "", + clusterv1.ClusterTopologyMachineDeploymentNameLabel: "workers1", + }).WithVersion("v1.19.1").WithSelector(*metav1.SetAsLabelSelector(labels.Set{clusterv1.ClusterTopologyMachineDeploymentNameLabel: "workers1"})).Build(), + builder.Machine("fooboo", "cluster1-workers1-1").WithLabels(map[string]string{ + clusterv1.ClusterNameLabel: "cluster1", + // clusterv1.ClusterTopologyOwnedLabel: "", + clusterv1.ClusterTopologyMachineDeploymentNameLabel: "workers1", + }).WithVersion("v1.18.1").Build(), + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + fldPath := field.NewPath("spec", "topology", "version") + fakeClient := fake.NewClientBuilder(). + WithObjects(tt.additionalObjects...). + WithScheme(fakeScheme). + Build() + + oldVersion, err := semver.ParseTolerant(tt.old.Spec.Topology.Version) + g.Expect(err).ToNot(HaveOccurred()) + + err = validateTopologyMachineDeploymentVersions(ctx, fldPath, fakeClient, tt.old, oldVersion) + if tt.expectErr { + g.Expect(err).To(HaveOccurred()) + return + } + g.Expect(err).ToNot(HaveOccurred()) + + }) + } +} + +func Test_validateTopologyMachinePoolVersions(t *testing.T) { + tests := []struct { + name string + expectErr bool + old *clusterv1.Cluster + additionalObjects []client.Object + workloadObjects []client.Object + }{ + { + name: "should update if no machine pool is exists", + expectErr: false, + old: builder.Cluster("fooboo", "cluster1"). + WithTopology(builder.ClusterTopology(). + WithClass("foo"). + WithVersion("v1.19.1"). + Build()). + Build(), + additionalObjects: []client.Object{}, + workloadObjects: []client.Object{}, + }, + { + name: "should update if machine pools are fully upgraded and up to date", + expectErr: false, + old: builder.Cluster("fooboo", "cluster1"). + WithTopology(builder.ClusterTopology(). + WithClass("foo"). + WithVersion("v1.19.1"). + WithMachinePool( + builder.MachinePoolTopology("pool1"). + WithClass("aa"). + Build()). + Build()). + Build(), + additionalObjects: []client.Object{ + builder.MachinePool("fooboo", "cluster1-pool1").WithLabels(map[string]string{ + clusterv1.ClusterNameLabel: "cluster1", + clusterv1.ClusterTopologyOwnedLabel: "", + clusterv1.ClusterTopologyMachinePoolNameLabel: "pool1", + }).WithVersion("v1.19.1").Build(), + }, + workloadObjects: []client.Object{}, + }, + { + name: "should block update if machine pool is not yet upgraded", + expectErr: true, + old: builder.Cluster("fooboo", "cluster1"). + WithTopology(builder.ClusterTopology(). + WithClass("foo"). + WithVersion("v1.19.1"). + WithMachinePool( + builder.MachinePoolTopology("pool1"). + WithClass("aa"). + Build()). + Build()). + Build(), + additionalObjects: []client.Object{ + builder.MachinePool("fooboo", "cluster1-pool1").WithLabels(map[string]string{ + clusterv1.ClusterNameLabel: "cluster1", + clusterv1.ClusterTopologyOwnedLabel: "", + clusterv1.ClusterTopologyMachinePoolNameLabel: "pool1", + }).WithVersion("v1.18.1").Build(), + }, + workloadObjects: []client.Object{}, + }, + { + name: "should block update machine pool is upgrading", + expectErr: true, + old: builder.Cluster("fooboo", "cluster1"). + WithTopology(builder.ClusterTopology(). + WithClass("foo"). + WithVersion("v1.19.1"). + WithMachinePool( + builder.MachinePoolTopology("pool1"). + WithClass("aa"). + Build()). + Build()). + Build(), + additionalObjects: []client.Object{ + builder.MachinePool("fooboo", "cluster1-pool1").WithLabels(map[string]string{ + clusterv1.ClusterNameLabel: "cluster1", + clusterv1.ClusterTopologyOwnedLabel: "", + clusterv1.ClusterTopologyMachinePoolNameLabel: "pool1", + }).WithVersion("v1.19.1").WithStatus(expv1.MachinePoolStatus{NodeRefs: []corev1.ObjectReference{{Name: "mp-node-1"}}}).Build(), + }, + workloadObjects: []client.Object{ + &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: "mp-node-1"}, + Status: corev1.NodeStatus{NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.18.1"}}, + }, + }, + }, + { + name: "should block update if it cannot get the node of a machine pool", + expectErr: true, + old: builder.Cluster("fooboo", "cluster1"). + WithTopology(builder.ClusterTopology(). + WithClass("foo"). + WithVersion("v1.19.1"). + WithMachinePool( + builder.MachinePoolTopology("pool1"). + WithClass("aa"). + Build()). + Build()). + Build(), + additionalObjects: []client.Object{ + builder.MachinePool("fooboo", "cluster1-pool1").WithLabels(map[string]string{ + clusterv1.ClusterNameLabel: "cluster1", + clusterv1.ClusterTopologyOwnedLabel: "", + clusterv1.ClusterTopologyMachinePoolNameLabel: "pool1", + }).WithVersion("v1.19.1").WithStatus(expv1.MachinePoolStatus{NodeRefs: []corev1.ObjectReference{{Name: "mp-node-1"}}}).Build(), + }, + workloadObjects: []client.Object{}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + fldPath := field.NewPath("spec", "topology", "version") + fakeClient := fake.NewClientBuilder(). + WithObjects(tt.additionalObjects...). + WithScheme(fakeScheme). + Build() + + oldVersion, err := semver.ParseTolerant(tt.old.Spec.Topology.Version) + g.Expect(err).ToNot(HaveOccurred()) + + fakeClusterCacheTracker := &fakeClusterCacheTracker{ + client: fake.NewClientBuilder(). + WithObjects(tt.workloadObjects...). + Build(), + } + + err = validateTopologyMachinePoolVersions(ctx, fldPath, fakeClient, fakeClusterCacheTracker, tt.old, oldVersion) + if tt.expectErr { + g.Expect(err).To(HaveOccurred()) + return + } + g.Expect(err).ToNot(HaveOccurred()) + + }) + } +} + func refToUnstructured(ref *corev1.ObjectReference) *unstructured.Unstructured { gvk := ref.GetObjectKind().GroupVersionKind() output := &unstructured.Unstructured{}