Experimental conformance promotion to standard #2868

mlavacca · 2024-03-13T15:22:55Z

What type of PR is this?

/area conformance

What this PR does / why we need it:

Which issue(s) this PR fixes:

Does this PR introduce a user-facing change?:

The Experimental Conformance test suite has been promoted to standard. Furthermore, the `ConformanceReport` API has been promoted to `v1`.

mlavacca · 2024-03-13T15:23:41Z

/cc @shaneutt @robscott @youngnick

mlavacca · 2024-03-13T15:23:48Z

/ok-to-test

conformance/conformance_test.go

conformance/utils/flags/flags.go

conformance/utils/suite/conformance-test.go

dprotaso · 2024-03-13T16:00:20Z

conformance/utils/suite/profiles.go

@@ -67,7 +67,9 @@ var (
 			SupportReferenceGrant,
 			SupportHTTPRoute,
 		),
-		ExtendedFeatures: HTTPRouteExtendedFeatures,
+		ExtendedFeatures: sets.New[SupportedFeature]().
+			Insert(GatewayExtendedFeatures.UnsortedList()...).


Adding Gateway Extended Features to HTTP profile seems off

I could imagine there being an Extended feature that has nothing to do with HTTPRoute

The current Gateway API extended features are GatewayPort8080 and GatewayStaticAddresses. I think both are totally relevant to the HTTP profile and should be included here.

Woud gateway infrastructure be something that we want in the HTTP profile?

#2845

I think so, WDYT?

I think @dprotaso's concerns are actually the same as mine - how do we avoid exponential growth in terms of profiles here?

For example, you could imagine multiples of the following:

mesh+gateway * each unique combination of route types (by protocol, layer, or all) * core + extended

One option would be to start exclusively with profiles for core support of GA resources in v1.1 (Gateway, HTTPRoute, and GRPCRoute) - I think Gateway is inseparable from GatewayClass so it's probably fine to merge those together. Maybe to take that one step further, does that just become another way to express the SupportHTTPRoute supported feature?

To be clear, I'm very excited about the updated conformance reporting which I think is what the vast majority of this PR is about, I'm just worried that if we're not careful with the profiles terminology here, we'll end up with an unmaintainable number of profile names alongside an ever-expanding set of feature names that could become overly confusing when layered on top of each other.

As a reminder, the idea of profiles is that they're just convenient groupings of features, e.g.:

Individual Tests |_ SupportedFeatures |_ ConformanceProfiles

The profiles are meant to cover the most common combinations of feature groupings, providing high level composability for implementations to subscribe to. This has been documented as the intent for over a year, and the currently expected profiles are common ones such as "HTTP", "Mesh", "L4", e.t.c.

I wonder if trying to change this now (especially as we were just about to go standard with it) would just be treating a symptom of a problem no where near the source of the problem, like how pain in us humans can radiate so we feel it somewhere it is not: perhaps Gateway APIs scope is way too large, and we're trying to cover too many things as a single project. So perhaps it is not "there's potential for too many conformance profiles" but instead "there's too many things we're trying to support as a single sub-project".

Hey @shaneutt thanks for the clarification. I'm good with this then. I also missed the fact that extended features need to be activated with the SupportedFeatures flag when running the test

Yes, gotcha. There are a couple other things to keep in mind here too:

Network Policy is (without any complaints thus far) using this as well and hoping to standardize on it once we do, so if we make any last minute large changes to the GEP this effects them as well.

Unlike the APIs or regular Go code, this doesn't have direct user implications. So if we do find in time we need a revision because we start experiencing some real world problems, it's not as hard to make some backwards incompatible changes.

Since this PR is simply the last PR implementing the existing GEP, because we're 2 weeks before release, and given the above context I think we should move forward as is and judge this based on it's adherence to the current GEP, and if needed make grander changes to the GEP itself separately.

robscott

Thanks @mlavacca!

conformance/apis/v1beta1/doc.go

conformance/utils/flags/flags.go

conformance/utils/suite/conformance.go

conformance/utils/suite/conformance_test.go

conformance/utils/suite/suite.go

conformance/apis/v1beta1/profilereport.go

shaneutt

Besides the comments already provided, looks good 👍

/approve

youngnick · 2024-03-28T00:15:33Z

LGTM once the merge conflict is fixed.

conformance/utils/suite/conformance.go

k8s-ci-robot · 2024-03-28T08:22:53Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dprotaso, mlavacca, shaneutt, tao12345666333

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:

