Add agentgateway to CI & rename dataplane flag - #1598
Add agentgateway to CI & rename dataplane flag#1598Keith Mattix II (keithmattix) wants to merge 5 commits into
Conversation
Signed-off-by: Keith Mattix II <keithmattix2@gmail.com>
Signed-off-by: Keith Mattix II <keithmattix2@gmail.com>
Signed-off-by: Keith Mattix II <keithmattix2@gmail.com>
Signed-off-by: Keith Mattix II <keithmattix2@gmail.com>
|
Current failure is a postgres timeout. I think CI just needs to be re-kicked |
Eitan Yarmush (EItanya)
left a comment
There was a problem hiding this comment.
🤖 Relocated with AI assistance; the original inline comment wording is preserved.
Moved from my review on #1628, which is stacked on this PR. The three inline comments and the ingress-contract portion of that review belong here:
I have some clarifying questions about what the actual ingress contract is since there are definitely dataplane specific checks in the tests
The benchmark-specific questions remain on #1628. The CI matrix suggestion is already addressed here.
| t.Fatalf("creating statusz client: %v", err) | ||
| var statusz *e2e.StatuszClient | ||
| if os.Getenv("E2E_DATAPLANE") != "agentgateway" { | ||
| statusz, err = e2e.NewStatuszClient(ctx) |
There was a problem hiding this comment.
Moved from my comment on #1628; this feedback belongs to #1598.
Can we use a more generic solution here?
There was a problem hiding this comment.
My plan is to just get both data planes to a working state in CI then iterate on a contract that both data planes implement. New contracts need would need to be persisted there instead of one off. So agree with the comment but will do it in a follow up
| t.Fatalf("timed out waiting for the parking gauge to satisfy the condition (last active=%d)", last) | ||
| } | ||
|
|
||
| func agentGatewayParkingCount(ctx context.Context) (int, bool) { |
There was a problem hiding this comment.
Moved from my comment on #1628; this feedback belongs to #1598.
Same question about unifying the data gathering mechanism here
Keith’s reply on the original thread points to this PR’s description and its goal of defining a shared proxy contract.
There was a problem hiding this comment.
Yeah +1 to my comment above; we'll define it in a shared contract
Signed-off-by: Keith Mattix II <keithmattix2@gmail.com>
Run agentgateway data plane tests as a part of substrate CI (non-blocking to start so we can confirm it's not flaky). Also, change the
--atenet-routerflag to--dataplaneto make it clearer that the flag controls ingress and egress.I've run the e2es locally across gVisor and microVM plus the MITM variants for both. The only skip we do for agentgateway is
TestIngressProtocolDowngradebecause 1. the behavior its testing only exists on the non-CONNECT atunnel ingress path and agentgateway only sends CONNECT to atunnel and 2. I'm not sure that we want this to be a part of the contract that substrate is bound by (e.g. do we really want to commit to atunnel always parsing HTTP?).My goal with getting both dataplanes into CI is to start taking steps to codify the proxy (router + egress PEP) contract for substrate. The telemetry they emit, atunnel expectations, etc. are all important contracts to explicitly call out so that they don't become too coupled to a single dataplane implementation.