Add rootless Docker support and update documentation#1549
Add rootless Docker support and update documentation#1549sireeshajonnalagadda wants to merge 13 commits intodevcontainers:mainfrom
Conversation
Kaniska244
left a comment
Kaniska244
left a comment
There was a problem hiding this comment.
I have left few comments. Please get back in case of any issues.
| buildx=(docker-buildx-plugin) | ||
| fi | ||
| apt-get -y install --no-install-recommends ${cli_package_name}${cli_version_suffix} "${buildx[@]}" docker-compose-plugin | ||
| if [ "${DOCKER_DASH_COMPOSE_VERSION}" != "v1" ]; then |
There was a problem hiding this comment.
What is the relevance of this check for rootless Docker support?
There was a problem hiding this comment.
+1 @sireeshajonnalagadda Did we not need this before; if so, how? Also, since they are both very similiar, can we do:
install cli package + buildx
if not Docker v1
install docker-compose-plugin
There was a problem hiding this comment.
Why do we have this documentation in the test directory? If any change required, couldn't this be done in the readme file or even as PR comment if applicable?
There was a problem hiding this comment.
Sorry, the file was misplaced. The rootless socket information has already been added to README.md. ROOTLESS_DOCKER.md provides clear examples on how to configure d-o-d feature with rootless docker setup. I will remove it from here and add a summary in the PR comment.
Thanks!
| ], | ||
| "containerUser": "vscode" | ||
| }, | ||
| "xdg_runtime_dir_socket": { |
There was a problem hiding this comment.
Why do we really need this test when it appears to be quite similar to rootless_docker_socket scenario?
There was a problem hiding this comment.
This just another variant of using custom socket path. If it appears redundant I will remove this.
There was a problem hiding this comment.
Agreed, could you please remove this too?
| "moby": false | ||
| } | ||
| }, | ||
| "mounts": [ |
There was a problem hiding this comment.
Do we really need the explicit mounts for this case as I guess this should be the default behavior. isn't it?
…eSwitch and remove obsolete ROOTLESS_DOCKER.md documentation
| }, | ||
| "containerUser": "vscode" | ||
| }, | ||
| "custom_rootless_socket_path": { |
There was a problem hiding this comment.
Isn't this identical to rootless_docker_socket (just with different socketPaths)? Both are custom.
abdurriq
left a comment
abdurriq
left a comment
There was a problem hiding this comment.
Implementation LGTM, some test changes requested.
3 participants
This PR fixes the issue for rootless docker
Checklist
Changes works as expected for both root and rootless docker environments.
Key Points
socketPathoption value/run/user/{uid}for rootless DockerTest Scenarios
The test scenarios demonstrate:
rootless_docker_socket: Standard rootless configurationcustom_rootless_socket_path: Custom socket pathxdg_runtime_dir_socket: XDG runtime directory styleroot_docker_socket: Standard root Docker (for comparison)