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
buildkite-agent service: declarative hooks and extraConfig option #35129
Conversation
50cfdfc
to
21c030b
Compare
@@ -101,7 +197,7 @@ in | |||
|
|||
systemd.services.buildkite-agent = | |||
let copy = x: target: perms: | |||
"cp -f ${x} ${target}; ${pkgs.coreutils}/bin/chmod ${toString perms} ${target}; "; | |||
"${pkgs.coreutils}/bin/cp -f ${x} ${target}; ${pkgs.coreutils}/bin/chmod ${toString perms} ${target}; "; |
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.
(nitpick) Maybe we could just add coreutils to path = cfg.runtimePackages ++ [ pkgs.coreutils]
and we don't need to hardcode them here (for readability)?
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.
Good idea, thanks. I have fixed that.
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 one nitpick, looking good! :)
This is useful for things like enabling debugging and increasing agent priority, which don't warrant extra module options.
Instead of having to set up a directory containing hook scripts, you can now directly set module options to add hooks.
I assumed they were space-separated, which was wrong. In future it might be better to allow specifying an attrset of strings for the option.
Newer versions of buildkite-agent can find the bootstrap script themselves.
Use the default name value from: https://buildkite.com/docs/agent/configuration
This simplifies the service script and it's probable that many builds will need coreutils anyway.
21c030b
to
e552633
Compare
Thanks @zimbatm @domenkozar :-) |
Motivation for this change
I thought it would be better to be able to set hook scripts as module options rather than manually creating a directory with script files.
There are other possible settings in the buildkite-agent config file which aren't covered by module options, so I have added an
extraConfig
option to handle those.There are also other minor tweaks to the module.
The option changes are backwards compatible.
Things done
build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
/cc @deepfire @zimbatm @domenkozar