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: reload on config change #74379

Merged
merged 1 commit into from Mar 31, 2020

Conversation

bachp
Copy link
Member

@bachp bachp commented Nov 27, 2019

The current module on master is broken with gitlab-runner 12.5.0 as it tries to create a Config.toml lock next to it's config file, which is located in the nix store and thus not writable.

This fixes this issue.

Also with this change it is no longer required to restart the runner on every
change. Instead it can just reload it's config while running.

This is an alternative to #53048 with the additional advantage that the config is located in the usual location under /etc.

Motivation for this change
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 nix-review --run "nix-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.
Notify maintainers

cc @max-wittig @zimbatm @globin @makefu @fpletz @flokli @bgamari

With this change it is no longer required to restart the runner on every
change. Instead it can just reload it's config while running.
@max-wittig
Copy link
Contributor

@zimbatm @globin @makefu @fpletz @flokli @bgamari The runner is currently broken. Any chance you could take a look?

@zimbatm
Copy link
Member

zimbatm commented Dec 2, 2019

Is it possible to specify to the gitlab-runner where to put the lockfile? /etc is not really meant to be writable on NixOS.

@flokli
Copy link
Contributor

flokli commented Dec 2, 2019

Yeah, writing to /etc will probably not work.

We could do some libredirect hackery to point the lock file elsewhere, but this really should be fixed properly upstream - gitlab-runner doesn't really play nice with CasC in general…

I left feedback upstream at https://gitlab.com/gitlab-org/gitlab-runner/issues/4876#note_253544148.

If somebody would be up to do a MR implementing https://gitlab.com/gitlab-org/gitlab-runner/issues/5024, we could fetchpatch and apply to gitlab-runner until it's merged…

@bachp
Copy link
Member Author

bachp commented Dec 3, 2019

We could also put the config file to /var/lib as proposed in #53048 or even put it into /run.

@flokli
Copy link
Contributor

flokli commented Dec 3, 2019 via email

@max-wittig
Copy link
Contributor

@flokli I've started to work on this: https://gitlab.com/gitlab-org/gitlab-runner/merge_requests/1700

@max-wittig
Copy link
Contributor

Do we want to just patch this? https://gitlab.com/gitlab-org/gitlab-runner/merge_requests/1700/diffs?commit_id=5be5ab6eeaad6c10ef3b692df662547e60a99e88

It does seem like nobody is reacting on GitLab side.

@flokli
Copy link
Contributor

flokli commented Dec 6, 2019 via email

@bachp
Copy link
Member Author

bachp commented Dec 22, 2019

Seems like upstream decided to remove the lock file again https://gitlab.com/gitlab-org/gitlab-runner/issues/4876#note_257319457

@flokli Do you know what state changes gitlab-runner does? The only one I know of is if you run gitlab-runner register which is not usable with the current nixos module.

@flokli
Copy link
Contributor

flokli commented Dec 22, 2019

@bachp

gitlab-runner register asks the user a lot of things (https://gitlab.com/gitlab-org/gitlab-runner/blob/master/commands/register.go#L72), but I assume all that can be pre-populated from the config file.

The registration token is passed down to the RegisterRunner command. For now, only Token is stored back to the config struct (and the private boolean registered, which doesn't end up in config):
https://gitlab.com/gitlab-org/gitlab-runner/blob/master/commands/register.go#L254

@zimbatm
Copy link
Member

zimbatm commented Dec 23, 2019

gitlab-runner has been updated: #76190

This release does no longer contain the lock file that broke the NixOS
module in 12.5.0

@bachp
Copy link
Member Author

bachp commented Mar 31, 2020

Does anybody still want dynamic config reload?

@flokli
Copy link
Contributor

flokli commented Mar 31, 2020

Yes please :-)

I assume we can merge this even with https://gitlab.com/gitlab-org/gitlab-runner/issues/5024#note_273315256 being stuck.

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

Successfully merging this pull request may close these issues.

None yet

5 participants