-
Notifications
You must be signed in to change notification settings - Fork 468
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
Initial Mesh (GAMMA) conformance tests #1878
Initial Mesh (GAMMA) conformance tests #1878
Conversation
Skipping CI for Draft Pull Request. |
595bc40
to
46cd068
Compare
46cd068
to
ef75c8a
Compare
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.
I'm OK with this based on the machinery and the decisions from discussions here that led to the follow up PR #1894.
I haven't done significant scrutiny of the tests or the test helpers personally, I've invited several mesh-focused community members to review and hopefully test this out themselves. We should give them a little bit of time to check this out before we merge.
/hold
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.
I personally don't have any reservations about this, but I'll let someone else be the final LGTM
@@ -0,0 +1,70 @@ | |||
/* | |||
Copyright 2022 The Kubernetes Authors. |
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.
A lot of these files have old copyright dates, are they just being copied from Istio codebase?
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.
copied from other files in this repo
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.
Great! LGTM
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: howardjohn, keithmattix, michaelbeaumont, shaneutt The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Alright, enough people have seen this now so we can /unhold |
Thanks all. Need 1 lgtm to get this in |
Looks good from here, FTR. |
Thanks @howardjohn! /lgtm |
* With kubernetes-sigs#1878 the existing flag `EnableAllSupportedFeatures` used in implementations to run conformance tests pertaining to all Gateway/Ingress features will now also run Mesh conformance tests which might not be the intent. * rm `EnableAllSupportedFeatures` and ask Ingress implementations to set `SupportedFeatures` to `AllGatewayFeatures` to achieve the same intent as before. Mesh implementations that also support Gateway features can set `SupportedFeatures` to `AllFeatures`. Signed-off-by: Arko Dasgupta <[email protected]>
What type of PR is this?
/area conformance
What this PR does / why we need it:
This PR introduces the basic scaffolding for Mesh testing. Mesh testing is fundamentally different from ingress testing, as we need to send requests from pod-to-pod instead of "go test" to "reachable IP address".
This PR introduces the basic scaffolding to run this test. With this PR, you can run a test against GAMMA tests.
This will:
Architecturally, we are deploying
gcr.io/istio-testing/app:latest
to each app. This is a client+server pair. We then on-demandexec
into each pod to runclient http://some-request
, and parse the result and validate against expectations.Likely desired improvements:
Opening this PR to get early feedback before moving further.
I think we could also merge this as-is, as it doesn't impact non-Mesh cases and will allow further iteration.
Which issue(s) this PR fixes:
Fixes #1340
Does this PR introduce a user-facing change?: