From a0277405c4d6fe0dd828f34ce11fc606bd2b96b1 Mon Sep 17 00:00:00 2001 From: Benjamin Schimke Date: Thu, 11 Jul 2024 19:48:33 +0200 Subject: [PATCH 1/2] Ignore kubernetes version prefix for machine filter --- pkg/machinefilters/machine_filters.go | 3 +- pkg/machinefilters/machine_filters_test.go | 57 ++++++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/pkg/machinefilters/machine_filters.go b/pkg/machinefilters/machine_filters.go index c1fc854d..de64844e 100644 --- a/pkg/machinefilters/machine_filters.go +++ b/pkg/machinefilters/machine_filters.go @@ -18,6 +18,7 @@ package machinefilters import ( "reflect" + "strings" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" @@ -78,7 +79,7 @@ func MatchesKubernetesVersion(kubernetesVersion string) Func { if machine.Spec.Version == nil { return false } - return *machine.Spec.Version == kubernetesVersion + return strings.TrimPrefix(*machine.Spec.Version, "v") == strings.TrimPrefix(kubernetesVersion, "v") } } diff --git a/pkg/machinefilters/machine_filters_test.go b/pkg/machinefilters/machine_filters_test.go index 12c5b4cd..22c586b2 100644 --- a/pkg/machinefilters/machine_filters_test.go +++ b/pkg/machinefilters/machine_filters_test.go @@ -7,6 +7,7 @@ import ( "google.golang.org/protobuf/proto" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" bootstrapv1 "github.com/canonical/cluster-api-k8s/bootstrap/api/v1beta2" @@ -283,3 +284,59 @@ func TestMatchesCK8sBootstrapConfig(t *testing.T) { }) }) } +func TestMatchesKubernetesVersion(t *testing.T) { + t.Run("returns true if machine's version matches", func(t *testing.T) { + g := NewWithT(t) + kubernetesVersion := "v1.30.0" + machine := &clusterv1.Machine{ + Spec: clusterv1.MachineSpec{ + Version: ptr.To("v1.30.0"), + }, + } + match := MatchesKubernetesVersion(kubernetesVersion)(machine) + g.Expect(match).To(BeTrue()) + }) + + t.Run("ignores 'v' prefix", func(t *testing.T) { + g := NewWithT(t) + kubernetesVersion := "1.30.0" + machine := &clusterv1.Machine{ + Spec: clusterv1.MachineSpec{ + Version: ptr.To("v1.30.0"), + }, + } + match := MatchesKubernetesVersion(kubernetesVersion)(machine) + g.Expect(match).To(BeTrue()) + }) + + t.Run("returns false if machine's version does not match", func(t *testing.T) { + g := NewWithT(t) + kubernetesVersion := "v1.30.0" + machine := &clusterv1.Machine{ + Spec: clusterv1.MachineSpec{ + Version: ptr.To("v1.29.0"), + }, + } + match := MatchesKubernetesVersion(kubernetesVersion)(machine) + g.Expect(match).To(BeFalse()) + }) + + t.Run("returns false if machine's version is nil", func(t *testing.T) { + g := NewWithT(t) + kubernetesVersion := "v1.30.0" + machine := &clusterv1.Machine{ + Spec: clusterv1.MachineSpec{ + Version: nil, + }, + } + match := MatchesKubernetesVersion(kubernetesVersion)(machine) + g.Expect(match).To(BeFalse()) + }) + + t.Run("returns false if machine is nil", func(t *testing.T) { + g := NewWithT(t) + kubernetesVersion := "v1.30.0" + match := MatchesKubernetesVersion(kubernetesVersion)(nil) + g.Expect(match).To(BeFalse()) + }) +} From 13295cdf48ad20334053dfbb7e96ec10bda2804d Mon Sep 17 00:00:00 2001 From: Benjamin Schimke Date: Fri, 12 Jul 2024 06:09:40 +0200 Subject: [PATCH 2/2] refactor test to be table-driven --- pkg/machinefilters/machine_filters_test.go | 96 ++++++++++------------ 1 file changed, 43 insertions(+), 53 deletions(-) diff --git a/pkg/machinefilters/machine_filters_test.go b/pkg/machinefilters/machine_filters_test.go index 22c586b2..ae78e751 100644 --- a/pkg/machinefilters/machine_filters_test.go +++ b/pkg/machinefilters/machine_filters_test.go @@ -285,58 +285,48 @@ func TestMatchesCK8sBootstrapConfig(t *testing.T) { }) } func TestMatchesKubernetesVersion(t *testing.T) { - t.Run("returns true if machine's version matches", func(t *testing.T) { - g := NewWithT(t) - kubernetesVersion := "v1.30.0" - machine := &clusterv1.Machine{ - Spec: clusterv1.MachineSpec{ - Version: ptr.To("v1.30.0"), - }, - } - match := MatchesKubernetesVersion(kubernetesVersion)(machine) - g.Expect(match).To(BeTrue()) - }) - - t.Run("ignores 'v' prefix", func(t *testing.T) { - g := NewWithT(t) - kubernetesVersion := "1.30.0" - machine := &clusterv1.Machine{ - Spec: clusterv1.MachineSpec{ - Version: ptr.To("v1.30.0"), - }, - } - match := MatchesKubernetesVersion(kubernetesVersion)(machine) - g.Expect(match).To(BeTrue()) - }) - - t.Run("returns false if machine's version does not match", func(t *testing.T) { - g := NewWithT(t) - kubernetesVersion := "v1.30.0" - machine := &clusterv1.Machine{ - Spec: clusterv1.MachineSpec{ - Version: ptr.To("v1.29.0"), - }, - } - match := MatchesKubernetesVersion(kubernetesVersion)(machine) - g.Expect(match).To(BeFalse()) - }) - - t.Run("returns false if machine's version is nil", func(t *testing.T) { - g := NewWithT(t) - kubernetesVersion := "v1.30.0" - machine := &clusterv1.Machine{ - Spec: clusterv1.MachineSpec{ - Version: nil, - }, - } - match := MatchesKubernetesVersion(kubernetesVersion)(machine) - g.Expect(match).To(BeFalse()) - }) + tests := []struct { + name string + kubernetesVersion string + expectedMatch bool + machine *clusterv1.Machine + }{ + { + name: "returns true if machine's version matches", + kubernetesVersion: "v1.30.0", + expectedMatch: true, + machine: &clusterv1.Machine{Spec: clusterv1.MachineSpec{Version: ptr.To("v1.30.0")}}, + }, + { + name: "ignores 'v' prefix", + kubernetesVersion: "1.30.0", + expectedMatch: true, + machine: &clusterv1.Machine{Spec: clusterv1.MachineSpec{Version: ptr.To("v1.30.0")}}, + }, + { + name: "returns false if machine's version does not match", + kubernetesVersion: "v1.30.0", + expectedMatch: false, + machine: &clusterv1.Machine{Spec: clusterv1.MachineSpec{Version: ptr.To("v1.29.0")}}, + }, + { + name: "returns false if machine's version is nil", + kubernetesVersion: "v1.30.0", + expectedMatch: false, + machine: &clusterv1.Machine{Spec: clusterv1.MachineSpec{Version: nil}}, + }, + { + name: "returns false if machine is nil", + expectedMatch: false, + machine: nil, + }, + } - t.Run("returns false if machine is nil", func(t *testing.T) { - g := NewWithT(t) - kubernetesVersion := "v1.30.0" - match := MatchesKubernetesVersion(kubernetesVersion)(nil) - g.Expect(match).To(BeFalse()) - }) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + match := MatchesKubernetesVersion(test.kubernetesVersion)(test.machine) + g.Expect(match).To(Equal(test.expectedMatch)) + }) + } }