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

nixos/gitlab-runner: support multiple services #84139

Merged
merged 1 commit into from May 1, 2020

Conversation

@misuzu
Copy link
Contributor

misuzu commented Apr 2, 2020

Motivation for this change

A more declarative module for gitlab-runner with support for multiple services.
Inspired by @arianvp's module.

Main changes:

  1. Multiple runner services can be registered.
  2. Automatic registration via registration token so same config can be used on many machines.
  3. Runner services is registered/unregistered only when needed (idempotent).
  4. Dead runner services are removed from config on start/reload.
  5. DynamicUser is used so no need for static user and group.
  6. Support for running nix in docker via host nix-daemon.

What is missing:

  1. Global configuration could only be changed by editing /var/lib/gitlab-runner/.gitlab-runner/config.toml. Fixed.

Please comment if you think that something else is missing or would be great to have.

Example config with four runner services:

{ config, pkgs, lib, ...}:
with lib;
{
  services.gitlab-runner = {
    enable = true;
    services = {
      # runner for building via nix in docker
      nix = {
        registrationConfigFile = pkgs.writeText "gitlab-runner-nix-registration" ''
          CI_SERVER_URL=https://gitlab.com/
          REGISTRATION_TOKEN=<token1>
        '';
        dockerImage = "alpine";
        dockerVolumes = [
          "/nix/store:/nix/store:ro"
          "/nix/var/nix/db:/nix/var/nix/db:ro"
          "/nix/var/nix/daemon-socket:/nix/var/nix/daemon-socket:ro"
        ];
        dockerDisableCache = true;
        preBuildScript = pkgs.writeScript "setup-container" ''
          mkdir -p -m 0755 /nix/var/log/nix/drvs
          mkdir -p -m 0755 /nix/var/nix/gcroots
          mkdir -p -m 0755 /nix/var/nix/profiles
          mkdir -p -m 0755 /nix/var/nix/temproots
          mkdir -p -m 0755 /nix/var/nix/userpool
          mkdir -p -m 1777 /nix/var/nix/gcroots/per-user
          mkdir -p -m 1777 /nix/var/nix/profiles/per-user
          mkdir -p -m 0755 /nix/var/nix/profiles/per-user/root
          mkdir -p -m 0700 "$HOME/.nix-defexpr"

          . ${pkgs.nix}/etc/profile.d/nix.sh

          ${pkgs.nix}/bin/nix-env -i ${concatStringsSep " " (with pkgs; [ nix cacert git openssh ])}

          ${pkgs.nix}/bin/nix-channel --add https://nixos.org/channels/nixpkgs-unstable
          ${pkgs.nix}/bin/nix-channel --update nixpkgs
        '';
        environmentVariables = {
          ENV = "/etc/profile";
          USER = "root";
          NIX_REMOTE = "daemon";
          PATH = "/nix/var/nix/profiles/default/bin:/nix/var/nix/profiles/default/sbin:/bin:/sbin:/usr/bin:/usr/sbin";
          NIX_SSL_CERT_FILE = "/nix/var/nix/profiles/default/etc/ssl/certs/ca-bundle.crt";
        };
        tagList = [ "nix" ];
      };
      # runner for building docker images
      docker-images = {
        registrationConfigFile = pkgs.writeText "gitlab-runner-docker-images-registration" ''
          CI_SERVER_URL=https://gitlab.com/
          REGISTRATION_TOKEN=<token2>
        '';
        dockerImage = "docker:stable";
        dockerVolumes = [
          "/var/run/docker.sock:/var/run/docker.sock"
        ];
        tagList = [ "docker-images" ];
      };
      # runner for executing stuff on host system
      # make sure to add required packages (including git!)
      # to `environment.systemPackages`
      shell = {
        registrationConfigFile = pkgs.writeText "gitlab-runner-shell-registration" ''
          CI_SERVER_URL=https://gitlab.com/
          REGISTRATION_TOKEN=<token3>
        '';
        executor = "shell";
        tagList = [ "shell" ];
      };
      # runner for everything else
      default = {
        registrationConfigFile = pkgs.writeText "gitlab-runner-default-registration" ''
          CI_SERVER_URL=https://gitlab.com/
          REGISTRATION_TOKEN=<token4>
        '';
        dockerImage = "debian:stable";
      };
    };
  };
  environment.systemPackages = with pkgs; [ git ];
}
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
@rissson
Copy link
Contributor

