feat: ping pong support for istio #3371
Conversation
Signed-off-by: Zach Aller <[email protected]>
Signed-off-by: Zach Aller <[email protected]>
Go Published Test Results2 144 tests 2 144 ✅ 2m 51s ⏱️ Results for commit f9bc1d9. ♻️ This comment has been updated with latest results. |
E2E Tests Published Test Results 4 files 4 suites 3h 44m 56s ⏱️ For more details on these failures, see this check. Results for commit 1546fc7. ♻️ This comment has been updated with latest results. |
Signed-off-by: Zach Aller <[email protected]>
Signed-off-by: Zach Aller <[email protected]>
Signed-off-by: Zach Aller <[email protected]>
Signed-off-by: Zach Aller <[email protected]>
|
…pong-istio Signed-off-by: Zach Aller <[email protected]>
Signed-off-by: Zach Aller <[email protected]>
There was a problem hiding this comment.
LGTM, but is there a minimum istio version that needs to be deployed for the feature to work? I think the doc should probably be updated because currently the ping-pong doc is in the ALB page
| @@ -284,6 +284,10 @@ func TestValidateRolloutStrategyCanary(t *testing.T) { | |||
| validRo.Spec.Strategy.Canary.Steps[0].SetWeight = pointer.Int32(10) | |||
| validRo.Spec.Strategy.Canary.CanaryService = "" | |||
| validRo.Spec.Strategy.Canary.StableService = "" | |||
| validRo.Spec.Strategy.Canary.PingPong = &v1alpha1.PingPongSpec{ | |||
There was a problem hiding this comment.
Shouldn't we add a new testcase for istio?
There was a problem hiding this comment.
added new test instead of piggy backing off one
| @@ -263,6 +298,72 @@ spec: | |||
| host: canary | |||
| weight: 0` | |||
|
|
|||
| const regularMixedVsvcPingPong = `apiVersion: networking.istio.io/v1alpha3 | |||
There was a problem hiding this comment.
I prefer using go:embed directive to accomplish the same goal of having a large file instantiating its contents in a variable. This approach is cleaner/easier to maintain and doesn't require any additional tooling.
See the following PR as an example:
https://github.com/argoproj/argo-cd/pull/17404/files
There was a problem hiding this comment.
Yea, I kept it the way it is because it is consistent with all the other tests for istio
There was a problem hiding this comment.
sure.. but we have to start somewhere ¯\_(ツ)_/¯
Signed-off-by: Zach Aller <[email protected]>
Good callout docs added. |
Signed-off-by: Zach Aller <[email protected]>
Signed-off-by: Zach Aller <[email protected]>
|
Checklist:
"fix(controller): Updates such and such. Fixes #1234".