<arianvp>
Personally I've just embraced reboot-to-activate
<arianvp>
Its not great for desktops (though if it's as quick as Chromebook I'm actually ok with it)
<arianvp>
All these things are so hard to get right :(
<andi->
This is a non-issue for all services that symlink their config to a static place. If you then tell the unit which DRV triggers a rebuild it just works. I am pretty sure the old system was build with this in mind. Now we have an increasing amount of services that no longer use such an indirection. For services that only (usually) have one instance that isn't a problem. If we talk about
<andi->
services that have multiple we just need a directory per service where the config is at. Not the cleanest approach but it would make reload just work. Restarts only happen on binary upgrades or cmdline argument changes.
<andi->
Obviously our complex pre-start script aren't helping with the situation. There is no way to tell if they are just there to setup a skeleton for the service or if that setups changes when some config options change. We would have to be very careful about this. IMHO we should provide some abstraction for the obvious configuration file case and we can't properly deal with the others without
<andi->
imposing too much manual care.
pbb has quit [Remote host closed the connection]
pbb has joined #nixos-systemd
pbb has quit [Quit: No Ping reply in 180 seconds.]
pbb has joined #nixos-systemd
pbb has quit [Remote host closed the connection]
<flokli>
hmm, I started diving into why the "virtual" networking test doesn't work with networkd (https://github.com/NixOS/nixpkgs/pull/81013/commits/be6598a6b69b8e5548383b8a67a8b3d59803d280) - turns out network-link-${i.name}.service wasn't fully migrated, and also lurks around on networkd systems. It uses the deprecated network-interfaces.target and should probably just be integrated into createTunDevice for
<flokli>
scripted, and the systemd.netdevs code for networkd
<flokli>
anyone interested in playing rubberduck to bounce some ideas around?
<aanderse>
andi-: i understand how to make reload work, my issue is that when the binary changes a reload is no longer sufficient - we need to stop, reload systemd unit, and start again
<andi->
aanderse: well then we probably need some X-RestartIfChanged that should be the list of paths that should trigger a restart
<aanderse>
i agree
<aanderse>
I'll give a specific example and you all tell me if I'm crazy
<aanderse>
apache httpd runs 1 master process which takes requests and spawns child processes to handle those requests (nginx does the same)
<aanderse>
out you reload then all existing child processes will eventually end, and the master process will re-read config, them btw child processes will have your changes
<aanderse>
great
<aanderse>
but if you nixos-rebuild and you updated httpd from version x to y...
<aanderse>
reload is called
<aanderse>
the master process continues to run
<aanderse>
it was never restarted
pbb has joined #nixos-systemd
<aanderse>
so it's still version x running
<aanderse>
:-\
<aanderse>
this should roughly apply to any service
<aanderse>
so if any service has reload instead of restart and a cve comes out
<aanderse>
everyone updates and thinks they are ok
<aanderse>
they are not
<flokli>
I think we need to always restart every time the ExecStart line has changed
<flokli>
which means, another binary is called, the `script` changed, or some command line parameter has changed
<aanderse>
flokli: i strongly disagree :-\
<aanderse>
httpd execstart passes config file so will always change
<aanderse>
we need to listen to restartTriggers and pass the binary in there
<flokli>
well, then how do you tell apache "there's a new config file at THIS location now?
<gchristensen>
one option would be, say, writing the version of the daemon somewhere and depending on that file to change (but that isn't so nice)
<flokli>
I assume we'd need to have the symlink andi- was talking about
<gchristensen>
but that wouldn't cover dependencies needing updates, like openssl
<aanderse>
exactly as andi- said: symlink to /etc
<flokli>
aanderse: yeah, this will trigger a reload, not restart, as the ExecStart line hasn't changed
<flokli>
only the config file the symlink is pointing to
<aanderse>
if you don't like /etc
<gchristensen>
another option is using an announcement process to say "You must fully restart your webserver." I think this is probably acceptable
<andi->
not /etc but yeah :) I wouldn't mind having it elsewhere if the only reason we have it is systemd workarounds
<aanderse>
gchristensen: it most certainly would
<gchristensen>
what would be what?
<aanderse>
I'll look into the perl script to support both restarts and reloads
<aanderse>
it will at least give us some more talking points
<gchristensen>
I'm not sure there is a fine enough way to slice it down
<gchristensen>
like to do it automatically you need to decide if a change was significant enough to warrant a restart, but the CVE could be anywhere in its chain of deps -- runtime or buildtime
<gchristensen>
so then pretty much every deploy with any software update would be a reboot
<gchristensen>
but maybe that is fine and people won't update their webserver's version of nixpkgs often?
<aanderse>
gchristensen: if i ever run nix-channel --update i can expect i need to restart software, that is fine
<flokli>
or "tun" or "tap" devices, …= setting is not currently supported
<aanderse>
BUT
<andi->
I think you are both talking about different issues. We should probably always restart if the ExecStart line has changed. We can decide to not restart the process only if the ExecStart line is stable.
<flokli>
andi-++
<{^_^}>
andi-'s karma got increased to 27
<aanderse>
for every time i run nix-channell --update i run nixops deploy a thousand times
<aanderse>
that is to say... i'm asked to tweak httpd configurations multiple times a day, usually
<gchristensen>
cool
<gchristensen>
sgtm sorry for confusin gthings
<andi->
I am peeling potatoes and just had a brief thought (don't expect me to reply soon): Structured service configuration. AKA no longer just stupid strings that carry context but actually having a way (type?) to tell our nix expression "this is a config file" and "this is an executable"
<aanderse>
gchristensen: definitely not, all conversation for this topic is more than welcome and much appreciated :)
pbb has quit [Ping timeout: 265 seconds]
<aanderse>
also, your mention of simply notifying the user to restart services is a good idea
<gchristensen>
if we can make it auto-restart if the binary changed, that'dbe good
<flokli>
eeeh… I doubt people will manually ssh in to restart services. I'd expect "switch-to-configuration" to also restart services
<aanderse>
gchristensen: yes, *if* :)
<gchristensen>
applicable to most software, though some would probablywant to stay up and have users need to know
<aanderse>
i'm kinda shocked this is an issue tbh :\
<flokli>
so auto-restart if the ExecStart line changed sounds good (plus move config file through a symlink over etc indirection)
<gchristensen>
me too aanderse :)
<aanderse>
and now i need to do something which seems counterintuitive to my entire being... read (and possibly write) perl on my day off T_T
<aanderse>
lol
<gchristensen>
hey I did that yesterday lol
<aanderse>
rough
<flokli>
gchristensen++ aanderse++
<{^_^}>
aanderse's karma got increased to 21, gchristensen's karma got increased to 266
<aanderse>
oh i suddenly get why this wasn't noticed... there are only like ~12 instances of reloadIfChanged when i grep through the module systems
<andi->
it is probably working well enough for most users :/
<aanderse>
restartTriggers actually forces a restart, reloadTriggers is just to make the module reload (and should only be used when reloadIfChanged = true, i need to put an assert in there still to enforce that)
<aanderse>
take a peak and let me know if you think i'm on the right track at all
<aanderse>
ps. i wrote that pretty quickly while playing withkids on and off so i probably missed a few things :-)
<globin>
flokli: I think nobody ever looked at the networking virtual test with networkd properly, I think linus once tried to fix it but didn't have the time in the end.. essentially it was the only networking test that didn't enable networkd when it should've and I fixed that behaviour at some point to make it correctly fail
<flokli>
globin: I'm pretty far with it I think. Currently rubberducking with NinjaTrappeur on it, PR will be out soon
hmpffff has joined #nixos-systemd
<ma27[m]>
aaron: would you mind pinging me when you file a PR with this? I'm actually quite interested in this since this makes parts of the awful hackery in this draft-PR obsolete: https://github.com/NixOS/nixpkgs/pull/84608
<{^_^}>
#84608 (by Ma27, 5 days ago, open): nixos/systemd-nspawn: reload or restart machines on config change