Support GitHub Enterprise Server using ARC #59
Loading…
Reference in a new issue
No description provided.
Delete branch "graham/fh-82-nix-installer-action-explore-how-to-support-arc"
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?
Description
This PR improves the action's support for environments unlike GitHub's own hosted runners. In particular, runners that don't expose systemd as PID1. This is common for GitHub Enterprise Server users, which often use something called ARC: https://github.com/actions/actions-runner-controller.
As a happy coincidence, this also closely matches other third party runners, like Namespace.
The implementation here is to fall back to borrowing Docker as a process supervisor if all three of the following are true:
This PR includes a pre-built Docker image for arm64 and amd64 in the repository, since the images are completely empty other than some configuration. This patch also updates the
trusted-usersconfiguration option, to use the OS to get the username instead of using an environment variable, which is not always available.This patch has been tested in GHES and on Namespace, and confirmed to work.
Checklist
@ -0,0 +16,4 @@--timeout=3s \CMD ["/nix/var/nix/profiles/default/bin/nix", "store", "ping", "--store", "daemon"]COPY ./Dockerfile /README.md...seems weird to be copying this Dockerfile into a
README.mdfile in the root of the image...?This looks wrong. Error message says "shim set to true" but this is checking the opposite?
@ -99,0 +145,4 @@}},},});This seems like it should be an error to me...?
@ -99,1 +175,4 @@actions_core.endGroup();}private async executionEnvironment(): Promise<ExecuteEnvironment> {Just double-checking: this will only show up if the action is being run in GHA's "debug" mode (i.e. it failed and you checked the box saying "pls run in debug mode"), right?
@ -0,0 +16,4 @@--timeout=3s \CMD ["/nix/var/nix/profiles/default/bin/nix", "store", "ping", "--store", "daemon"]COPY ./Dockerfile /README.mdIt turns out if the Docker image is actually empty, a lot of Docker's tools break. So I figured the most friendly thing to do is to copy in the Dockerfile to give context about what the thing is. I didn't overthink it, just wanted to put something not entirely useless inside the container.
@ -0,0 +47,4 @@```tar --options gzip:compression-level=9 -zcf ../arm64.tar.gz .```nit/question: I guess this is just building the arm64 version but ... "manually", thus not requiring an arm64 builder?
Do you want a
--restart alwaysor--restart on-failuremaybe?@ -99,1 +175,4 @@actions_core.endGroup();}private async executionEnvironment(): Promise<ExecuteEnvironment> {I should double check that.
Nice idea.
@ -0,0 +16,4 @@--timeout=3s \CMD ["/nix/var/nix/profiles/default/bin/nix", "store", "ping", "--store", "daemon"]COPY ./Dockerfile /README.mdRight, it just seems weird that you're copying in the Dockerfile and not the, y'know,
README.mdin the same directory 😆@ -0,0 +47,4 @@```tar --options gzip:compression-level=9 -zcf ../arm64.tar.gz .```Right. Unfortunately there is no way to say "hey I promise this ~empty image can run a bunch of places :)"
Neat.
@ -0,0 +16,4 @@--timeout=3s \CMD ["/nix/var/nix/profiles/default/bin/nix", "store", "ping", "--store", "daemon"]COPY ./Dockerfile /README.mdThe README is for us :)
@ -99,0 +145,4 @@}},},});I could make it a hard error. My thinking is if someone forces it true, then who am I to say they shouldn't be allowed to keep both pieces? But I can also imagine a thing about being conservative about our inputs. wdyt?
@ -0,0 +16,4 @@--timeout=3s \CMD ["/nix/var/nix/profiles/default/bin/nix", "store", "ping", "--store", "daemon"]COPY ./Dockerfile /README.mdI think it would make more sense if the Dockerfile were copied to
/Dockerfile, but I won't block on that.@ -99,0 +145,4 @@}},},});If it's going to be a warning, it'd be nice to make the "keep both pieces" more explicit somehow ("unsupported" is a lot softer than "this may break at any moment, we're not responsible if it blows up your cat or locks your key inside your car").
@ -99,0 +145,4 @@}},},});I'm inclined to leave it as a warning and indicate we won't setup the docker shim if you're not on Linux.
Fixed, thanks!
@ -99,1 +175,4 @@actions_core.endGroup();}private async executionEnvironment(): Promise<ExecuteEnvironment> {It was in fact doing that, but now it is not :).
Seems fine.