machinesetsync: refactor to a generalized differ which can work independent of types

Author: chrischdi

pkg/controllers/machinesetsync/machineset_sync_controller.go

@@ -787,7 +787,10 @@ func (r *MachineSetSyncReconciler) createOrUpdateCAPIMachineSet(ctx context.Cont
 	// If there is an existing CAPI machine set, work out if it needs updating.
 
 	// Compare the existing CAPI machine set with the converted CAPI machine set to check for changes.
-	capiMachineSetsDiff := compareCAPIMachineSets(existingCAPIMachineSet, convertedCAPIMachineSet)
+	capiMachineSetsDiff, err := compareCAPIMachineSets(existingCAPIMachineSet, convertedCAPIMachineSet)
+	if err != nil {
+		return fmt.Errorf("failed to compare CAPI machine sets: %w", err)
+	}
 
 	// Make a deep copy of the converted CAPI machine set to avoid modifying the original.
 	updatedOrCreatedCAPIMachineSet := convertedCAPIMachineSet.DeepCopy()
@@ -1397,7 +1400,7 @@ func compareCAPIInfraMachineTemplates(platform configv1.PlatformType, infraMachi
 }
 
 // compareCAPIMachineSets compares CAPI machineSets a and b, and returns a list of differences, or none if there are none.
-func compareCAPIMachineSets(capiMachineSet1, capiMachineSet2 *clusterv1.MachineSet) map[string]any {
+func compareCAPIMachineSets(capiMachineSet1, capiMachineSet2 *clusterv1.MachineSet) (map[string]any, error) {
 	diff := make(map[string]any)
 
 	if diffSpec := deep.Equal(capiMachineSet1.Spec, capiMachineSet2.Spec); len(diffSpec) > 0 {
@@ -1408,11 +1411,13 @@ func compareCAPIMachineSets(capiMachineSet1, capiMachineSet2 *clusterv1.MachineS
 		diff[".metadata"] = diffObjectMeta
 	}
 
-	if diffStatus := util.CAPIMachineSetStatusEqual(capiMachineSet1.Status, capiMachineSet2.Status); len(diffStatus) > 0 {
+	if diffStatus, err := util.CAPIMachineSetStatusEqual(capiMachineSet1.Status, capiMachineSet2.Status); err != nil {
+		return nil, fmt.Errorf("failed to compare CAPI machine set status: %w", err)
+	} else if len(diffStatus) > 0 {
 		diff[".status"] = diffStatus
 	}
 
-	return diff
+	return diff, nil
 }
 
 // compareMAPIMachineSets compares MAPI machineSets a and b, and returns a list of differences, or none if there are none.

pkg/util/diff.go

@@ -0,0 +1,89 @@
+/*
+Copyright 2025 Red Hat, Inc.
+
+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 util
+
+import (
+	"fmt"
+
+	"github.com/go-test/deep"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
+)
+
+type UnstructuredDiffer[T any] struct {
+	customDiff   []func(a, b T) ([]string, error)
+	ignoreFields [][]string
+}
+
+func (d *UnstructuredDiffer[T]) Diff(a, b T) (map[string]any, error) {
+	diffs := map[string]any{}
+
+	for i, customDiff := range d.customDiff {
+		diff, err := customDiff(a, b)
+		if err != nil {
+			return nil, err
+		}
+
+		if len(diff) > 0 {
+			diffs[fmt.Sprintf("customDiff.%d", i)] = diff
+		}
+	}
+
+	unstructuredA, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&a)
+	if err != nil {
+		return nil, fmt.Errorf("failed to convert b to unstructured: %w", err)
+	}
+
+	unstructuredB, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&b)
+	if err != nil {
+		return nil, fmt.Errorf("failed to convert b to unstructured: %w", err)
+	}
+
+	for _, nestedField := range d.ignoreFields {
+		unstructured.RemoveNestedField(unstructuredA, nestedField...)
+		unstructured.RemoveNestedField(unstructuredB, nestedField...)
+	}
+
+	diff := deep.Equal(unstructuredA, unstructuredB)
+	if len(diff) > 0 {
+		diffs["deep.Equal"] = diff
+	}
+
+	return diffs, nil
+}
+
+type diffopts[T any] func(*UnstructuredDiffer[T])
+
+func NewUnstructuredDiffer[T any](opts ...diffopts[T]) *UnstructuredDiffer[T] {
+	d := &UnstructuredDiffer[T]{}
+	for _, opt := range opts {
+		opt(d)
+	}
+
+	return d
+}
+
+func WithIgnoreField[T any](path ...string) diffopts[T] {
+	return func(d *UnstructuredDiffer[T]) {
+		d.ignoreFields = append(d.ignoreFields, path)
+	}
+}
+
+func WithCustomDiff[T any](diff func(a, b T) ([]string, error)) diffopts[T] {
+	return func(d *UnstructuredDiffer[T]) {
+		d.customDiff = append(d.customDiff, diff)
+	}
+}

pkg/util/sync.go

@@ -81,34 +81,25 @@ func ObjectMetaEqual(a, b metav1.ObjectMeta) map[string]any {
 // CAPIMachineSetStatusEqual compares variables a and b,
 // and returns a list of differences, or nil if there are none,
 // for the fields we care about when synchronising MAPI and CAPI Machines.
-func CAPIMachineSetStatusEqual(a, b clusterv1.MachineSetStatus) map[string]any { //nolint:dupl
-	diff := map[string]any{}
-
-	if diffConditions := compareCAPIMachineSetConditions(a.Conditions, b.Conditions); len(diffConditions) > 0 {
-		diff[".conditions"] = diffConditions
-	}
-
-	if diffReadyReplicas := deep.Equal(a.ReadyReplicas, b.ReadyReplicas); len(diffReadyReplicas) > 0 {
-		diff[".readyReplicas"] = diffReadyReplicas
-	}
-
-	if diffAvailableReplicas := deep.Equal(a.AvailableReplicas, b.AvailableReplicas); len(diffAvailableReplicas) > 0 {
-		diff[".availableReplicas"] = diffAvailableReplicas
-	}
-
-	if diffFullyLabeledReplicas := deep.Equal(a.FullyLabeledReplicas, b.FullyLabeledReplicas); len(diffFullyLabeledReplicas) > 0 {
-		diff[".fullyLabeledReplicas"] = diffFullyLabeledReplicas
-	}
-
-	if diffFailureReason := deep.Equal(a.FailureReason, b.FailureReason); len(diffFailureReason) > 0 {
-		diff[".failureReason"] = diffFailureReason
-	}
-
-	if diffFailureMessage := deep.Equal(a.FailureMessage, b.FailureMessage); len(diffFailureMessage) > 0 {
-		diff[".failureMessage"] = diffFailureMessage
-	}
-
-	return diff
+func CAPIMachineSetStatusEqual(a, b clusterv1.MachineSetStatus) (map[string]any, error) {
+	// Maybe we can also have it this way:
+	// differ := NewUnstructuredDiffer(
+	// 	WithIgnoreField[clusterv1.MachineSetStatus]("conditions"),
+	// 	WithCustomDiff(func(a, b clusterv1.MachineSetStatus) ([]string, error) {
+	// 		return compareCAPIMachineSetConditions(a.Conditions, b.Conditions), nil
+	// 	}),
+	// )
+
+	differ := UnstructuredDiffer[clusterv1.MachineSetStatus]{
+		customDiff: []func(a clusterv1.MachineSetStatus, b clusterv1.MachineSetStatus) ([]string, error){
+			func(a, b clusterv1.MachineSetStatus) ([]string, error) {
+				return compareCAPIMachineSetConditions(a.Conditions, b.Conditions), nil
+			},
+		},
+		ignoreFields: [][]string{{"conditions"}},
+	}
+
+	return differ.Diff(a, b)
 }
 
 func compareCAPIMachineSetConditions(a, b []clusterv1.Condition) []string {
@@ -158,7 +149,7 @@ func compareMAPIMachineSetConditions(a, b []mapiv1beta1.Condition) []string {
 // MAPIMachineSetStatusEqual compares variables a and b,
 // and returns a list of differences, or nil if there are none,
 // for the fields we care about when synchronising MAPI and CAPI Machines.
-func MAPIMachineSetStatusEqual(a, b mapiv1beta1.MachineSetStatus) map[string]any { //nolint:dupl
+func MAPIMachineSetStatusEqual(a, b mapiv1beta1.MachineSetStatus) map[string]any {
 	diff := map[string]any{}
 
 	if diffConditions := compareMAPIMachineSetConditions(a.Conditions, b.Conditions); len(diffConditions) > 0 {