Skip to content

Use default network type#2085

Merged
2uasimojo merged 1 commit into
openshift:masterfrom
2uasimojo:default-network-type
Aug 11, 2023
Merged

Use default network type#2085
2uasimojo merged 1 commit into
openshift:masterfrom
2uasimojo:default-network-type

Conversation

@2uasimojo
Copy link
Copy Markdown
Member

Installer will default the network type to OVNKubernetes if not specified. We have been overriding it to OpenShiftSDN in the clusterresource builder used for hiveutil and thus e2e. This commit removes that override, letting the installer default it for resources provisioned through those code paths.

Installer will default the network type to OVNKubernetes if not
specified. We have been overriding it to OpenShiftSDN in the
clusterresource builder used for hiveutil and thus e2e. This commit
removes that override, letting the installer default it for resources
provisioned through those code paths.
@openshift-ci openshift-ci Bot requested review from abutcher and dlom August 9, 2023 21:27
@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 9, 2023
@2uasimojo
Copy link
Copy Markdown
Member Author

/assign @jstuever

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 10, 2023

Codecov Report

Merging #2085 (c0ae8f3) into master (01532c6) will increase coverage by 0.13%.
Report is 14 commits behind head on master.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2085      +/-   ##
==========================================
+ Coverage   57.38%   57.52%   +0.13%     
==========================================
  Files         186      186              
  Lines       25712    25860     +148     
==========================================
+ Hits        14756    14877     +121     
- Misses       9717     9733      +16     
- Partials     1239     1250      +11     
Files Changed Coverage Δ
pkg/clusterresource/builder.go 66.75% <ø> (-0.10%) ⬇️
pkg/clusterresource/aws.go 90.62% <100.00%> (ø)
...g/controller/clusterpool/clusterpool_controller.go 60.02% <100.00%> (+3.72%) ⬆️

... and 2 files with indirect coverage changes

@2uasimojo
Copy link
Copy Markdown
Member Author

Um, I don't know why that failed. The output looks copacetic. The time frame is reasonable. The time stamp on the last output before cleanup is an hour before the timeout message from the job runner. Did something hang while we were gathering logs/artifacts?

/test e2e-pool

@2uasimojo
Copy link
Copy Markdown
Member Author

Did something hang while we were gathering logs/artifacts?

Yeah, this is what it looks like: oc delete clusterclaim --all never returned. And because we didn't save logs again at that point, we really have no way of tracking down why. Threw up #2086 to improve gathering in this scenario.

@jstuever
Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 10, 2023
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Aug 10, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 2uasimojo, jstuever

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details 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

@openshift-ci-robot
Copy link
Copy Markdown

/retest-required

Remaining retests: 0 against base HEAD 7461d16 and 2 for PR HEAD c0ae8f3 in total

@2uasimojo
Copy link
Copy Markdown
Member Author

/test e2e-pool

Incidental error during cleanup, that we shouldn't allow to fail the whole test. Closing that gap via #2088.

@2uasimojo
Copy link
Copy Markdown
Member Author

Well boo, my test succeeded, but the cleanup workflow failed :(

/test e2e-pool

@openshift-ci-robot
Copy link
Copy Markdown

/retest-required

Remaining retests: 0 against base HEAD da36286 and 1 for PR HEAD c0ae8f3 in total

@2uasimojo
Copy link
Copy Markdown
Member Author

/test e2e-pool

infra flake

@2uasimojo
Copy link
Copy Markdown
Member Author

/test unit

seems stuck?

@2uasimojo
Copy link
Copy Markdown
Member Author

Well, it passed previously, and I don't feel like retesting everything.

/override ci/prow/unit

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Aug 11, 2023

@2uasimojo: Overrode contexts on behalf of 2uasimojo: ci/prow/unit

Details

In response to this:

Well, it passed previously, and I don't feel like retesting everything.

/override ci/prow/unit

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.

@2uasimojo
Copy link
Copy Markdown
Member Author

/test coverage

known taint sorting flake

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Aug 11, 2023

@2uasimojo: 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/e2e-pool c0ae8f3 link unknown /test e2e-pool

Full PR test history. Your PR dashboard.

Details

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.

@2uasimojo
Copy link
Copy Markdown
Member Author

infra flake after test succeeded

@2uasimojo 2uasimojo merged commit 70b666e into openshift:master Aug 11, 2023
@2uasimojo 2uasimojo deleted the default-network-type branch August 11, 2023 22:06
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. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants