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/systemd: allow preStart with other ExecStartPre cmdlines #109976

Merged

Conversation

roberth
Copy link
Member

@roberth roberth commented Jan 19, 2021

Declaring them as lists enables the concatenation, supporting
lib.mkBefore, lib.mkOrder, etc.

This leaves all existing configurations as they are.

New configurations will be possible that have both preStart and serviceConfig.ExecStartPre = lib.mkBefore [ ...... ] in services.

Same for ExecStartPost.

Motivation for this change

This is useful when you need to extend a service with a pre-start
script that needs to run as root.
For example, when a service configuration needs custom filesystem initialization.

It may also be useful for merging external secrets into configuration, for non-dynamicuser services or depending on the outcome of NixOS/rfcs#59 (comment)

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.

Declaring them as lists enables the concatenation, supporting
lib.mkBefore, lib.mkOrder, etc.

This is useful when you need to extend a service with a pre-start
script that needs to run as root.
@roberth
Copy link
Member Author

roberth commented Jan 19, 2021

@GrahamcOfBorg test redis

@aanderse
Copy link
Member

Any interest in ExecStartPost while you're in there?

@roberth
Copy link
Member Author

roberth commented Jan 19, 2021

Any interest in ExecStartPost while you're in there?

✔️

@roberth roberth requested a review from aanderse January 21, 2021 22:00
Copy link
Member

@aanderse aanderse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome! I've seen a couple issues come up over the past year that this would have solved. Thanks ✨

@roberth roberth merged commit bbaff89 into NixOS:master Jan 22, 2021
7c6f434c added a commit that referenced this pull request Feb 12, 2021
Follow-up to: nixos/systemd: allow preStart with other ExecStartPre cmdlines #109976

As the additional ExecStartPre and ExecStartPost are now lists, update
their processing by service-runner.nix
mweinelt pushed a commit to mweinelt/nixpkgs that referenced this pull request May 22, 2021
Follow-up to: nixos/systemd: allow preStart with other ExecStartPre cmdlines NixOS#109976

As the additional ExecStartPre and ExecStartPost are now lists, update
their processing by service-runner.nix

(cherry picked from commit 9486375)
ivanbakel added a commit to ivanbakel/nixpkgs that referenced this pull request Mar 23, 2023
The definition of `ExecStartPre` used for redis is non-composable, and
causes issues if a module tries to define `redis.preStart` (despite the
fact that these definitions can be compatible.)

This uses the approach suggested by
NixOS#109976
to allow for composing use of `preStart` and `ExecStartPre`, by making
the default value of `ExecStartPre` defined internally a list with
`mkBefore`.
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

2 participants