Skip to content

🌱 E2E: Use UEFI #2770

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
Open
lentzi90 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

🌱 E2E: Use UEFI #2770

lentzi90 wants to merge 3 commits into from

Conversation

@lentzi90
Copy link
Member

@lentzi90 lentzi90 commented Nov 12, 2025

What this PR does / why we need it:

Switch e2e tests to UEFI instead of Legacy.
(Second attempt)

Fixes #1607

Checklist:

  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • E2E tests have been added, if necessary.
  • Integration tests have been added, if necessary.

@metal3-io-bot
Copy link
Contributor

metal3-io-bot commented Nov 12, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign honza for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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

@metal3-io-bot metal3-io-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 12, 2025
@lentzi90 lentzi90 force-pushed the lentzi90/e2e-uefi branch 2 times, most recently from 55be93f to c709b4e Compare November 13, 2025 06:57
@metal3-io-bot metal3-io-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 13, 2025
@lentzi90
Copy link
Member Author

lentzi90 commented Nov 13, 2025

/hold
WIP

@metal3-io-bot metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 13, 2025
@metal3-io-bot metal3-io-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 17, 2025
@metal3-io-bot metal3-io-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 18, 2025
MahnoorAsghar
MahnoorAsghar reviewed Dec 12, 2025
View reviewed changes
@lentzi90
Copy link
Member Author

lentzi90 commented Dec 19, 2025

Ok I think I have figured out all issues now. The part I was missing for virtualmedia was that the VMs run with secure boot. That doesn't work well with cirros. I have now disabled secure boot.

The second issue is for network boot. I think it boils down to #2792. Our DHCP config is not as it should be. I have managed to get around it locally but it is not a pretty workaround. I'll see if I can come up with something better here.

@lentzi90
E2E: Use UEFI
b057101 
Switch e2e tests to UEFI instead of Legacy.

Signed-off-by: Lennart Jern <[email protected]>
@lentzi90
Clean up sushy-tools config
5bdf34b 
Signed-off-by: Lennart Jern <[email protected]>
@lentzi90
Copy link
Member Author

lentzi90 commented Dec 19, 2025

Good! Now virtualmedia is working properly. DHCP is left to fix to make IPMI green.

@metal3-io-bot metal3-io-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 19, 2025
@lentzi90
E2E: Cleanup libvit DHCP config
d8e0d52 
Use Ironic dnsmasq container for DHCP.
This requires a workaround for now, since the keepalived image we use
doesn't support any other IP range than /32.

Signed-off-by: Lennart Jern <[email protected]>
@lentzi90
Copy link
Member Author

lentzi90 commented Dec 19, 2025

All green! 🎉
Checking optional (upgrade) jobs.
/test ?

@metal3-io-bot
Copy link
Contributor

metal3-io-bot commented Dec 19, 2025

@lentzi90: The following commands are available to trigger required jobs:

/test generate

/test gomod

/test manifestlint

/test markdownlint

/test shellcheck

/test test

The following commands are available to trigger optional jobs:

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

/test metal3-centos-e2e-basic-test-main

/test metal3-centos-e2e-feature-test-main-features

/test metal3-centos-e2e-feature-test-main-pivoting

/test metal3-centos-e2e-feature-test-main-remediation

/test metal3-centos-e2e-integration-k8s-pre-release-test-main

/test metal3-centos-e2e-integration-test-main

/test metal3-dev-env-integration-test-centos-main

/test metal3-dev-env-integration-test-ubuntu-main

/test metal3-e2e-1-33-1-34-upgrade-test-main

/test metal3-e2e-clusterctl-upgrade-test-main

/test metal3-ubuntu-e2e-basic-test-main

/test metal3-ubuntu-e2e-feature-test-main-features

/test metal3-ubuntu-e2e-feature-test-main-pivoting

/test metal3-ubuntu-e2e-feature-test-main-remediation

/test metal3-ubuntu-e2e-integration-k8s-pre-release-test-main

/test metal3-ubuntu-e2e-integration-test-main

Use /test all to run the following jobs that were automatically triggered:

generate

gomod

manifestlint

test
Details

In response to this:

All green! 🎉
Checking optional (upgrade) jobs.
/test ?

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-sigs/prow repository.

@lentzi90
Copy link
Member Author

lentzi90 commented Dec 19, 2025

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

@lentzi90
Copy link
Member Author

lentzi90 commented Dec 19, 2025

First upgrade timed out in AfterEach while deleting the namespace. I think this is a flake. After that, all tests fail because the ironic node conflicts with the next test...
/test metal3-bmo-e2e-test-optional-pull

@lentzi90
Copy link
Member Author

lentzi90 commented Dec 19, 2025

/hold cancel
/cc @Rozzii @adilGhaffarDev

@metal3-io-bot metal3-io-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 19, 2025
@lentzi90
Copy link
Member Author

lentzi90 commented Dec 19, 2025

This now also fixes #2792.
The three commits have descriptive commit messages that explain a bit more what and why. I can also push these as separate PRs if you think that is preferable. Then the DHCP fix has to merge before UEFI.

@dtantsur
Copy link
Member

dtantsur commented Dec 20, 2025

/lgtm

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Dec 20, 2025
adilGhaffarDev
adilGhaffarDev reviewed Dec 22, 2025
View reviewed changes
Copy link
Member

@adilGhaffarDev adilGhaffarDev left a comment

Choose a reason for hiding this comment

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

/lgtm

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

Reviewers

@s3rj1k s3rj1k Awaiting requested review from s3rj1k

@zaneb zaneb Awaiting requested review from zaneb

@Rozzii Rozzii Awaiting requested review from Rozzii

2 more reviewers

@adilGhaffarDev adilGhaffarDev adilGhaffarDev left review comments

@MahnoorAsghar MahnoorAsghar MahnoorAsghar left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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.

Use UEFI boot mode instead of legacy

5 participants

@lentzi90 @metal3-io-bot @dtantsur @adilGhaffarDev @MahnoorAsghar