fix(controller): Skip no-op infra resource updates, and use actual mysql server status

internal/controller/common/resource.go

@@ -5,7 +5,9 @@ import (
 	"reflect"
 
 	"github.com/wandb/operator/internal/logx"
+	"k8s.io/apimachinery/pkg/api/equality"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -64,8 +66,13 @@ func CrudResource[T client.Object](ctx context.Context, c client.Client, desired
 	actualExists := !IsNil(actual) && actual.GetName() != ""
 
 	if actualExists && desiredExists {
-		action = UpdateAction
+		// Avoid no-op updates while ignoring runtime-managed fields such as status and
+		// resource metadata that differ between desired and actual.
+		if desiredMatchesActual(desired, actual) {
+			return NoAction, nil
+		}
 
+		action = UpdateAction
 		desired.SetResourceVersion(actual.GetResourceVersion())
 		err = c.Update(ctx, desired)
 	}
@@ -90,6 +97,37 @@ func CrudResource[T client.Object](ctx context.Context, c client.Client, desired
 	return action, err
 }
 
+func desiredMatchesActual(desired client.Object, actual client.Object) bool {
+	desiredMap, desiredErr := toComparableUnstructured(desired)
+	actualMap, actualErr := toComparableUnstructured(actual)
+	if desiredErr == nil && actualErr == nil {
+		return equality.Semantic.DeepDerivative(desiredMap, actualMap)
+	}
+
+	// Fallback for unexpected conversion failures.
+	return equality.Semantic.DeepDerivative(desired, actual)
+}
+
+func toComparableUnstructured(obj client.Object) (map[string]any, error) {
+	m, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj)
+	if err != nil {
+		return nil, err
+	}
+	delete(m, "status")
+
+	metadata, ok := m["metadata"].(map[string]any)
+	if ok {
+		delete(metadata, "creationTimestamp")
+		delete(metadata, "resourceVersion")
+		delete(metadata, "generation")
+		delete(metadata, "uid")
+		delete(metadata, "managedFields")
+		delete(metadata, "selfLink")
+	}
+
+	return m, nil
+}
+
 // IsNil checks if the generic value v is a pointer and if that pointer is nil.
 // It returns false if true is a non-pointer type, or if it's a non-nil pointer.
 func IsNil[T any](v T) bool {

internal/controller/common/resource_test.go

@@ -0,0 +1,152 @@
+package common
+
+import (
+	"context"
+	"testing"
+
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+)
+
+func TestCrudResourceNoActionForSubsetDesired(t *testing.T) {
+	scheme := runtime.NewScheme()
+	if err := corev1.AddToScheme(scheme); err != nil {
+		t.Fatalf("failed to add corev1 scheme: %v", err)
+	}
+
+	existing := &corev1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:        "cm",
+			Namespace:   "default",
+			Annotations: map[string]string{"controller-added": "true"},
+		},
+		Data: map[string]string{"key": "value"},
+	}
+
+	client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(existing).Build()
+
+	actual := &corev1.ConfigMap{}
+	if err := client.Get(context.Background(), types.NamespacedName{Name: "cm", Namespace: "default"}, actual); err != nil {
+		t.Fatalf("failed to get configmap: %v", err)
+	}
+
+	desired := &corev1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "cm",
+			Namespace: "default",
+		},
+		Data: map[string]string{"key": "value"},
+	}
+
+	action, err := CrudResource(context.Background(), client, desired, actual)
+	if err != nil {
+		t.Fatalf("CrudResource returned error: %v", err)
+	}
+	if action != NoAction {
+		t.Fatalf("expected no action, got %s", action)
+	}
+}
+
+func TestCrudResourceUpdateWhenDesiredDiffers(t *testing.T) {
+	scheme := runtime.NewScheme()
+	if err := corev1.AddToScheme(scheme); err != nil {
+		t.Fatalf("failed to add corev1 scheme: %v", err)
+	}
+
+	existing := &corev1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "cm",
+			Namespace: "default",
+		},
+		Data: map[string]string{"key": "old"},
+	}
+
+	client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(existing).Build()
+
+	actual := &corev1.ConfigMap{}
+	if err := client.Get(context.Background(), types.NamespacedName{Name: "cm", Namespace: "default"}, actual); err != nil {
+		t.Fatalf("failed to get configmap: %v", err)
+	}
+
+	desired := &corev1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "cm",
+			Namespace: "default",
+		},
+		Data: map[string]string{"key": "new"},
+	}
+
+	action, err := CrudResource(context.Background(), client, desired, actual)
+	if err != nil {
+		t.Fatalf("CrudResource returned error: %v", err)
+	}
+	if action != UpdateAction {
+		t.Fatalf("expected update action, got %s", action)
+	}
+
+	updated := &corev1.ConfigMap{}
+	if err := client.Get(context.Background(), types.NamespacedName{Name: "cm", Namespace: "default"}, updated); err != nil {
+		t.Fatalf("failed to get updated configmap: %v", err)
+	}
+	if updated.Data["key"] != "new" {
+		t.Fatalf("expected updated data, got %q", updated.Data["key"])
+	}
+}
+
+func TestCrudResourceNoActionIgnoresStatusAndRuntimeMetadata(t *testing.T) {
+	scheme := runtime.NewScheme()
+	if err := corev1.AddToScheme(scheme); err != nil {
+		t.Fatalf("failed to add corev1 scheme: %v", err)
+	}
+
+	existing := &corev1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:            "pod",
+			Namespace:       "default",
+			ResourceVersion: "12",
+			Generation:      3,
+			UID:             "pod-uid",
+			Annotations: map[string]string{
+				"controller-added": "true",
+			},
+		},
+		Spec: corev1.PodSpec{
+			Containers: []corev1.Container{
+				{Name: "main", Image: "busybox"},
+			},
+		},
+		Status: corev1.PodStatus{
+			Phase: corev1.PodRunning,
+		},
+	}
+
+	client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(existing).Build()
+
+	actual := &corev1.Pod{}
+	if err := client.Get(context.Background(), types.NamespacedName{Name: "pod", Namespace: "default"}, actual); err != nil {
+		t.Fatalf("failed to get pod: %v", err)
+	}
+
+	desired := &corev1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "pod",
+			Namespace: "default",
+		},
+		Spec: corev1.PodSpec{
+			Containers: []corev1.Container{
+				{Name: "main", Image: "busybox"},
+			},
+		},
+	}
+
+	action, err := CrudResource(context.Background(), client, desired, actual)
+	if err != nil {
+		t.Fatalf("CrudResource returned error: %v", err)
+	}
+	if action != NoAction {
+		t.Fatalf("expected no action, got %s", action)
+	}
+}

