-
Notifications
You must be signed in to change notification settings - Fork 880
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: argo rollout compatibility with emissary and edge stack v2.0 #1330
feat: argo rollout compatibility with emissary and edge stack v2.0 #1330
Conversation
Signed-off-by: Alix Cook <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #1330 +/- ##
==========================================
- Coverage 81.40% 81.27% -0.14%
==========================================
Files 106 107 +1
Lines 9778 9824 +46
==========================================
+ Hits 7960 7984 +24
- Misses 1278 1297 +19
- Partials 540 543 +3
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to update the ambassador docs to say Emissary / Edge Stack v2.0 is supported with the --ambassador-api-version x.getambassador.io/v3alpha1
(with an "available since v1.1" caveat).
utils/defaults/defaults.go
Outdated
DefaultAmbassadorAPIGroup = "getambassador.io" | ||
DefaultAmbassadorVersion = "v2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it seems this change now accepts API version with or without the group in it. e.g.:
rollouts-controller --ambassador-api-version v2
rollouts-controller --ambassador-api-version getambassador.io/v2
rollouts-controller --ambassador-api-version x.getambassador.io/v3alpha1
Now that we support supplying a different group, can we change DefaultAmbassadorVersion
to be getambassador.io/v2
so that when users see the --help
of the rollout controller, they know that this is possible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense.
Signed-off-by: Alix Cook <[email protected]>
be2c41b
to
1a0922e
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Signed-off-by: Alix Cook [email protected]
Allow argo rollouts to work with emissary and edge stack v2, recently announced as developer preview.
See our release notes for more information.
I'd like to merge this feature, and then update our docs and the argo-docs in a subsequent PR so that I don't have to worry about releasing these things in lockstep.
Checklist:
"fix(controller): Updates such and such. Fixes #1234"
.