Skip to content

fix: preserve existing ArgoCD spec fields during update#627

Open
mohtork wants to merge 1 commit into
validatedpatterns:mainfrom
mohtork:fix/preserve-argocd-spec-fields-on-update
Open

fix: preserve existing ArgoCD spec fields during update#627
mohtork wants to merge 1 commit into
validatedpatterns:mainfrom
mohtork:fix/preserve-argocd-spec-fields-on-update

Conversation

@mohtork
Copy link
Copy Markdown

@mohtork mohtork commented May 15, 2026

PR Description


Summary

patterns-operator currently performs a full Update() 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 .spec updates, rapidly increasing metadata.generation, repeated reconciliations, and unnecessary API server and etcd activity.

This PR preserves existing spec fields that are not explicitly managed by patterns-operator before performing the update.


Root Cause

createOrUpdateArgoCD() currently builds a new ArgoCD object and performs a full object update:

argo.SetResourceVersion(oldArgo.GetResourceVersion())

obj, errConvert := runtime.DefaultUnstructuredConverter.ToUnstructured(argo)
newArgo := &unstructured.Unstructured{Object: obj}

_, err = client.Resource(gvr).Namespace(namespace).Update(
    context.TODO(),
    newArgo,
    metav1.UpdateOptions{},
)

Because this is a full Update(), any spec field not present in newArgo is removed from the live resource.

In environments where another controller manages additional ArgoCD spec fields — for example networkPolicy added 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:

// Preserve spec fields not known to this vendored argocd-operator version
// (e.g. networkPolicy added in gitops-operator v1.20.3) to avoid
// stripping them and causing infinite reconciliation loops.
oldUnstructured, errGet2 := client.Resource(gvr).Namespace(namespace).Get(context.TODO(), name, metav1.GetOptions{})
if errGet2 == nil {
    oldSpec, _, _ := unstructured.NestedMap(oldUnstructured.Object, "spec")
    newSpec, _, _ := unstructured.NestedMap(newArgo.Object, "spec")
    for key, val := range oldSpec {
        if _, exists := newSpec[key]; !exists {
            _ = unstructured.SetNestedField(newArgo.Object, val, "spec", key)
        }
    }
}

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:

  • OCP 4.20.x
  • OpenShift GitOps Operator v1.20.3
  • patterns-operator v0.0.72

Steps:

# 1. Install OpenShift GitOps Operator v1.20.3 on OCP 4.20.x

# 2. Install patterns-operator v0.0.72
cat <<EOF | oc apply -f -
apiVersion: operators.coreos.com/v1alpha1
kind: Subscription
metadata:
  name: patterns-operator
  namespace: openshift-operators
spec:
  channel: fast
  name: patterns-operator
  source: community-operators
  sourceNamespace: openshift-marketplace
  startingCSV: patterns-operator.v0.0.72
EOF

# 3. Create a Pattern CR
cat <<EOF | oc apply -f -
apiVersion: gitops.hybrid-cloud-patterns.io/v1alpha1
kind: Pattern
metadata:
  name: test-pattern
  namespace: openshift-operators
spec:
  clusterGroupName: hub
  gitSpec:
    targetRepo: https://github.com/validatedpatterns/multicloud-gitops.git
    targetRevision: main
  multiSourceConfig:
    enabled: true
EOF

# 4. Watch generation counter — increments at ~12/second
watch -n1 "oc get argocd openshift-gitops -n openshift-gitops \
  -o jsonpath='{.metadata.generation}{\"\\n\"}'"

Expected: Generation counter stable
Actual: Generation counter increments continuously at ~12/second

Confirm root cause — scale patterns-operator to 0, generation freezes immediately:

oc scale deployment patterns-operator-controller-manager \
  -n openshift-operators --replicas=0

Scale back to 1, loop immediately resumes:

oc scale deployment patterns-operator-controller-manager \
  -n openshift-operators --replicas=1

Observe the field oscillation directly — one of the reads will catch np: empty, confirming the strip/restore cycle between the two controllers:

for i in $(seq 1 10); do
  oc get argocd openshift-gitops -n openshift-gitops \
    -o jsonpath='{.metadata.generation} np:{.spec.networkPolicy} iu:{.spec.imageUpdater}{"\n"}'
done

Example output showing the oscillation:

2552 np:{"enabled":true} iu:{"enabled":false}
2557 np: iu:                                    ← patterns-operator stripped the fields
2562 np:{"enabled":true} iu:{"enabled":false}  ← gitops-operator restored them

Result After Fix

After applying this change:

  • Existing ArgoCD spec fields are preserved across updates
  • networkPolicy and similar fields managed by other controllers remain stable
  • metadata.generation stops continuously incrementing
  • Reconciliation loop is eliminated
  • No behaviour change for fields explicitly managed by patterns-operator

Verification:

# Scale down installed operator
oc scale deployment patterns-operator-controller-manager \
  -n openshift-operators --replicas=0

# Clone repo with fix applied and run locally
ENABLE_WEBHOOKS=false make run

Watch generation in another terminal — generation stays frozen, fields remain stable.


Impact

  • Affects any cluster running both patterns-operator and GitOps Operator v1.20.3 simultaneously
  • metadata.generation counter observed exceeding 5.2 million over 49 days in production
  • Continuous log storm from all ArgoCD sub-components (dex, repo-server, application-controller) reacting to Watch events at ~12 writes/second
  • Sustained etcd write pressure
  • No impact on ArgoCD functionality — applications continue syncing normally throughout

Notes

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 — so patterns-operator only modifies the ArgoCD spec fields it explicitly owns, making field conflicts structurally impossible regardless of what other controllers do.


Related

  • GitOps Operator v1.20.3 introduced networkPolicy and imageUpdater spec fields via argoproj-labs/argocd-operator PR #2049, which unconditionally writes these fields on every reconcile pass, amplifying any external spec write into a high-frequency loop

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.
@openshift-ci openshift-ci Bot requested review from dminnear-rh and mbaldessari May 15, 2026 13:45
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 15, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mohtork
Once this PR has been reviewed and has the lgtm label, please assign oaharoni-redhat for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 15, 2026

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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant