fix: Incorrect comparison logic when comparing lists

[aali309]

What type of PR is this?
See: GITOPS-8018

/kind bug

What does this PR do / why we need it:
This change sorts deployment lists before comparison to prevent false updates when
etcd returns containers, volumes, or tolerations in different order.

Have you updated the necessary documentation?

• Documentation update is required by this PR.
• Documentation has been updated.

Which issue(s) this PR fixes:

Fixes: GITOPS-8018

Test acceptance criteria:

• Unit Test
• E2E Test

How to test changes / Special notes to the reviewer:

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign jannfis 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:

• OWNERS

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

[ranakan19]

Do we have any data on the impact of sorting before comparison here? Maybe we have any performance tests that can be run to measure the impact?

My concern is that sorting generally has a higher time complexity (O(nlogn) in the best cases). Unless size is bound or is not performance-sensitive, I'm cautious of sorting beforehand. So looking for more context about this decision

[olivergondza]

Question, not an objection: Can we assume that list order is insignificant in all those cases? For instance, EgressNetworkPolicy is using list where order absolutely matters.

Thinking about EnvVars that override one another, or VolumeMounts that either override, or mount inside one another's mount path. If I, as an admin, get them in a wrong order and then fix it, the operator may refuse to update it therefore not letting me fix it.

[aali309]

Question, not an objection: Can we assume that list order is insignificant in all those cases? For instance, EgressNetworkPolicy is using list where order absolutely matters.

Thinking about EnvVars that override one another, or VolumeMounts that either override, or mount inside one another's mount path. If I, as an admin, get them in a wrong order and then fix it, the operator may refuse to update it therefore not letting me fix it.

From what I understand so far, the deployment generated by the operator doesn’t support duplicate env var names or overlapping volume mount paths. Because of that, the order of these lists doesn’t seem to carry semantic meaning in this context. Sorting them helps avoid false positives caused by non-deterministic ordering from etcd, without hiding any meaningful changes.

[aali309]

Do we have any data on the impact of sorting before comparison here? Maybe we have any performance tests that can be run to measure the impact?

My concern is that sorting generally has a higher time complexity (O(nlogn) in the best cases). Unless size is bound or is not performance-sensitive, I'm cautious of sorting beforehand. So looking for more context about this decision

Thought of this, these lists(containers, env, volumes, tolerations) are very small and this code runs only during reconciliation, not in a hot path.

So I think In this case, sorting is more efficient than the alternative(unnecessary updates and API calls during reconciliation caused by false positives due to non-deterministic list ordering from etcd).

[aali309]

/retest

[aali309]

I have tried to test 1-123_validate_list_order_comparison_test locally with make run and also by installing the operator via OLM and its passing consistently. I have no idea why its failing on Github workflow. Is this expected?
Please take a look @svghadi @anandf. I believe the reason is this code here

	if realMajorVersion < startMajorVersion || (realMajorVersion == startMajorVersion && realMinorVersion < startMinorVersion) {
		// Skip plugin reconciliation if real OCP version is less than dynamic plugin start OCP version
		return reconcile.Result{}, nil
	} else {
		return r.reconcilePlugin(instance, request)
	}
}

So skipping the test if the OCP version running the test is lower than startMajorVersion (4.14 in GitHub CI)

CI=prow ./bin/ginkgo -v --trace --timeout 30m ./test/openshift/e2e/ginkgo/sequential -- -ginkgo.focus="1-123_validate_list_order_comparison"
Running Suite: Sequential Suite - /Users/atali/Desktop/workStation/Repos/gitops-operator/test/openshift/e2e/ginkgo/sequential
=============================================================================================================================
Random Seed: 1769749615

Will run 2 of 76 specs
------------------------------
[BeforeSuite] 
/Users/atali/Desktop/workStation/Repos/gitops-operator/test/openshift/e2e/ginkgo/sequential/suite_test.go:42
[BeforeSuite] PASSED [0.000 seconds]
------------------------------
SSSSSSSSSSSSSSSSSSSSSSSSS
------------------------------
GitOps Operator Sequential E2E Tests 1-123_validate_list_order_comparison Should not trigger updates when only list order differs
/Users/atali/Desktop/workStation/Repos/gitops-operator/test/openshift/e2e/ginkgo/sequential/1-123_validate_list_order_comparison_test.go:56
  Object exists in ExistByName: openshift-gitops
  ArgoCD status is now Available
  STEP: capturing initial state before simulating etcd order change @ 01/30/26 00:07:21.274
  STEP: simulating etcd returning lists in different order @ 01/30/26 00:07:21.314
  STEP: triggering reconciliation @ 01/30/26 00:07:21.46
  STEP: verifying no unnecessary update was triggered @ 01/30/26 00:07:55.947
