diff --git a/doc/build-helpers/testers.chapter.md b/doc/build-helpers/testers.chapter.md index a616742fbb3b..241acb439ea6 100644 --- a/doc/build-helpers/testers.chapter.md +++ b/doc/build-helpers/testers.chapter.md @@ -758,6 +758,7 @@ An attribute set of derivations which perform the tests during their build. - Output attribute `config` is the resulting evaluated services attrset (e.g., the value of the `system.services` option in NixOS). This attribute must be available even if `checkDrv` would fail. - Output attribute `checkDrv` is a representative derivation whose existence and buildability prove the eval is sound (e.g., `system.build.toplevel` in NixOS, but could perhaps be more specific in the case of another process manager integration). + - The generic tester only reads `config` and `checkDrv`. An integration may return additional attributes for its own integration-specific eval checks. Such extra attributes are optional. `mkTest` (function) @@ -772,6 +773,14 @@ An attribute set of derivations which perform the tests during their build. : Path to a directory writable by service processes and readable by `testExe`. The integration must ensure this directory is available when the services and `testExe` run. +`callReload` (function) + +: `path -> string`. + Given a service's name `path` (the list of service names from the top-level service down to the target sub-service, e.g. `[ "reload" "inner" ]`), returns a shell command that reloads that service. + The command is embedded in `testExe` and executed with sufficient privilege to reload the service (e.g. as root in the test VM). + There is no manager-agnostic reload command, so every integration must provide this; the integration joins the `path` per its own unit-naming convention (the suite does not assume one). + On NixOS the `path` dash-joins into the systemd unit name with a `.service` suffix, so the command is `systemctl reload ${lib.concatStringsSep "-" path}.service` (a top-level service is a single-element path `[ "svc" ]` -> `svc.service`; a nested sub-service `[ "parent" "child" ]` -> `parent-child.service`). + :::{.example #ex-modularServiceCompliance-nixos} # NixOS invocation of the compliance suite @@ -802,6 +811,7 @@ recurseIntoAttrs ( config = machine.config.system.services; checkDrv = machine.config.system.build.toplevel; }; + callReload = path: "systemctl reload ${lib.concatStringsSep "-" path}.service"; mkTest = { name, diff --git a/lib/services/service.nix b/lib/services/service.nix index 449c35426039..5911202547fe 100644 --- a/lib/services/service.nix +++ b/lib/services/service.nix @@ -7,10 +7,11 @@ { lib, config, + options, ... }: let - inherit (lib) mkOption types; + inherit (lib) mkEnableOption mkOption types; pathOrStr = types.coercedTo types.path (x: "${x}") types.str; in { @@ -40,7 +41,7 @@ in visible = "shallow"; }; process = { - argv = lib.mkOption { + argv = mkOption { type = types.listOf pathOrStr; example = lib.literalExpression ''[ (lib.getExe config.package) "--nobackground" ]''; description = '' @@ -72,15 +73,12 @@ in }; notificationProtocol = mkOption { - type = types.listOf ( - types.enum [ - "systemd" - "s6" - ] - ); - - default = [ ]; - apply = v: lib.unique v; + type = types.submodule { + options = { + systemd = mkEnableOption "Whether the service supports systemd-notify."; + s6 = mkEnableOption "Whether the service supports s6-notify."; + }; + }; description = '' Notification protocol that this service supports with the underlying service manager. ''; @@ -90,13 +88,20 @@ in config = { assertions = [ { - assertion = config.process.reloadSignal != null && config.process.reloadCommand != null; + # `reloadSignal` derives `reloadCommand` at `mkDefault` priority below, so a + # conflict only exists when the user *also* set `reloadCommand` explicitly. + # An explicit (non-`mkDefault`) definition has `defaultOverridePriority`. + assertion = + !( + config.process.reloadSignal != null + && options.process.reloadCommand.highestPrio <= lib.modules.defaultOverridePriority + ); message = "reloadSignal conflicts with reloadCommand. Please either use reloadSignal or reloadCommand."; } ]; - process.reloadCommand = (lib.mkIf config.process.reloadSignal != null) ( - lib.mkForce "${pkgs.coreutils}/bin/kill -${config.process.reloadSignal} $MAINPID" + process.reloadCommand = lib.mkIf (config.process.reloadSignal != null) ( + lib.mkDefault "${pkgs.coreutils}/bin/kill -${config.process.reloadSignal} $MAINPID" ); }; } diff --git a/nixos/modules/system/service/systemd/service.nix b/nixos/modules/system/service/systemd/service.nix index 63e80aa19e0c..e7fa338ff97b 100644 --- a/nixos/modules/system/service/systemd/service.nix +++ b/nixos/modules/system/service/systemd/service.nix @@ -126,33 +126,29 @@ in `%i` (instance), `%t` (runtime directory), and environment variables using `''${VAR}` syntax in your command line. - By default, it is either set to null or this is set to the escaped version of {option}`process.reloadCommand` - when specified to prevent systemd substitution. Set this option explicitly to enable - systemd's substitution features. + By default, it is set to {option}`process.reloadCommand` when specified, or an + empty string otherwise. Because {option}`process.reloadCommand` is already a + command line (not an argument list), it is used verbatim so that references + like `$MAINPID` are preserved. To extend {option}`process.reloadCommand` with systemd specifiers, you can append - to the escaped arguments: + to the command line: ```nix systemd.mainExecReload = - config.systemd.lib.escapeSystemdExecArgs config.process.reload + " --systemd-unit %n"; + config.process.reloadCommand + " --systemd-unit %n"; ``` This pattern allows you to pass the unit name (or other systemd specifiers) - as additional arguments while keeping the base command from {option}`process.argv` - properly escaped. + as additional arguments. See {manpage}`systemd.service(5)` (section "COMMAND LINES") for details on variable substitution and {manpage}`systemd.unit(5)` (section "SPECIFIERS") for available specifiers like `%n`, `%i`, `%t`. ''; type = types.nullOr types.str; - default = - if config.process.reloadCommand then - config.systemd.lib.escapeSystemdExecArgs config.process.reloadCommand - else - ""; - defaultText = lib.literalExpression "config.systemd.lib.escapeSystemdExecArgs config.process.reloadCommand"; + default = if config.process.reloadCommand != null then config.process.reloadCommand else ""; + defaultText = lib.literalExpression "config.process.reloadCommand"; }; systemd.services = mkOption { @@ -213,9 +209,7 @@ in wantedBy = lib.mkDefault [ "multi-user.target" ]; serviceConfig = { ExecReload = config.systemd.mainExecReload; - Type = lib.mkDefault ( - if (config.serviceManager.notificationProtocol == "systemd") then "notify" else "simple" - ); + Type = lib.mkDefault (if config.notificationProtocol.systemd then "notify" else "simple"); Restart = lib.mkDefault "always"; RestartSec = lib.mkDefault "5"; ExecStart = [ diff --git a/nixos/tests/system-services-compliance.nix b/nixos/tests/system-services-compliance.nix index 0a40d18fc4fe..add3593fd3a8 100644 --- a/nixos/tests/system-services-compliance.nix +++ b/nixos/tests/system-services-compliance.nix @@ -7,6 +7,23 @@ let sharedDir = "/tmp/modular-service-compliance"; + + inherit (pkgs) lib coreutils; + + evalSystemServices = + services: + evalSystem ( + { ... }: + { + system.services = services; + system.stateVersion = "25.05"; + fileSystems."/" = { + device = "/test/dummy"; + fsType = "auto"; + }; + boot.loader.grub.enable = false; + } + ); in let suite = pkgs.testers.modularServiceCompliance { @@ -15,23 +32,13 @@ let evalConfig = { services }: let - machine = evalSystem ( - { ... }: - { - system.services = services; - system.stateVersion = "25.05"; - fileSystems."/" = { - device = "/test/dummy"; - fsType = "auto"; - }; - boot.loader.grub.enable = false; - } - ); + machine = evalSystemServices services; in { config = machine.config.system.services; checkDrv = machine.config.system.build.toplevel; }; + callReload = path: "systemctl reload ${lib.concatStringsSep "-" path}.service"; mkTest = { name, @@ -49,6 +56,80 @@ let meta.maintainers = with pkgs.lib.maintainers; [ roberth ]; }; }; + + # systemd-specific eval assertions. serviceConfig.Type/ExecReload only exist on + # the resolved host units, so a fresh eval is used per case. + systemdEvalTests = + let + defaultUnits = + (evalSystemServices { + service.process.argv = [ "${coreutils}/bin/true" ]; + }).config.systemd.services; + + notifyUnits = + (evalSystemServices { + service = { + process.argv = [ "${coreutils}/bin/true" ]; + notificationProtocol.systemd = true; + }; + }).config.systemd.services; + + reloadUnits = + (evalSystemServices { + service = { + process.argv = [ "${coreutils}/bin/true" ]; + process.reloadSignal = "HUP"; + }; + }).config.systemd.services; + in + { + testDefaultType = { + expr = defaultUnits.service.serviceConfig.Type; + expected = "simple"; + }; + + testNotifyType = { + expr = notifyUnits.service.serviceConfig.Type; + expected = "notify"; + }; + + testReloadExecReload = { + expr = reloadUnits.service.serviceConfig.ExecReload; + expected = "${coreutils}/bin/kill -HUP $MAINPID"; + }; + }; + + systemdEval = pkgs.stdenvNoCC.mkDerivation (finalAttrs: { + __structuredAttrs = true; + name = "system-services-compliance-systemd-eval-report"; + passthru = { + tests = systemdEvalTests; + failures = lib.runTests finalAttrs.finalPackage.tests; + }; + testResults = lib.mapAttrs (_: test: test.expr == test.expected) finalAttrs.finalPackage.tests; + buildCommand = '' + touch $out + for testName in "''${!testResults[@]}"; do + if [[ -n "''${testResults[$testName]}" ]]; then + echo "PASS $testName" + else + echo "FAIL $testName" + fi + done + '' + + lib.optionalString (lib.any (v: !v) (lib.attrValues finalAttrs.testResults)) '' + { + echo "" + echo "systemd-specific eval-level compliance failures:" + for testName in "''${!testResults[@]}"; do + if [[ -z "''${testResults[$testName]}" ]]; then + echo "- $testName" + fi + done + } >&2 + exit 1 + ''; + }); in # Please the callTest pattern. @@ -64,4 +145,4 @@ pkgs.lib.mapAttrs ( test = v; driver = v; } -) suite +) (suite // { systemd-eval = systemdEval; }) diff --git a/pkgs/build-support/testers/modular-service-compliance.nix b/pkgs/build-support/testers/modular-service-compliance.nix index ece674583eff..2349e9e96a8f 100644 --- a/pkgs/build-support/testers/modular-service-compliance.nix +++ b/pkgs/build-support/testers/modular-service-compliance.nix @@ -15,6 +15,7 @@ evalConfig, mkTest, sharedDir, + callReload, namePrefix ? "modular-service-compliance", }: @@ -45,6 +46,11 @@ let ]; warnings = [ "compliance child warning" ]; }; + # A sub-service exercising reload derivation from a reload signal. + services.reloadee = { + process.argv = [ "${coreutils}/bin/true" ]; + process.reloadSignal = "HUP"; + }; }; }; }; @@ -65,7 +71,7 @@ let }; testAssertions = { - expr = builtins.elem { + expr = lib.elem { assertion = true; message = "compliance test assertion"; } c.assertions; @@ -73,12 +79,12 @@ let }; testWarnings = { - expr = builtins.elem "compliance test warning" c.warnings; + expr = lib.elem "compliance test warning" c.warnings; expected = true; }; testSubServiceAssertions = { - expr = builtins.elem { + expr = lib.elem { assertion = true; message = "compliance child assertion"; } c.services.child.assertions; @@ -86,14 +92,58 @@ let }; testSubServiceWarnings = { - expr = builtins.elem "compliance child warning" c.services.child.warnings; + expr = lib.elem "compliance child warning" c.services.child.warnings; + expected = true; + }; + + # The reload-conflict assertion must not fire (its `assertion` must hold) for a + # service that sets only reloadSignal (guards the inverted-assertion fix, and the + # priority-aware conflict detection: reloadSignal derives reloadCommand internally, + # which must not be mistaken for a user-set conflict). + testNoReloadConflict = { + expr = lib.any ( + a: + a.message + == "reloadSignal conflicts with reloadCommand. Please either use reloadSignal or reloadCommand." + && !a.assertion + ) c.services.reloadee.assertions; + expected = false; + }; + + # Setting process.reloadSignal derives process.reloadCommand + # (guards the misplaced-paren mkIf fix). + testReloadSignalDerivesCommand = { + expr = c.services.reloadee.process.reloadCommand; + expected = "${coreutils}/bin/kill -HUP $MAINPID"; + }; + + # notificationProtocol submodule bools default to false. + testNotificationProtocolSystemdDefault = { + expr = c.notificationProtocol.systemd; + expected = false; + }; + + testNotificationProtocolS6Default = { + expr = c.notificationProtocol.s6; + expected = false; + }; + + # Setting both reloadSignal and reloadCommand explicitly is a genuine conflict, + # so the assertion must fire. Separate eval: checkDrv would fail on this. + testReloadConflictFires = { + expr = lib.any ( + a: + a.message + == "reloadSignal conflicts with reloadCommand. Please either use reloadSignal or reloadCommand." + && !a.assertion + ) conflictEval.config.conflict.assertions; expected = true; }; # Separate eval for a failing assertion — checkDrv would fail here, # so we only access config. testFailingAssertionValue = { - expr = builtins.elem { + expr = lib.elem { assertion = false; message = "compliance failing assertion"; } failingEval.config.failing.assertions; @@ -101,6 +151,16 @@ let }; }; + conflictEval = evalConfig { + services = { + conflict = { + process.argv = [ "${coreutils}/bin/true" ]; + process.reloadSignal = "HUP"; + process.reloadCommand = "${coreutils}/bin/kill -HUP $MAINPID"; + }; + }; + }; + failingEval = evalConfig { services = { failing = { @@ -129,6 +189,24 @@ let exec "${coreutils}/bin/sleep" infinity ''; + /** + A reloadable service script. Like `svc`, it namespaces a comms subdirectory + by its first argument and records the remaining arguments. It traps SIGHUP + and appends a marker line to `$dir/reloads` on each reload, then stays alive + as the trapping shell itself (no `exec`, so the trap and MAINPID are kept). + */ + reloadableSvc = writeShellScript "${namePrefix}-reloadable-svc" '' + id="$1"; shift + dir="${sharedDir}/$id" + mkdir -p "$dir" + : > "$dir/reloads" + reload() { printf 'reloaded\n' >> "$dir/reloads"; } + trap reload HUP + echo "$$" > "$dir/pid" + printf '%s\n' "$@" > "$dir/args" + while true; do "${coreutils}/bin/sleep" 1; done + ''; + mkArgv = id: extraArgs: [ @@ -171,6 +249,27 @@ let inherit text; }); + # The reload runtime test's service tree. The reloadable unit is the *sub*-service, + # so its name path `[ "reload" "inner" ]` handed to `callReload` exercises the + # integration's nested unit naming (NixOS dash-joins to `reload-inner.service`). + reloadServices = { + reload = { + # Parent must run something; a bare keep-alive is enough. + process.argv = [ + reloadableSvc + "reload-parent" + ]; + services.inner.process = { + argv = [ + reloadableSvc + "reload-inner" + ]; + # The script traps SIGHUP; reloadSignal derives the manager's reload command. + reloadSignal = "HUP"; + }; + }; + }; + in { # Eval-level tests: config structure, evaluated in the integration's @@ -186,9 +285,9 @@ in representative = evalResult.checkDrv; passthru = { tests = evalTestDefs; - failures = lib.runTests finalAttrs.passthru.tests; + failures = lib.runTests finalAttrs.finalPackage.tests; }; - testResults = lib.mapAttrs (_: test: test.expr == test.expected) finalAttrs.passthru.tests; + testResults = lib.mapAttrs (_: test: test.expr == test.expected) finalAttrs.finalPackage.tests; buildCommand = '' touch $out for testName in "''${!testResults[@]}"; do @@ -247,5 +346,26 @@ in ''; }; + # Runtime reload: start a reloadable service, ask the integration to reload it, + # and assert the service observed the reload (recorded a SIGHUP marker). + reload = mkTest { + name = "${namePrefix}-reload"; + services = reloadServices; + testExe = mkTestScript "reload" '' + ${waitAndCheck "reload-inner" [ ]} + ${callReload [ + "reload" + "inner" + ]} + timeout=30; elapsed=0 + while ! grep -qx "reloaded" "${sharedDir}/reload-inner/reloads" && [ "$elapsed" -lt "$timeout" ]; do + sleep 1; elapsed=$((elapsed + 1)) + done + grep -qx "reloaded" "${sharedDir}/reload-inner/reloads" \ + || { echo "reload: no reload recorded after ''${timeout}s"; cat "${sharedDir}/reload-inner/reloads"; exit 1; } + echo "reload: reload observed" + ''; + }; + # See also the manual compliance items in doc/build-helpers/testers.chapter.md. }