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.

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

Update to latest iPXE #142

Merged
merged 3 commits into from
Mar 1, 2024
Merged

Update to latest iPXE #142

merged 3 commits into from
Mar 1, 2024

Conversation

nshalman
Copy link
Member

@nshalman nshalman commented Feb 27, 2024

Description

Update to the latest iPXE

This release includes ipxe/ipxe#1152 which resolves #117.

I've also included the workaround for #115 as specified from ipxe/ipxe#1091 (comment) to keep Mellanox cards working.

Why is this needed

Fixes: #117
Relates to: #115

How Has This Been Tested?

I am drafting this PR as the first step towards testing. The plan is:

  • Test on an affected Mellanox card
  • Confirm that LetsEncrypt is working

Resolves tinkerbell#117
Relates to tinkerbell#115

This release includes
ipxe/ipxe#1152 which resolves tinkerbell#117

I've also included the workaround for tinkerbell#115 as specified from
ipxe/ipxe#1091 (comment)

Signed-off-by: Nahum Shalman <[email protected]>
Copy link

codecov bot commented Feb 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.82%. Comparing base (36ad044) to head (62314e0).
Report is 20 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #142      +/-   ##
==========================================
+ Coverage   97.28%   97.82%   +0.53%     
==========================================
  Files           5        5              
  Lines         442      367      -75     
==========================================
- Hits          430      359      -71     
+ Misses          8        4       -4     
  Partials        4        4              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nshalman
Copy link
Member Author

Forgot to revert the patching in of the certificates... Need a bit more testing to confirm and will update this PR.

@nshalman nshalman marked this pull request as ready for review February 29, 2024 21:40
@nshalman
Copy link
Member Author

Let me know if you want the changes in 7f6cde1 split into more than one commit.
I will sadly also have a follow up issue to file related to ipxe/ipxe#1115 and a follow up PR akin to what we did for the Mellanox issue, but at least we can still still stick to only a single ipxe revert commit now that there is a better workaround for the Mellanox EAPOL issue.

Copy link
Member

@jacobweinstock jacobweinstock left a comment

Choose a reason for hiding this comment

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

Thanks, @nshalman !

@jacobweinstock jacobweinstock added the ready-to-merge Mergify: Ready for Merging label Mar 1, 2024
@mergify mergify bot merged commit 3cd7f25 into tinkerbell:main Mar 1, 2024
7 checks passed
mergify bot added a commit that referenced this pull request Mar 1, 2024
Follow up for #142 that includes a workaround for #143

This once again points us at a commit that I pushed to my fork. Reviewers should verify that the commit looks legit and is correctly a child of the commit it is replacing.

- [x] tested on an affected NIC
@nshalman nshalman deleted the updates-20240227 branch June 25, 2024 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge Mergify: Ready for Merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue with LetsEncrypt certificates
2 participants