Skip to content
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

Conformance tests for static Gateway addresses #2412

Conversation

shaneutt
Copy link
Member

@shaneutt shaneutt commented Sep 18, 2023

What type of PR is this?

/area conformance
/kind test

What this PR does / why we need it:

Going into GA we have an extended field for supplying static addresses for Gateways, but no conformance tests for that feature. This patch adds the test, while also tweaking some of the related condition reasons (see the explanation in the CHANGELOG.md).

Which issue(s) this PR fixes:

Resolves #2035

Does this PR introduce a user-facing change?:

NONE

Warning: The way we're injecting the address pools into the test is bad. I don't like it. However, I think at this point with GA in the next few weeks, trying to make this better would require some kind of fundamental change to the test suite which we don't have time for. If you have any great ideas for alternatives that are simple, that would be lovely. If your PR comments are going to be something like "man we really need proper templating in the test suite" please note that this will probably have to wait for later.

@shaneutt shaneutt added area/conformance release-blocker MUST be completed to complete the milestone labels Sep 18, 2023
@shaneutt shaneutt added this to the v1.0.0 milestone Sep 18, 2023
@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. kind/test labels Sep 18, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 18, 2023
@shaneutt shaneutt added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Sep 18, 2023
@shaneutt shaneutt force-pushed the shaneutt/conformance-tests-for-gateway-addresses branch 2 times, most recently from 0452c86 to 3cf8046 Compare September 19, 2023 18:57
@shaneutt shaneutt added the tide/merge-method-rebase Denotes a PR that should be rebased by tide when it merges. label Sep 20, 2023
@shaneutt shaneutt force-pushed the shaneutt/conformance-tests-for-gateway-addresses branch from 3cf8046 to d147653 Compare September 20, 2023 16:29
@kubernetes-sigs kubernetes-sigs deleted a comment from k8s-ci-robot Sep 20, 2023
@shaneutt shaneutt force-pushed the shaneutt/conformance-tests-for-gateway-addresses branch 3 times, most recently from 247f14a to f9ea74b Compare September 20, 2023 20:11
apis/v1beta1/gateway_types.go Outdated Show resolved Hide resolved
conformance/tests/gateway-static-addresses.go Outdated Show resolved Hide resolved
conformance/utils/kubernetes/apply.go Show resolved Hide resolved
conformance/utils/kubernetes/apply.go Outdated Show resolved Hide resolved
conformance/utils/kubernetes/helpers.go Outdated Show resolved Hide resolved
@shaneutt shaneutt force-pushed the shaneutt/conformance-tests-for-gateway-addresses branch from 4c5425f to da94172 Compare September 25, 2023 20:15
@shaneutt shaneutt marked this pull request as ready for review September 25, 2023 21:57
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 25, 2023
@shaneutt shaneutt force-pushed the shaneutt/conformance-tests-for-gateway-addresses branch from da94172 to 4b14d70 Compare September 25, 2023 22:16
Signed-off-by: Shane Utt <shaneutt@linux.com>
… suite

Signed-off-by: Shane Utt <shaneutt@linux.com>
@shaneutt shaneutt force-pushed the shaneutt/conformance-tests-for-gateway-addresses branch 2 times, most recently from 9a4938c to bf2d448 Compare September 26, 2023 22:37
@shaneutt shaneutt force-pushed the shaneutt/conformance-tests-for-gateway-addresses branch 2 times, most recently from 0ba6401 to 8310caa Compare September 26, 2023 23:05
@robscott robscott self-requested a review September 26, 2023 23:11
@shaneutt shaneutt force-pushed the shaneutt/conformance-tests-for-gateway-addresses branch 4 times, most recently from a3be3c8 to 8628ab3 Compare September 27, 2023 15:48
Signed-off-by: Shane Utt <shaneutt@linux.com>
@shaneutt shaneutt force-pushed the shaneutt/conformance-tests-for-gateway-addresses branch from 8628ab3 to 277e3f8 Compare September 27, 2023 16:54
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 27, 2023
@shaneutt
Copy link
Member Author

This is now passing in Blixt, albeit I had to add several hacks to deal with the fact that our test suite is currently expecting if you do Gateway you must support HTTP (filed an issue for this in #2403). Ready for review.

@shaneutt shaneutt marked this pull request as ready for review September 27, 2023 17:03
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 27, 2023
@robscott robscott added tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. and removed tide/merge-method-rebase Denotes a PR that should be rebased by tide when it merges. labels Oct 9, 2023
@robscott
Copy link
Member

robscott commented Oct 9, 2023

Thanks @shaneutt!

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 9, 2023
@k8s-ci-robot k8s-ci-robot merged commit 335be6a into kubernetes-sigs:main Oct 9, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/conformance cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/test lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-blocker MUST be completed to complete the milestone release-note-none Denotes a PR that doesn't merit a release note. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Conformance tests for addresses
4 participants