feat: Add unit tests and add matching logic for the rest of kthreesco…

…ntrolplanespec Signed-off-by: Richard Draycott <[email protected]>
k3s-io · May 14, 2024 · 4a4e8d2 · 4a4e8d2
1 parent ed4a401
commit 4a4e8d2
Show file tree

Hide file tree

Showing 2 changed files with 213 additions and 2 deletions.
diff --git a/pkg/machinefilters/machine_filters.go b/pkg/machinefilters/machine_filters.go
@@ -102,7 +102,20 @@ func MatchesKThreesBootstrapConfig(machineConfigs map[string]*bootstrapv1.KThree
 			return true
 		}
 
-		return true
+		machineConfig, found := machineConfigs[machine.Name]
+		if !found {
+			// Return true here because failing to get KThreesConfig should not be considered as unmatching.
+			// This is a safety precaution to avoid rolling out machines if the client or the api-server is misbehaving.
+			return true
+		}
+
+		kcpConfig := kcp.Spec.KThreesConfigSpec.DeepCopy()
+		// KCP bootstrapv1.KThreesServerConfig will only be compared with a machine's ServerConfig annotation, so
+		// we are cleaning up from the reflect.DeepEqual comparison.
+		kcpConfig.ServerConfig = bootstrapv1.KThreesServerConfig{}
+		machineConfig.Spec.ServerConfig = bootstrapv1.KThreesServerConfig{}
+
+		return reflect.DeepEqual(&machineConfig.Spec, kcpConfig)
 	}
 }
 
@@ -132,7 +145,7 @@ func matchKThreesServerConfig(kcp *controlplanev1.KThreesControlPlane, machine *
 		kThreesServerConfig = &bootstrapv1.KThreesServerConfig{}
 	}
 
-	kcpLocalKThreesServerConfig := kcp.Spec.KThreesConfigSpec.ServerConfig
+	kcpLocalKThreesServerConfig := &kcp.Spec.KThreesConfigSpec.ServerConfig
 
 	// Compare and return.
 	return reflect.DeepEqual(kThreesServerConfig, kcpLocalKThreesServerConfig)

