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

🌱 Enable parallel testing in E2E #1527

Merged
merged 1 commit into from Jan 29, 2024
Merged

🌱 Enable parallel testing in E2E #1527

merged 1 commit into from Jan 29, 2024

Conversation

mquhuy
Copy link
Member

@mquhuy mquhuy commented Jan 22, 2024

Fixes #1443

@metal3-io-bot metal3-io-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 22, 2024
@mquhuy
Copy link
Member Author

mquhuy commented Jan 22, 2024

/test metal3-bmo-e2e-test-pull

@mquhuy
Copy link
Member Author

mquhuy commented Jan 22, 2024

/test metal3-bmo-e2e-test-pull
/test metal3-bmo-e2e-optional-test-pull

@mquhuy
Copy link
Member Author

mquhuy commented Jan 22, 2024

/test metal3-bmo-e2e-test-optional-pull

@mquhuy
Copy link
Member Author

mquhuy commented Jan 22, 2024

/test metal3-bmo-e2e-test-pull
/test metal3-bmo-e2e-test-optional-pull

tuminoid
tuminoid reviewed Jan 23, 2024
View reviewed changes
Copy link
Member

@tuminoid tuminoid left a comment

Choose a reason for hiding this comment

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

Minor nits, and also a question.

hack/ci-e2e.sh Outdated Show resolved Hide resolved
hack/ci-e2e.sh Outdated Show resolved Hide resolved
test/e2e/config/ironic-vbmc.yaml Outdated Show resolved Hide resolved
test/e2e/config/ironic-vbmc.yaml Outdated Show resolved Hide resolved
@mquhuy mquhuy force-pushed the mquhuy/e2e-parallel-test branch 3 times, most recently from 3e6d58a to f57b2ba Compare January 24, 2024 10:21
@mquhuy
Copy link
Member Author

mquhuy commented Jan 24, 2024

/test metal3-bmo-e2e-test-pull
/test metal3-bmo-e2e-test-optional-pull

@mquhuy
Copy link
Member Author

mquhuy commented Jan 24, 2024

/retest-required

@mquhuy
Copy link
Member Author

mquhuy commented Jan 24, 2024

/test metal3-bmo-e2e-test-pull

@mquhuy
Copy link
Member Author

mquhuy commented Jan 24, 2024

/test metal3-bmo-e2e-test-pull

@mquhuy
Copy link
Member Author

mquhuy commented Jan 24, 2024

/test metal3-bmo-e2e-test-pull

@mquhuy mquhuy force-pushed the mquhuy/e2e-parallel-test branch 3 times, most recently from 73db2a9 to 6e61fbf Compare January 24, 2024 11:43
@mquhuy
Copy link
Member Author

mquhuy commented Jan 24, 2024

/test metal3-bmo-e2e-test-pull

@mquhuy
Copy link
Member Author

mquhuy commented Jan 24, 2024

/test metal3-bmo-e2e-test-pull

@mquhuy mquhuy force-pushed the mquhuy/e2e-parallel-test branch 2 times, most recently from aedcb30 to 0b33a8b Compare January 24, 2024 13:07
@mquhuy
Copy link
Member Author

mquhuy commented Jan 24, 2024

/test metal3-bmo-e2e-test-pull

1 similar comment
@mquhuy
Copy link
Member Author

mquhuy commented Jan 25, 2024

/test metal3-bmo-e2e-test-pull

tuminoid
tuminoid reviewed Jan 26, 2024
View reviewed changes
Copy link
Member

@tuminoid tuminoid left a comment

Choose a reason for hiding this comment

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

Few nits.

hack/create_bmcs.sh Outdated Show resolved Hide resolved
hack/create_bmcs.sh Outdated Show resolved Hide resolved
hack/e2e/ensure_yq.sh Outdated Show resolved Hide resolved
@mquhuy
Copy link
Member Author

mquhuy commented Jan 26, 2024

Few nits.

Thanks a lot. I fixed them now.

@mquhuy
Copy link
Member Author

mquhuy commented Jan 26, 2024

/test metal3-bmo-e2e-test-pull

1 similar comment
@mquhuy
Copy link
Member Author

mquhuy commented Jan 26, 2024

/test metal3-bmo-e2e-test-pull

lentzi90
lentzi90 reviewed Jan 26, 2024
View reviewed changes
Copy link
Member

@lentzi90 lentzi90 left a comment

Choose a reason for hiding this comment

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

