NE-1260: Add Makefile target to run Gateway API conformance tests #1176

lihongan · 2025-01-08T13:54:02Z

This PR adds new Makefile target and enables the upstream gateway-api conformance test.

Once this get merged, the follow up openshift/release#60217 will add optional pre-submits job to run conformance test.

openshift-ci-robot · 2025-01-08T13:55:32Z

@lihongan: This pull request references NE-1260 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the epic to target the "4.19.0" version, but no target version was set.

In response to this:

This PR adds new Makefile target and enables the upstream gateway-api conformance test.

Once this get merged, the follow up openshift/release#60217 will add optional pre-submits job to run conformance test.

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 openshift-eng/jira-lifecycle-plugin repository.

lihongan · 2025-01-08T14:10:19Z

cc @Miciah @candita @shaneutt

candita · 2025-01-08T16:36:12Z

/assign @grzpiotrowski
/assign @alebedev87

lihongan · 2025-01-09T10:57:02Z

/test e2e-aws-operator

grzpiotrowski · 2025-01-09T12:35:18Z

hack/gatewayapi-conformance.sh

+
+oc wait --for=condition=Accepted=true gatewayclass/gateway-conformance --timeout=300s
+
+echo "All gatewayclass ststus:"


Typo in status

grzpiotrowski · 2025-01-09T12:35:57Z

hack/gatewayapi-conformance.sh

+fi
+echo "Gateway API CRD bundle-version: ${BUNDLE_VERSION}"
+
+echo "Creat GatewayClass gateway-conformance"


Typo in Create

grzpiotrowski · 2025-01-09T12:42:42Z

hack/gatewayapi-conformance.sh

+sed -i "s/MaxTimeToConsistency:              30/MaxTimeToConsistency:              90/g" conformance/utils/config/timeout.go
+
+echo "Start Gateway API Conformance Testing"
+go test ./conformance -v -timeout 0 -run TestConformance -args --supported-features=Gateway,HTTPRoute


I was wondering should we also include the ReferenceGrant in the arg for testing, besides the Gateway and HTTPRoute?

Yes, we should include the ReferenceGrant in the arg.

candita · 2025-01-10T19:15:44Z

/retest

lihongan · 2025-01-13T08:52:43Z

/test e2e-gcp-ovn

lihongan · 2025-01-13T13:03:45Z

Step e2e-gcp-ovn-ipi-install-install failed after 54m23s

lihongan · 2025-01-16T09:27:41Z

/retest

hack/gatewayapi-conformance.sh

alebedev87 · 2025-01-21T08:48:08Z

hack/gatewayapi-conformance.sh

+cd gateway-api
+go mod vendor
+
+# modify default timeout of "MaxTimeToConsistency" to make tests pass on AWS


Can you please elaborate what slows the test down on AWS specifically?

Yes, the root cause is the DNS propagation of AWS ELB needs about extra 60s before the actual convergence process. The default MaxTimeToConsistency is 30s so we need to increase it to 90s here.
I ever proposed a change in PR kubernetes-sigs/gateway-api#3542 and it was closed, seems many implementations use kind cluster and metallb to run conformance and the LB can be available in short time, so they don't think the change is necessary.
But when we run the test on Cloud platforms (including Azure/GCP although they use IP instead of alias) it needs more time until the LB is reachable.

Added one more line comment to explain this.

alebedev87 · 2025-01-21T08:51:54Z

hack/gatewayapi-conformance.sh

+sed -i "s/MaxTimeToConsistency:              30/MaxTimeToConsistency:              90/g" conformance/utils/config/timeout.go
+
+echo "Start Gateway API Conformance Testing"
+go test ./conformance -v -timeout 0 -run TestConformance -args --supported-features=Gateway,HTTPRoute,ReferenceGrant


I think that we should limit the test run time to a specific value even if this value is in hours. We should have an opinion about how long the test should take.

Agree, generally the test run time is about 500s so we could limit it to 10m at present, but it might be changed in future.