rissson commented Apr 2, 2020

You probably want to document the method to use the host's nix-daemon in the runner somewhere

@misuzu
Copy link
Contributor Author

misuzu commented Apr 2, 2020

You probably want to document the method to use the host's nix-daemon in the runner somewhere

Maybe adding a module for this would be even better?

@rissson
Copy link
Contributor

rissson commented Apr 2, 2020

Maybe adding a module for this would be even better?

You could add an option for this, with the proper warning that it exposes the host's store to anything running on that runner

EDIT: actually, as it is, it might cause problems for people with a custom store path

@misuzu misuzu force-pushed the misuzu:gitlab-runner-multi branch 2 times, most recently from 44f3855 to 02d8bcf Apr 2, 2020
@misuzu
Copy link
Contributor Author

misuzu commented Apr 7, 2020

I've added examples for services option, built manual with nix-build nixos/release.nix -A manualHTML.x86_64-linux and checked that there is no display issues.

@flokli
Copy link
Contributor

flokli commented Apr 7, 2020

@arianvp, could you review this?

@misuzu misuzu force-pushed the misuzu:gitlab-runner-multi branch from 02d8bcf to 24a5034 Apr 9, 2020
@misuzu
Copy link
Contributor Author

misuzu commented Apr 9, 2020

Global check_interval and concurrent is now configurable.

@misuzu misuzu force-pushed the misuzu:gitlab-runner-multi branch 5 times, most recently from 882c765 to 93f86b9 Apr 9, 2020
@bachp
Copy link
Member

bachp commented Apr 10, 2020

@misuzu Can't we combine this with the existing gitlab-runner module.

Both modules have some features the other one is missing. Here is a short list of what I think those are.

gitlab-runner-multi

  • multiple services
  • declartive
  • automatic registration 🍾

gitlab-runner

  • runner executable can be selected via services.gitlab-runner.package attribute
  • graceful termination where it can configured how long gitlab-runner should wait till jobs get killed if the runner service is stoped (e.g machine shutdown/reboot).
  • allows changing the working directory
  • better module name

My proposal would be:

  1. Replace the current module with this one but keeping all the currently supported options.
  2. Add the .services attribute as proposed here to gitlab-runner
  3. Allow .configFile to override the config. I think that would cover most usecases of the current module.
  4. Deprecate .configOptions
  5. Maybe rename packages to extraPackages as proposed here.

What do you think?

I think it would be better to have only one gitlab-runner module in nixpkgs.

@misuzu
Copy link
Contributor Author

misuzu commented Apr 10, 2020

Replace the current module with this one but keeping all the currently supported options.

workDir option will not work correctly with DynamicUser. Should DynamicUser be dropped?

Allow .configFile to override the config. I think that would cover most usecases of the current module.

I'll look into this.

I think it would be better to have only one gitlab-runner module in nixpkgs.

The idea was to drop current one since it's broken anyway, but suggested changes sounds reasonable.

@flokli
Copy link
Contributor

flokli commented Apr 11, 2020

I'm also much in favour of improving/merging the existing module.

@misuzu workDir would still work in combination with DynamicUser=true, as long as it's inside /var/lib/gitlab-runner (which is the default).

I'd prefer to restrict the runner to this location over introducing all sort of system-wide requirements.
We could add an assertion asking the user to move things before switching if workDir doesn't currently point into a subdirectory of /var/lib/gitlab-runner.

@bachp
Copy link
Member

bachp commented Apr 11, 2020

As far as I remember workDir is only useful in combination with the shell runner, which I have no experience with. So from my point of view we can also drop the workDir option if nobody has a usecase for it.

@misuzu
Copy link
Contributor Author

misuzu commented Apr 11, 2020

@misuzu workDir would still work in combination with DynamicUser=true, as long as it's inside /var/lib/gitlab-runner (which is the default).

We could add an assertion asking the user to move things before switching if workDir doesn't currently point into a subdirectory of /var/lib/gitlab-runner.

What's the difference if workDir in /var/lib/gitlab-runner or /var/lib/gitlab-runner/some/other/path? Don't really see the point of this option in this case.

As far as I remember workDir is only useful in combination with the shell runner, which I have no experience with. So from my point of view we can also drop the workDir option if nobody has a usecase for it.

I agree.

@flokli
Copy link
Contributor

