gitea
/
act_runner
56
87
Fork 66
Code Issues 109 Pull Requests 10 Actions Releases 17 Activity

Run as a container (#8) including Docker-in-Docker. #84

Merged
wolfogre merged 2 commits from telackey/act_runner:telackey/dind into main 2023-04-11 02:58:13 +00:00
Conversation 17 Commits 2 Files Changed 6 +74 -1
telackey commented 2023-03-27 18:42:52 +00:00
Contributor
Copy Link

This adds a very simple Dockerfile and run script for running act_runner as a container.

It also allows setting Privileged and ContainerOptions flags via the new config file when spawning task containers. The combination makes it possible to use Docker-in-Docker (if installed in the select task executor image), which requires privileged mode, as well as pass any other options child Docker containers may require.

For example, if Gitea is running in Docker on the same machine, for the checkout action to behave as expected from a task container launched by act_runner, it might be necessary to map the hostname via something like:

container:
  network_mode: bridge
  privileged: true
  options: --add-host=my.gitea.hostname:host-gateway

NOTE: Description updated to reflect latest code.
NOTE: Description updated to reflect latest code (again).

This adds a very simple Dockerfile and run script for running `act_runner` as a container. It also allows setting `Privileged` and `ContainerOptions` flags via the new config file when spawning task containers. The combination makes it possible to use Docker-in-Docker (if installed in the select task executor image), which requires `privileged` mode, as well as pass any other options child Docker containers may require. For example, if Gitea is running in Docker on the same machine, for the `checkout` action to behave as expected from a task container launched by `act_runner`, it might be necessary to map the hostname via something like: ``` container: network_mode: bridge privileged: true options: --add-host=my.gitea.hostname:host-gateway ``` > NOTE: Description updated to reflect latest code. > NOTE: Description updated to reflect latest code (again).
❤️ 3 🎉 2
telackey added 5 commits 2023-03-27 18:42:52 +00:00
Some checks failed
check and test
Details
9364ee71fa s/+/a/
telackey added 1 commit 2023-03-27 19:08:29 +00:00
Some checks failed
check and test
Details
7ddec9699a Fix spacing.
lunny added the
kind
feature
 label 2023-03-28 00:15:16 +00:00
lunny reviewed 2023-03-28 00:16:21 +00:00
Dockerfile Outdated
@ -0,0 +5,4 @@
RUN make build
FROM ubuntu:22.04 as runner
lunny commented 2023-03-28 00:16:21 +00:00
Owner
Copy Link

The base image looks too big? Maybe we could use a smaller image.

The base image looks too big? Maybe we could use a smaller image.
telackey marked this conversation as resolved
lunny reviewed 2023-03-28 16:28:23 +00:00
runtime/task.go Outdated
@ -79,6 +79,7 @@ func NewTask(forgeInstance string, buildID int64, client client.Client, runnerEn
Input: &TaskInput{
envs: runnerEnvs,
containerNetworkMode: "bridge", // TODO should be configurable
privileged: true, // TODO should be configurable
lunny commented 2023-03-28 16:28:23 +00:00
Owner
Copy Link

Is this safe enough for forked repository pull request?

Is this safe enough for forked repository pull request?
telackey marked this conversation as resolved
telackey added 1 commit 2023-03-28 20:22:39 +00:00
All checks were successful
check and test
Details
fdd29d2eeb Use more compact image.
telackey added 1 commit 2023-03-28 20:43:28 +00:00
telackey added 1 commit 2023-03-28 20:48:59 +00:00
telackey added 1 commit 2023-03-28 20:51:13 +00:00
All checks were successful
check and test
Details
0ba4f94676 Merge
telackey added 1 commit 2023-03-28 20:53:03 +00:00
All checks were successful
check and test
Details
de6abab9a8 network mode
telackey commented 2023-03-28 21:02:00 +00:00
Author
Contributor
Copy Link

In response to comments, I updated the the code to add the following configuration options:

dockerContainerOptions / GITEA_RUNNER_DOCKER_CONTAINER_OPTIONS
dockerNetworkMode / GITEA_RUNNER_DOCKER_NETWORK_MODE default:"bridge"
dockerPrivileged / GITEA_RUNNER_DOCKER_PRIVILEGED default:"false"

That way privileged mode can remain disabled by default, for those not needing dind support, and additional container options can be specified more easily (eg, --add-host=my.gitea.host:host-gateway).

NOTE: Outdated by config file.

In response to comments, I updated the the code to add the following configuration options: ``` dockerContainerOptions / GITEA_RUNNER_DOCKER_CONTAINER_OPTIONS dockerNetworkMode / GITEA_RUNNER_DOCKER_NETWORK_MODE default:"bridge" dockerPrivileged / GITEA_RUNNER_DOCKER_PRIVILEGED default:"false" ``` That way `privileged` mode can remain disabled by default, for those not needing dind support, and additional container options can be specified more easily (eg, `--add-host=my.gitea.host:host-gateway`). > NOTE: Outdated by config file.
telackey commented 2023-03-28 21:04:35 +00:00
Author
Contributor
Copy Link

I also switched to alpine for the act_runner base image, to slim it down. The final size dropped from 159MB to 38MB.

I also switched to alpine for the `act_runner` base image, to slim it down. The final size dropped from 159MB to 38MB.
❤️ 1
telackey added 1 commit 2023-03-28 21:07:54 +00:00
All checks were successful
check and test
Details
f5206b44d4 Typo
telackey commented 2023-03-28 21:10:41 +00:00
Author
Contributor
Copy Link

Incidentally, I'm open to leaving Dockerfile.task-executor out of this PR, as that exact image is not required, it is simply a working example.

Incidentally, I'm open to leaving `Dockerfile.task-executor` out of this PR, as that exact image is not required, it is simply a working example.
telackey added 1 commit 2023-03-28 21:26:12 +00:00
lunny commented 2023-03-29 00:08:54 +00:00
Owner
Copy Link

Incidentally, I'm open to leaving Dockerfile.task-executor out of this PR, as that exact image is not required, it is simply a working example.

Yes, at least not this PR.

> Incidentally, I'm open to leaving `Dockerfile.task-executor` out of this PR, as that exact image is not required, it is simply a working example. Yes, at least not this PR.
telackey added 1 commit 2023-03-29 01:07:03 +00:00
All checks were successful
check and test
Details
0cf83294c7 Remove task executor Dockefile.
telackey added 1 commit 2023-03-29 01:07:59 +00:00
All checks were successful
check and test
Details
3350b21606 Remove env.example
telackey commented 2023-03-29 01:08:31 +00:00
Author
Contributor
Copy Link

Incidentally, I'm open to leaving Dockerfile.task-executor out of this PR, as that exact image is not required, it is simply a working example.

Yes, at least not this PR.

Done.

> > Incidentally, I'm open to leaving `Dockerfile.task-executor` out of this PR, as that exact image is not required, it is simply a working example. > > Yes, at least not this PR. Done.
lunny reviewed 2023-03-29 01:45:01 +00:00
runtime/runtime.go Outdated
@ -33,3 +36,2 @@
}
if s.CacheHandler != nil {
env["ACTIONS_CACHE_URL"] = s.CacheHandler.ExternalURL() + "/"
env["ACTIONS_CACHE_URL"] = s.CacheHandler.ExternalURL() + "/"
lunny commented 2023-03-29 01:45:01 +00:00
Owner
Copy Link

Why this changed? s.CacheHandler maybe nil?

Why this changed? s.CacheHandler maybe nil?
telackey marked this conversation as resolved
telackey added 1 commit 2023-03-29 15:27:37 +00:00
All checks were successful
check and test
Details
1bfe39fd2e Fix merge error.
telackey added 1 commit 2023-03-29 15:29:32 +00:00
All checks were successful
check and test
Details
fc45ca8f78 Merge branch 'main' into telackey/dind
telackey commented 2023-03-29 15:31:47 +00:00
Author
Contributor
Copy Link

For some reason my replies to the comment about ACTIONS_CACHE_URL are not being saved. The short answer is that was a good catch, as it was an inadvertent change, the result of a merge error when I rebased.

It is fixed now.

For some reason my replies to the comment about `ACTIONS_CACHE_URL` are not being saved. The short answer is that was a good catch, as it was an inadvertent change, the result of a merge error when I rebased. It is fixed now.
telackey added 1 commit 2023-03-29 15:37:38 +00:00
All checks were successful
check and test
Details
b2d02a4a4d Add comment
telackey commented 2023-04-06 19:03:18 +00:00
Author
Contributor
Copy Link

Are there any other changes or improvements you would like me to make to this PR?

Are there any other changes or improvements you would like me to make to this PR?
telackey requested review from lunny 2023-04-06 19:03:36 +00:00
telackey requested review from wolfogre 2023-04-06 19:04:15 +00:00
lunny commented 2023-04-07 00:06:18 +00:00
Owner
Copy Link

Please resolve the conflicts. I will have a test again after that.

Please resolve the conflicts. I will have a test again after that.
wolfogre commented 2023-04-07 06:41:01 +00:00
Owner
Copy Link

Sorry for causing conflicts in this PR. They mainly come from Refactor environment variables to configuration and registration, but it has been in my mind for a long time, see #21.

Sorry for causing conflicts in this PR. They mainly come from [Refactor environment variables to configuration and registration](https://gitea.com/gitea/act_runner/pulls/90), but it has been in my mind for a long time, see #21.
telackey force-pushed telackey/dind from b2d02a4a4d to c94db953fe 2023-04-11 02:29:55 +00:00 Compare
telackey commented 2023-04-11 02:32:09 +00:00
Author
Contributor
Copy Link

It was more like a port than a merge, but it seems to be working for me. The new code also simplified things a good deal.

It was more like a port than a merge, but it seems to be working for me. The new code also simplified things a good deal.
telackey added 1 commit 2023-04-11 02:47:57 +00:00
All checks were successful
checks / check and test (pull_request) Successful in 1m34s
Details
8d9bc970d5 Show the default as false (which it is)
lunny approved these changes 2023-04-11 02:55:37 +00:00
wolfogre approved these changes 2023-04-11 02:57:51 +00:00
wolfogre left a comment
Owner
Copy Link

LGTM

LGTM
wolfogre merged commit 5a8134410d into main 2023-04-11 02:58:13 +00:00
telackey deleted branch telackey/dind 2023-04-11 17:27:21 +00:00
telackey commented 2023-04-11 17:27:39 +00:00
Author
Contributor
Copy Link

Great, thanks!

Great, thanks!
vbrandl commented 2023-04-13 10:25:05 +00:00
Contributor
Copy Link

Are there plans to publish this to a public registry, so it can be used in docker-compose setups?

Are there plans to publish this to a public registry, so it can be used in docker-compose setups?
lunny commented 2023-04-13 10:33:24 +00:00
Owner
Copy Link

I found there are some problems, I think we can release it to dockerhub or other places once it fixed.

I found there are some problems, I think we can release it to dockerhub or other places once it fixed.
telackey commented 2023-04-13 16:01:39 +00:00
Author
Contributor
Copy Link

Let me know if there is anything I need to do/fix.

Let me know if there is anything I need to do/fix.
lunny commented 2023-04-13 16:32:36 +00:00
Owner
Copy Link

Let me know if there is anything I need to do/fix.

The running container cannot be shutdown with ctrl + C

> Let me know if there is anything I need to do/fix. The running container cannot be shutdown with ctrl + C
telackey commented 2023-04-13 18:05:32 +00:00
Author
Contributor
Copy Link

Let me know if there is anything I need to do/fix.

The running container cannot be shutdown with ctrl + C

I'm not able to reproduce this, at least not at first glance:

❯ docker run -it cerc/act-runner:local
level=info msg="Registering runner, arch=amd64, os=linux, version=0.1.1."
level=error msg="Invalid input, please re-run act command." error="instance address is empty"
Waiting to retry ...
^C
❯ echo $?
130

One thing that might be worth trying is to explicitly trap Ctrl-C:

diff --git a/run.sh b/run.sh
index aa8f456..b2f8208 100755
--- a/run.sh
+++ b/run.sh
@@ -1,5 +1,12 @@
 #!/usr/bin/env bash

+trap ctrl_c INT
+
+function ctrl_c() {
+  echo "aborted" 1>&2
+  exit 129
+}
+
 if [[ ! -d /data ]]; then
   mkdir -p /data
 fi

After which I get:

❯ docker run -it cerc/act-runner:local
level=info msg="Registering runner, arch=amd64, os=linux, version=0.1.1."
level=error msg="Invalid input, please re-run act command." error="instance address is empty"
Waiting to retry ...
^Caborted
❯ echo $?
129
> > Let me know if there is anything I need to do/fix. > > The running container cannot be shutdown with ctrl + C I'm not able to reproduce this, at least not at first glance: ``` ❯ docker run -it cerc/act-runner:local level=info msg="Registering runner, arch=amd64, os=linux, version=0.1.1." level=error msg="Invalid input, please re-run act command." error="instance address is empty" Waiting to retry ... ^C ❯ echo $? 130 ``` One thing that might be worth trying is to explicitly trap Ctrl-C: ``` diff --git a/run.sh b/run.sh index aa8f456..b2f8208 100755 --- a/run.sh +++ b/run.sh @@ -1,5 +1,12 @@ #!/usr/bin/env bash +trap ctrl_c INT + +function ctrl_c() { + echo "aborted" 1>&2 + exit 129 +} + if [[ ! -d /data ]]; then mkdir -p /data fi ``` After which I get: ``` ❯ docker run -it cerc/act-runner:local level=info msg="Registering runner, arch=amd64, os=linux, version=0.1.1." level=error msg="Invalid input, please re-run act command." error="instance address is empty" Waiting to retry ... ^Caborted ❯ echo $? 129 ```
vbrandl commented 2023-04-13 23:05:22 +00:00
Contributor
Copy Link

Using podman with the docker compat layer, I can reproduce the described behavior when using docker run local/act_runner without the -it flag.

Trapping INT in run.sh did fix the problem for me.

Using podman with the docker compat layer, I can reproduce the described behavior when using `docker run local/act_runner` without the `-it` flag. Trapping `INT` in `run.sh` did fix the problem for me.
Sign in to join this conversation.
Reviewers
No reviewers
lunny
wolfogre
Labels
Clear labels   
kind
bug
Something is not working

  
kind
build

  
kind
dependencies

  
kind
docs

  
kind
enhancement
New feature

  
kind
feature

  
kind
help wanted
Need some help

  
kind
proposal

  
kind
refactor

  
related
act
Related to act.

  
related
environment
Related to the environment.

  
related
exec

  
related
gitea
Related to Gitea.

  
related
workflow
Related to the workflow file or actions scripts.

  
reviewed
confirmed
confirmed issue

  
reviewed
duplicate
This issue or pull request already exists

  
reviewed
invalid
Something is wrong

  
reviewed
needs feedback

  
reviewed
wontfix
This won't be fixed

No Label
kind
bug
kind
build
kind
dependencies
kind
docs
kind
enhancement
kind
feature
kind
help wanted
kind
proposal
kind
refactor
related
act
related
environment
related
exec
related
gitea
related
workflow
reviewed
confirmed
reviewed
duplicate
reviewed
invalid
reviewed
needs feedback
reviewed
wontfix
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
4 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_runner#84
No description provided.
Delete Branch "telackey/act_runner:telackey/dind"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?