-
Notifications
You must be signed in to change notification settings - Fork 5.9k
feat: user-mode docker #192
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
Signed-off-by: Hikari <[email protected]>
Wonderful change. ping @nhooyr |
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 think this is a good change and should debate the semantics on an issue later. This seems to be the last step for code-server
integration in Che so it'd be nice for people to be able to start using it.
The adduser
portion doesn't look that bad to me though, although I know that's not the whole argument. It's minor enough we can reevaluate later
Once the conflicts are resolved I'm good to merge @sr229 |
merge issues fixed, CI seems stuck but PR build reports fine |
I feel like this is worth mentioning, but I made my own changes to shift over to a non-root user that mimic the changes in this PR. The Dockerfile in question is here: https://gist.github.com/davefinster/39825c0fa7cf168b8114d24bd3b3df53 When I first deployed the container and everything worked except Workspace creation. The container was setup via Kube using these params: command: ["code-server"]
args:
- --allow-http
- --no-auth
- --port=8440
- --data-dir=/home/coder/.code-server
- /home/coder
workingDir: /home/coder Despite that, I was getting file write errors (due to permissions - non-root user writing into /root so not surprising) whenever VS Code tried to anything Workspace related since it was always attempting to write into /root/.code-server/Workspaces/number/workspace.json regardless of what I did. I ended up having to add these lines to get it to work: mkdir -p /root/.code-server/Workspaces && \
chown -R 3000 /root/.code-server && \
chmod -R 777 /root/.code-server && \
chmod 755 /root |
@davefinster interesting. @sr229 have you experienced that with this dockerfile? |
@davefinster @kylecarbs This is a mirror of my Dockerfile, particularly the Vanilla dockerfile. Every operation works, and you shouldn't be mounting on |
Ignore my comment - turns out it was due to a dirty workspace key in localstorage. |
im abusing the word slap today please help me
Request for another review for merge @coadler @kylecarbs |
@sr229 you need to merge master into your branch again. 😓 |
Should be ready to merge now. |
Describe in detail the problem you had and how this PR fixes it
Prior to discussion to #65, this runs all file operations in the user
coder
in container along with some refactoring of the Dockerfile to make it easier to read.Is there an open issue you can link to?
This resolves #65. This will be blocked until general consensus that this merit as a merge.