• [46.860 seconds]
------------------------------
GitOps Operator Sequential E2E Tests 1-123_validate_list_order_comparison Should trigger updates when actual changes are made
/Users/atali/Desktop/workStation/Repos/gitops-operator/test/openshift/e2e/ginkgo/sequential/1-123_validate_list_order_comparison_test.go:147
  Object exists in ExistByName: openshift-gitops
  ArgoCD status is now Available
  STEP: capturing initial state before making actual change @ 01/30/26 00:08:07.897
  STEP: making an actual change to the deployment @ 01/30/26 00:08:07.937
  STEP: triggering reconciliation @ 01/30/26 00:08:23.173
  STEP: verifying operator corrected the image back to the expected image @ 01/30/26 00:08:52.756
• [56.802 seconds]
------------------------------
SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS
------------------------------
[AfterSuite] 
/Users/atali/Desktop/workStation/Repos/gitops-operator/test/openshift/e2e/ginkgo/sequential/suite_test.go:47
[AfterSuite] PASSED [0.000 seconds]
------------------------------

Ran 2 of 76 Specs in 103.663 seconds
SUCCESS! -- 2 Passed | 0 Failed | 0 Pending | 74 Skipped
PASS

Ginkgo ran 1 suite in 1m57.154506792s
Test Suite Passed

and

LOCAL_RUN=true ./bin/ginkgo -v -focus "1-123_validate_list_order_comparison" -r ./test/openshift/e2e/ginkgo/sequential/
Running Suite: Sequential Suite - /Users/atali/desktop/workStation/Repos/gitops-operator/test/openshift/e2e/ginkgo/sequential
=============================================================================================================================
Random Seed: 1769533065

Will run 2 of 76 specs
------------------------------
[BeforeSuite] 
/Users/atali/desktop/workStation/Repos/gitops-operator/test/openshift/e2e/ginkgo/sequential/suite_test.go:42
[BeforeSuite] PASSED [0.000 seconds]
------------------------------
SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS
------------------------------
GitOps Operator Sequential E2E Tests 1-123_validate_list_order_comparison Should not trigger updates when only list order differs
/Users/atali/desktop/workStation/Repos/gitops-operator/test/openshift/e2e/ginkgo/sequential/1-123_validate_list_order_comparison_test.go:56
  Object exists in ExistByName: openshift-gitops
  ArgoCD status is now Available
  STEP: verifying ArgoCD is available @ 01/27/26 11:58:06.684
  ArgoCD status is now Available
  STEP: capturing initial state before simulating etcd order change @ 01/27/26 11:58:17.246
  STEP: simulating etcd returning lists in different order @ 01/27/26 11:58:17.297
  STEP: triggering reconciliation @ 01/27/26 11:58:17.419
  STEP: verifying no unnecessary update was triggered @ 01/27/26 11:58:51.93
• [56.596 seconds]
------------------------------
GitOps Operator Sequential E2E Tests 1-123_validate_list_order_comparison Should trigger updates when actual changes are made
/Users/atali/desktop/workStation/Repos/gitops-operator/test/openshift/e2e/ginkgo/sequential/1-123_validate_list_order_comparison_test.go:149
  Object exists in ExistByName: openshift-gitops
  ArgoCD status is now Available
  ArgoCD status is now Available
  STEP: capturing initial Generation before making actual change @ 01/27/26 11:59:13.657
  STEP: making an actual change to the deployment @ 01/27/26 11:59:13.697
  STEP: triggering reconciliation @ 01/27/26 11:59:13.932
  STEP: verifying update was triggered @ 01/27/26 11:59:53.424
• [61.486 seconds]
------------------------------
SSSSSSSSSSSSSSS
------------------------------
[AfterSuite] 
/Users/atali/desktop/workStation/Repos/gitops-operator/test/openshift/e2e/ginkgo/sequential/suite_test.go:47
[AfterSuite] PASSED [0.000 seconds]
------------------------------

Ran 2 of 76 Specs in 118.083 seconds
SUCCESS! -- 2 Passed | 0 Failed | 0 Pending | 74 Skipped
PASS

[aali309]

/test v4.14-kuttl-parallel

[aali309]

/retest

[aali309]

/retest

[openshift-ci]

@aali309: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/v4.14-kuttl-parallel	6f0da8a	link	true	/test v4.14-kuttl-parallel
ci/prow/v4.14-kuttl-sequential	6f0da8a	link	true	/test v4.14-kuttl-sequential

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.