Skip to content

Mark network tests that don't require node's additional interfaces #724

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.

Sign up for GitHub

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

Jump to bottom
Merged
vsibirsk merged 3 commits into from
May 4, 2025
Merged

Mark network tests that don't require node's additional interfaces #724

vsibirsk merged 3 commits into from
May 4, 2025

Conversation

@yossisegev
Copy link
Contributor

@yossisegev yossisegev commented Apr 10, 2025

Acceptance tests for new architectures, platforms etc. shouldn't depend on having extra NICs on the cluster nodes, except for the primary interface. In order to have satisfactory coveage in these tests, we mark all the these tests with the a new marker, which can and should be used for these types of tests.

@ghost
Copy link

ghost commented Apr 10, 2025

Report bugs in Issues

The following are automatically added:

  • Add reviewers from OWNER file (in the root of the repository) under reviewers section.
  • Set PR size label.
  • New issue is created for the PR. (Closed when PR is merged/closed)
  • Run pre-commit if .pre-commit-config.yaml exists in the repo.

Available user actions:

  • To mark PR as WIP comment /wip to the PR, To remove it from the PR comment /wip cancel to the PR.
  • To block merging of PR comment /hold, To un-block merging of PR comment /hold cancel.
  • To mark PR as verified comment /verified to the PR, to un-verify comment /verified cancel to the PR.
    verified label removed on each new commit push.
  • To cherry pick a merged PR comment /cherry-pick <target branch to cherry-pick to> in the PR.
    • Multiple target branches can be cherry-picked, separated by spaces. (/cherry-pick branch1 branch2)
    • Cherry-pick will be started when PR is merged
  • To build and push container image command /build-and-push-container in the PR (tag will be the PR number).
    • You can add extra args to the Podman build command
      • Example: /build-and-push-container --build-arg OPENSHIFT_PYTHON_WRAPPER_COMMIT=<commit_hash>
  • To add a label by comment use /<label name>, to remove, use /<label name> cancel
  • To assign reviewers based on OWNERS file use /assign-reviewers
  • To check if PR can be merged use /check-can-merge
  • to assign reviewer to PR use /assign-reviewer @<reviewer>
Supported /retest check runs
  • /retest tox: Retest tox
  • /retest build-container: Retest build-container
  • /retest all: Retest all
Supported labels
  • hold
  • verified
  • wip
  • lgtm

@ghost ghost added the size/S label Apr 10, 2025
@ghost ghost requested a review from dbasunag April 10, 2025 14:06
@ghost ghost mentioned this pull request Apr 10, 2025
Closed
@ghost ghost added the branch-main label Apr 10, 2025
@ghost ghost requested review from Anatw, RoniKishner, dshchedr, sbahar619 and vsibirsk April 10, 2025 14:06
@yossisegev
Copy link
Contributor Author

yossisegev commented Apr 10, 2025

/build-and-push-container

@ghost
Copy link

ghost commented Apr 10, 2025

New container for quay.io/openshift-cnv/openshift-virtualization-tests:pr-724 published

EdDev
EdDev reviewed Apr 11, 2025
View reviewed changes
Copy link
Contributor

@EdDev EdDev left a comment

Choose a reason for hiding this comment

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

@yossisegev , I think there is a need to update the condition in the network sanity check to allow such tests to run.

@ghost ghost added the commented-EdDev label Apr 11, 2025
Anatw
Anatw reviewed Apr 14, 2025
View reviewed changes
Copy link
Contributor

@Anatw Anatw left a comment

Choose a reason for hiding this comment

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

Yossi, how were the marked test gathered?

@ghost ghost added the commented-Anatw label Apr 14, 2025
@yossisegev
Copy link
Contributor Author

yossisegev commented Apr 23, 2025

@yossisegev , I think there is a need to update the condition in the network sanity check to allow such tests to run.

@EdDev I prefer handling it in another PR because:

  1. The tests that are marked in this PR don't rely on the multi NIC verification.
  2. Changing this condition affects the bond scenarios as well. While 1 extra NIC is still enough for most of the L2 cases, bond cases still require 2 extra NICs. So my concern is that changing this condition might break some of the bond cases, requiring to specially handling them, which is out of the scope of this PR.

@yossisegev yossisegev force-pushed the mark-single-nic-tests branch from 4451652 to 78faf84 Compare April 23, 2025 15:32
EdDev
EdDev approved these changes Apr 29, 2025
View reviewed changes
Copy link
Contributor

@EdDev EdDev left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm

@ghost ghost added the lgtm-EdDev label Apr 29, 2025
@dshchedr
Copy link
Collaborator

dshchedr commented Apr 29, 2025

/approve

@dshchedr dshchedr enabled auto-merge (squash) April 29, 2025 17:06
@ghost ghost added the approved-dshchedr label Apr 29, 2025
@EdDev
Copy link
Contributor

