From 2c4e2bc33f787e54d6724eef5c1e4d22d9592538 Mon Sep 17 00:00:00 2001
From: David Joshy <djoshy@redhat.com>
Date: Thu, 3 Sep 2026 12:44:59 -0400
Subject: [PATCH] operator: retry on conflict in syncMachineConfigNodes

Wrap ApplyMachineConfigNode with retry.OnError/DefaultRetry to prevent transient
conflict errors from surfacing a Degraded=True condition on the ClusterOperator.
---
 lib/resourceapply/machineconfig_test.go | 92 +++++++++++++++++++++++++
 pkg/operator/sync.go                    |  8 ++-
 2 files changed, 98 insertions(+), 2 deletions(-)

diff --git a/lib/resourceapply/machineconfig_test.go b/lib/resourceapply/machineconfig_test.go
index 3103fc78d9..4638e67713 100644
--- a/lib/resourceapply/machineconfig_test.go
+++ b/lib/resourceapply/machineconfig_test.go
@@ -10,8 +10,10 @@ import (
 	"github.com/openshift/client-go/machineconfiguration/clientset/versioned/fake"
 	"github.com/openshift/machine-config-operator/test/helpers"
 	"k8s.io/apimachinery/pkg/api/equality"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/diff"
 	clienttesting "k8s.io/client-go/testing"
 )
@@ -296,3 +298,93 @@ func TestApplyMachineConfig(t *testing.T) {
 		})
 	}
 }
+
+// TestIsApplyErrorRetriable verifies all cases that IsApplyErrorRetriable considers retriable,
+// and that unrelated errors are not.
+func TestIsApplyErrorRetriable(t *testing.T) {
+	mcnGR := schema.GroupResource{Group: "machineconfiguration.openshift.io", Resource: "machineconfignodes"}
+
+	tests := []struct {
+		name      string
+		err       error
+		retriable bool
+	}{
+		{
+			name:      "conflict error is retriable",
+			err:       apierrors.NewConflict(mcnGR, "node-1", fmt.Errorf("the object has been modified")),
+			retriable: true,
+		},
+		{
+			name:      "timeout error is retriable",
+			err:       apierrors.NewTimeoutError("request timed out", 10),
+			retriable: true,
+		},
+		{
+			name:      "rpc error is retriable",
+			err:       fmt.Errorf("rpc error: code = Unavailable desc = transport is closing"),
+			retriable: true,
+		},
+		{
+			name:      "not found error is not retriable",
+			err:       apierrors.NewNotFound(mcnGR, "node-1"),
+			retriable: false,
+		},
+		{
+			name:      "forbidden error is not retriable",
+			err:       apierrors.NewForbidden(mcnGR, "node-1", fmt.Errorf("access denied")),
+			retriable: false,
+		},
+		{
+			name:      "unrelated error is not retriable",
+			err:       fmt.Errorf("something went wrong"),
+			retriable: false,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := IsApplyErrorRetriable(tc.err); got != tc.retriable {
+				t.Errorf("IsApplyErrorRetriable(%v) = %v, want %v", tc.err, got, tc.retriable)
+			}
+		})
+	}
+}
+
+// TestApplyMachineConfigNodeConflictIsRetriable verifies that ApplyMachineConfigNode surfaces a
+// conflict error that IsApplyErrorRetriable recognises, so the retry.OnError wrapper in
+// syncMachineConfigNodes can retry it correctly.
+func TestApplyMachineConfigNodeConflictIsRetriable(t *testing.T) {
+	existing := &mcfgv1.MachineConfigNode{
+		ObjectMeta: metav1.ObjectMeta{Name: "node-1", ResourceVersion: "100"},
+		Spec: mcfgv1.MachineConfigNodeSpec{
+			Pool: mcfgv1.MCOObjectReference{Name: "worker"},
+		},
+	}
+	required := &mcfgv1.MachineConfigNode{
+		ObjectMeta: metav1.ObjectMeta{Name: "node-1"},
+		Spec: mcfgv1.MachineConfigNodeSpec{
+			Pool: mcfgv1.MCOObjectReference{Name: "master"},
+		},
+	}
+
+	client := fake.NewSimpleClientset(existing)
+	conflictErr := apierrors.NewConflict(
+		schema.GroupResource{Group: "machineconfiguration.openshift.io", Resource: "machineconfignodes"},
+		"node-1",
+		fmt.Errorf("the object has been modified; please apply your changes to the latest version and try again"),
+	)
+	client.PrependReactor("update", "machineconfignodes", func(_ clienttesting.Action) (bool, runtime.Object, error) {
+		return true, nil, conflictErr
+	})
+
+	_, _, err := ApplyMachineConfigNode(client.MachineconfigurationV1(), required)
+	if err == nil {
+		t.Fatal("expected conflict error, got nil")
+	}
+	if !apierrors.IsConflict(err) {
+		t.Fatalf("expected conflict error, got: %v", err)
+	}
+	if !IsApplyErrorRetriable(err) {
+		t.Fatal("conflict error should be retriable via IsApplyErrorRetriable")
+	}
+}
diff --git a/pkg/operator/sync.go b/pkg/operator/sync.go
index 12cd0a8e24..4d330fa6a7 100644
--- a/pkg/operator/sync.go
+++ b/pkg/operator/sync.go
@@ -892,8 +892,12 @@ func (optr *Operator) syncMachineConfigNodes(_ *renderConfig, _ *configv1.Cluste
 			return err
 		}
 		p := mcoResourceRead.ReadMachineConfigNodeV1OrDie(mcsBytes)
-		mcn, _, err := mcoResourceApply.ApplyMachineConfigNode(optr.client.MachineconfigurationV1(), p)
-		if err != nil {
+		var mcn *mcfgv1.MachineConfigNode
+		if err := retry.OnError(retry.DefaultRetry, mcoResourceApply.IsApplyErrorRetriable, func() error {
+			var applyErr error
+			mcn, _, applyErr = mcoResourceApply.ApplyMachineConfigNode(optr.client.MachineconfigurationV1(), p)
+			return applyErr
+		}); err != nil {
 			return err
 		}
 		// if this is the first time we are applying the MCN and the node is ready, set the config version probably