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.

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

Add a test for configuring maxDevices on virt-handler #6792

Merged
merged 3 commits into from
Dec 24, 2021
Merged

Add a test for configuring maxDevices on virt-handler #6792

merged 3 commits into from
Dec 24, 2021

Conversation

zcahana
Copy link
Contributor

@zcahana zcahana commented Nov 14, 2021

What this PR does / why we need it:

Adds func test for configuring VirtualMachingInstancesPerNode (added in #6105).

This PR picks up the pending work from #6170 by @ashleyschuett (which is no longer involved in the project), and adds a tiny fix that addresses the last remaining review comment (+rebase).

Release note:

NONE

@kubevirt-bot kubevirt-bot added release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Nov 14, 2021
@zcahana zcahana mentioned this pull request Nov 14, 2021
Closed
@zcahana
Copy link
Contributor Author

zcahana commented Nov 14, 2021

/retest

@zcahana
Copy link
Contributor Author

zcahana commented Nov 18, 2021

/test pull-kubevirt-check-tests-for-flakes

1 similar comment
@zcahana
Copy link
Contributor Author

zcahana commented Nov 18, 2021

/test pull-kubevirt-check-tests-for-flakes

iholder101
iholder101 reviewed Nov 22, 2021
View reviewed changes
Copy link
Contributor

@iholder101 iholder101 left a comment

Choose a reason for hiding this comment

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

Thanks @zcahana!

tests/operator_test.go Show resolved Hide resolved
tests/operator_test.go Outdated Show resolved Hide resolved
tests/operator_test.go Outdated Show resolved Hide resolved
@kubevirt-bot kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 25, 2021
@kubevirt-bot kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 25, 2021
@iholder101
Copy link
Contributor

Thank you @zcahana!
/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 25, 2021
@zcahana
Copy link
Contributor Author

zcahana commented Nov 25, 2021

/test pull-kubevirt-e2e-k8s-1.22-sig-storage

@zcahana
Copy link
Contributor Author

zcahana commented Nov 28, 2021

/retest-required

@zcahana
Copy link
Contributor Author

zcahana commented Nov 28, 2021

@enp0s3 Hi Igor, can you also take a look and see if you can /approve this? Thanks!

@kubevirt-bot kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 30, 2021
@kubevirt-bot kubevirt-bot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Dec 1, 2021
ashleyschuett and others added 3 commits December 21, 2021 11:56
@ashleyschuett @zcahana
add a test for configuring maxDevices on virt-handler
ff97f9c 
Signed-off-by: Ashley Schuett <aschuett@redhat.com>
@zcahana
Fix VirtualMachineInstancesPerNode comparison
d4cd2e6 
Signed-off-by: Zvi Cahana <zvic@il.ibm.com>
@zcahana
Address review comments in maxDevices test
055c44a 
Signed-off-by: Zvi Cahana <zvic@il.ibm.com>
@zcahana
Copy link
Contributor Author

zcahana commented Dec 21, 2021

Rebased since this has been lying around for quite some time.
/cc @rmohr who have approved the original PR #6105, maybe you can take a look at this too?

@iholder101
Copy link
Contributor

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Dec 21, 2021
@zcahana
Copy link
Contributor Author

zcahana commented Dec 21, 2021

/test pull-kubevirt-unit-test

@rmohr
Copy link
Member

rmohr commented Dec 21, 2021

/approve
/retest

@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rmohr

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

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 21, 2021
@zcahana
Copy link
Contributor Author

zcahana commented Dec 21, 2021

/test pull-kubevirt-e2e-k8s-1.20-cgroupsv2

@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

1 similar comment
@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@zcahana
Copy link
Contributor Author

zcahana commented Dec 22, 2021

/test pull-kubevirt-e2e-k8s-1.20-cgroupsv2

@dhiller
Copy link
Contributor

dhiller commented Dec 22, 2021

/hold

Disabling retest-bot due to high cluster load.

@kubevirt-bot kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 22, 2021
@dhiller
Copy link
Contributor

dhiller commented Dec 22, 2021

/unhold

@kubevirt-bot kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 22, 2021
@dhiller
Copy link
Contributor

dhiller commented Dec 22, 2021

/retest

@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

4 similar comments
@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@rmohr
Copy link
Member

rmohr commented Dec 24, 2021

/retest

test results look ok. Seems to be a mixture of flaky tests plus bad luck when finally in the merge queue.

@kubevirt-bot kubevirt-bot merged commit 2472052 into kubevirt:main Dec 24, 2021
@zcahana zcahana deleted the max-vmi branch December 25, 2021 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iholder101 iholder101 iholder101 left review comments

@kbidarkar kbidarkar Awaiting requested review from kbidarkar

@rnetser rnetser Awaiting requested review from rnetser

Assignees

@iholder101 iholder101

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@zcahana @iholder101 @rmohr @kubevirt-bot @kubevirt-commenter-bot @dhiller @ashleyschuett