feat: support additional services for blue-green. fixes #451

[d3adb5]

Add support for additional preview and active services for the BlueGreen
strategy.

Checklist:

• Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this is a chore.
• The title of the PR is (a) conventional with a list of types and scopes found here, (b) states what changed, and (c) suffixes the related issues number. E.g. "fix(controller): Updates such and such. Fixes #1234".
• I've signed my commits with DCO
• I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
• My builds are green. Try syncing with master if they are not.
• My organization is added to USERS.md.

This is my first contribution to Argo Rollouts, and I feel like #451 isn't
really closed if this doesn't encompass the Canary strategy. I want to work on
it, but first I wanted to write E2E tests for this feature. I'm rather new to
Go, so I'm not sure if I'm doing this right. Still combing through the available
tests to see if I can find a good example to follow.

Support for multiple services is added as a couple new optional fields called
additionalPreviewServices and additionalActiveServices. When populated with
service names, these lists will cause the services in them to be updated as
though they were the preview service or the active service.

Help with this would be much appreciated.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
0.0% Duplication

[github-actions]

Go Published Test Results

1 984 tests   1 984 ✔️  2m 37s ⏱️
   118 suites         0 💤
       1 files           0 ❌

Results for commit 0e94be5.

♻️ This comment has been updated with latest results.

[github-actions]

E2E Tests Published Test Results

    4 files      4 suites   3h 34m 19s ⏱️
  96 tests   80 ✔️   5 💤 11 ❌
400 runs  364 ✔️ 20 💤 16 ❌

For more details on these failures, see this check.

Results for commit 0e94be5.

♻️ This comment has been updated with latest results.

[meeech]

Would it make sense to add activeServices/previewServices, and just make them mutually exclusive?

[d3adb5]

Would it make sense to add activeServices/previewServices, and just make them mutually exclusive?

If I understand your suggestion correctly, I don't think so. My original use case was I had two Services for a specific service: one attached to a Network Load Balancer, redirecting UDP traffic, and a simple ClusterIP referenced by Ingress. There were two active and two preview Services.

There are more use cases reported in #451. The problem with just using a list is it's not explicit which Service might be the "origin of the truth" in the case of recovering from a crash. Not that that behavior is really made explicit here.

[meeech]

@d3adb5 Sorry, i realize re-reading it that I wasn't clear there. I didn't mean to make them mutually exclusive to each other (i know you need active and preview).
i meant to make the plural and singular (eg: activeService and activeServices) mutually exclusive.
Which I now realize is similar to the suggestion @huikang #451 (comment) made on #451 - though i don't know if they meant to suggest making them mutually exclusive (basically, can only use 1 - the singular, or the plural) with the plural being preferred going forward - since it works with 1 or more services.

As far as source of truth, i would make the first in the list the SoT, and document that.

[github-actions]

This PR is stale because it has been open 90 days with no activity.

[d3adb5]

I realize it's been a while since I touched this PR. Life's been busy, now there are even merge conflicts. I'll get to it sooner or later.

As far as source of truth, i would make the first in the list the SoT, and document that.

My goal with the additional* parameters was to avoid having to document which one will be the SoT. My hopes were reading the spec would make it immediately clear which one would be used in case of recovery and which ones would become secondary updates, rather than let the user guess which one would be the SoT and have to read the documentation to confirm their assumptions.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
11.4% Duplication

[codecov]

Codecov Report

Patch coverage: 39.39% and project coverage change: -0.07 ⚠️

Comparison is base (6ac1533) 81.68% compared to head (0e94be5) 81.61%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2603      +/-   ##
==========================================
- Coverage   81.68%   81.61%   -0.07%     
==========================================
  Files         133      133              
  Lines       20178    20211      +33     
==========================================
+ Hits        16483    16496      +13     
- Misses       2843     2857      +14     
- Partials      852      858       +6     

Impacted Files	Coverage Δ
rollout/bluegreen.go	65.94% <21.42%> (-2.36%)	⬇️
rollout/service.go	76.82% <52.63%> (-2.15%)	⬇️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[github-actions]

This PR is stale because it has been open 90 days with no activity.