Skip to content

fix runAsNonRoot (true) efa device plugin bug#6302

Merged
cPu1 merged 1 commit intoeksctl-io:mainfrom
researchapps:update/efa-device-plugin
Jul 4, 2024
Merged

fix runAsNonRoot (true) efa device plugin bug#6302
cPu1 merged 1 commit intoeksctl-io:mainfrom
researchapps:update/efa-device-plugin

Conversation

@vsoch
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

vsoch commented Mar 28, 2023

Why was this closed? The bug is still there.

@DanielJuravski
Copy link
Copy Markdown

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

@bollig
Copy link
Copy Markdown

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
Copy Markdown
Contributor

@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.

@vsoch vsoch force-pushed the update/efa-device-plugin branch from 49bf3eb to 53425e5 Compare June 21, 2024 11:45
@cPu1
Copy link
Copy Markdown
Contributor

cPu1 commented Jun 21, 2024

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

@cPu1
Copy link
Copy Markdown
Contributor

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 <vsoch@users.noreply.github.com>
@vsoch vsoch force-pushed the update/efa-device-plugin branch from 53425e5 to d83fd8a Compare July 4, 2024 15:07
@vsoch
Copy link
Copy Markdown
Contributor Author

vsoch commented Jul 4, 2024

@cPu1 all set.

@cPu1
Copy link
Copy Markdown
Contributor

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
@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