internal/controller/infra/mysql/mysql/read.go

@@ -2,8 +2,10 @@ package mysql
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"strconv"
+	"strings"
 
 	ctrlcommon "github.com/wandb/operator/internal/controller/common"
 	"github.com/wandb/operator/internal/controller/translator"
@@ -126,21 +128,21 @@ func computeMySQLReportedReadyCondition(_ context.Context, clusterCR *v2.InnoDBC
 		return []metav1.Condition{}
 	}
 
-	status := metav1.ConditionUnknown
-	reason := "Unknown"
-
-	// Map InnoDBCluster status to conditions.
-	// InnoDBClusterStatus.Status is a RawExtension, but we can check if it's ready
-	// based on the documented behavior or by looking at the actual status if we had more info.
-	// For now, we'll assume it's ready if we can find it, or try to infer from common patterns.
-	// Actually, looking at innodbcluster_types.go, Status is runtime.RawExtension.
-	// We might need to unmarshal it or find another way to check readiness.
-
-	// If we don't have a clear way to check readiness from the Go types yet,
-	// we might just mark it as True if found for now, or look for standard conditions.
-
-	status = metav1.ConditionTrue
-	reason = "Online"
+	status := metav1.ConditionFalse
+	reason := ctrlcommon.NoResourceReason
+
+	clusterStatus := extractInnoDBClusterStatus(clusterCR)
+	switch {
+	case strings.EqualFold(clusterStatus, "ONLINE"):
+		status = metav1.ConditionTrue
+		reason = "Online"
+	case clusterStatus != "":
+		status = metav1.ConditionFalse
+		reason = clusterStatus
+	default:
+		status = metav1.ConditionFalse
+		reason = ctrlcommon.NoResourceReason
+	}
 
 	return []metav1.Condition{
 		{
@@ -150,3 +152,24 @@ func computeMySQLReportedReadyCondition(_ context.Context, clusterCR *v2.InnoDBC
 		},
 	}
 }