diff --git a/pkg/machinefilters/machine_filters_test.go b/pkg/machinefilters/machine_filters_test.go
@@ -0,0 +1,198 @@
+package machinefilters
+
+import (
+	"testing"
+
+	. "github.com/onsi/gomega"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+
+	bootstrapv1 "github.com/k3s-io/cluster-api-k3s/bootstrap/api/v1beta2"
+	controlplanev1 "github.com/k3s-io/cluster-api-k3s/controlplane/api/v1beta2"
+)
+
+func TestMatchesKubeadmBootstrapConfig(t *testing.T) {
+	t.Run("returns true if ClusterConfiguration is equal", func(t *testing.T) {
+		g := NewWithT(t)
+		kcp := &controlplanev1.KThreesControlPlane{
+			Spec: controlplanev1.KThreesControlPlaneSpec{
+				KThreesConfigSpec: bootstrapv1.KThreesConfigSpec{
+					ServerConfig: bootstrapv1.KThreesServerConfig{
+						ClusterDomain: "foo",
+					},
+				},
+			},
+		}
+		m := &clusterv1.Machine{
+			ObjectMeta: metav1.ObjectMeta{
+				Annotations: map[string]string{
+					controlplanev1.KThreesServerConfigurationAnnotation: "{\n  \"clusterDomain\": \"foo\"\n}",
+				},
+			},
+		}
+		machineConfigs := map[string]*bootstrapv1.KThreesConfig{
+			m.Name: {},
+		}
+		match := MatchesKThreesBootstrapConfig(machineConfigs, kcp)(m)
+		g.Expect(match).To(BeTrue())
+	})
+	t.Run("returns false if ClusterConfiguration is NOT equal", func(t *testing.T) {
+		g := NewWithT(t)
+		kcp := &controlplanev1.KThreesControlPlane{
+			Spec: controlplanev1.KThreesControlPlaneSpec{
+				KThreesConfigSpec: bootstrapv1.KThreesConfigSpec{
+					ServerConfig: bootstrapv1.KThreesServerConfig{
+						ClusterDomain: "foo",
+					},
+				},
+			},
+		}
+		m := &clusterv1.Machine{
+			ObjectMeta: metav1.ObjectMeta{
+				Annotations: map[string]string{
+					controlplanev1.KThreesServerConfigurationAnnotation: "{\n  \"clusterDomain\": \"bar\"\n}",
+				},
+			},
+		}
+		machineConfigs := map[string]*bootstrapv1.KThreesConfig{
+			m.Name: {},
+		}
+		match := MatchesKThreesBootstrapConfig(machineConfigs, kcp)(m)
+		g.Expect(match).To(BeFalse())
+	})
+
+	t.Run("returns false if some other configurations are not equal", func(t *testing.T) {
+		g := NewWithT(t)
+		kcp := &controlplanev1.KThreesControlPlane{
+			Spec: controlplanev1.KThreesControlPlaneSpec{
+				KThreesConfigSpec: bootstrapv1.KThreesConfigSpec{
+					Files: []bootstrapv1.File{}, // This is a change
+				},
+			},
+		}
+
+		m := &clusterv1.Machine{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       "KThreesConfig",
+				APIVersion: clusterv1.GroupVersion.String(),
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: "default",
+				Name:      "test",
+			},
+			Spec: clusterv1.MachineSpec{
+				Bootstrap: clusterv1.Bootstrap{
+					ConfigRef: &corev1.ObjectReference{
+						Kind:       "KThreesConfig",
+						Namespace:  "default",
+						Name:       "test",
+						APIVersion: bootstrapv1.GroupVersion.String(),
+					},
+				},
+			},
+		}
+		machineConfigs := map[string]*bootstrapv1.KThreesConfig{
+			m.Name: {
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "KThreesConfig",
+					APIVersion: bootstrapv1.GroupVersion.String(),
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "default",
+					Name:      "test",
+				},
+				Spec: bootstrapv1.KThreesConfigSpec{},
+			},
+		}
+		match := MatchesKThreesBootstrapConfig(machineConfigs, kcp)(m)
+		g.Expect(match).To(BeFalse())
+	})
+
+	t.Run("should match on labels and annotations", func(t *testing.T) {
+		kcp := &controlplanev1.KThreesControlPlane{
+			Spec: controlplanev1.KThreesControlPlaneSpec{
+				MachineTemplate: controlplanev1.KThreesControlPlaneMachineTemplate{
+					ObjectMeta: clusterv1.ObjectMeta{
+						Annotations: map[string]string{
+							"test": "annotation",
+						},
+						Labels: map[string]string{
+							"test": "labels",
+						},
+					},
+				},
+				KThreesConfigSpec: bootstrapv1.KThreesConfigSpec{
+					ServerConfig: bootstrapv1.KThreesServerConfig{},
+				},
+			},
+		}
+		m := &clusterv1.Machine{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       "KThreesConfig",
+				APIVersion: clusterv1.GroupVersion.String(),
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: "default",
+				Name:      "test",
+			},
+			Spec: clusterv1.MachineSpec{
+				Bootstrap: clusterv1.Bootstrap{
+					ConfigRef: &corev1.ObjectReference{
+						Kind:       "KThreesConfig",
+						Namespace:  "default",
+						Name:       "test",
+						APIVersion: bootstrapv1.GroupVersion.String(),
+					},
+				},
+			},
+		}
+		machineConfigs := map[string]*bootstrapv1.KThreesConfig{
+			m.Name: {
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "KThreesConfig",
+					APIVersion: bootstrapv1.GroupVersion.String(),
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "default",
+					Name:      "test",
+				},
+				Spec: bootstrapv1.KThreesConfigSpec{
+					ServerConfig: bootstrapv1.KThreesServerConfig{},
+				},
+			},
+		}
+
+		t.Run("by returning true if neither labels or annotations match", func(t *testing.T) {
+			g := NewWithT(t)
+			machineConfigs[m.Name].Annotations = nil
+			machineConfigs[m.Name].Labels = nil
+			match := MatchesKThreesBootstrapConfig(machineConfigs, kcp)(m)
+			g.Expect(match).To(BeTrue())
+		})
+
+		t.Run("by returning true if only labels don't match", func(t *testing.T) {
+			g := NewWithT(t)
+			machineConfigs[m.Name].Annotations = kcp.Spec.MachineTemplate.ObjectMeta.Annotations
+			machineConfigs[m.Name].Labels = nil
+			match := MatchesKThreesBootstrapConfig(machineConfigs, kcp)(m)
+			g.Expect(match).To(BeTrue())
+		})
+
+		t.Run("by returning true if only annotations don't match", func(t *testing.T) {
+			g := NewWithT(t)
+			machineConfigs[m.Name].Annotations = nil
+			machineConfigs[m.Name].Labels = kcp.Spec.MachineTemplate.ObjectMeta.Labels
+			match := MatchesKThreesBootstrapConfig(machineConfigs, kcp)(m)
+			g.Expect(match).To(BeTrue())
+		})
+
+		t.Run("by returning true if both labels and annotations match", func(t *testing.T) {
+			g := NewWithT(t)
+			machineConfigs[m.Name].Labels = kcp.Spec.MachineTemplate.ObjectMeta.Labels
+			machineConfigs[m.Name].Annotations = kcp.Spec.MachineTemplate.ObjectMeta.Annotations
+			match := MatchesKThreesBootstrapConfig(machineConfigs, kcp)(m)
+			g.Expect(match).To(BeTrue())
+		})
+	})
+}