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

use tcp DOCKER_HOST instead of sharing docker.sock #177

Merged
merged 2 commits into from Nov 16, 2020

Conversation

Warashi
Copy link
Contributor

@Warashi Warashi commented Nov 12, 2020

docker:dind container creates /var/run/docker.sock with root user and root group.
so, docker command in runner container needs root privileges to use docker.sock and docker action fails because lack of permission.

in this PR, use tcp connection between runner and docker container, so runner container doesn't need root privileges to run docker, and can run docker action.

@Warashi
Copy link
Contributor Author

Warashi commented Nov 12, 2020

this fixes #174

@Warashi
Copy link
Contributor Author

Warashi commented Nov 15, 2020

Is there any difficulties to merge this PR?
If there is, I will try to solve, please tell me.

Copy link
Collaborator

@mumoshu mumoshu left a comment

Choose a reason for hiding this comment

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

Thank you so much for your contribution @Warashi ☺️

I believe that this way should be much better than stumbling to setup consistent group IDs across containers. Let's go ahead.

If possible, would you mind contributing to setup TLS between the runner and dind container, by setting DOCKER_TLS_CERTDIR to a non-empty, shared directory? (Probably you have expertise in it than me, but it seems like explained in the TLS section of https://hub.docker.com/_/docker/

You can also find the context around enabling TLS in my comment at #174 (comment)

@mumoshu mumoshu merged commit 1fd752f into actions:master Nov 16, 2020
@Warashi
Copy link
Contributor Author

Warashi commented Nov 16, 2020

Thank you to merge this 😄

If possible, would you mind contributing to setup TLS between the runner and dind container, by setting DOCKER_TLS_CERTDIR to a non-empty, shared directory?

OK I try. Please give me time.

@Warashi
Copy link
Contributor Author

Warashi commented Nov 16, 2020

Opened #192 to use TLS between the runner and dind container.

@Warashi Warashi deleted the feature/dont-share-dockersock branch November 19, 2020 00:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants