-
-
Notifications
You must be signed in to change notification settings - Fork 15k
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
nebula: Add restartTrigger
for config file
#253491
Conversation
# Restart nebula when the config file changes. | ||
# Without this, we may get "Current command vanished from the unit file" | ||
# when the file changes and systemd will _not_ restart it. | ||
restartTriggers = [ configFile ]; |
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.
I am not sure what "Current command vanished from the unit file" means and how it affects service restarting.
However, I do not think adding restartTriggers
makes any difference. All restartTriggers
does is adding X-Restart-Triggers = your-triggers
to the service file.
- Since this directive starts with
X-
, systemd ignores it. - Also, the logic of restarting services in
switch-to-configuration.pl
does not do anything special for it. AndconfigFile
has already been used inExecStart
. So there is no difference after addingrestartTriggers
from the perspective ofswitch-to-configuration.pl
.
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.
I am not sure what "Current command vanished from the unit file" means and how it affects service restarting.
It is an error/issue with systemd, see e.g. #49415. Sometimes, when a systemd service's unit file changes (e.g. when the command line changes as part of a NixOS configuration switch), there is a race where systemd notices that change, and then seems to run some "somebody changed my file under me, I'll do nothing about it" logic. Unfortunately it isn't documented in systemd, thus my vague explanation.
The effect of that is that sometimes on NixOS configuration switches, services aren't restarted (#49415), and in this case for Nebula, I noticed that when I change the Nebula config file via configuration.nix
, Nebula was not restarted (thus not observing the config change).
- the logic of restarting services in
switch-to-configuration.pl
does not do anything special for it. AndconfigFile
has already been used inExecStart
. So there is no difference after addingrestartTriggers
from the perspective ofswitch-to-configuration.pl
.
Hm, this is correct. From b606165 the only thing X-Restart-Triggers
does is to be present in the unit file, so that the unit file changes. So you're right that should already tbe the case due to ExecStart = "${netCfg.package}/bin/nebula -config ${configFile}";
.
I am wondering now whether I was just lucky regarding the race and this change indeed had no additional effect on my systems, or whether the fact that a string in the unit file changes that's outside the ExecStart
line ("current command ...") helps systemd avoid the problem somehow.
I'm inclined to close this PR, until we have some proof that the change actually does something.
From my comment:
|
Description of changes
Upstreaming this from my production setup.
Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)CC
nebula
package maintainers @Br1ght0ne @numinit