~~OWNERS~~ [shaneutt]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

conformance/conformance_test.go

Signed-off-by: Mattia Lavacca <lavacca.mattia@gmail.com>

Signed-off-by: Mattia Lavacca <lavacca.mattia@gmail.com> Co-authored-by: Dave Protasowski <dprotaso@gmail.com>

Signed-off-by: Mattia Lavacca <lavacca.mattia@gmail.com>

dprotaso · 2024-03-28T13:26:13Z

/lgtm

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. area/conformance cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 13, 2024

k8s-ci-robot requested review from keithmattix and michaelbeaumont March 13, 2024 15:23

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 13, 2024

k8s-ci-robot requested review from robscott, shaneutt and youngnick March 13, 2024 15:23

k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Mar 13, 2024

mlavacca force-pushed the promote-conformance-suite branch from aa4e92b to 68233ac Compare March 13, 2024 15:53

k8s-ci-robot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 13, 2024

dprotaso reviewed Mar 13, 2024

View reviewed changes

mlavacca force-pushed the promote-conformance-suite branch 2 times, most recently from 5e62da6 to 4ea1e10 Compare March 13, 2024 17:01

mlavacca mentioned this pull request Mar 14, 2024

Fix conformance test flags are exported to other libraries that use the conformance test suite #2843

Merged

robscott reviewed Mar 15, 2024

View reviewed changes

mlavacca force-pushed the promote-conformance-suite branch from 458eabb to 39fb7f1 Compare March 16, 2024 09:11

shaneutt approved these changes Mar 17, 2024

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 17, 2024

mlavacca force-pushed the promote-conformance-suite branch from 39fb7f1 to 266a224 Compare March 18, 2024 14:47

dprotaso approved these changes Mar 23, 2024

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 26, 2024

shaneutt requested a review from robscott March 27, 2024 11:03

tao12345666333 approved these changes Mar 28, 2024

View reviewed changes

conformance/utils/suite/conformance.go Show resolved Hide resolved

tao12345666333 reviewed Mar 28, 2024

View reviewed changes

conformance/conformance_test.go Show resolved Hide resolved

mlavacca and others added 5 commits March 28, 2024 14:12

chore: experimental conformance promotion

665866b

Signed-off-by: Mattia Lavacca <lavacca.mattia@gmail.com>

file renamed

b71285c

Signed-off-by: Mattia Lavacca <lavacca.mattia@gmail.com>

Allow single test to be run

377189a

Signed-off-by: Mattia Lavacca <lavacca.mattia@gmail.com> Co-authored-by: Dave Protasowski <dprotaso@gmail.com>

address review comments

6c60b6a

Signed-off-by: Mattia Lavacca <lavacca.mattia@gmail.com>

chore: minor changes in the CLI

1fd59a4

Signed-off-by: Mattia Lavacca <lavacca.mattia@gmail.com>

mlavacca force-pushed the promote-conformance-suite branch from 266a224 to 1fd59a4 Compare March 28, 2024 13:14

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 28, 2024

k8s-ci-robot assigned dprotaso Mar 28, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 28, 2024

k8s-ci-robot merged commit 204abaf into kubernetes-sigs:main Mar 28, 2024
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Experimental conformance promotion to standard #2868

Experimental conformance promotion to standard #2868

mlavacca commented Mar 13, 2024 •

edited

Loading

mlavacca commented Mar 13, 2024

mlavacca commented Mar 13, 2024

dprotaso Mar 13, 2024

mlavacca Mar 13, 2024

dprotaso Mar 13, 2024

mlavacca Mar 13, 2024

robscott Mar 19, 2024

shaneutt Mar 21, 2024

dprotaso Mar 22, 2024

shaneutt Mar 23, 2024

robscott left a comment

shaneutt left a comment

youngnick commented Mar 28, 2024

k8s-ci-robot commented Mar 28, 2024

dprotaso commented Mar 28, 2024

Experimental conformance promotion to standard #2868

Experimental conformance promotion to standard #2868

Conversation

mlavacca commented Mar 13, 2024 • edited Loading

mlavacca commented Mar 13, 2024

mlavacca commented Mar 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robscott left a comment

Choose a reason for hiding this comment

shaneutt left a comment

Choose a reason for hiding this comment

youngnick commented Mar 28, 2024

k8s-ci-robot commented Mar 28, 2024

dprotaso commented Mar 28, 2024

mlavacca commented Mar 13, 2024 •

edited

Loading