`cmainstallprogression` controller PatchStatus calls missing RetryOnConflict — MCAs stuck Unknown after hub restart

[mcphersonwhite-axon]

Describe the bug

After a hub cluster restart, ManagedClusterAddOn objects for enrolled clusters remain in Unknown or ConfigurationUnsupported state indefinitely. The only recovery is manual intervention (patch status.supportedConfigs by hand or restart the addon-manager).

Root cause

pkg/addon/controllers/cmainstallprogression/controller.go contains three bare PatchStatus calls (one per code path: manual, self-managed, and placement install strategies) with no RetryOnConflict around them.

On hub restart, the informer cache is rebuilt and a reconcile event fires for every ClusterManagementAddOn simultaneously. Multiple reconcile goroutines race to patch the same CMA's status.supportedConfigs. Because the sdk-go patcher makes a single client.Patch() call with no retry, concurrent attempts fail with 409 Conflict — the resourceVersion has already changed between the GET (lister cache) and the PATCH.

The 409 causes the controller to requeue the CMA, but by that time the other goroutines have all failed too. The field never gets written.

Downstream effect: ClusterManagementAddOn.status.supportedConfigs stays empty → any spec.configs patch on a ManagedClusterAddOn is rejected with ConfigurationUnsupported → the addontemplate controller cannot render ManifestWorks → all addons remain Dead/Unknown on every spoke.

Affected versions

• Confirmed on 1.3.1
• Likely also affects 1.2.x (same code path)

Reproduction steps

1. Enroll 3+ spoke clusters to a hub (enough to generate concurrent CMA reconcile events)
2. Restart the cluster-manager deployment: kubectl rollout restart deployment/cluster-manager -n open-cluster-management
3. Observe: kubectl get managedclusteraddon -A shows many addons in Unknown / ConfigurationUnsupported
4. Observe: kubectl get clustermanagementaddon -o json | jq '.[].status.supportedConfigs' shows null on affected CMAs
5. Observe cluster-manager logs for lines like: failed to patch cma status ... the object has been modified; please apply your changes to the latest version

Proposed fix

Wrap each of the three PatchStatus call sites in retry.RetryOnConflict(retry.DefaultRetry, ...). Because PatchStatus takes both newObject and oldObject and creates a merge patch (it does not re-fetch internally), each retry closure must re-fetch a fresh object from the API server before re-applying the status mutation.

A fix with a unit test is available at: (add PR link once opened)

The unit test (TestPatchStatusRetriesOn409Conflict) injects 2 consecutive 409 Conflicts via a PrependReactor, runs sync(), and asserts the reconciler succeeds and writes status.defaultConfigReferences.

Workaround

Until this is fixed upstream, operators can add a "status healer" controller that bootstraps status.supportedConfigs from spec.supportedConfigs after detecting the empty-status condition:

kubectl patch clustermanagementaddon <name> \
  --subresource=status --type=merge \
  --patch '{"status":{"supportedConfigs":[{"group":"addon.open-cluster-management.io","resource":"addondeploymentconfigs"},{"group":"addon.open-cluster-management.io","resource":"addontemplates"}]}}'

(Repeat for each affected CMA.)

[dosubot]

Hi @mcphersonwhite-axon! I'm Dosu and I'm helping the Open-Cluster-Management team.

Your analysis is accurate. I can confirm that pkg/addon/controllers/cmainstallprogression/controller.go has 3 bare PatchStatus calls (at lines ~70, ~76, and ~85) with no RetryOnConflict wrapper around any of them [1]. Each call follows the pattern:

_, err = c.patcher.PatchStatus(ctx, mgmtAddonCopy, mgmtAddonCopy.Status, mgmtAddon.Status)
return err

This means a 409 immediately terminates reconciliation with no retry or fallback.

This is a known area of pain — there's an existing thread documenting persistent "object has been modified" conflicts across multiple addon controllers (addon-configuration-controller, addon-registration-controller, addon-deploy-controller) that concurrently update the same resource [2]. The issue was only partially mitigated by waiting for configReferences to be set before processing.

Notably, RetryOnConflict has already been added to other parts of the addon framework — for example, PR #1142 added it to CreateManagedClusterAddOn status updates. Additionally, PR #1573 (currently open) is working on improving conflict-handling in this area as well. You may want to coordinate with that PR to avoid overlapping changes.

Your proposed fix (wrapping each call site in retry.RetryOnConflict with a fresh re-fetch inside the retry closure) aligns with the pattern already established elsewhere in the codebase. Looking forward to your PR!

To reply, just mention @dosu.

---

Docs are dead. Just use Dosu.

[mikeshng]

This seems like a pretty serious bug to me. I wonder why this has not come up until now.

CC @qiujian16 @tesshuflower

[tesshuflower]

This does sound like a serious issue. I'd have to defer to @qiujian16 as he understands this problem space much better than me, however my 2 cents:

• The ClusterManagementAddOn.status.supportedConfigs - I'm not clear on why this would be empty on a hub restart. Does this issue primarily happen when the hub is restarted while adding new ManagedClusters?
• If we add retries (with re-load) on the patch - This will bypass code in the controller that is already looking at the resource status before determining whether it should do a patch or not (for example here: https://github.com/open-cluster-management-io/ocm/blob/main/pkg/addon/controllers/cmainstallprogression/controller.go#L102-L105). Generally the reconcile logic is setup to fix this eventually by retrying when the initial patch fails. However maybe we're getting into a loop here of many failures that are backing off.

Perhaps retries (with re-load) on the patching is fine, but I don't want to say it's a trivial fix - @qiujian16 will probably be able to provide a more insightful analysis.

[mcphersonwhite-axon]

• The ClusterManagementAddOn.status.supportedConfigs - I'm not clear on why this would be empty on a hub restart. Does this issue primarily happen when the hub is restarted while adding new ManagedClusters?

We are seeing this issue even upon adding new clusters to our placement (via CEL matching). When a new cluster arrives, we have 3-4 addons that we deliver to the cluster. The addons themselves need templating so we use AddonTemplates. Those templates contain cluster specific data. To deliver the cluster specific data to the templating engine, we add an AddonDeploymentConfig that has the cluster specific customized variable. We wrote a small controller to patch the ManagedClusterAddon so that the cluster scoped variables would apply to the AddonTemplate and into the ManifestWork.

This system worked great when we had just a few clusters and a few addons. As we have grown in clusters and addons, we find that the MCAs enter an unknown state that they cannot recover from. We have a work around, but think that allowing retryon the PatchStatus calls will allow the MCA to continue to progress and not remain in an Unknown status.

Honestly this was deep to figure out and while we can operate a bit with our work around, we would prefer if the MCA could not reach a state that it cannot recover from.

Additionally will this bypass the code or just allow the PatchStatus check to have the current state rather than a cached state?

Lastly we considered making a mutating web hook for MCA admission into the cluster in order to append the cluster-specific AddonDeploymentConfig.

If you take the unit tests from this PR and run them against current code you should be able to see the issue, or at least that was the intent. PR #1585