fix(rbg): use SSA Apply with distinct field managers on RBG writes

internal/controller/workloads/discovery_config_mode_test.go

@@ -37,17 +37,16 @@ import (
 
 func TestEnsureDiscoveryConfigMode(t *testing.T) {
 	type testCase struct {
-		name               string
-		mutateRBG          func(*workloadsv1alpha2.RoleBasedGroup)
-		buildExtraObjects  func(*workloadsv1alpha2.RoleBasedGroup) []runtime.Object
-		wantRequeue        bool
-		wantMode           constants.DiscoveryConfigMode
-		wantModeAnnotation bool
+		name              string
+		mutateRBG         func(*workloadsv1alpha2.RoleBasedGroup)
+		buildExtraObjects func(*workloadsv1alpha2.RoleBasedGroup) []runtime.Object
+		wantRequeue       bool
+		wantMode          constants.DiscoveryConfigMode
 	}
 
 	tests := []testCase{
 		{
-			name: "missing annotation with legacy role configmap should set legacy mode without requeue",
+			name: "missing annotation with legacy role configmap sets legacy mode",
 			buildExtraObjects: func(rbg *workloadsv1alpha2.RoleBasedGroup) []runtime.Object {
 				return []runtime.Object{
 					&corev1.ConfigMap{
@@ -58,33 +57,37 @@ func TestEnsureDiscoveryConfigMode(t *testing.T) {
 					},
 				}
 			},
-			wantRequeue:        false,
-			wantMode:           constants.LegacyDiscoveryConfigMode,
-			wantModeAnnotation: true,
+			wantRequeue: false,
+			wantMode:    constants.LegacyDiscoveryConfigMode,
 		},
 		{
-			name:               "missing annotation without legacy signal should set refine mode without requeue",
-			wantRequeue:        false,
-			wantMode:           constants.RefineDiscoveryConfigMode,
-			wantModeAnnotation: true,
+			name: "missing annotation with observed generation sets legacy mode",
+			mutateRBG: func(rbg *workloadsv1alpha2.RoleBasedGroup) {
+				rbg.Status.ObservedGeneration = 1
+			},
+			wantRequeue: false,
+			wantMode:    constants.LegacyDiscoveryConfigMode,
+		},
+		{
+			name:        "missing annotation without legacy signal sets refine mode",
+			wantRequeue: false,
+			wantMode:    constants.RefineDiscoveryConfigMode,
 		},
 		{
-			name: "existing legacy annotation should not requeue",
+			name: "existing legacy annotation is a no-op",
 			mutateRBG: func(rbg *workloadsv1alpha2.RoleBasedGroup) {
 				rbg.SetDiscoveryConfigMode(constants.LegacyDiscoveryConfigMode)
 			},
-			wantRequeue:        false,
-			wantMode:           constants.LegacyDiscoveryConfigMode,
-			wantModeAnnotation: true,
+			wantRequeue: false,
+			wantMode:    constants.LegacyDiscoveryConfigMode,
 		},
 		{
-			name: "existing refine annotation should not requeue",
+			name: "existing refine annotation is a no-op",
 			mutateRBG: func(rbg *workloadsv1alpha2.RoleBasedGroup) {
 				rbg.SetDiscoveryConfigMode(constants.RefineDiscoveryConfigMode)
 			},
-			wantRequeue:        false,
-			wantMode:           constants.RefineDiscoveryConfigMode,
-			wantModeAnnotation: true,
+			wantRequeue: false,
+			wantMode:    constants.RefineDiscoveryConfigMode,
 		},
 	}
 
@@ -111,7 +114,7 @@ func TestEnsureDiscoveryConfigMode(t *testing.T) {
 			current := &workloadsv1alpha2.RoleBasedGroup{}
 			key := types.NamespacedName{Name: rbg.Name, Namespace: rbg.Namespace}
 			if err := client.Get(context.Background(), key, current); err != nil {
-				t.Fatalf("get rbg error: %v", err)
+				t.Fatalf("get rbg: %v", err)
 			}
 
 			requeue, err := reconciler.ensureDiscoveryConfigMode(context.Background(), current)
@@ -122,16 +125,16 @@ func TestEnsureDiscoveryConfigMode(t *testing.T) {
 				t.Fatalf("requeue = %v, want %v", requeue, tt.wantRequeue)
 			}
 
+			if got := current.GetDiscoveryConfigMode(); got != tt.wantMode {
+				t.Fatalf("in-memory mode = %s, want %s", got, tt.wantMode)
+			}
+
 			persisted := &workloadsv1alpha2.RoleBasedGroup{}
 			if err := client.Get(context.Background(), key, persisted); err != nil {
-				t.Fatalf("get persisted rbg error: %v", err)
+				t.Fatalf("get persisted rbg: %v", err)
 			}
 			if got := persisted.GetDiscoveryConfigMode(); got != tt.wantMode {
-				t.Fatalf("mode = %s, want %s", got, tt.wantMode)
-			}
-			_, hasModeAnnotation := persisted.Annotations[constants.DiscoveryConfigModeAnnotationKey]
-			if hasModeAnnotation != tt.wantModeAnnotation {
-				t.Fatalf("has discovery-config-mode annotation = %v, want %v", hasModeAnnotation, tt.wantModeAnnotation)
+				t.Fatalf("persisted mode = %s, want %s", got, tt.wantMode)
 			}
 		})
 	}

internal/controller/workloads/rolebasedgroup_controller.go

@@ -55,6 +55,7 @@ import (
 	lwsv1 "sigs.k8s.io/lws/api/leaderworkerset/v1"
 	"sigs.k8s.io/rbgs/api/workloads/constants"
 	workloadsv1alpha2 "sigs.k8s.io/rbgs/api/workloads/v1alpha2"
+	applyconfiguration "sigs.k8s.io/rbgs/client-go/applyconfiguration/workloads/v1alpha2"
 	"sigs.k8s.io/rbgs/pkg/coordination/coordinationscaling"
 	"sigs.k8s.io/rbgs/pkg/dependency"
 	"sigs.k8s.io/rbgs/pkg/discovery"
@@ -339,11 +340,19 @@ func (r *RoleBasedGroupReconciler) ensureDiscoveryConfigMode(
 		mode = constants.RefineDiscoveryConfigMode
 	}
 
-	old := rbg.DeepCopy()
-	rbg.SetDiscoveryConfigMode(mode)
-	if err := r.client.Patch(ctx, rbg, client.MergeFrom(old)); err != nil {
+	gvk := utils.GetRbgGVK()
+	applyCfg := applyconfiguration.RoleBasedGroup(rbg.Name, rbg.Namespace).
+		WithKind(gvk.Kind).
+		WithAPIVersion(gvk.GroupVersion().String()).
+		WithAnnotations(map[string]string{
+			constants.DiscoveryConfigModeAnnotationKey: string(mode),
+		})
+	if err := utils.PatchObjectApplyConfigurationWithFieldManager(
+		ctx, r.client, applyCfg, utils.PatchSpec, utils.RBGDiscoveryFieldManager,
+	); err != nil {
 		return true, err
 	}
+	rbg.SetDiscoveryConfigMode(mode)
 
 	log.FromContext(ctx).Info("Initialized discovery config mode", "mode", mode)
 	// Don't requeue here - continue to reconcile ConfigMap and workloads in the same loop

internal/controller/workloads/rolebasedgroupscalingadapter_controller.go

@@ -24,15 +24,13 @@ import (
 
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	metaapplyv1 "k8s.io/client-go/applyconfigurations/meta/v1"
 	"k8s.io/client-go/tools/record"
-	"k8s.io/client-go/util/retry"
 	"k8s.io/klog/v2"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
@@ -498,28 +496,25 @@ func (r *RoleBasedGroupScalingAdapterReconciler) GetTargetRbgFromAdapter(
 func (r *RoleBasedGroupScalingAdapterReconciler) updateRoleReplicas(
 	ctx context.Context, rbg *workloadsv1alpha2.RoleBasedGroup, targetRoleName string, newReplicas *int32,
 ) error {
-	return retry.RetryOnConflict(
-		retry.DefaultBackoff, func() error {
-			for index, role := range rbg.Spec.Roles {
-				if role.Name == targetRoleName {
-					role.Replicas = newReplicas
-					rbg.Spec.Roles[index] = role
-					break
-				}
-			}
-			if err := r.client.Update(ctx, rbg); err != nil {
-				if apierrors.IsConflict(err) {
-					if err := r.client.Get(
-						ctx, types.NamespacedName{Name: rbg.Name, Namespace: rbg.Namespace}, rbg,
-					); err != nil {
-						return err
-					}
-				}
-				return err
-			}
-			return nil
-		},
-	)
+	gvk := utils.GetRbgGVK()
+	applyCfg := applyconfiguration.RoleBasedGroup(rbg.Name, rbg.Namespace).
+		WithKind(gvk.Kind).
+		WithAPIVersion(gvk.GroupVersion().String()).
+		WithSpec(
+			applyconfiguration.RoleBasedGroupSpec().
+				WithRoles(
+					applyconfiguration.RoleSpec().
+						WithName(targetRoleName).
+						WithReplicas(*newReplicas),
+				),
+		)
+
+	if err := utils.PatchObjectApplyConfigurationWithFieldManager(
+		ctx, r.client, applyCfg, utils.PatchSpec, utils.RBGReplicaFieldManager,
+	); err != nil {
+		return fmt.Errorf("apply replica update for role %q: %w", targetRoleName, err)
+	}
+	return nil
 }
 
 // extractLabelSelectorDefault extracts a LabelSelector string from the given role's scale subresource.

internal/controller/workloads/rolebasedgroupscalingadapter_controller_test.go

@@ -1110,3 +1110,42 @@ func TestRBGRoleStatusPredicate(t *testing.T) {
 		}))
 	})
 }
