-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
Conversation
nixos/modules/services/continuous-integration/gitlab-runner-multi.nix
Outdated
Show resolved
Hide resolved
You probably want to document the method to use the host's |
Maybe adding a module for this would be even better? |
nixos/modules/services/continuous-integration/gitlab-runner-multi.nix
Outdated
Show resolved
Hide resolved
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 |
44f3855
to
02d8bcf
Compare
I've added examples for |
@arianvp, could you review this? |
02d8bcf
to
24a5034
Compare
Global |
882c765
to
93f86b9
Compare
@misuzu Can't we combine this with the existing Both modules have some features the other one is missing. Here is a short list of what I think those are.
My proposal would be:
What do you think? I think it would be better to have only one gitlab-runner module in nixpkgs. |
I'll look into this.
The idea was to drop current one since it's broken anyway, but suggested changes sounds reasonable. |
I'm also much in favour of improving/merging the existing module. @misuzu I'd prefer to restrict the runner to this location over introducing all sort of system-wide requirements. |
As far as I remember |
What's the difference if
I agree. |
Yeah, okay, let's drop it, and in the mkRemovedOption module mention state (if any) would need to be migrated to |
93f86b9
to
f9010c8
Compare
ce3aea3
to
221791f
Compare
nixos/modules/services/continuous-integration/gitlab-runner.nix
Outdated
Show resolved
Hide resolved
221791f
to
4aff1f7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With my proposed change this modules works as a drop in replacement for the use cases I have tested.
All docker based runners tough.
nixos/modules/services/continuous-integration/gitlab-runner.nix
Outdated
Show resolved
Hide resolved
4aff1f7
to
7095937
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for the improvements to the module. All good from my side now!
FYI @max-wittig
9620830
to
3b80f04
Compare
Added |
3b80f04
to
ac8d006
Compare
Added ability to change user for service using |
ac8d006
to
c4c855f
Compare
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 |
nixos/modules/services/continuous-integration/gitlab-runner.nix
Outdated
Show resolved
Hide resolved
c4c855f
to
d241f5a
Compare
nixos/modules/services/continuous-integration/gitlab-runner.nix
Outdated
Show resolved
Hide resolved
This looks great! what is left? I think this is ready to merge no? |
nixos/modules/services/continuous-integration/gitlab-runner.nix
Outdated
Show resolved
Hide resolved
d241f5a
to
3853c27
Compare
I think it's feature-complete and people are saying it's working fine so yes, i think it is. |
I'm still not a fan of running the configuration script as root, but that can be improved on in a later PR. |
Thanks everyone! |
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 |
Motivation for this change
A more declarative module for
gitlab-runner
with support for multiple services.Inspired by @arianvp's module.
Main changes:
DynamicUser
is used so no need for static user and group.nix
in docker via hostnix-daemon
.What is missing:
Global configuration could only be changed by editing. Fixed./var/lib/gitlab-runner/.gitlab-runner/config.toml
Please comment if you think that something else is missing or would be great to have.
Example config with four runner services:
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)