From fb50adc8885852c337fa3ca6600b8e49b6dbd044 Mon Sep 17 00:00:00 2001 From: nispriha Date: Tue, 18 Aug 2026 10:06:58 +0530 Subject: [PATCH] OCPBUGS-100366: Re-queue ContainerRuntimeConfig on status update failure --- .../container_runtime_config_controller.go | 5 +- ...ontainer_runtime_config_controller_test.go | 63 +++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/pkg/controller/container-runtime-config/container_runtime_config_controller.go b/pkg/controller/container-runtime-config/container_runtime_config_controller.go index e0e55eda2b..d5f633498a 100644 --- a/pkg/controller/container-runtime-config/container_runtime_config_controller.go +++ b/pkg/controller/container-runtime-config/container_runtime_config_controller.go @@ -686,11 +686,12 @@ func (ctrl *Controller) syncStatusOnly(cfg *mcfgv1.ContainerRuntimeConfig, err e _, updateErr := ctrl.client.MachineconfigurationV1().ContainerRuntimeConfigs().UpdateStatus(context.TODO(), newcfg, metav1.UpdateOptions{}) return updateErr }) - // If an error occurred in updating the status just log it if statusUpdateErr != nil { klog.Warningf("error updating container runtime config status: %v", statusUpdateErr) + if err == nil { + return statusUpdateErr + } } - // Want to return the actual error received from the sync function return err } diff --git a/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go b/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go index 7abbbe4967..5027730372 100644 --- a/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go +++ b/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go @@ -16,6 +16,7 @@ import ( "github.com/stretchr/testify/require" "k8s.io/klog/v2" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -2430,3 +2431,65 @@ func TestCrioCredentialProviderConfigCreateEmpty(t *testing.T) { }) } } + +// TestStatusUpdateConflictRequeues verifies that when the final status write +// in syncContainerRuntimeConfig fails with a 409 Conflict error (e.g., due to +// a concurrent finalizer update), the sync function returns an error so the +// workqueue re-queues the item. This is a regression test for: +// MCO render controller permanently stuck due to ContainerRuntimeConfig +// failure during SNO bootstrap. +// Without the fix, syncStatusOnly swallows the status update failure (logs a +// warning and returns nil), leaving the CR with no .status and permanently +// blocking the render controller. +func TestStatusUpdateConflictRequeues(t *testing.T) { + for _, platform := range []apicfgv1.PlatformType{apicfgv1.AWSPlatformType, apicfgv1.NonePlatformType} { + t.Run(string(platform), func(t *testing.T) { + f := newFixture(t) + f.skipActionsValidation = true + + cc := newControllerConfig(ctrlcommon.ControllerConfigName, platform) + mcp := helpers.NewMachineConfigPool("master", nil, helpers.MasterSelector, "v0") + mcp2 := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v0") + ctrcfg := newContainerRuntimeConfig("enable-crun-master", &mcfgv1.ContainerRuntimeConfiguration{LogLevel: "debug"}, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "pools.operator.machineconfiguration.openshift.io/master", "")) + + f.ccLister = append(f.ccLister, cc) + f.mcpLister = append(f.mcpLister, mcp, mcp2) + f.mccrLister = append(f.mccrLister, ctrcfg) + f.objects = append(f.objects, ctrcfg) + + c := f.newController() + + // Inject a reactor that makes every UpdateStatus call on + // containerruntimeconfigs return a 409 Conflict error, simulating + // the race where a concurrent finalizer patch bumps the + // resourceVersion between the lister read and the status write. + ctrRuntimeResource := schema.GroupVersionResource{ + Group: "machineconfiguration.openshift.io", + Version: "v1", + Resource: "containerruntimeconfigs", + } + f.client.PrependReactor("update", "containerruntimeconfigs", func(action core.Action) (bool, runtime.Object, error) { + if action.GetSubresource() != "status" { + return false, nil, nil + } + return true, nil, apierrors.NewConflict( + ctrRuntimeResource.GroupResource(), + "enable-crun-master", + fmt.Errorf("the object has been modified; please apply your changes to the latest version and try again"), + ) + }) + + err := c.syncHandler(getKey(ctrcfg, t)) + + // The sync function should return an error when the status write + // fails, so the workqueue re-queues the item. If err is nil, the + // bug is present: the controller silently swallowed the status + // update failure. + if err == nil { + t.Errorf("syncHandler returned nil; expected an error when status update fails with 409 Conflict. " + + "This means the item will NOT be re-queued and the CR will be left without .status, " + + "permanently blocking the render controller.") + } + }) + } +}