nixos/github-runner: init at v2.277.1 #116775

veehaitch · 2021-03-18T20:01:18Z

Motivation for this change

Adds GitHub Actions Runner to nixpkgs. Only x86_64-linux is supported right now. The .NET tests are run in the check phase and pass (apart from some online tests).

Required quite a few source patches to make it work with Nix:

Tries to obtain the hash of the latest Git commit. Patched out and replaced by passing the rev from fetchFromGitHub to the property GitInfoCommitHash directly.
Updates itself if a new release is detected. This cannot be disabled (see Disable runner auto update actions/runner#485), hence, it's patched out.
Assumes that its root is writable which is not the case for Nix as it resolves to the read-only Nix store. Introduced an environment variable RUNNER_ROOT which allows changing the root directory.
Tries to install a systemd service when registering a new runner. Also patched out.
The original release also includes Node. Replaced by a symlink to Nix's Node.

The output derivation tries to resemble the structure of the upstream releases where it makes sense. Also, the scripts config.sh and run.sh are fully usable.

Also adds a NixOS module:

If any of the options name, url, runnerGroup, extraLabels, tokenFile or its content changes, a registration is triggered.
Relies on systemd life cycle management and isolation where possible.

Thanks @Trundle for helping out 🙏🏻

Things done

pkgs/development/tools/continuous-integration/github-runner/default.nix

r-rmcgibbo · 2021-03-18T21:15:33Z

Result of nixpkgs-review pr 116775 at 73456884 run on x86_64-linux 1

1 package built successfully:

github-runner

1 suggestion:

warning: missing-phase-hooks
checkPhase should probably contain runHook preCheck and runHook postCheck.

Near pkgs/development/tools/continuous-integration/github-runner/default.nix:174:3:
```
    |
174 |   checkPhase = ''
    |   ^
```

Also note that not only a change to the option value will trigger a reconfiguration but also modifications to the file's content.

Adds a parent directory "github-runner/" to all of the systemd lifecycle directories StateDirectory=, RuntimeDirectory= and LogDirectory=. Doing this has two motivations: 1. Something like this would required if we want to support multiple runners configurations. Please note that this is already possible using NixOS containers. 2. Having an additional parent directory makes it easier to remap any of the directories. Without a parent, systemd is going to complain if, for example, the given StateDirectory= is a symlink.

kfollesdal · 2021-03-28T22:01:05Z

nixos/modules/services/continuous-integration/github-runner.nix

+        Changing this option triggers a new runner registration.
+      '';
+      example = "nixos";
+      default = null;


I have testet your PR, work fine so far :-) But get following error if I do not set option name.

error: cannot coerce null to a string at /nix/store/rfxj941pv6yf3p2rikhx8wsw3y1q7m34-source/nixos/modules/services/continuous-integration/github-runner.nix:6:18: 5| svcName = "github-runner"; 6| systemdUser = "${svcName}-${cfg.name}"; | ^ 7| systemdDir = "${svcName}/${cfg.name}";

Looks like hostname is not set as default as described. But maybe I have done something wrong? But after setting name it works fine for me.

Good catch, thanks! Hopefully this is fixed in 9dcd2ec.

Until now, the runner registration did not set the `--name` argument if the configuration option was `null`, the default for the option. According to GitHub's documentation, this instructs the registration script to use the machine's hostname. This commit causes the registration to always pass the `--name` argument to the runner configuration script. The option now defaults to `networking.hostName` which should be always set on NixOS. This change becomes necessary as the systemd service name includes the name of the runner since fcfa809 and, hence, expects it to be set. Thus, an unset `name` option leads to an error.

Forcing a `name` option to comply with a pattern which could also be used as a hostname is probably not required by GitHub.

User and group naming restrictions are a complex topic [1] that I don't even want to touch. Let systemd figure out the username and group and reference it in our scripts through the USER environment variable. [1] https://systemd.io/USER_NAMES/

The escaping applied to the subdirectory paths given to StateDirectory=, RuntimeDirectory= and LogsDirectory= apparently doesn't use the same strategy that is used to escape unit names (cf. systemd-escape(1)). This makes it unreasonably hard to construct reliable paths which work for StateDirectory=/RuntimeDirectory=/LogsDirectory= and ExecStartPre=. Against this background, I decided to (re-)apply restrictions to the name a user might give for the GitHub runner. The pattern for `networking.hostName` seems like a reasonable choice, also as its value is the default if the `name` option isn't set. This reverts commit 193ac67.

domenkozar · 2021-04-10T10:00:12Z

@veehaitch is this ready?

veehaitch · 2021-04-10T10:07:48Z

@veehaitch is this ready?

I'd say: yes! I've been using the runner without any issues since opening this PR.

Obviously, there's room for improvement. Mainly support for multiple runner definitions (currently, we can achieve this with containers) and runner registration using a personal access token. I'd suggest, however, to implement those improvements in follow-ups. This PR is pretty big already.

