syslog-ng: fix reload service #45078
Conversation
Izorkin
force-pushed
the
syslog-ng-reload-fix
branch
from
1806d53
to
300fe1c
Compare
|
Not sure how I didn't spot that, sorry for that. But you should also test your PR's properly before submitting them. These checkmarks are there for a reason. |
| @@ -88,7 +88,7 @@ in { | |||
| StandardOutput = "null"; | |||
| Restart = "on-failure"; | |||
| ExecStart = "${cfg.package}/sbin/syslog-ng ${concatStringsSep " " syslogngOptions}"; | |||
| ExecReload = "${pkgs.coreutils}/bin/kill -HUP ${pidFile}"; | |||
| ExecReload = "${pkgs.coreutils}/bin/kill -HUP `${pkgs.coreutils}/bin/cat ${pidFile}`"; | |||
There was a problem hiding this comment.
Use
{
PIDFile = pidFile;
ExecReload = "${pkgs.coreutils}/bin/kill -HUP $MAINPID";
}instead
(And make sure to test)
There was a problem hiding this comment.
Both options worked for me.
|
Updated PR and tested. |
| @@ -85,10 +85,11 @@ in { | |||
| after = [ "multi-user.target" ]; # makes sure hostname etc is set | |||
| serviceConfig = { | |||
| Type = "notify"; | |||
| PIDFile = "${pidFile}"; | |||
There was a problem hiding this comment.
Is pidfile required for this to work?
There was a problem hiding this comment.
Worked without PIDFile = "${pidFile}"; Remove?
There was a problem hiding this comment.
systemd does some auto-detection for the main pid through forks, but I think since we have the pidfile already, we might as well pass it to be sure.
Is there a problem with PIDFile = pidFile;? I'm pretty sure the "${..} isn't needed, which is why I proposed this code in the first place.
There was a problem hiding this comment.
Variant ExecReload = "${pkgs.coreutils}/bin/kill -HUP ${pkgs.coreutils}/bin/cat ${pidFile}"; fully working, for the first time and made such a.
There was a problem hiding this comment.
No, use this:
{
PIDFile = pidFile;
ExecReload = "${pkgs.coreutils}/bin/kill -HUP $MAINPID";
}As I said initially
There was a problem hiding this comment.
Updated to PIDFile = pidFile;
Izorkin
force-pushed
the
syslog-ng-reload-fix
branch
from
fa79fbc
to
e4f4589
Compare
|
Whoops I or you should have squashed the commits, I didn't notice. That's 2 (minor) mess ups of me in the last 24 hours :o |
|
Already nothing can be done? |
Motivation for this change
Sorry, PR #45073 it`s wrong
Things done
sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)nix path-info -Sbefore and after)