+
+func extractInnoDBClusterStatus(clusterCR *v2.InnoDBCluster) string {
+	if clusterCR == nil || len(clusterCR.Status.Raw) == 0 {
+		return ""
+	}
+
+	var raw map[string]any
+	if err := json.Unmarshal(clusterCR.Status.Raw, &raw); err != nil {
+		return ""
+	}
+
+	cluster, ok := raw["cluster"].(map[string]any)
+	if !ok {
+		return ""
+	}
+	value, ok := cluster["status"].(string)
+	if !ok {
+		return ""
+	}
+	return value
+}

internal/controller/infra/mysql/mysql/read_test.go

@@ -0,0 +1,73 @@
+package mysql
+
+import (
+	"context"
+	"testing"
+
+	"github.com/wandb/operator/internal/controller/common"
+	"github.com/wandb/operator/pkg/vendored/mysql-operator/v2"
+	"k8s.io/apimachinery/pkg/runtime"
+)
+
+func TestComputeMySQLReportedReadyConditionOnline(t *testing.T) {
+	cluster := &v2.InnoDBCluster{
+		Status: v2.InnoDBClusterStatus{
+			RawExtension: runtime.RawExtension{
+				Raw: []byte(`{"cluster":{"status":"ONLINE"}}`),
+			},
+		},
+	}
+
+	conditions := computeMySQLReportedReadyCondition(context.Background(), cluster)
+	if len(conditions) != 1 {
+		t.Fatalf("expected one condition, got %d", len(conditions))
+	}
+	if conditions[0].Status != "True" {
+		t.Fatalf("expected ready condition true, got %s", conditions[0].Status)
+	}
+	if conditions[0].Reason != "Online" {
+		t.Fatalf("expected reason Online, got %s", conditions[0].Reason)
+	}
+}
+
+func TestComputeMySQLReportedReadyConditionPending(t *testing.T) {
+	cluster := &v2.InnoDBCluster{
+		Status: v2.InnoDBClusterStatus{
+			RawExtension: runtime.RawExtension{
+				Raw: []byte(`{"cluster":{"status":"PENDING"}}`),
+			},
+		},
+	}
+
+	conditions := computeMySQLReportedReadyCondition(context.Background(), cluster)
+	if len(conditions) != 1 {
+		t.Fatalf("expected one condition, got %d", len(conditions))
+	}
+	if conditions[0].Status != "False" {
+		t.Fatalf("expected ready condition false, got %s", conditions[0].Status)
+	}
+	if conditions[0].Reason != "PENDING" {
+		t.Fatalf("expected reason PENDING, got %s", conditions[0].Reason)
+	}
+}
+
+func TestComputeMySQLReportedReadyConditionMissingStatus(t *testing.T) {
+	cluster := &v2.InnoDBCluster{
+		Status: v2.InnoDBClusterStatus{
+			RawExtension: runtime.RawExtension{
+				Raw: []byte(`{"kopf":{"progress":{}}}`),
+			},
+		},
+	}
+
+	conditions := computeMySQLReportedReadyCondition(context.Background(), cluster)
+	if len(conditions) != 1 {
+		t.Fatalf("expected one condition, got %d", len(conditions))
+	}
+	if conditions[0].Status != "False" {
+		t.Fatalf("expected ready condition false, got %s", conditions[0].Status)
+	}
+	if conditions[0].Reason != common.NoResourceReason {
+		t.Fatalf("expected reason %s, got %s", common.NoResourceReason, conditions[0].Reason)
+	}
+}