fix: rs conflict with fallback to patch

[zachaller]

When updating a replicaset if we get a conflict with the replicaset, we fallback to a patch. This prevents us from getting into a loop of conflicts and preventing scaling from occurring. This is more of a temporary hot fix until a larger refactor can happen around how rollouts manages replicasets.

[github-actions]

Go Published Test Results

2 162 tests   2 162 ✅  2m 54s ⏱️
  119 suites      0 💤
    1 files        0 ❌

Results for commit ec9d9b0.

♻️ This comment has been updated with latest results.

[github-actions]

E2E Tests Published Test Results

  4 files    4 suites   3h 24m 40s ⏱️
110 tests  99 ✅  6 💤  5 ❌
452 runs  416 ✅ 24 💤 12 ❌

For more details on these failures, see this check.

Results for commit ec9d9b0.

♻️ This comment has been updated with latest results.

[codecov]

Codecov Report

Attention: Patch coverage is 65.71429% with 24 lines in your changes missing coverage. Please review.

Project coverage is 80.26%. Comparing base (8405f2e) to head (e52400d).
Report is 114 commits behind head on master.

❗ Current head e52400d differs from pull request most recent head ec9d9b0

Please upload reports for the commit ec9d9b0 to get more accurate results.

Files	Patch %	Lines
rollout/controller.go	75.00%	7 Missing and 7 partials ⚠️
rollout/sync.go	42.85%	4 Missing ⚠️
utils/controller/controller.go	0.00%	2 Missing and 1 partial ⚠️
rollout/ephemeralmetadata.go	33.33%	2 Missing ⚠️
rollout/canary.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3559      +/-   ##
==========================================
- Coverage   81.83%   80.26%   -1.58%     
==========================================
  Files         135      156      +21     
  Lines       20688    17970    -2718     
==========================================
- Hits        16931    14424    -2507     
+ Misses       2883     2634     -249     
- Partials      874      912      +38     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[leoluz]

Please check my comments

[leoluz]

we should probably add several test cases to validate the new updateReplicaSetFallbackToPatch method

[zachaller]

There are two test that cover most the happy paths to lock in the behavior on the two conflict points here. I don't think it's worth going down a lot of the error paths like incorrect json marshal's etc.

[zachaller]

codecovs percentages don't always make sense to me, here is the coverage of that function https://app.codecov.io/gh/argoproj/argo-rollouts/pull/3559?src=pr&el=tree&filepath=rollout%2Fcontroller.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=argoproj#6b8a01c38fada35656503847a6f4a00d-R951

[leoluz]

It seems that the coverage is comparing the wrong base from the link above.

[leoluz]

It seems that the coverage is comparing the wrong base from the link above.

[sonarcloud]

Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
9.5% Duplication on New Code

See analysis details on SonarCloud

[leoluz]

LGTM

[SleepyBrett]

hey @zachaller It seems like this has had me chasing my tail for a few weeks now in relation to: #3569

We've been using the notification engine separately to create sqs events back to the deploying software on rollout progression and other factors. We've been seeing, as the ticket describes, rollouts that seem to hang for a very long time once they've completed their steps and then eventually they will suddenly resolve themselves, or we can go into the rollout and make some manual status block changes to unblock them. It wasn't every rollout, but it was a significant percentage of them.

I'm wondering if the updates on the rollout to the notified.notifications.argoproj.io: annotations that then automagically propagate down to the replicaset are what is causing our issue. We were seeing a lot of this kind of looping:

argo-rollouts-6bff4d767b-fv26j argo-rollouts time="2024-06-13T23:57:49Z" level=info msg="Reconciliation completed" generation=4 namespace=fastlane-synthetic-health resourceVersion=3497127176 rollout=happy-path-canary-microservice time_ms=15.57145
argo-rollouts-6bff4d767b-fv26j argo-rollouts time="2024-06-13T23:57:49Z" level=error msg="rollout syncHandler error: failed to getAllReplicaSetsAndSyncRevision in isScalingEvent: error updating replicaset revision: Operation cannot be fulfilled on replicasets.apps \"happy-path-canary-microservice-7767565bcf\": the object has been modified; please apply your changes to the latest version and try again" namespace=fastlane-synthetic-health rollout=happy-path-canary-microservice
argo-rollouts-6bff4d767b-fv26j argo-rollouts time="2024-06-13T23:57:49Z" level=info msg="rollout syncHandler queue retries: 6128 : key \"fastlane-synthetic-health/happy-path-canary-microservice\"" namespace=fastlane-synthetic-health rollout=happy-path-canary-microservice
argo-rollouts-6bff4d767b-fv26j argo-rollouts time="2024-06-13T23:57:49Z" level=error msg="failed to getAllReplicaSetsAndSyncRevision in isScalingEvent: error updating replicaset revision: Operation cannot be fulfilled on replicasets.apps \"happy-path-canary-microservice-7767565bcf\": the object has been modified; please apply your changes to the latest version and try again\n" error="<nil>"
argo-rollouts-6bff4d767b-fv26j argo-rollouts time="2024-06-13T23:57:59Z" level=info msg="Started syncing rollout" generation=4 namespace=fastlane-synthetic-health resourceVersion=3497127176 rollout=happy-path-canary-microservice
argo-rollouts-6bff4d767b-fv26j argo-rollouts time="2024-06-13T23:57:59Z" level=info msg="Updating replica set 'happy-path-canary-microservice-7767565bcf' revision from 11406 to 11408" namespace=fastlane-synthetic-health rollout=happy-path-canary-microservice
argo-rollouts-6bff4d767b-fv26j argo-rollouts time="2024-06-13T23:57:59Z" level=error msg="Error: updating replicaset revision" error="Operation cannot be fulfilled on replicasets.apps \"happy-path-canary-microservice-7767565bcf\": the object has been modified; please apply your changes to the latest version and try again" namespace=fastlane-synthetic-health rollout=happy-path-canary-microservice
argo-rollouts-6bff4d767b-fv26j argo-rollouts time="2024-06-13T23:57:59Z" level=error msg="roCtx.reconcile err failed to getAllReplicaSetsAndSyncRevision in isScalingEvent: error updating replicaset revision: Operation cannot be fulfilled on replicasets.apps \"happy-path-canary-microservice-7767565bcf\": the object has been modified; please apply your changes to the latest version and try again" generation=4 namespace=fastlane-synthetic-health resourceVersion=3497127176 rollout=happy-path-canary-microservice
argo-rollouts-6bff4d767b-fv26j argo-rollouts time="2024-06-13T23:57:59Z" level=info msg="Reconciliation completed" generation=4 namespace=fastlane-synthetic-health resourceVersion=3497127176 rollout=happy-path-canary-microservice time_ms=14.442395
argo

I'm now excited to try out 1.7.* once it's had a few days for other people to find any bugs for me ;) maybe 1.7.1.

Though it looks like maybe this got cherry picked into 1.6, i guess i can wait for a 1.6.7 as well.

Do you think my hypothesis makes sense?