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

Add runtimeClassName #105

Merged
merged 3 commits into from
Jun 5, 2024
Merged

Conversation

thomaspetit
Copy link
Contributor

@thomaspetit thomaspetit commented Jun 3, 2024

This option might be interesting for those that are running a GPU within their kubernetes cluster and want to be able to specify the runtimeClassName manually.

---
apiVersion: v1
kind: Pod
metadata:
  name: nbody-gpu-benchmark
  namespace: default
spec:
  restartPolicy: OnFailure
  runtimeClassName: nvidia
  containers:
  - name: cuda-container
    image: nvcr.io/nvidia/k8s/cuda-sample:nbody
    args: ["nbody", "-gpu", "-benchmark"]
    resources:
      limits:
        nvidia.com/gpu: 1
    env:
    - name: NVIDIA_VISIBLE_DEVICES
      value: all
    - name: NVIDIA_DRIVER_CAPABILITIES
      value: all

@thomaspetit thomaspetit requested a review from a team as a code owner June 3, 2024 18:09
Copy link
Member

@cilindrox cilindrox left a comment

Choose a reason for hiding this comment

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

LGTM 👍 - thanks @thomaspetit

Could you bump the chart's version to 0.4.0?

@cilindrox cilindrox self-assigned this Jun 3, 2024
@thomaspetit
Copy link
Contributor Author

thomaspetit commented Jun 3, 2024

Fixed and also signed my commits (as per requirement for allowing this PR)

@thomaspetit
Copy link
Contributor Author

@cilindrox can you have a look at the latest commit? I'd love to re-align my homelab to the latest version with gpu acceleration 😄

Copy link
Member

@cilindrox cilindrox left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@thomaspetit
Copy link
Contributor Author

I'm validating some local stuff and just noticed that the securityContext is not configureable which basically means that the GPU can be detected but not used by Plex. Probably need to make it configureable too.

@cilindrox
Copy link
Member

I'm validating some local stuff and just noticed that the securityContext is not configureable which basically means that the GPU can be detected but not used by Plex. Probably need to make it configureable too.

Good call - we went with something a bit too lenient but maybe that whole block can be made configurable and the default value just uses the current settings?

LMK and I can keep this PR open

@thomaspetit
Copy link
Contributor Author

The security context that you're referring to ise indeed required for rclone but I you don't plan to use the rclone or gpu acceleration you maybe do not want to make your plex container privileged from a security pov.

I'll check how I can nicely make it fit and ensure we have something non-breaking 👍

@cilindrox
Copy link
Member

whoops, bad container - even more straightforward then ;)

@thomaspetit
Copy link
Contributor Author

I made an error locally by not adding my encoding package libnvidia-encode-550. It works nicely now without the privileged mode.

We can merge it if ok for you 🥳

@cilindrox cilindrox merged commit 4d68c05 into plexinc:master Jun 5, 2024
1 check passed
@thomaspetit thomaspetit deleted the feature/runtimeclass branch June 6, 2024 18:08
@thomaspetit
Copy link
Contributor Author

🥳

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.

2 participants