gateway-api
gateway-api copied to clipboard
Add conformance report for Application Gateway for Containers
What type of PR is this? /area conformance
What this PR does / why we need it: Adds conformance report and updates implementation page for Azure Application Gateway for Containers v1.0.0
Which issue(s) this PR fixes: N/A
Does this PR introduce a user-facing change?: None
Hi @snehachhabria. Thanks for your PR.
I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test label.
I understand the commands that are listed here.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
/ok-to-test
As FYI, the tests skipped in the core implementation are because they use port 8080 and Application Gateway For Containers currently supports only port 80 and 443.
@robscott Are the tests in https://github.com/kubernetes-sigs/gateway-api/blob/264546815534e45a777c7ae48ecd3c493d7aaa86/conformance/tests/gateway-with-attached-routes-with-port-8080.yaml something we could consider scoping differently so they could be skipped more granularly without skipping the rest of that (relatively large and important) test?
@mikemorris @snehachhabria I think if you mark SupportGatewayPort8080 as an unsupported feature when generating this test report, any tests dependent on that should automatically be skipped:
https://github.com/kubernetes-sigs/gateway-api/blob/264546815534e45a777c7ae48ecd3c493d7aaa86/conformance/tests/gateway-with-attached-routes.go#L138 https://github.com/kubernetes-sigs/gateway-api/blob/264546815534e45a777c7ae48ecd3c493d7aaa86/conformance/utils/suite/suite.go#L114
If that doesn't work, we're probably missing something in our conformance test runner.
@mikemorris @snehachhabria I think if you mark
SupportGatewayPort8080as an unsupported feature when generating this test report, any tests dependent on that should automatically be skipped:https://github.com/kubernetes-sigs/gateway-api/blob/264546815534e45a777c7ae48ecd3c493d7aaa86/conformance/tests/gateway-with-attached-routes.go#L138
https://github.com/kubernetes-sigs/gateway-api/blob/264546815534e45a777c7ae48ecd3c493d7aaa86/conformance/utils/suite/suite.go#L114
If that doesn't work, we're probably missing something in our conformance test runner.
thanks for the suggestion @robscott. I was able to do this and regenerated our conformance report. PTAL
/retest
@robscott , unfortunately we have an implementation specific annotation that is required for the gateway objects in order for our controller to process it. An example for the annotation can be found here: https://learn.microsoft.com/en-us/azure/application-gateway/for-containers/how-to-traffic-splitting-gateway-api?tabs=alb-managed#deploy-t[…]esources
Internally in our codebase, we have modified the kube client used by the conformance to inject this annotation on the fly for all gateway objects created by the conformance and that's how we run the conformance tests in our repo. Since our project is a managed offering from a cloud provider, we have not publicly exposed anything. There might be a few solutions such as creating a cli tool for our project and adding a command to run conformance tests, but it would take time. We would like to think a bit on what the best way to expose reproducing the conformance report would be.
In the meantime would it be possible to merge the current PR? and I can take an actionable item on our end to come up with a way to reproduce the report generation that would work for both Gateway API and our project.
I think that, in the spirit of encouraging folks to submit conformance sooner rather than later, and accept partial conformance reports, we should accept this as-is. This means that we have some time to sort out the details of how we handle problems with conformance that lie outside the actual suite (which #3036 is discussing), while the Azure folks look at how to pass the ObservedGenerationBump GatewayClass test.
Thanks @snehachhabria!
/approve
Will defer to @youngnick or @mlavacca for final LGTM.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: robscott, snehachhabria
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~conformance/OWNERS~~ [robscott]
- ~~site-src/OWNERS~~ [robscott]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
Agreed, let's get this in.
/lgtm
/release-note-none