nixos/ups: shutdown UPS at host shutdown #344940
Conversation
They're listed twice (documentation and implementation) and this change makes it easier to compare the attrsets.
| @@ -334,6 +335,7 @@ let | |||
| MINSUPPLIES = lib.mkDefault 1; | |||
| MONITOR = lib.flip lib.mapAttrsToList cfg.upsmon.monitor (name: monitor: with monitor; [ system powerValue user "\"@upsmon_password_${name}@\"" type ]); | |||
| NOTIFYCMD = lib.mkDefault "${pkgs.nut}/bin/upssched"; | |||
| POWERDOWNFLAG = "/run/killpower"; | |||
There was a problem hiding this comment.
What do you think about putting this in /run/nut/ along with the configs?
There was a problem hiding this comment.
It might work now, because of how /run/nut gets created. But I think it should be a RuntimeDirectory=, which means it'll be unmounted by the time the killpower service runs. We need to store the flag somewhere that outlives all normal services.
Note that upstream suggests /etc/killpower. I opted for /run because I prefer ramfs when that is possible. (And upsmon supports both volatile and non-volatile filesystems -- it'll remove the POWERDOWNFLAG on startup if it exists.)
| @@ -575,6 +577,23 @@ in | |||
| ]; | |||
| }; | |||
|
|
|||
| systemd.services.ups-killpower = { | |||
| enable = cfg.upsd.enable; | |||
There was a problem hiding this comment.
Should we also check cfg.upsmon.settings.POWERDOWNFLAG != null here, so that users can easily disable this behavior?
There was a problem hiding this comment.
Maybe. Or if we want to make this configurable (and discoverable), maybe it's better to have an explicit option? power.ups.allowUpsKillPower? Or tell users to set systemd.services.ups-killpower.enable = false?
Is it overdesigning though? Why would someone not want to kill the UPS power? It's needed to ensure the systems come back up, so...
But I guess allowing null is an easy way to handle it -- it's not very obvious, but at least it's doable.
Data point: the apcupsd module does not have option to disable killpower. Nobody complained in 10+ years.
I guess a risk could be if the UPS firmware is broken and turns power off immediately, instead of waiting a few seconds? But the system should have unmounted all the normal filesystems by then, so I don't think it should cause any damage. But maybe I should write a release note entry and mention the POWERDOWNFLAG = null as a way to keep the old functionality? |
bjornfor
force-pushed
the
nixos-ups-fix-shutdown
branch
from
75d3001
to
20cb90d
Compare
|
The latest push has these changes:
|
|
Forgot the release notes entry. Added now. |
bjornfor
force-pushed
the
nixos-ups-fix-shutdown
branch
from
aebfcfb
to
d8f8c83
Compare
Implement the missing bit of the NUT shutdown design[1]. This ensures that machines come back up automatically after a power outage. (Without this change they will only come back up if the UPS completely empties its battery.) [1] https://networkupstools.org/docs/user-manual.chunked/Configuration_notes.html#Shutdown_design
bjornfor
force-pushed
the
nixos-ups-fix-shutdown
branch
from
d8f8c83
to
e3cdbe8
Compare
|
Last two pushes:
|
Description of changes
Implement the missing bit of the NUT shutdown design[1]. This ensures
that machines come back up automatically after a power outage. (Without
this change they will only come back up if the UPS completely empties
its battery.)
[1] https://networkupstools.org/docs/user-manual.chunked/Configuration_notes.html#Shutdown_design
Tested out-of-tree in NixOS 24.05. (This PR cannot be cherry-picked to
release-24.05 / has conflicts.)
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.