-
Notifications
You must be signed in to change notification settings - Fork 905
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: support additional services for blue-green. fixes #451 #2603
base: master
Are you sure you want to change the base?
Conversation
7a138a8
to
471e40d
Compare
Kudos, SonarCloud Quality Gate passed!
|
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 There are more use cases reported in #451. The problem with just using a list is it's not explicit which |
@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). As far as source of truth, i would make the first in the list the SoT, and document that. |
This PR is stale because it has been open 90 days with no activity. |
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.
My goal with the |
Kudos, SonarCloud Quality Gate passed!
|
Codecov ReportPatch coverage:
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
☔ View full report in Codecov by Sentry. |
This PR is stale because it has been open 90 days with no activity. |
Hello @d3adb5 any updates on this? :) We are in need of the same feature and wonder if we can help contribute and push for the feature. |
Hey @fdfzcq, I haven't had the time to look back at this, and since I've moved on from the project where the use case came up it took a hit in my list of priorities. Last I remember I was a little confused about how to add E2E tests for this and still wondering if I'm going about the type change the right way. I have a rough week or two ahead of me at work right now, but afterwards I'll be able to commit myself to this. If you want, you can always take over this implementation. Want to explain your use case here or elsewhere so I get a clearer picture of what you guys need? |
@d3adb5 Our use case is kind of similar to the one you have. We want to use Argo Rollouts in a situation where we route traffic to the same deployment through multiple service resources. We need multiple service resources, because each service defines a different protocol (UDP and TCP) for the same L4 pass-through LB (GKE docs). |
So, it has been a while since there were changes made here. @d3adb5 if you are still on this, please let me/us know if/how I/we can support. This is a feature that I would really love to see in Argo, and that would solve a big pain point for me. If you cannot find time to contribute to this any more, that is fair. Life moves on and so do we. Just let us know, so that the rest of the community can take over. And thanks for creating this PR in the first place. |
0e94be5
to
01b48af
Compare
Sorry for the delay getting back to everyone. @eduardOrthopy, it would help me quite a bit to understand how to properly run and write E2E tests for this. For some reason E2E tests with a kind or k3d cluster on my machine are always failing intermittently. I'm new to Go, too, so any feedback on what I have here would be well appreciated. I rebased on |
Published E2E Test Results 4 files 4 suites 3h 9m 4s ⏱️ For more details on these failures, see this check. Results for commit 4cd5ad0. ♻️ This comment has been updated with latest results. |
Published Unit Test Results2 296 tests 2 296 ✅ 2m 59s ⏱️ Results for commit 4cd5ad0. ♻️ This comment has been updated with latest results. |
Add support for additional preview and active services for the BlueGreen strategy. Signed-off-by: d3adb5 <me@d3adb5.net>
01b48af
to
4cd5ad0
Compare
|
Add support for additional preview and active services for the BlueGreen
strategy.
Checklist:
"fix(controller): Updates such and such. Fixes #1234"
.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
andadditionalActiveServices
. When populated withservice 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.