NE-1208: Gateway API E2E Testing #1023
Conversation
openshift-ci-robot
added
the
jira/valid-reference
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
@candita: This pull request references NE-1208 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 story to target the "4.16.0" version, but no target version was set. In response to this:
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. |
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test verify |
|
/test e2e-aws-operator |
candita
force-pushed
the
NE-1208-E2E-Testing-GWAPI
branch
3 times, most recently
from
b8b5ff7
to
9deaecf
Compare
|
/test verify |
|
@candita: This pull request references NE-1208 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 story to target the "4.16.0" version, but no target version was set. In response to this:
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. |
|
/test e2e-aws-operator |
candita
force-pushed
the
NE-1208-E2E-Testing-GWAPI
branch
from
9deaecf
to
42f74d6
Compare
|
/retest |
candita
force-pushed
the
NE-1208-E2E-Testing-GWAPI
branch
from
42f74d6
to
73e6cb1
Compare
|
/retest-required |
candita
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
/assign @rfredette |
|
Seems to be a network issue that made e2e-aws-operator test fail:
/test e2e-aws-operator |
|
CI may depend on #1037 |
|
/retest-required |
|
Maybe transient:
/test e2e-aws-operator |
|
This is failing again? See if it happens repeatedly.
/test e2e-aws-operator |
|
@candita: all tests passed! 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/test-infra repository. I understand the commands that are listed here. |
| testGatewayAPIResources(t) | ||
| testGatewayAPIObjects(t) | ||
| testGatewayAPIIstioInstallation(t) |
There was a problem hiding this comment.
| testGatewayAPIResources(t) | |
| testGatewayAPIObjects(t) | |
| testGatewayAPIIstioInstallation(t) | |
| t.Run("testGatewayAPIResources", testGatewayAPIResources) | |
| t.Run("testGatewayAPIObjects", testGatewayAPIObjects) | |
| t.Run("testGatewayAPIIstioInstallation", testGatewayAPIIstioInstallation) |
If testGatewayAPIResources, testGatewayAPIObjects, and testGatewayAPIIstioInstallation are intended to be treated as separate tests, using t.Run to run them as subtests would help separate their output and success/failure conditions
| t.Fatalf("failed to find expected CatalogSource %s", expectedCatalogSourceName) | ||
| } | ||
| if err := assertOSSMOperator(t); err != nil { | ||
| t.Fatalf("failed to find expected Istio operator") |
There was a problem hiding this comment.
Use t.Fatal() when there are no printf-style arguments
| t.Fatalf("failed to find expected Istio operator") | ||
| } | ||
| if err := assertIstiodProxy(t); err != nil { | ||
| t.Fatalf("failed to find expected Istiod proxy") |
There was a problem hiding this comment.
Same as above; use t.Fatal()
| } | ||
| // TODO - In OSSM 3.x the configuration object to check will be different. | ||
| if err := assertSMCP(t); err != nil { | ||
| t.Fatalf("failed to find expected SMCP: %s", err.Error()) |
There was a problem hiding this comment.
| t.Fatalf("failed to find expected SMCP: %s", err.Error()) | |
| t.Fatalf("failed to find expected SMCP: %v", err) |
This is a nit, but it brings it in line with the way the majority of error messages are printed in these tests.
| ok, gatewayClass := assertCanCreateGatewayClass(t, gatewayclass.OpenShiftDefaultGatewayClassName, gatewayclass.OpenShiftGatewayClassControllerName) | ||
| if !ok { | ||
| return fmt.Errorf("feature gate was enabled, but gateway class object could not be created") | ||
| } |
There was a problem hiding this comment.
| ok, gatewayClass := assertCanCreateGatewayClass(t, gatewayclass.OpenShiftDefaultGatewayClassName, gatewayclass.OpenShiftGatewayClassControllerName) | |
| if !ok { | |
| return fmt.Errorf("feature gate was enabled, but gateway class object could not be created") | |
| } | |
| gatewayClass, err := createGatewayClass(t, gatewayclass.OpenShiftDefaultGatewayClassName, gatewayclass.OpenShiftGatewayClassControllerName) | |
| if err != nil { | |
| return fmt.Errorf("feature gate was enabled, but gateway class object could not be created: %v", err) | |
| } |
I'm not sure the assertCanCreate... functions are useful. Wouldn't this essentially be the same?
| if err = kclient.Get(context.TODO(), name, gateway); err == nil { | ||
| return gateway, nil | ||
| } |
There was a problem hiding this comment.
This returns gateway, nil if the get is successful, but if it fails (which I'm sure is very unlikely), it still ends up returning gateway, nil
| if err = kclient.Get(context.TODO(), name, gateway); err == nil { | |
| return gateway, nil | |
| } | |
| if err = kclient.Get(context.TODO(), name, gateway); err == nil { | |
| return gateway, nil | |
| } else { | |
| return nil, err | |
| } |
| } | ||
|
|
||
| // getClusterVersion returns the ClusterVersion if found. If one is not found, it returns an error. | ||
| func getClusterVersion() (*configv1.ClusterVersion, error) { |
There was a problem hiding this comment.
This doesn't seem to be gateway API specific. I think it'd be great to put it in util_test.go for better visibility.
| } | ||
|
|
||
| // assertSubscription checks if the Subscription of the given name exists and returns an error if not. | ||
| func assertSubscription(t *testing.T, namespace, subName string) error { |
There was a problem hiding this comment.
Same as above; this could go in util_test.go
| ok, testGateway = assertCanCreateGateway(t, gatewayClass, testGatewayName, openshiftIngressNamespace, domain) | ||
| if !ok { | ||
| return fmt.Errorf("feature gate was enabled, but gateway object could not be created") | ||
| } |
There was a problem hiding this comment.
| ok, testGateway = assertCanCreateGateway(t, gatewayClass, testGatewayName, openshiftIngressNamespace, domain) | |
| if !ok { | |
| return fmt.Errorf("feature gate was enabled, but gateway object could not be created") | |
| } | |
| testGateway, err = createGateway(gatewayClass, testGatewayName, openshiftIngressNamespace, domain) | |
| if err != nil { | |
| return fmt.Errorf("feature gate was enabled, but gateway object could not be created: %w", err) | |
| } |
Same as above
| ok, _ = assertCanCreateHttpRoute(t, ns.Name, "test-httproute", openshiftIngressNamespace, default_routename, testGatewayName+"-"+gatewayclass.OpenShiftDefaultGatewayClassName, testGateway) | ||
| if !ok { | ||
| return fmt.Errorf("feature gate was enabled, but http route object could not be created") | ||
| // The http route is automatically deleted because its namespace is automatically deleted. | ||
| } |
There was a problem hiding this comment.
| ok, _ = assertCanCreateHttpRoute(t, ns.Name, "test-httproute", openshiftIngressNamespace, default_routename, testGatewayName+"-"+gatewayclass.OpenShiftDefaultGatewayClassName, testGateway) | |
| if !ok { | |
| return fmt.Errorf("feature gate was enabled, but http route object could not be created") | |
| // The http route is automatically deleted because its namespace is automatically deleted. | |
| } | |
| _, err = createHttpRoute(ns.Name, "test-httproute", openshiftIngressNamespace, default_routename, testGatewayName+"-"+gatewayclass.OpenShiftDefaultGatewayClassName, testGateway) | |
| if err != nil { | |
| return fmt.Errorf("feature gate was enabled, but http route object could not be created: %w", err) | |
| // The http route is automatically deleted because its namespace is automatically deleted. | |
| } |
Same as above
|
It seems like this test relies on OSSM 2.5 and the Gateway API v0.6.2 CRDs in #1018. When I ran it against a cluster on the latest nightly, the tests failed waiting for /hold until #1018 is merged. |
openshift-ci
bot
added
the
do-not-merge/hold
Add three new tests and a few helper functions for testing GatewayAPI.
Currently, the FeatureGate for GatewayAPI must be enabled first.
Working on a way to fake that for testing because it roll nodes and the router pods.