-
Notifications
You must be signed in to change notification settings - Fork 43
fix(remount): ensure mountpoint is a file for files #249
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
Conversation
8df37bb
to
4d94484
Compare
4d94484
to
7ecee4a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution!
Apparently that's not enough, maybe files need extra flags or something, I'll try to investigate:
|
What are you trying to accomplish exactly? Can you provide steps to reproduce? |
Fixed. The file mountpoint must exist The main use case would probably usage of But I'm experimenting with GPUs, the NVIDIA container runtime mounts a lot of things. Now I'm back to the known issue with Kaniko pod.yamlapiVersion: v1
kind: Pod
metadata:
name: envbuilder
spec:
containers:
- name: envbuilder
image: ghcr.io/coder/envbuilder-preview
env:
- name: FALLBACK_IMAGE
value: busybox
- name: INIT_SCRIPT
value: sh
resources:
limits:
nvidia.com/gpu: "1"
securityContext:
privileged: true These paths need privileges to be remounted, I am thinking Line 436 in b065656
Edit: I have a workaround for the Kaniko issue, I'll open a PR for that too |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a couple of changes in a separate PR:
- Modified our existing integration test to correctly reproduce the issue you mention (we weren't mounting RO) 734cefc
- Added IgnorePaths when performing temp remount 20c0145
Let me know if this looks good to you! If you're happy, you can cherry-pick those commits into this PR or we can continue with the other one!
if options.LayerCacheDir != "" { | ||
ignorePaths = append(ignorePaths, options.LayerCacheDir) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, nice catch 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hehe yes, it was ignoring everything 😅
Thank you @johnstcn, this looks good to me. There is more to consider for NVIDIA and I am not sure |
Thanks for the collaboration @maxbrunet! |
Hi, how did you solve this? I'm running into the same issue at the moment and I'm stuck :S |
Fixes errors like:
Follow up to #183