Repository navigation
fix(chart): make containers and vgpulock dirs writable by non-root workloads - #22
dttung2905 wants to merge 1 commit into
Conversation
Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: dttung2905 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
No actionable comments were generated in the recent review. ð âđïļ Recent review infoâïļ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ð Files selected for processing (1)
ð WalkthroughWalkthroughThe DaemonSet startup script now creates the host library destination, ChangesLibrary Sync Directory Setup
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested labels: Suggested reviewers: Poem
ðĨ Pre-merge checks | â 5â Passed checks (5 passed)
âĻ Finishing Touchesð§Š Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| chmod 0644 "${DST}/libvgpu.so" "${DST}/ld.so.preload" | ||
| # Workload containers often run as non-root (e.g. e2e UID 1000); | ||
| # libvgpu must be able to mkdir {POD_UID}_{CONTAINER_NAME} here. | ||
| chmod 1777 "${DST}/containers" /host-root/tmp/vgpulock |
There was a problem hiding this comment.
/tmp/vgpulock is hardcoded here too, same as vgpuLockPath in webhook/main.go. could drift later, np for now tho
|
Old PRs and issues are being closed because they are no longer relevant. Or it was closed because of th |
The libsync DaemonSet only created /usr/local/vgpu. The webhook mounts
{containerVgpuMount}/containers and the host /tmp/vgpulock into workload
containers as DirectoryOrCreate hostPaths, so the kubelet created both
as root 0755. libvgpu then mkdirs its synthesized cache dir
{CONTAINER_VGPU_MOUNT}/containers/{POD_UID}_{CONTAINER_NAME} from inside
the workload container and, running as non-root, fails with errno=13 and
falls back to the container-local /tmp/cudevshr.cache, which
kai-vgpu-monitor cannot see.
Create both directories in the libsync startup script, which already
runs privileged as root with / mounted at /host-root, and leave them
1777 so any workload UID can create its own cache dir on the shared host
path. mkdir -p plus chmod is idempotent, so an in-place upgrade over
existing 0755 directories converts them on the libsync restart.
This is the change proposed in #22 by dttung2905, closed unmerged by a
stale-PR sweep. Required by the maintainer decision in
kai-scheduler/KAI-Scheduler#1821 (libsync creates /tmp/vgpulock on host
at install) and verified by the merged kai-scheduler/KAI-Scheduler#2033,
which asserts 1777 for both directories.
No values, image, mount, securityContext or webhook change; the rendered
manifest differs from main only inside the libsync command block.
Signed-off-by: blackdragoon26 <sankalp.jha9643@gmail.com>
What type of PR is this?
Chart fix
What this PR does / why we need it:
When I carried out e2e test on kai-scheduler side for item 4 in this kai-scheduler/KAI-Scheduler#1821 , I notice this issue
libvgpu synthesizes its metrics cache under
{CONTAINER_VGPU_MOUNT}/containers/{POD_UID}_{CONTAINER_NAME}. When that host directory is755root, non-root workload containers (common in e2e, UID 1000) fail witherrno=13and fall back to in-container/tmp/cudevshr.cache, so kai-vgpu-monitor never sees per-containerhami_*metrics.This change has libsync create
containers/and/tmp/vgpulockup front andchmod 1777them so non-root pods can create their cache dirs while still writing the shared cache on the host path the monitor scrapes.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Summary by CodeRabbit