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

NE-1492: Enable Gateway API feature gate in CI and run E2E tests #48873

Merged
merged 1 commit into from
Apr 11, 2024

Conversation

candita
Copy link
Contributor

@candita candita commented Feb 15, 2024

Add a new test which enables the GatewayAPI feature gate and then runs the E2E tests.

@openshift-ci-robot openshift-ci-robot added the rehearsals-ack Signifies that rehearsal jobs have been acknowledged label Feb 15, 2024
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 15, 2024
@candita candita marked this pull request as draft February 15, 2024 23:22
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 15, 2024
@candita
Copy link
Contributor Author

candita commented Feb 15, 2024

/retest-required

@openshift-ci-robot openshift-ci-robot removed the rehearsals-ack Signifies that rehearsal jobs have been acknowledged label Feb 15, 2024
@candita candita marked this pull request as ready for review February 16, 2024 00:01
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 16, 2024
@candita
Copy link
Contributor Author

candita commented Feb 16, 2024

/pj-rehearse

@openshift-ci openshift-ci bot requested a review from miheer February 16, 2024 00:04
@candita
Copy link
Contributor Author

candita commented Feb 16, 2024

The new test ran successfully, albeit slowly, during pj-rehearse:

[36mINFO�[0m[2024-02-16T00:49:24Z] Running step e2e-aws-gatewayapi-test.
[36mINFO�[0m[2024-02-16T01:37:17Z] Step e2e-aws-gatewayapi-test succeeded after 47m53s.
[36mINFO�[0m[2024-02-16T01:37:17Z] Step phase test succeeded after 47m53s.

@candita
Copy link
Contributor Author

candita commented Feb 16, 2024

ci-operator-config-metadata failure shows something is out of sync. But if I run the suggestion listed at:
https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_release/48873/pull-ci-openshift-release-master-ci-operator-config-metadata/1758280584397852672, then I lose the steps and env variables I added to the new test.

@candita candita marked this pull request as draft February 16, 2024 02:37
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 16, 2024
@candita
Copy link
Contributor Author

candita commented Feb 16, 2024

/test ci-operator-config-metadata

@candita
Copy link
Contributor Author

candita commented Feb 16, 2024

/pj-rehearse

@candita
Copy link
Contributor Author

candita commented Feb 27, 2024

/pj-rehearse

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 27, 2024
@openshift-ci-robot
Copy link
Contributor

@candita, pj-rehearse: unable to determine affected jobs. This could be due to a branch that needs to be rebased. ERROR:

couldn't prepare candidate: couldn't rebase candidate onto master: %!w(<nil>)
Interacting with pj-rehearse

Comment: /pj-rehearse to run up to 5 rehearsals
Comment: /pj-rehearse skip to opt-out of rehearsals
Comment: /pj-rehearse {test-name}, with each test separated by a space, to run one or more specific rehearsals
Comment: /pj-rehearse more to run up to 10 rehearsals
Comment: /pj-rehearse max to run up to 25 rehearsals
Comment: /pj-rehearse auto-ack to run up to 5 rehearsals, and add the rehearsals-ack label on success
Comment: /pj-rehearse abort to abort all active rehearsals

Once you are satisfied with the results of the rehearsals, comment: /pj-rehearse ack to unblock merge. When the rehearsals-ack label is present on your PR, merge will no longer be blocked by rehearsals.
If you would like the rehearsals-ack label removed, comment: /pj-rehearse reject to re-block merging.

@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 27, 2024
@candita
Copy link
Contributor Author

candita commented Feb 27, 2024

/pj-rehearse

@candita
Copy link
Contributor Author

candita commented Feb 29, 2024

/retest-required

test:
- as: test
cli: latest
commands: TEST=TestGatewayAPI;make test-e2e
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this work without export?

Suggested change
commands: TEST=TestGatewayAPI;make test-e2e
commands: export TEST=TestGatewayAPI;make test-e2e

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, it does seem to be at least running according to https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/openshift_release/48873/rehearse-48873-pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-gatewayapi/1775666420378505216/artifacts/e2e-aws-gatewayapi/test/build-log.txt

go generate ./pkg/manifests
CGO_ENABLED=1 GO111MODULE=on GOFLAGS=-mod=vendor go test -timeout 1h -count 1 -v -tags e2e -run "TestGatewayAPI" ./test/e2e
testing: warning: no tests to run
PASS
ok github.com/openshift/cluster-ingress-operator/test/e2e 0.657s [no tests to run]

@candita
Copy link
Contributor Author

candita commented Mar 27, 2024

/assign @frobware

test:
- as: test
cli: latest
commands: TEST=TestGatewayAPI;make test-e2e
Copy link
Contributor

Choose a reason for hiding this comment

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

make TEST=TestGatewayAPI is generally the better option. This approach is more direct and aligns with overriding make variables for the duration of a specific make invocation. It keeps the override scoped to the make process, avoiding potential side effects on the environment and other processes that do not need to be aware of the TEST variable.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point. It would be useful to explain why using an environment variable instead of a command-line argument is necessary, if it is.

There is a difference in semantics between using an environment variable and using a command-line argument—quoting the manual:

If a variable has been set with a command argument (*note Overriding
Variables: Overriding.), then ordinary assignments in the makefile are
ignored.  If you want to set the variable in the makefile even though it
was set with a command argument, you can use an 'override' directive,

It isn't obvious to me why we would want an assignment in the Makefile to override the TEST=TestGatewayAPI command-line argument.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, thanks for the feedback.

@openshift-ci-robot openshift-ci-robot removed the rehearsals-ack Signifies that rehearsal jobs have been acknowledged label Mar 29, 2024
@openshift-ci-robot
Copy link
Contributor

[REHEARSALNOTIFIER]
@candita: the pj-rehearse plugin accommodates running rehearsal tests for the changes in this PR. Expand 'Interacting with pj-rehearse' for usage details. The following rehearsable tests have been affected by this change:

Test name Repo Type Reason
pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-gatewayapi openshift/cluster-ingress-operator presubmit Presubmit changed
Interacting with pj-rehearse

Comment: /pj-rehearse to run up to 5 rehearsals
Comment: /pj-rehearse skip to opt-out of rehearsals
Comment: /pj-rehearse {test-name}, with each test separated by a space, to run one or more specific rehearsals
Comment: /pj-rehearse more to run up to 10 rehearsals
Comment: /pj-rehearse max to run up to 25 rehearsals
Comment: /pj-rehearse auto-ack to run up to 5 rehearsals, and add the rehearsals-ack label on success
Comment: /pj-rehearse abort to abort all active rehearsals

Once you are satisfied with the results of the rehearsals, comment: /pj-rehearse ack to unblock merge. When the rehearsals-ack label is present on your PR, merge will no longer be blocked by rehearsals.
If you would like the rehearsals-ack label removed, comment: /pj-rehearse reject to re-block merging.

@candita
Copy link
Contributor Author

candita commented Apr 1, 2024

/pj-rehearse

@candita
Copy link
Contributor Author

candita commented Apr 2, 2024

Some temporary etcd issue?

level=error msg=Cluster operator authentication Degraded is True with APIServerDeployment_PreconditionNotFulfilled::IngressStateEndpoints_MissingSubsets::OAuthAPIServerConfigObservation_Error::OAuthServerServiceEndpointAccessibleController_SyncError::OAuthServerServiceEndpointsEndpointAccessibleController_SyncError: APIServerDeploymentDegraded: waiting for observed configuration to have mandatory apiServerArguments.etcd-servers
level=error msg=APIServerDeploymentDegraded:
level=error msg=IngressStateEndpointsDegraded: No subsets found for the endpoints of oauth-server
level=error msg=OAuthAPIServerConfigObservationDegraded: configmaps openshift-etcd/etcd-endpoints: no etcd endpoint addresses found

/pj-rehearse

Copy link
Contributor

openshift-ci bot commented Apr 2, 2024

@candita: The following tests 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/rehearse/openshift/cluster-ingress-operator/release-4.16/e2e-aws-gatewayapi a074410 link unknown /pj-rehearse pull-ci-openshift-cluster-ingress-operator-release-4.16-e2e-aws-gatewayapi
ci/rehearse/openshift/cluster-ingress-operator/release-4.17/e2e-aws-gatewayapi a074410 link unknown /pj-rehearse pull-ci-openshift-cluster-ingress-operator-release-4.17-e2e-aws-gatewayapi

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.

@candita
Copy link
Contributor Author

candita commented Apr 3, 2024

level=error msg=Cluster operator operator-lifecycle-manager-packageserver Available is False with ClusterServiceVersionNotSucceeded: ClusterServiceVersion openshift-operator-lifecycle-manager/packageserver observed in phase Failed with reason: InstallCheckFailed, message: install timeout

/pj-rehearse

@candita
Copy link
Contributor Author

candita commented Apr 4, 2024

/pj-rehearse ack

@openshift-ci-robot openshift-ci-robot added the rehearsals-ack Signifies that rehearsal jobs have been acknowledged label Apr 4, 2024
@candita
Copy link
Contributor Author

candita commented Apr 4, 2024

Depends on openshift/cluster-ingress-operator#1023

@frobware
Copy link
Contributor

/lgtm
/hold
Should openshift/cluster-ingress-operator#1023 merge first?

@openshift-ci 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 Apr 11, 2024
Copy link
Contributor

openshift-ci bot commented Apr 11, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: candita, frobware

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

@candita
Copy link
Contributor Author

candita commented Apr 11, 2024

Should openshift/cluster-ingress-operator#1023 merge first?

@frobware I was hoping to have this framework ready for when the tests go in.

@frobware
Copy link
Contributor

Should openshift/cluster-ingress-operator#1023 merge first?

@frobware I was hoping to have this framework ready for when the tests go in.

I see:

% make TEST=TestGatewayAPI test-e2e
go generate ./pkg/manifests
CGO_ENABLED=1 GO111MODULE=on GOFLAGS=-mod=vendor go test -timeout 1h -count 1 -v -tags e2e -run "TestGatewayAPI" ./test/e2e
testing: warning: no tests to run
PASS
ok  	github.com/openshift/cluster-ingress-operator/test/e2e	2.347s [no tests to run

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 11, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit c978053 into openshift:master Apr 11, 2024
19 checks passed
saifshaikh48 pushed a commit to saifshaikh48/release that referenced this pull request Apr 23, 2024
@candita
Copy link
Contributor Author

candita commented Apr 26, 2024

/jira refresh

@candita
Copy link
Contributor Author

candita commented Apr 26, 2024

/honk

Copy link
Contributor

openshift-ci bot commented Apr 26, 2024

@candita:
goose image

In response to this:

/honk

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.

@candita
Copy link
Contributor Author

candita commented Apr 26, 2024

/jira refresh

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. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. rehearsals-ack Signifies that rehearsal jobs have been acknowledged
Projects
None yet
5 participants