EdDev commented Apr 29, 2025

/approve

@ghost ghost added the approved-EdDev label Apr 29, 2025
@EdDev
Copy link
Contributor

EdDev commented Apr 29, 2025

/check-can-merge

sbahar619
sbahar619 reviewed Apr 30, 2025
View reviewed changes
@ghost ghost added the commented-sbahar619 label Apr 30, 2025
sbahar619
sbahar619 reviewed Apr 30, 2025
View reviewed changes
@ghost ghost added the commented-EdDev label Apr 30, 2025
@yossisegev
Copy link
Contributor Author

yossisegev commented May 4, 2025

/verified

@yossisegev yossisegev requested a review from sbahar619 May 4, 2025 10:22
@EdDev
Copy link
Contributor

EdDev commented May 4, 2025

/check-can-merge

@ghost ghost added the can-be-merged label May 4, 2025
@vsibirsk vsibirsk disabled auto-merge May 4, 2025 12:51
@vsibirsk vsibirsk merged commit e559a94 into RedHatQE:main May 4, 2025
7 checks passed
@ghost
Copy link

ghost commented May 4, 2025

Successfully removed PR tag: quay.io/openshift-cnv/openshift-virtualization-tests:pr-724.

@yossisegev
Copy link
Contributor Author

yossisegev commented May 4, 2025

/cherry-pick cnv-4.18

@ghost
Copy link

ghost commented May 4, 2025

Manual cherry-pick is needed
Cherry pick failed for e559a94 to cnv-4.18:
To cherry-pick run:

git remote update
git checkout cnv-4.18
git pull origin cnv-4.18
git checkout -b mark-single-nic-tests-cnv-4.18
git cherry-pick e559a949b92fb80937c989d4d271e03a8eb66df0
git push origin mark-single-nic-tests-cnv-4.18

@yossisegev
Copy link
Contributor Author

yossisegev commented Jun 9, 2025

/cherry-pick cnv-4.18

@openshift-virtualization-qe-bot-2
Copy link
Contributor

openshift-virtualization-qe-bot-2 commented Jun 9, 2025

Manual cherry-pick is needed
Cherry pick failed for e559a94 to cnv-4.18:
To cherry-pick run:

git remote update
git checkout cnv-4.18
git pull origin cnv-4.18
git checkout -b mark-single-nic-tests-cnv-4.18
git cherry-pick e559a949b92fb80937c989d4d271e03a8eb66df0
git push origin mark-single-nic-tests-cnv-4.18

@yossisegev
Copy link
Contributor Author

yossisegev commented Jun 12, 2025

/cherry-pick cnv-4.18

@openshift-virtualization-qe-bot
Copy link

openshift-virtualization-qe-bot commented Jun 12, 2025

Manual cherry-pick is needed
Cherry pick failed for e559a94 to cnv-4.18:
To cherry-pick run:

git remote update
git checkout cnv-4.18
git pull origin cnv-4.18
git checkout -b mark-single-nic-tests-cnv-4.18
git cherry-pick e559a949b92fb80937c989d4d271e03a8eb66df0
git push origin mark-single-nic-tests-cnv-4.18

@yossisegev
Copy link
Contributor Author

yossisegev commented Jul 14, 2025

/cherry-pick cnv-4.18

@openshift-virtualization-qe-bot
Copy link

openshift-virtualization-qe-bot commented Jul 14, 2025

Manual cherry-pick is needed
Cherry pick failed for e559a94 to cnv-4.18:
To cherry-pick run:

git remote update
git checkout cnv-4.18
git pull origin cnv-4.18
git checkout -b mark-single-nic-tests-cnv-4.18
git cherry-pick e559a949b92fb80937c989d4d271e03a8eb66df0
git push origin mark-single-nic-tests-cnv-4.18

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dshchedr dshchedr dshchedr approved these changes

@polarion-jenkins polarion-jenkins polarion-jenkins approved these changes

@dbasunag dbasunag Awaiting requested review from dbasunag

@vsibirsk vsibirsk Awaiting requested review from vsibirsk

@RoniKishner RoniKishner Awaiting requested review from RoniKishner

@Anatw Anatw Awaiting requested review from Anatw

@sbahar619 sbahar619 Awaiting requested review from sbahar619

+1 more reviewer

@EdDev EdDev EdDev approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

approved-dshchedr approved-EdDev branch-main can-be-merged commented-EdDev commented-sbahar619 commented-yossisegev lgtm-dshchedr lgtm-EdDev lgtm-polarion-jenkins sig-network size/S tox:verify-bugs-are-open:passed D/S Testing tox:verify-tc-requirement-polarion:passed D/S Testing verified

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@yossisegev @EdDev @dshchedr @openshift-virtualization-qe-bot-2 @openshift-virtualization-qe-bot @Anatw @sbahar619 @polarion-jenkins @vsibirsk @dbasunag1