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

fix runAsNonRoot (true) efa device plugin bug #6302

Merged
merged 1 commit into from
Jul 4, 2024

Conversation

vsoch
Copy link
Contributor

@vsoch vsoch commented Feb 19, 2023

The plugins were globally changed to have runAsNonRoot set to true. This breaks the efa plugin, which
currently requires it. This PR was tested and confirmed to fix the bug in several cases. This will close #6222, and the other suggestions tried are also mentioned there.

Thanks for working on eksctl, we are using it a lot!

Checklist

  • Added tests that cover your change (if possible) NA
  • Added/modified documentation as required (such as the README.md, or the userdocs directory)
  • Manually tested
  • Made sure the title of the PR is a good description that can go into the release notes
  • (Core team) Added labels for change area (e.g. area/nodegroup) and kind (e.g. kind/improvement)

BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯

  • Backfilled missing tests for code in same general area 🎉
  • Refactored something and made the world a better place 🌟

@vsoch
Copy link
Contributor Author

vsoch commented Mar 28, 2023

Why was this closed? The bug is still there.

@DanielJuravski
Copy link

This helm chart resolved the issue https://github.com/aws-samples/efa-device-plugin-helm

@bollig
Copy link

bollig commented Jun 26, 2023

Redirecting anyone who lands here or #6435 to #6222. Both issues closed due to timeout, not resolution in eksctl codebase. efaEnabled: true does NOT function as expected.

@cPu1 cPu1 reopened this Jun 21, 2024
Copy link
Collaborator

@cPu1 cPu1 left a comment

Choose a reason for hiding this comment

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

@vsoch, really apologize for the delay. This got slipped through the cracks.

Given that the official Helm chart also defaults to runAsNonRoot: false, we should do that too.

@cPu1
Copy link
Collaborator

cPu1 commented Jun 21, 2024

The unit tests are failing due to #7845. We're working on a fix.

@cPu1
Copy link
Collaborator

cPu1 commented Jul 4, 2024

@vsoch can you rebase with main? This is good to merge.

The plugins were globally changed to have runAsNonRoot
set to true. This breaks the efa plugin, which
currently requires it. This PR was tested and confirmed
to fix the bug in several cases.

Signed-off-by: vsoch <[email protected]>
@vsoch vsoch force-pushed the update/efa-device-plugin branch from 53425e5 to d83fd8a Compare July 4, 2024 15:07
@vsoch
Copy link
Contributor Author

vsoch commented Jul 4, 2024

@cPu1 all set.

@cPu1
Copy link
Collaborator

cPu1 commented Jul 4, 2024

@cPu1 all set.

Great. Thanks! Appreciate your patience.

@cPu1 cPu1 merged commit 80e0c8d into eksctl-io:main Jul 4, 2024
9 checks passed
@vsoch vsoch deleted the update/efa-device-plugin branch July 4, 2024 15:31
@vsoch vsoch changed the title fix runAsRoot (true) efa device plugin bug fix runAsNonRoot (true) efa device plugin bug Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Issue with efa device plugin running as root
4 participants