Pass 'sleep' as container command rather than entrypoint #86

Merged
wolfogre merged 2 commits from telackey/act:main into main 2024-03-27 10:17:49 +00:00
Contributor

The current code overrides the container's entrypoint with sleep. Unfortunately, that prevents initialization scripts, such as to initialize Docker-in-Docker, from running.

The change simply moves the sleep to the command, rather than entrypoint, directive.

For most containers of this sort, the entrypoint script performs initialization, and then ends with $@ to execute whatever command is passed.

If the container has no entrypoint, the command is executed directly. As a result, this should be a transparent change for most use cases, while allowing the container's entrypoint to be used when present.

The current code overrides the container's entrypoint with `sleep`. Unfortunately, that prevents initialization scripts, such as to initialize Docker-in-Docker, from running. The change simply moves the `sleep` to the command, rather than entrypoint, directive. For most containers of this sort, the entrypoint script performs initialization, and then ends with `$@` to execute whatever command is passed. If the container has no entrypoint, the command is executed directly. As a result, this should be a transparent change for most use cases, while allowing the container's entrypoint to be used when present.
telackey added 1 commit 2024-01-10 04:56:11 +00:00
All checks were successful
checks / check and test (pull_request) Successful in 1m42s
9ba257f6b3
Pass 'sleep' as container command rather than entrypoint. (#1)
The current code overrides the container's entrypoint with `sleep`.  Unfortunately, that prevents initialization scripts, such as to initialize Docker-in-Docker, from running.

The change simply moves the `sleep` to the command, rather than entrypoint, directive.

For most containers of this sort, the entrypoint script performs initialization, and then ends with `$@` to execute whatever command is passed.

If the container has no entrypoint, the command is executed directly.  As a result, this should be a transparent change for most use cases, while allowing the container's entrypoint to be used when present.

Reviewed-on: telackey/act#1
Co-authored-by: Thomas E Lackey <telackey@bozemanpass.com>
Co-committed-by: Thomas E Lackey <telackey@bozemanpass.com>
lunny requested review from Zettat123 2024-01-15 06:40:42 +00:00
First-time contributor

Thanks for this PR! I found this is extremely useful for starting a dind container for each job, using eg. https://github.com/cruizba/ubuntu-dind/ . This can solve the docker cleanup problem (e.g. gitea/act_runner#189 (comment)), which doesn't appear in Github because there the job is running in a new VM each time.

I tried run the act_runner built with this branch, and it works fine to use docker run in the steps.
However, to use the mentioned addnab/docker-run-action I must manually mount /var/run to share docker socket each time (may be resolved by #80 ):

container:
  image: ubuntu-dind
  volumes: 
    - dockerdind:/var/run

or starting the dockerd with -H tcp://0.0.0.0:2375 --tls=false and put these config for the runner.

runner:
  envs:
    DOCKER_HOST: tcp://${{ env.JOB_CONTAINER }}:2375
Thanks for this PR! I found this is extremely useful for starting a dind container for each job, using eg. https://github.com/cruizba/ubuntu-dind/ . This can solve the docker cleanup problem (e.g. https://gitea.com/gitea/act_runner/issues/189#issuecomment-740636), which doesn't appear in Github because there the job is running in a new VM each time. I tried run the `act_runner` built with this branch, and it works fine to use `docker run` in the steps. However, to use the mentioned `addnab/docker-run-action` I must manually mount `/var/run` to share docker socket each time (may be resolved by #80 ): ```yaml container: image: ubuntu-dind volumes: - dockerdind:/var/run ``` or starting the `dockerd` with `-H tcp://0.0.0.0:2375 --tls=false` and put these config for the runner. ```yaml runner: envs: DOCKER_HOST: tcp://${{ env.JOB_CONTAINER }}:2375 ```
Author
Contributor

I think the latter, setting DOCKER_HOST, makes the most sense. Elsewhere in act/act_runner/gitea the host Docker socket file is bound explicitly to the container under /var/run, and it is best to just avoid confusion and battles over the socket.

I do something similar to your second example, but for my custom container I have ENV DOCKER_HOST "unix:///var/run/dind.sock" in the Dockerfile itself.

I think the latter, setting `DOCKER_HOST`, makes the most sense. Elsewhere in act/act_runner/gitea the host Docker socket file is bound explicitly to the container under /var/run, and it is best to just avoid confusion and battles over the socket. I do something similar to your second example, but for my custom container I have `ENV DOCKER_HOST "unix:///var/run/dind.sock"` in the Dockerfile itself.
First-time contributor

I think the latter, setting DOCKER_HOST, makes the most sense. Elsewhere in act/act_runner/gitea the host Docker socket file is bound explicitly to the container under /var/run, and it is best to just avoid confusion and battles over the socket.

For the first usage, I will set

container:
  docker_host: "-"

In the runner's config file. This will prevent it binding the host Docker socket file. Though, it is still annoying that there is no way to clean that volume between jobs. Not sure if this will cause some problems. So I prefer the second way, too.

I do something similar to your second example, but for my custom container I have ENV DOCKER_HOST "unix:///var/run/dind.sock" in the Dockerfile itself.

I can see this will work for steps directly runing in the job container, what about the actions that use a step container, like addnab/docker-run-action mentioned above?

> I think the latter, setting `DOCKER_HOST`, makes the most sense. Elsewhere in act/act_runner/gitea the host Docker socket file is bound explicitly to the container under /var/run, and it is best to just avoid confusion and battles over the socket. For the first usage, I will set ```yaml container: docker_host: "-" ``` In the runner's config file. This will prevent it binding the host Docker socket file. Though, it is still annoying that there is no way to clean that volume between jobs. Not sure if this will cause some problems. So I prefer the second way, too. > I do something similar to your second example, but for my custom container I have `ENV DOCKER_HOST "unix:///var/run/dind.sock"` in the Dockerfile itself. I can see this will work for steps directly runing in the job container, what about the actions that use a step container, like `addnab/docker-run-action` mentioned above?
wolfogre added 1 commit 2024-03-27 10:16:43 +00:00
All checks were successful
checks / check and test (pull_request) Successful in 31s
f7885ccd48
Merge branch 'main' into main
wolfogre changed title from Pass 'sleep' as container command rather than entrypoint. (#1) to Pass 'sleep' as container command rather than entrypoint 2024-03-27 10:16:58 +00:00
wolfogre approved these changes 2024-03-27 10:17:16 +00:00
wolfogre merged commit a79d81989f into main 2024-03-27 10:17:49 +00:00
Owner

Sorry I didn't consider it thoroughly when reviewing it, now I need to revert this PR. See #107

Sorry I didn't consider it thoroughly when reviewing it, now I need to revert this PR. See #107
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: gitea/act#86
No description provided.