lihongan · 2025-01-22T09:46:14Z

hack/gatewayapi-conformance.sh

+# because the AWS ELB needs extra ~60s for DNS propagation
+sed -i "s/MaxTimeToConsistency:              30/MaxTimeToConsistency:              90/g" conformance/utils/config/timeout.go
+
+SUPPORTED_FEATURES="Gateway,HTTPRoute,ReferenceGrant,GatewayPort8080,HTTPRouteQueryParamMatching,HTTPRouteMethodMatching,HTTPRouteResponseHeaderModification,HTTPRoutePortRedirect,HTTPRouteSchemeRedirect,HTTPRoutePathRedirect,HTTPRouteHostRewrite,HTTPRoutePathRewrite,HTTPRouteRequestMirror,HTTPRouteRequestMultipleMirrors,HTTPRouteBackendProtocolH2C,HTTPRouteBackendProtocolWebSocket"


I added more supported extended features in latest commit, because if we specify "Gateway,HTTPRoute,ReferenceGrant" then just core tests will be covered (29 tests). After adding supported extended features we could cover 44 tests and they are:

--- PASS: TestConformance/GatewayInvalidRouteKind (3.48s) --- PASS: TestConformance/GatewayInvalidTLSConfiguration (5.49s) --- PASS: TestConformance/GatewayModifyListeners (31.00s) --- PASS: TestConformance/GatewayObservedGenerationBump (28.96s) --- PASS: TestConformance/GatewaySecretInvalidReferenceGrant (9.01s) --- PASS: TestConformance/GatewaySecretMissingReferenceGrant (1.54s) --- PASS: TestConformance/GatewaySecretReferenceGrantAllInNamespace (2.59s) --- PASS: TestConformance/GatewaySecretReferenceGrantSpecific (2.33s) --- PASS: TestConformance/GatewayWithAttachedRoutes (8.94s) --- PASS: TestConformance/GatewayWithAttachedRoutesWithPort8080 (2.43s) --- PASS: TestConformance/GatewayClassObservedGenerationBump (2.98s) --- PASS: TestConformance/HTTPRouteBackendProtocolH2C (2.87s) --- PASS: TestConformance/HTTPRouteBackendProtocolWebSocket (3.34s) --- PASS: TestConformance/HTTPRouteCrossNamespace (3.48s) --- PASS: TestConformance/HTTPExactPathMatching (2.70s) --- PASS: TestConformance/HTTPRouteHeaderMatching (5.12s) --- PASS: TestConformance/HTTPRouteHostnameIntersection (103.51s) --- PASS: TestConformance/HTTPRouteInvalidNonExistentBackendRef (3.44s) --- PASS: TestConformance/HTTPRouteInvalidBackendRefUnknownKind (3.22s) --- PASS: TestConformance/HTTPRouteInvalidCrossNamespaceBackendRef (3.07s) --- PASS: TestConformance/HTTPRouteInvalidCrossNamespaceParentRef (3.19s) --- PASS: TestConformance/HTTPRouteInvalidParentRefNotMatchingSectionName (2.18s) --- PASS: TestConformance/HTTPRouteInvalidReferenceGrant (11.32s) --- PASS: TestConformance/HTTPRouteListenerHostnameMatching (21.70s) --- PASS: TestConformance/HTTPRouteMatchingAcrossRoutes (4.48s) --- PASS: TestConformance/HTTPRouteMatching (2.72s) --- PASS: TestConformance/HTTPRouteMethodMatching (2.71s) --- PASS: TestConformance/HTTPRouteObservedGenerationBump (3.97s) --- PASS: TestConformance/HTTPRoutePartiallyInvalidViaInvalidReferenceGrant (4.51s) --- PASS: TestConformance/HTTPRoutePathMatchOrder (2.56s) --- PASS: TestConformance/HTTPRouteQueryParamMatching (5.02s) --- PASS: TestConformance/HTTPRouteRedirectHostAndStatus (2.86s) --- PASS: TestConformance/HTTPRouteRedirectPath (2.68s) --- PASS: TestConformance/HTTPRouteRedirectPortAndScheme (16.16s) --- PASS: TestConformance/HTTPRouteRedirectPort (2.71s) --- PASS: TestConformance/HTTPRouteRedirectScheme (2.87s) --- PASS: TestConformance/HTTPRouteReferenceGrant (16.48s) --- PASS: TestConformance/HTTPRouteRequestHeaderModifier (2.92s) --- PASS: TestConformance/HTTPRouteRequestMirror (2.18s) --- PASS: TestConformance/HTTPRouteRequestMultipleMirrors (2.14s) --- PASS: TestConformance/HTTPRouteResponseHeaderModifier (2.47s) --- PASS: TestConformance/HTTPRouteRewriteHost (2.66s) --- PASS: TestConformance/HTTPRouteRewritePath (2.76s) --- PASS: TestConformance/HTTPRouteSimpleSameNamespace (3.09s)