+
+// TestUpdateRoleReplicas only covers behaviors the controller-runtime fake
+// client can faithfully model: that the apply config is accepted and the
+// targeted role's replicas reflect the desired value. SSA list-merge by
+// name and sibling-field preservation are real API-server behaviors and
+// must be verified via envtest or kind.
+func TestUpdateRoleReplicas(t *testing.T) {
+	scheme := runtime.NewScheme()
+	_ = workloadsv1alpha2.AddToScheme(scheme)
+	_ = corev1.AddToScheme(scheme)
+
+	rbg := wrappersv2.BuildBasicRoleBasedGroup("test-rbg", "default").
+		WithRoles([]workloadsv1alpha2.RoleSpec{
+			wrappersv2.BuildStandaloneRole("worker").WithReplicas(1).Obj(),
+		}).Obj()
+
+	cl := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(rbg).Build()
+	reconciler := &RoleBasedGroupScalingAdapterReconciler{
+		client:   cl,
+		recorder: record.NewFakeRecorder(10),
+	}
+	ctx := ctrl.LoggerInto(context.Background(), zap.New().WithValues("env", "unit-test"))
+
+	if err := reconciler.updateRoleReplicas(ctx, rbg, "worker", ptr.To(int32(5))); err != nil {
+		t.Fatalf("updateRoleReplicas: %v", err)
+	}
+
+	updated := &workloadsv1alpha2.RoleBasedGroup{}
+	if err := cl.Get(ctx, types.NamespacedName{Name: rbg.Name, Namespace: rbg.Namespace}, updated); err != nil {
+		t.Fatalf("get updated RBG: %v", err)
+	}
+	role, err := updated.GetRole("worker")
+	if err != nil {
+		t.Fatalf("worker role missing after update: %v", err)
+	}
+	if role.Replicas == nil || *role.Replicas != 5 {
+		t.Errorf("worker replicas = %v, want 5", role.Replicas)
+	}
+}