flokli commented Apr 11, 2020

Yeah, okay, let's drop it, and in the mkRemovedOption module mention state (if any) would need to be migrated to /var/lib/gitlab-runner.

@misuzu misuzu force-pushed the misuzu:gitlab-runner-multi branch from 93f86b9 to f9010c8 Apr 11, 2020
@misuzu misuzu changed the title nixos/gitlab-runner-multi: init nixos/gitlab-runner: support multiple services Apr 11, 2020
@misuzu misuzu force-pushed the misuzu:gitlab-runner-multi branch from 221791f to 4aff1f7 Apr 11, 2020
Copy link
Member

bachp left a comment

With my proposed change this modules works as a drop in replacement for the use cases I have tested.

All docker based runners tough.

@misuzu misuzu force-pushed the misuzu:gitlab-runner-multi branch from 4aff1f7 to 7095937 Apr 11, 2020
@bachp
bachp approved these changes Apr 12, 2020
Copy link
Member

bachp left a comment

Thanks a lot for the improvements to the module. All good from my side now!

FYI @max-wittig

@misuzu misuzu force-pushed the misuzu:gitlab-runner-multi branch 3 times, most recently from 9620830 to 3b80f04 Apr 13, 2020
@misuzu
Copy link
Contributor Author

misuzu commented Apr 17, 2020

Added registrationFlags option for advanced use-cases.

@misuzu misuzu force-pushed the misuzu:gitlab-runner-multi branch from 3b80f04 to ac8d006 Apr 18, 2020
@misuzu
Copy link
Contributor Author

misuzu commented Apr 18, 2020

Added ability to change user for service using systemd.services.gitlab-runner.serviceConfig options. Configuration script will still be executed as root.
Tested changing User to root, unprivileged user and rolling back to DynamicUser, no manual chown's was needed.
Not sure if there should be dedicated options for this.

@misuzu misuzu force-pushed the misuzu:gitlab-runner-multi branch from ac8d006 to c4c855f Apr 18, 2020
@nixos-discourse
Copy link

nixos-discourse commented Apr 18, 2020

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/call-for-testing-gitlab-runner-module/6765/1

@misuzu misuzu force-pushed the misuzu:gitlab-runner-multi branch from c4c855f to d241f5a Apr 30, 2020
@arianvp
Copy link
Member

arianvp commented May 1, 2020

This looks great! what is left? I think this is ready to merge no?

@misuzu misuzu force-pushed the misuzu:gitlab-runner-multi branch from d241f5a to 3853c27 May 1, 2020
@misuzu
Copy link
Contributor Author

misuzu commented May 1, 2020

This looks great! what is left? I think this is ready to merge no?

I think it's feature-complete and people are saying it's working fine so yes, i think it is.
New options should be easy to add if necessary.

@flokli
Copy link
Contributor

flokli commented May 1, 2020

I'm still not a fan of running the configuration script as root, but that can be improved on in a later PR.

@flokli flokli merged commit 4e14ff6 into NixOS:master May 1, 2020
14 checks passed
14 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="3853c27"; rev="3853c2711126d2c1518514222d19be631b609bac"; } ./pkgs/t
Details
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="3853c27"; rev="3853c2711126d2c1518514222d19be631b609bac"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="3853c27"; rev="3853c2711126d2c1518514222d19be631b609bac"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="3853c27"; rev="3853c2711126d2c1518514222d19be631b609bac"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="3853c27"; rev="3853c2711126d2c1518514222d19be631b609bac"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="3853c27"; rev="3853c2711126d2c1518514222d19be631b609bac"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="3853c27"; rev="3853c2711126d2c1518514222d19be631b609bac"; } ./pkgs/t
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@flokli
Copy link
Contributor

flokli commented May 1, 2020

Thanks everyone!

@FRidh
Copy link
Member

FRidh commented May 2, 2020

I reverted this when merging master into staging-next to avoid the merge conflict. Please rebase it unto staging-next and submit again.

@FRidh FRidh mentioned this pull request May 2, 2020
0 of 10 tasks complete
@misuzu misuzu mentioned this pull request May 2, 2020
3 of 10 tasks complete
@misuzu
Copy link
Contributor Author

misuzu commented May 2, 2020

I reverted this when merging master into staging-next to avoid the merge conflict. Please rebase it unto staging-next and submit again.

Done! #86561

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
You can’t perform that action at this time.