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

github-runners: move workDir outside of /run #1018

Merged
merged 3 commits into from
Jul 31, 2024

Conversation

Enzime
Copy link
Collaborator

@Enzime Enzime commented Jul 27, 2024

Turns out I messed up when testing #1013 and forgot to reboot when testing and the PR didn't actually work.

As /run gets recreated every reboot and we can't specify dependencies for launchd, creating the workDir every reboot will require extra complexity with a separate daemon that runs as root otherwise it won't have sufficient privileges.

As we clean the workDir when the service first starts anyway, it ends up being the same.

As `/run` gets recreated every reboot and we can't specify dependencies
for launchd, creating the `workDir` every reboot will require extra
complexity with a separate daemon that runs as `root` otherwise it won't
have sufficient privileges.

As we clean the `workDir` when the service first starts anyway, it ends
up being the same.
@Enzime Enzime marked this pull request as draft July 27, 2024 01:16
@Enzime
Copy link
Collaborator Author

Enzime commented Jul 27, 2024

Marking as draft until I've finished testing this properly this time :)

@Enzime
Copy link
Collaborator Author

Enzime commented Jul 28, 2024

Currently, changing workDir won't have any effect on existing setups that don't have ephemeral enabled.

# Clean the $RUNNER_ROOT if we are in ephemeral mode
if ${boolToString cfg.ephemeral}; then
echo "Cleaning $RUNNER_ROOT"
${pkgs.findutils}/bin/find "$RUNNER_ROOT" -mindepth 1 -delete
fi
# If the `.runner` file does not exist, we assume the runner is not configured
if [[ ! -f "$RUNNER_ROOT/.runner" ]]; then
${getExe configure}
fi

This is because the logic in NixOS that ensures runner registration is rerun if any configuration changes hasn't been ported yet:

https://github.com/NixOS/nixpkgs/blob/6d8391a3ce154bdf1870d998f187b26de8147065/nixos/modules/services/continuous-integration/github-runner/service.nix#L123-L140

My previous PR makes setting up new GitHub Runners broken as the configure script won't be run with sufficient permissions to create the work directory. This PR will fix setting up new GitHub Runners by changing the default directory and restoring the old logic of using the launchd activation script to create the folder, meaning new setups won't encounter this error.

As I'm short on time at the moment, migrating the logic to rerun runner registration on configuration changes will have to wait for another PR.

As a manual workaround, anyone with an existing setup can delete the file at /var/lib/github-runners/<name>/.runner to force runner registration, this will also require removing the runner in the GitHub UI if you have not set services.github-runners.<name>.replace = true;

@Enzime Enzime marked this pull request as ready for review July 28, 2024 03:31
@emilazy emilazy merged commit 7e08a9d into LnL7:master Jul 31, 2024
6 checks passed
@Enzime Enzime deleted the fix/github-runners-work-dir branch July 31, 2024 19:46
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.

2 participants