Can we use profiles (e.g. HTTPConformanceProfile) with conformance-profiles flag?

There's an example for envoygateway that might help: https://github.com/envoyproxy/gateway/blob/main/tools/make/kube.mk#L265

Yes, I also checked the code from other implementations. Because in v1.0.0 there are two sets flags, in this flags.go there is no conformance-profiles flag, but it is defined in the file experimental_flags.go. I believe those experimental flags are used to generate the test report (see flag report-output). And those experimental flags are merged into one file flags.go since v1.1.0.
Currently with v1.0.0 I'd like to just use the standard flags, and I'm planning to add more flags accordingly when we supporting v1.1.x and we could generate the report automatically.
For the example report we could refer to: https://github.com/kubernetes-sigs/gateway-api/tree/main/conformance/reports (use main branch so we could see all versions)

Anyway, if we prefer to use experimental flags/profiles with v.1.0.0 then I can update the go test ... to include them.
And I'm not very sure the flags of implementation info, they might be

implementation: contact: - github.com/openshift/cluster-ingress-operator organization: OpenShift project: ingress-gateway-api url: github.com/openshift/cluster-ingress-operator version: v4.19.0

but please help correct them if I'm wrong. Thanks.

BTW, in v1.0.0 three experimental profiles HTTP, TLS, MESH are defined and they are changed to 5 profiles in v1.1.0 as GATEWAY-HTTP, GATEWAY-TLS, GATEWAY-GRPC, MESH-HTTP, MESH-GRPC.

Because in v1.0.0 there are two sets flags, in this flags.go there is no conformance-profiles flag, but it is defined in the file experimental_flags.go.

I can see conformance-profiles flag on release-1.0 branch:

$ git branch --show-current release-1.0 $ git log -1 commit 093f6538409c071e906856bd1ce4072204fb3f08 (HEAD -> release-1.0, origin/release-1.0) Merge: 829284a0 1ed97491 Author: Kubernetes Prow Robot <[email protected]> Date: Mon Feb 5 10:03:20 2024 -0800 Merge pull request #2753 from k8s-infra-cherrypick-robot/cherry-pick-2736-to-release-1.0 [release-1.0] Update experimental channel kustomization with backendtlspolicy $ go test ./conformance -v -timeout 10m -run TestConformance -args -h | grep -A1 conformance-profiles -conformance-profiles string Comma-separated list of the conformance profiles to run

Yes, other experimental flags also can be seen in the help on release-1.0 branch, e.g

$ go test ./conformance -v -timeout 10m -run TestConformance -args -h | grep Implementation -B1 -organization string Implementation's Organization to issue conformance to -project string Implementation's project to issue conformance to -- -url string Implementation's url to issue conformance to -version string Implementation's version to issue conformance to

If just adding --conformance-profiles here I cannot see any change to the test result, it is likely to be ignored.
I still believe it is used with --report-output flag.

With release-1.0 branch we could run below command and can generate report

$ go test ./conformance -v -timeout 10m -run TestExperimentalConformance -report-output=./report.yaml -conformance-profiles="HTTP" -organization=a -project=b -url=c -version=d <...snip...> experimental_conformance_test.go:149: Conformance report: apiVersion: gateway.networking.k8s.io/v1alpha1 date: "2025-01-24T22:11:42+08:00" gatewayAPIVersion: TODO implementation: contact: - "" organization: a project: b url: c version: d kind: ConformanceReport profiles: - core: result: success statistics: Failed: 0 Passed: 29 Skipped: 0 summary: "" name: HTTP ...

I think the code explain it.

// if some conformance profiles have been set, run the experimental conformance suite... // ...otherwise run the standard conformance suite.

alebedev87 · 2025-01-22T10:32:34Z

/lgtm
/approve
/hold

@grzpiotrowski : unhold if you are fine with the changes.

openshift-ci · 2025-01-22T10:32:55Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alebedev87

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~~ [alebedev87]

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

lihongan · 2025-01-23T10:28:14Z

Squashed commits into one to keep it clean
cc @alebedev87 @grzpiotrowski

grzpiotrowski

The changes look good to me. Thank you.

/unhold

alebedev87 · 2025-01-23T12:47:28Z

hack/gatewayapi-conformance.sh

+CLONE_DIR=$(mktemp -d)
+cd "${CLONE_DIR}"


Suggested change

CLONE_DIR=$(mktemp -d)

cd "${CLONE_DIR}"

CLONE_DIR=$(mktemp -d)

echo "clone directory: ${CLONE_DIR}"

cd "${CLONE_DIR}"

Otherwise it will be more difficult to find where the repository was cloned.

Or even like this, to let the runner specify a pre-cloned repository too:

Suggested change

CLONE_DIR=$(mktemp -d)

cd "${CLONE_DIR}"

if [ -z "${CLONE_DIR}" ]; then

CLONE_DIR=$(mktemp -d)

fi

echo "clone directory: ${CLONE_DIR}"

cd "${CLONE_DIR}"

Yes, it looks good. And what do you think about specifying a template ? e.g

CLONE_DIR=$(mktemp -d -t gwapi-conformance-XXXXX)

And what do you think about specifying a template ?

I think it's a good idea.

alebedev87 · 2025-01-23T13:01:24Z

hack/gatewayapi-conformance.sh

+# because the AWS ELB needs extra ~60s for DNS propagation
+sed -i "s/MaxTimeToConsistency:              30/MaxTimeToConsistency:              90/g" conformance/utils/config/timeout.go
+
+SUPPORTED_FEATURES="Gateway,HTTPRoute,ReferenceGrant,GatewayPort8080,HTTPRouteQueryParamMatching,HTTPRouteMethodMatching,HTTPRouteResponseHeaderModification,HTTPRoutePortRedirect,HTTPRouteSchemeRedirect,HTTPRoutePathRedirect,HTTPRouteHostRewrite,HTTPRoutePathRewrite,HTTPRouteRequestMirror,HTTPRouteRequestMultipleMirrors,HTTPRouteBackendProtocolH2C,HTTPRouteBackendProtocolWebSocket"


Can we use profiles (e.g. HTTPConformanceProfile) with conformance-profiles flag?

alebedev87 · 2025-01-24T17:40:31Z

/lgtm

alebedev87 · 2025-01-24T18:02:06Z

/label acknowledge-critical-fixes-only

New Makefile command which doesn't impact the OCP payload in any way.

alebedev87 · 2025-01-24T18:02:16Z

/retest-required

openshift-ci-robot · 2025-01-24T23:19:55Z

/retest-required

Remaining retests: 0 against base HEAD cba3e44 and 2 for PR HEAD f6f1dbe in total

openshift-ci · 2025-01-25T01:27:52Z

@lihongan: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/okd-scos-e2e-aws-ovn	`f6f1dbe`	link	false	`/test okd-scos-e2e-aws-ovn`

