fix: preserve existing ArgoCD spec fields during update#627
Conversation
patterns-operator currently performs a full Update on the ArgoCD resource using a newly constructed spec. Any fields not explicitly included in the generated spec are removed during the update. When running alongside OpenShift GitOps Operator v1.20.3, this can cause a reconciliation loop where fields such as networkPolicy are continuously removed and restored by different controllers, resulting in rapid metadata.generation increments and sustained reconciliation activity. This change preserves spec fields already present on the existing ArgoCD resource when they are not explicitly managed by patterns-operator, preventing unintended field removal while keeping the current update flow unchanged. A future improvement could replace the full-object Update with a patch-based approach so patterns-operator only modifies the fields it explicitly owns.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mohtork The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @mohtork. Thanks for your PR. I'm waiting for a validatedpatterns member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
PR Description
Summary
patterns-operatorcurrently performs a fullUpdate()on the ArgoCD custom resource using a newly generated object. During this process, any existing spec fields not explicitly included in the generated object are removed.When running alongside OpenShift GitOps Operator v1.20.3, this creates a reconciliation loop where one controller removes fields such as
networkPolicy, while another controller restores them. This results in continuous.specupdates, rapidly increasingmetadata.generation, repeated reconciliations, and unnecessary API server and etcd activity.This PR preserves existing spec fields that are not explicitly managed by
patterns-operatorbefore performing the update.Root Cause
createOrUpdateArgoCD()currently builds a new ArgoCD object and performs a full object update:Because this is a full
Update(), any spec field not present innewArgois removed from the live resource.In environments where another controller manages additional ArgoCD spec fields — for example
networkPolicyadded by GitOps Operator v1.20.3 — the field is repeatedly removed and restored, causing continuous reconciliation.Fix
After converting to unstructured, copy existing spec fields from the live ArgoCD resource into the new object when those fields are absent from the generated spec:
This is general — it handles
networkPolicy,imageUpdater, and any future fields added by other operators without requiring changes to the vendored argocd-operator types.Reproduction
Environment:
Steps:
Expected: Generation counter stable
Actual: Generation counter increments continuously at ~12/second
Confirm root cause — scale patterns-operator to 0, generation freezes immediately:
Scale back to 1, loop immediately resumes:
Observe the field oscillation directly — one of the reads will catch
np:empty, confirming the strip/restore cycle between the two controllers:Example output showing the oscillation:
Result After Fix
After applying this change:
networkPolicyand similar fields managed by other controllers remain stablemetadata.generationstops continuously incrementingVerification:
Watch generation in another terminal — generation stays frozen, fields remain stable.
Impact
metadata.generationcounter observed exceeding 5.2 million over 49 days in productionNotes
This PR intentionally keeps the current full-object update flow to minimise behavioural changes.
A future improvement could replace the full
Update()with a patch-based approach — specifically server-side apply with field ownership — sopatterns-operatoronly modifies the ArgoCD spec fields it explicitly owns, making field conflicts structurally impossible regardless of what other controllers do.Related
networkPolicyandimageUpdaterspec fields viaargoproj-labs/argocd-operatorPR #2049, which unconditionally writes these fields on every reconcile pass, amplifying any external spec write into a high-frequency loop