Ok I'm happy with this if you can get the tests to pass!
I guess it is probably a matter of tweaking the intervals or we are a bit resource constrained in the Jenkins worker...

@metal3-io-bot
Copy link
Contributor

metal3-io-bot commented Jan 26, 2024 •
edited

@mquhuy: 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
metal3-bmo-e2e-test-optional-pull 38d59d0 link false /test metal3-bmo-e2e-test-optional-pull

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.

@mquhuy
Copy link
Member Author

mquhuy commented Jan 26, 2024

/test metal3-bmo-e2e-test-pull

@mquhuy
Copy link
Member Author

mquhuy commented Jan 26, 2024

Ok I'm happy with this if you can get the tests to pass! I guess it is probably a matter of tweaking the intervals or we are a bit resource constrained in the Jenkins worker...

Thank you. I reckon using 2 VMs might be more likely to succeed, I just want to test if 3 VMs is feasible. I'll test it out a couple of times more to figure out.

@mquhuy
Enable parallel testing in E2E
934e18b 
Signed-off-by: Huy Mai <huy.mai@est.tech>
@lentzi90
Copy link
Member

For reference, the workers we currently use have 8 CPU and 16 GB memory. We can change this later if we want 🙂

@mquhuy
Copy link
Member Author

mquhuy commented Jan 26, 2024

/test metal3-bmo-e2e-test-pull

@mquhuy
Copy link
Member Author

mquhuy commented Jan 26, 2024

For reference, the workers we currently use have 8 CPU and 16 GB memory. We can change this later if we want 🙂

@lentzi90 I guess it explained the random flakes in the tests. 3 VMs would take 6 CPUs, so it means we have to run 3 threads for the tests on 2 remaining CPUs.
Switching to 2 VMs made the tests pass. We can increase it later if we want though. A 2-VM setup take about 25mins, while 3-VM one takes 20 mins.

Btw, could you help me override the integration tests?

lentzi90
lentzi90 reviewed Jan 26, 2024
View reviewed changes
Copy link
Member

@lentzi90 lentzi90 left a comment

Choose a reason for hiding this comment

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

/lgtm

I don't have privileges to override context in this repo unfortunately

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 26, 2024
@tuminoid
Copy link
Member

/override test-centos-e2e-integration-main test-ubuntu-integration-main

@metal3-io-bot
Copy link
Contributor

@tuminoid: Overrode contexts on behalf of tuminoid: test-centos-e2e-integration-main, test-ubuntu-integration-main

In response to this:

/override test-centos-e2e-integration-main test-ubuntu-integration-main

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.

@mquhuy
Copy link
Member Author

mquhuy commented Jan 26, 2024

/cc @kashifest

maxrantil
maxrantil reviewed Jan 29, 2024
View reviewed changes
test/e2e/config/ironic.yaml
IMAGE_URL: "http://192.168.222.1/cirros-0.6.2-x86_64-disk.img"
IMAGE_CHECKSUM: "c8fc807773e5354afe61636071771906"
CERT_MANAGER_VERSION: "v1.13.1"
SSH_CHECK_PROVISIONED: "true"
IP_BMO_E2E_0: "192.168.222.122"
SSH_PORT: "22"
Copy link
Member

Choose a reason for hiding this comment

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

Do all tests pass without SSH_PORT ? It is used when checking to verify the node's boot source. Maybe it used 22 by default. But I think it would be good to still have it because the variable is used in the go code and someone might want to use another port sometime?

Copy link
Member

Choose a reason for hiding this comment

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

Oh NVM! I see that you have changed the code.

@maxrantil
Copy link
Member

LGTM

@kashifest
Copy link
Member

This is cool. Great work @mquhuy
/approve

@metal3-io-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kashifest

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

@metal3-io-bot metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 29, 2024
@metal3-io-bot metal3-io-bot merged commit e476d6d into metal3-io:main Jan 29, 2024
23 checks passed
@metal3-io-bot metal3-io-bot deleted the mquhuy/e2e-parallel-test branch January 29, 2024 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tuminoid tuminoid tuminoid left review comments

@lentzi90 lentzi90 lentzi90 left review comments

@maxrantil maxrantil maxrantil left review comments

@kashifest kashifest Awaiting requested review from kashifest

Assignees
No one assigned
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. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

E2e: enable parallel testing
6 participants
@mquhuy @lentzi90 @metal3-io-bot @tuminoid @maxrantil @kashifest