Full PR test history. Your PR dashboard.

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-sigs/prow repository. I understand the commands that are listed here.

openshift-ci-robot · 2025-01-27T10:05:54Z

/retest-required

Remaining retests: 0 against base HEAD cba3e44 and 2 for PR HEAD f6f1dbe in total

openshift-bot · 2025-01-27T16:20:26Z

[ART PR BUILD NOTIFIER]

Distgit: ose-cluster-ingress-operator
This PR has been included in build ose-cluster-ingress-operator-container-v4.19.0-202501271516.p0.g8ba9339.assembly.stream.el9.
All builds following this will include this PR.

openshift-ci bot requested review from alebedev87 and Thealisyed January 8, 2025 13:54

lihongan changed the title ~~Add Makefile target to run Gateway API conformance tests~~ NE-1260: Add Makefile target to run Gateway API conformance tests Jan 8, 2025

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jan 8, 2025

openshift-ci bot assigned alebedev87 and grzpiotrowski Jan 8, 2025

lihongan mentioned this pull request Jan 9, 2025

adding Gateway API conformance tests openshift/release#60217

Open

grzpiotrowski reviewed Jan 9, 2025

View reviewed changes

alebedev87 reviewed Jan 21, 2025

View reviewed changes

lihongan commented Jan 22, 2025

View reviewed changes

openshift-ci bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. labels Jan 22, 2025

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 22, 2025

Add Makefile target to run k8s Gateway API conformance test

f6f1dbe

lihongan force-pushed the gwapi-conformance branch from c64b7f5 to f6f1dbe Compare January 23, 2025 10:21

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 23, 2025

grzpiotrowski approved these changes Jan 23, 2025

View reviewed changes

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 23, 2025

alebedev87 reviewed Jan 23, 2025

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 24, 2025

openshift-ci bot added the acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. label Jan 24, 2025

openshift-merge-bot bot merged commit 8ba9339 into openshift:master Jan 27, 2025
18 of 19 checks passed


		oc wait --for=condition=Accepted=true gatewayclass/gateway-conformance --timeout=300s

		echo "All gatewayclass ststus:"

NE-1260: Add Makefile target to run Gateway API conformance tests #1176

NE-1260: Add Makefile target to run Gateway API conformance tests #1176

Conversation

lihongan commented Jan 8, 2025

openshift-ci-robot commented Jan 8, 2025 • edited by openshift-ci bot Loading

lihongan commented Jan 8, 2025

candita commented Jan 8, 2025

lihongan commented Jan 9, 2025

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

candita commented Jan 10, 2025

lihongan commented Jan 13, 2025

lihongan commented Jan 13, 2025

lihongan commented Jan 16, 2025

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

alebedev87 Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lihongan Jan 24, 2025 • edited Loading

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

lihongan Jan 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alebedev87 commented Jan 22, 2025

openshift-ci bot commented Jan 22, 2025

lihongan commented Jan 23, 2025

grzpiotrowski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alebedev87 Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

lihongan Jan 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alebedev87 Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

alebedev87 commented Jan 24, 2025

alebedev87 commented Jan 24, 2025

alebedev87 commented Jan 24, 2025

openshift-ci-robot commented Jan 24, 2025

openshift-ci bot commented Jan 25, 2025 • edited Loading

openshift-ci-robot commented Jan 27, 2025

openshift-bot commented Jan 27, 2025

openshift-ci-robot commented Jan 8, 2025 •

edited by openshift-ci bot

Loading

alebedev87 Jan 23, 2025 •

edited

Loading

lihongan Jan 24, 2025 •

edited

Loading

lihongan Jan 24, 2025 •

edited

Loading

alebedev87 Jan 23, 2025 •

edited

Loading

lihongan Jan 24, 2025 •

edited

Loading

alebedev87 Jan 23, 2025 •

edited

Loading

openshift-ci bot commented Jan 25, 2025 •

edited

Loading