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.

Sign up for GitHub

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

Jump to bottom

Fixes to allow the mesh feature to be tested, including alone #1949

Merged
merged 5 commits into from
Apr 24, 2023
Merged

Fixes to allow the mesh feature to be tested, including alone #1949

merged 5 commits into from
Apr 24, 2023

Conversation

frankbu
Copy link
Contributor

@frankbu frankbu commented Apr 18, 2023 •
edited
Loading

What type of PR is this?
/kind test
/area conformance

What this PR does / why we need it:
Some improvements to enable the conformance suite to run mesh tests and to allow the core gateway feature to be excluded from testing. This includes initializing RESTClient, needed to exec into pods for mesh tests.

With these changes, the mesh tests can be run by passing the following args:

"--supported-features", "Mesh", "--exempt-features", "Gateway"

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

NONE

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 18, 2023
@k8s-ci-robot
Copy link
Contributor

Hi @frankbu. 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.

sunjayBhatia
sunjayBhatia reviewed Apr 18, 2023
View reviewed changes
Copy link
Member

@sunjayBhatia sunjayBhatia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is a little conceptual overlap with #1907, something to keep track of in case theres any issues as these PRs move along

conformance/conformance_test.go Show resolved Hide resolved
conformance/utils/flags/flags.go Show resolved Hide resolved
conformance/conformance_test.go Outdated
GatewayClassName: *flags.GatewayClassName,
Debug: *flags.ShowDebug,
CleanupBaseResources: *flags.CleanupBaseResources,
SupportedFeatures: supportedFeatures,
EnableAllSupportedFeatures: *flags.EnableAllSupportedFeatures,
})
for feature := range exemptFeatures {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe this should be done in suite.New rather than here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering about that myself. I suppose it would be cleaner. I'll move it.

@shaneutt
Copy link
Member

I'll need to take a look at this one soon, please don't merge without me.

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 19, 2023
@frankbu frankbu changed the title Allow mesh feature to be tested alone, exclude gateway tests Fixes to allow the mesh feature to be tested, including alone Apr 19, 2023
shaneutt
shaneutt approved these changes Apr 21, 2023
View reviewed changes
Copy link
Member

@shaneutt shaneutt left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for trying the mesh tests and airing out some of these problems, we appreciate it.

It looks good to me:

/approve

But would like a couple more of the conformance reviewers to double check too:

/cc @mlavacca @arkodg

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 21, 2023
arkodg
arkodg approved these changes Apr 21, 2023
View reviewed changes
Copy link
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks

@robscott
Copy link
Member

Thanks @frankbu! This LGTM, but do you mind also updating the docs to reflect this change? (https://gateway-api.sigs.k8s.io/concepts/conformance/#mesh-tests)

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 23, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: arkodg, frankbu, 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

@frankbu
Copy link
Contributor Author

frankbu commented Apr 24, 2023

@robscott docs updated, PTAL

@robscott
Copy link
Member

Thanks @frankbu!

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 24, 2023
@shaneutt shaneutt removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 24, 2023
@k8s-ci-robot k8s-ci-robot merged commit 387072c into kubernetes-sigs:main Apr 24, 2023
@shaneutt shaneutt added this to the v0.7.1 milestone Apr 24, 2023
@frankbu frankbu mentioned this pull request Sep 15, 2023
Closed
9 tasks
@frankbu frankbu deleted the conform-features branch September 25, 2023 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sunjayBhatia sunjayBhatia sunjayBhatia left review comments

@shaneutt shaneutt shaneutt approved these changes

@arkodg arkodg arkodg approved these changes

@keithmattix keithmattix Awaiting requested review from keithmattix

@mlavacca mlavacca Awaiting requested review from mlavacca

Assignees

@robscott robscott

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. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
No open projects
Gateway API: The Road to GA
Status: Done
Milestone
v0.7.1
Development

Successfully merging this pull request may close these issues.
6 participants
@frankbu @k8s-ci-robot @shaneutt @robscott @sunjayBhatia @arkodg