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

added configuring of device mount permissions in udev discovery handler #726 #737

Merged
merged 19 commits into from
Jan 22, 2025

Conversation

bindsi
Copy link
Member

@bindsi bindsi commented Jan 17, 2025

What this PR does / why we need it:
This PR adds configuration options for the device mount permissions in the udev discovery handler.

Corresponding doc PR project-akri/akri-docs#116

If applicable:

  • this PR has an associated PR with documentation in akri-docs
  • this PR contains unit tests
  • added code adheres to standard Rust formatting (cargo fmt)
  • code builds properly (cargo build)
  • code is free of common mistakes (cargo clippy)
  • all Akri tests succeed (cargo test)
  • inline documentation builds (cargo doc)
  • all commits pass the DCO bot check by being signed off -- see the failing DCO check for instructions on how to retroactively sign commits

@bindsi bindsi requested a review from yujinkim-msft as a code owner January 17, 2025 15:52
project-akri#726

Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com>
Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com>
Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com>
Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com>
Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com>
Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com>
Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com>
Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com>
Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com>
Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com>
@bindsi
Copy link
Member Author

bindsi commented Jan 19, 2025

@yujinkim-msft can you add maybe @kate-goldenring as reviewer as she requested/suggested configuring of device mount permissions...I cannot do it so far...maybe we can discuss that I could become "maintainer" too.

  • would be great to merge this quickly as I also resolved some of the errors in the validation workflows caused by an old rust toolchain version, unfortunately using the latest 1.84 doesn´t work as it would require some code changes due to breaking changes and yaserde cannot be used as it...so was updating to 1.81 which is the min-version required by some of our dependencies.

Copy link
Contributor

@kate-goldenring kate-goldenring left a comment

Choose a reason for hiding this comment

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

Just one small improvement. Please also add default overriding configuration to the Helm lint files for pods and jobs. Thanks for adding this!

discovery-handlers/udev/src/discovery_handler.rs Outdated Show resolved Hide resolved
Co-authored-by: Kate Goldenring <kate.goldenring@gmail.com>
Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com>
Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com>
Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com>
Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com>
Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com>
Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com>
Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com>
@bindsi
Copy link
Member Author

bindsi commented Jan 22, 2025

Just one small improvement. Please also add default overriding configuration to the Helm lint files for pods and jobs. Thanks for adding this!

@kate-goldenring Thanks for the review and feedback. I added test test, your validation suggestion and the defaults for the lint files.
Created the issue for the webhook checks here: #738

Copy link
Contributor

@kate-goldenring kate-goldenring left a comment

Choose a reason for hiding this comment

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

One final nit suggestion on the test but LGTM

discovery-handlers/udev/src/discovery_handler.rs Outdated Show resolved Hide resolved
Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com>
Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com>
@yujinkim-msft yujinkim-msft merged commit 5e20ee4 into project-akri:main Jan 22, 2025
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable configuring device mount permissions in udev Discovery Handler
3 participants