nixos/frr: adapt to frr-9 #274425
nixos/frr: adapt to frr-9 #274425
Conversation
|
@GrahamcOfBorg test frr |
woffs
force-pushed
the
fix-274286
branch
2 times, most recently
from
e333655
to
ca739d9
Compare
|
@GrahamcOfBorg test frr |
woffs
force-pushed
the
fix-274286
branch
2 times, most recently
from
1c0071d
to
93c95ed
Compare
|
@GrahamcOfBorg test frr |
There was a problem hiding this comment.
LGTM
As NixOS currently generates frr daemon configs into store paths, which are read-only, we can keep separate daemon configs also for frr 10.0. A unified frr.conf would currently only be necessary, if writing to this file should also be supported via e.g. vtysh.
|
Probably needs to be rebased. |
done |
|
@GrahamcOfBorg test frr |
|
Regarding mgmtd reload: mgmtd keeps track of different config versions for other daemons. It cannot be reloaded with a different config on its own. I think its ok to block/ignore this operation. frr-reload.py also does not contain mgmtd in the list of allowed daemons to reload (https://github.com/FRRouting/frr/blob/master/tools/frr-reload.py#L1999). I guess from semantics side that's correct. |
|
@woffs What's your opinion regarding mgmtd? |
If it cannot be reloaded and does not need to be reloaded, we should not reload it 😉 |
|
I think #274286 is somehow a security issue (configured static routes getting ignored without much noise), so it would be great this PR could be finally reviewed and merged. 😁 |
- fix NixOS#274286 - remove `-f configfile` from ExecStart - use /etc/frr/${service}d.conf - enable mgmtd when staticd is enabled - don't frr-reload.py mgmtd - remove obsolete lib.mdDoc
|
Successfully created backport PR for |
|
Successfully created backport PR for |
-f configfilefrom ExecStartDescription of changes
In frr-9 staticd no longer accepts
-f configfileoption, but uses mgmtd for configuration management. Therefore we remove-f configfilefrom all frr services and use the default config location/etc/frr/${service}d.conf(NixOS used this withoutdbefore) and enable mgmtd automatically, if staticd is enabled. We have to follow upstream release notes carefully if they add more daemons to the new mgmtd method (or we start mgmtd, like zebra, if any service is enabled?).I had no leasure to read and understand mgmtd documentation thoroughly yet, sorry. I'm not sure how to handle mgmtd reload, I tried to disable reloading mgmtd for now, because frr-reload denies reloading mgmtd.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.