feat: Changes to controlplane spec should trigger a rollout

pkg/machinefilters/machine_filters.go

@@ -17,6 +17,9 @@ limitations under the License.
 package machinefilters
 
 import (
+	"encoding/json"
+	"reflect"
+
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/cluster-api/util/collections"
@@ -83,6 +86,70 @@ func MatchesKubernetesVersion(kubernetesVersion string) Func {
 // MatchesKThreesBootstrapConfig checks if machine's KThreesConfigSpec is equivalent with KCP's KThreesConfigSpec.
 func MatchesKThreesBootstrapConfig(machineConfigs map[string]*bootstrapv1.KThreesConfig, kcp *controlplanev1.KThreesControlPlane) Func {
 	return func(machine *clusterv1.Machine) bool {
+		if machine == nil {
+			return false
+		}
+
+		// Check if KCP and machine KThreesServerConfig matches, if not return
+		if !matchKThreesServerConfig(kcp, machine) {
+			return false
+		}
+
+		bootstrapRef := machine.Spec.Bootstrap.ConfigRef
+		if bootstrapRef == nil {
+			// Missing bootstrap reference should not be considered as unmatching.
+			// This is a safety precaution to avoid selecting machines that are broken, which in the future should be remediated separately.
+			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{}
+		// KCP version check is handled elsewhere
+		kcpConfig.Version = ""
+		machineConfig.Spec.Version = ""
+
+		return reflect.DeepEqual(&machineConfig.Spec, kcpConfig)
+	}
+}
+
+// matchKThreesServerConfig verifies if KCP and machine ClusterConfiguration matches.
+// NOTE: Machines that have KThreesServerConfigurationAnnotation will have to match with KCP KThreesServerConfig.
+// If the annotation is not present (machine is either old or adopted), we won't roll out on any possible changes
+// made in KCP's KThreesServerConfig given that we don't have enough information to make a decision.
+// Users should use KCP.Spec.RolloutAfter field to force a rollout in this case.
+func matchKThreesServerConfig(kcp *controlplanev1.KThreesControlPlane, machine *clusterv1.Machine) bool {
+	kThreesServerConfigStr, ok := machine.GetAnnotations()[controlplanev1.KThreesServerConfigurationAnnotation]
+	if !ok {
+		// We don't have enough information to make a decision; don't' trigger a roll out.
 		return true
 	}
+
+	kThreesServerConfig := &bootstrapv1.KThreesServerConfig{}
+
+	// ClusterConfiguration annotation is not correct, only solution is to rollout.
+	// The call to json.Unmarshal has to take a pointer to the pointer struct defined above,
+	// otherwise we won't be able to handle a nil ClusterConfiguration (that is serialized into "null").
+	if err := json.Unmarshal([]byte(kThreesServerConfigStr), &kThreesServerConfig); err != nil {
+		return false
+	}
+
+	// If any of the compared values are nil, treat them the same as an empty KThreesServerConfig.
+	if kThreesServerConfig == nil {
+		kThreesServerConfig = &bootstrapv1.KThreesServerConfig{}
+	}
+
+	kcpLocalKThreesServerConfig := &kcp.Spec.KThreesConfigSpec.ServerConfig
+
+	// Compare and return.
+	return reflect.DeepEqual(kThreesServerConfig, kcpLocalKThreesServerConfig)
 }

pkg/machinefilters/machine_filters_test.go

@@ -0,0 +1,283 @@
+package machinefilters
+
+import (
+	"testing"
+
+	. "github.com/onsi/gomega"
+	"google.golang.org/protobuf/proto"
+	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 other configurations", func(t *testing.T) {
+		kThreesConfigSpec := bootstrapv1.KThreesConfigSpec{
+			Files:           []bootstrapv1.File{},
+			PreK3sCommands:  []string{"test"},
+			PostK3sCommands: []string{"test"},
+			AgentConfig: bootstrapv1.KThreesAgentConfig{
+				NodeName:      "test-node",
+				NodeTaints:    []string{"node-role.kubernetes.io/control-plane:NoSchedule"},
+				KubeProxyArgs: []string{"metrics-bind-address=0.0.0.0"},
+			},
+			ServerConfig: bootstrapv1.KThreesServerConfig{
+				DisableComponents:         []string{"traefik"},
+				KubeControllerManagerArgs: []string{"bind-address=0.0.0.0"},
+				KubeSchedulerArgs:         []string{"bind-address=0.0.0.0"},
+			},
+		}
+		kcp := &controlplanev1.KThreesControlPlane{
+			Spec: controlplanev1.KThreesControlPlaneSpec{
+				Replicas: proto.Int32(3),
+				Version:  "v1.13.14+k3s1",
+				MachineTemplate: controlplanev1.KThreesControlPlaneMachineTemplate{
+					ObjectMeta: clusterv1.ObjectMeta{
+						Labels: map[string]string{"test-label": "test-value"},
+					},
+				},
+				KThreesConfigSpec: kThreesConfigSpec,
+			},
+		}
+
+		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: kThreesConfigSpec,
+			},
+		}
+
+		t.Run("by returning true if all configs match", func(t *testing.T) {
+			g := NewWithT(t)
+			match := MatchesKThreesBootstrapConfig(machineConfigs, kcp)(m)
+			g.Expect(match).To(BeTrue())
+		})
+
+		t.Run("by returning false if post commands don't match", func(t *testing.T) {
+			g := NewWithT(t)
+			machineConfigs[m.Name].Spec.PostK3sCommands = []string{"new-test"}
+			match := MatchesKThreesBootstrapConfig(machineConfigs, kcp)(m)
+			g.Expect(match).To(BeFalse())
+		})
+
+		t.Run("by returning false if agent configs don't match", func(t *testing.T) {
+			g := NewWithT(t)
+			machineConfigs[m.Name].Spec.AgentConfig.KubeletArgs = []string{"test-arg"}
+			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())
+		})
+	})
+}