zimbatm · 2021-04-10T10:17:17Z

sweet!

mweinelt · 2021-04-10T10:57:36Z

@zimbatm This causes an eval error, because the kerberos attribute isn't passed.

anonymous function at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/ofborg-evaluator-2/pkgs/development/tools/continuous-integration/github-runner/default.nix:1:1 called without required argument 'kerberos', at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/ofborg-evaluator-2/lib/customisation.nix:69:16

https://gist.github.com/GrahamcOfBorg/2f92163fa71808194f67a2b60974f5a3

domenkozar · 2021-04-10T11:49:04Z

How come CI didn't catch that?

veehaitch · 2021-04-10T11:52:51Z

dcb501f99325 wasn't included yet in this branch's base

SuperSandro2000 · 2021-04-10T12:08:50Z

How come CI didn't catch that?

The ofborg results are a week old I think.

dhess · 2021-04-10T18:10:21Z

Thanks, this is fantastic!

ofborg bot added 6.topic: nixos 8.has: module (update) 8.has: package (new) 11.by: package-maintainer 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 labels Mar 18, 2021

SuperSandro2000 reviewed Mar 18, 2021

View reviewed changes

zowoq requested a review from zimbatm March 18, 2021 21:45

veehaitch requested a review from rickynils March 19, 2021 00:22

veehaitch added 9 commits March 24, 2021 22:31

github-runner: init at 2.277.1

67da734

nixos/github-runner: initial version

a10f465

nixos/github-runner: add warning if tokenFile in Nix store

b4d9c31

github-runner: don't accept unexpected attrs

ac82082

github-runner: formatting nits

fd1f0d2

github-runner: add pre and post hooks to checkPhase

a12d9d8

nixos/github-runner: update ExecStartPre= comment

320479b

nixos/github-runner: adapt tokenFile option description

9a51add

Also note that not only a change to the option value will trigger a reconfiguration but also modifications to the file's content.

nixos/github-runner: remove mkDefault for DynamicUser=

08df6fa

zimbatm mentioned this pull request Mar 25, 2021

WIP: github-actions nix-community/infra#5

Closed

veehaitch added 2 commits March 27, 2021 16:14

nixos/github-runner: use specifier to get abs runtime path

fcfa809

kfollesdal reviewed Mar 28, 2021

View reviewed changes

veehaitch added 7 commits March 29, 2021 00:19

nixos/github-runner: use types.str for name option

193ac67

Forcing a `name` option to comply with a pattern which could also be used as a hostname is probably not required by GitHub.

nixos/github-runner: pass dir paths explicitly for ExecStartPre=

c4bd4d3

nixos/github-runner: update variable and script naming

929aa7a

nixos/github-runner: use types.path for tokenFile option

095f377

veehaitch added 3 commits March 29, 2021 15:18

nixos/github-runner: escape options used as shell arguments

64f4d4a

nixos/github-runner: wait for network-online.target

a68dac1

github-runner: ignore additional online tests

cd24f4f

zimbatm merged commit f4af2f2 into NixOS:master Apr 10, 2021

veehaitch mentioned this pull request Apr 10, 2021

Packaging Github actions runner #101426

Closed

veprbl mentioned this pull request Apr 10, 2021

github-runner: fix eval #119011

Merged

domenkozar mentioned this pull request May 21, 2021

Allow Nix to be installed inside Docker agents as root or without systemd cachix/install-nix-action#43

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos/github-runner: init at v2.277.1 #116775

nixos/github-runner: init at v2.277.1 #116775

veehaitch commented Mar 18, 2021

r-rmcgibbo commented Mar 18, 2021

kfollesdal Mar 28, 2021 •

edited

veehaitch Mar 28, 2021

domenkozar commented Apr 10, 2021

veehaitch commented Apr 10, 2021

zimbatm commented Apr 10, 2021

mweinelt commented Apr 10, 2021 •

edited

domenkozar commented Apr 10, 2021

veehaitch commented Apr 10, 2021

SuperSandro2000 commented Apr 10, 2021

dhess commented Apr 10, 2021

nixos/github-runner: init at v2.277.1 #116775

nixos/github-runner: init at v2.277.1 #116775

Conversation

veehaitch commented Mar 18, 2021

Motivation for this change

Things done

r-rmcgibbo commented Mar 18, 2021

kfollesdal Mar 28, 2021 • edited

Choose a reason for hiding this comment

veehaitch Mar 28, 2021

Choose a reason for hiding this comment

domenkozar commented Apr 10, 2021

veehaitch commented Apr 10, 2021

zimbatm commented Apr 10, 2021

mweinelt commented Apr 10, 2021 • edited

domenkozar commented Apr 10, 2021

veehaitch commented Apr 10, 2021

SuperSandro2000 commented Apr 10, 2021

dhess commented Apr 10, 2021

kfollesdal Mar 28, 2021 •

edited

mweinelt commented Apr 10, 2021 •

edited