From cc9fb5f0152424e96267a30fd9a9ce35837d9f63 Mon Sep 17 00:00:00 2001 From: Nikolay Amiantov Date: Thu, 25 Jul 2019 21:49:46 +0300 Subject: [PATCH 1/9] systemd: update revision --- pkgs/os-specific/linux/systemd/default.nix | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/os-specific/linux/systemd/default.nix b/pkgs/os-specific/linux/systemd/default.nix index 39aa855e3cc..b7f1af618fe 100644 --- a/pkgs/os-specific/linux/systemd/default.nix +++ b/pkgs/os-specific/linux/systemd/default.nix @@ -24,8 +24,8 @@ stdenv.mkDerivation rec { src = fetchFromGitHub { owner = "NixOS"; repo = "systemd"; - rev = "aa4c4d39d75ce52664cb28d569b1ceafda7b4c06"; - sha256 = "1ax94gzbdwdcf3wgj7f9cabdkvn2zynnnli7gkbz4isidlpis86g"; + rev = "5fb35fbc783516e2014115c3488134a2afb8494c"; + sha256 = "0pyjvzzh8nnxv4z58n82lz1mjnzv44sylcjgkvw8sp35vx1ryxfh"; }; outputs = [ "out" "lib" "man" "dev" ]; From fd405dab3ea0b5a2d3730c836dbcfabf2e820951 Mon Sep 17 00:00:00 2001 From: Nikolay Amiantov Date: Thu, 25 Jul 2019 16:26:28 +0300 Subject: [PATCH 2/9] systemd service: rename generator-packages --- nixos/modules/rename.nix | 3 +++ nixos/modules/system/boot/systemd.nix | 6 +++--- nixos/modules/tasks/filesystems/nfs.nix | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/nixos/modules/rename.nix b/nixos/modules/rename.nix index 4ae64222274..b01febe551f 100644 --- a/nixos/modules/rename.nix +++ b/nixos/modules/rename.nix @@ -257,6 +257,9 @@ with lib; (mkRenamedOptionModule [ "networking" "extraResolvconfConf" ] [ "networking" "resolvconf" "extraConfig" ]) (mkRenamedOptionModule [ "networking" "resolvconfOptions" ] [ "networking" "resolvconf" "extraOptions" ]) + # systemd + (mkRenamedOptionModule [ "systemd" "generator-packages" ] [ "systemd" "generatorPackages" ]) + ] ++ (flip map [ "blackboxExporter" "collectdExporter" "fritzboxExporter" "jsonExporter" "minioExporter" "nginxExporter" "nodeExporter" "snmpExporter" "unifiExporter" "varnishExporter" ] diff --git a/nixos/modules/system/boot/systemd.nix b/nixos/modules/system/boot/systemd.nix index cf35504e518..a89a200960e 100644 --- a/nixos/modules/system/boot/systemd.nix +++ b/nixos/modules/system/boot/systemd.nix @@ -497,7 +497,7 @@ in ''; }; - systemd.generator-packages = mkOption { + systemd.generatorPackages = mkOption { default = []; type = types.listOf types.package; example = literalExample "[ pkgs.systemd-cryptsetup-generator ]"; @@ -762,10 +762,10 @@ in environment.etc = let # generate contents for /etc/systemd/system-generators from - # systemd.generators and systemd.generator-packages + # systemd.generators and systemd.generatorPackages generators = pkgs.runCommand "system-generators" { preferLocalBuild = true; - packages = cfg.generator-packages; + packages = cfg.generatorPackages; } '' mkdir -p $out for package in $packages diff --git a/nixos/modules/tasks/filesystems/nfs.nix b/nixos/modules/tasks/filesystems/nfs.nix index d3a558738f4..c17cf804950 100644 --- a/nixos/modules/tasks/filesystems/nfs.nix +++ b/nixos/modules/tasks/filesystems/nfs.nix @@ -56,7 +56,7 @@ in boot.initrd.kernelModules = mkIf inInitrd [ "nfs" ]; systemd.packages = [ pkgs.nfs-utils ]; - systemd.generator-packages = [ pkgs.nfs-utils ]; + systemd.generatorPackages = [ pkgs.nfs-utils ]; environment.etc = { "idmapd.conf".source = idmapdConfFile; From a304fc5d753d0d2c134bf0925070bcead8650dff Mon Sep 17 00:00:00 2001 From: Nikolay Amiantov Date: Thu, 25 Jul 2019 16:27:28 +0300 Subject: [PATCH 3/9] systemd service: add support for shutdown packages Shutdown hooks are executed right before the shutdown, which is useful for some applications. Among other things this is needed for mdadm hook to run. --- nixos/modules/system/boot/systemd.nix | 39 ++++++++++++++++++++------- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/nixos/modules/system/boot/systemd.nix b/nixos/modules/system/boot/systemd.nix index a89a200960e..96c4ee30584 100644 --- a/nixos/modules/system/boot/systemd.nix +++ b/nixos/modules/system/boot/systemd.nix @@ -504,6 +504,23 @@ in description = "Packages providing systemd generators."; }; + systemd.shutdown = mkOption { + type = types.attrsOf types.path; + default = {}; + description = '' + Definition of systemd shutdown executables. + For each NAME = VALUE pair of the attrSet, a link is generated from + /etc/systemd/system-shutdown/NAME to VALUE. + ''; + }; + + systemd.shutdownPackages = mkOption { + default = []; + type = types.listOf types.package; + example = literalExample "[ pkgs.mdadm ]"; + description = "Packages providing systemd shutdown executables."; + }; + systemd.defaultUnit = mkOption { default = "multi-user.target"; type = types.str; @@ -761,18 +778,21 @@ in environment.systemPackages = [ systemd ]; environment.etc = let - # generate contents for /etc/systemd/system-generators from - # systemd.generators and systemd.generatorPackages - generators = pkgs.runCommand "system-generators" { + # generate contents for /etc/systemd/system-${type} from attrset of links and packages + hooks = type: links: packages: pkgs.runCommand "system-${type}" { preferLocalBuild = true; - packages = cfg.generatorPackages; - } '' + packages = packages; + } '' + set -e mkdir -p $out for package in $packages do - ln -s $package/lib/systemd/system-generators/* $out/ - done; - ${concatStrings (mapAttrsToList (generator: target: "ln -s ${target} $out/${generator};\n") cfg.generators)} + for hook in $package/lib/systemd/system-${type}/* + do + ln -s $hook $out/ + done + done + ${concatStrings (mapAttrsToList (exec: target: "ln -s ${target} $out/${exec};\n") links)} ''; in ({ "systemd/system".source = generateUnits "system" cfg.units upstreamSystemUnits upstreamSystemWants; @@ -834,7 +854,8 @@ in ${concatStringsSep "\n" cfg.tmpfiles.rules} ''; - "systemd/system-generators" = { source = generators; }; + "systemd/system-generators" = { source = hooks "generators" cfg.generators cfg.generatorPackages; }; + "systemd/system-shutdown" = { source = hooks "shutdown" cfg.shutdown cfg.shutdownPackages; }; }); services.dbus.enable = true; From 5636fe572bf0d7176eda11e4a2fd62ccd1577916 Mon Sep 17 00:00:00 2001 From: Nikolay Amiantov Date: Thu, 25 Jul 2019 21:50:02 +0300 Subject: [PATCH 4/9] systemd test: add test for systemd-shutdown scripts --- nixos/tests/systemd.nix | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/nixos/tests/systemd.nix b/nixos/tests/systemd.nix index 4d470126abe..fadea52f1db 100644 --- a/nixos/tests/systemd.nix +++ b/nixos/tests/systemd.nix @@ -1,4 +1,4 @@ -import ./make-test.nix { +import ./make-test.nix ({ pkgs, ... }: { name = "systemd"; machine = { lib, ... }: { @@ -21,6 +21,14 @@ import ./make-test.nix { services.journald.extraConfig = "Storage=volatile"; services.xserver.displayManager.auto.user = "alice"; + systemd.shutdown.test = pkgs.writeScript "test.shutdown" '' + #!${pkgs.stdenv.shell} + PATH=${lib.makeBinPath (with pkgs; [ utillinux coreutils ])} + mount -t 9p shared -o trans=virtio,version=9p2000.L /tmp/shared + touch /tmp/shared/shutdown-test + umount /tmp/shared + ''; + systemd.services.testservice1 = { description = "Test Service 1"; wantedBy = [ "multi-user.target" ]; @@ -69,5 +77,20 @@ import ./make-test.nix { # has a last mount time, because the file system wasn't checked. $machine->fail('dumpe2fs /dev/vdb | grep -q "^Last mount time: *n/a"'); }; + + # Regression test for https://github.com/NixOS/nixpkgs/issues/35268 + subtest "file system with x-initrd.mount is not unmounted", sub { + $machine->shutdown; + $machine->waitForUnit('multi-user.target'); + # If the file system was unmounted during the shutdown the file system + # has a last mount time, because the file system wasn't checked. + $machine->fail('dumpe2fs /dev/vdb | grep -q "^Last mount time: *n/a"'); + }; + + subtest "systemd-shutdown works", sub { + $machine->shutdown; + $machine->waitForUnit('multi-user.target'); + $machine->succeed('test -e /tmp/shared/shutdown-test'); + }; ''; -} +}) From 88d5b40d40cec83934dcbdf29a98e18ea4ff1b8c Mon Sep 17 00:00:00 2001 From: Nikolay Amiantov Date: Thu, 25 Jul 2019 16:29:39 +0300 Subject: [PATCH 5/9] mdadm: use absolute paths We also correct paths in udev rules. This is cleaner and allows for less unexpected behaviour. We still check for self-references, however we do it manually now and only for binaries as udev rules can have them. Rather, we patch them out during initrd generation now. --- pkgs/os-specific/linux/mdadm/default.nix | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/pkgs/os-specific/linux/mdadm/default.nix b/pkgs/os-specific/linux/mdadm/default.nix index 4e0202cebf3..764d43f6c2c 100644 --- a/pkgs/os-specific/linux/mdadm/default.nix +++ b/pkgs/os-specific/linux/mdadm/default.nix @@ -1,4 +1,4 @@ -{ stdenv, fetchurl, groff, system-sendmail }: +{ stdenv, utillinux, coreutils, fetchurl, groff, system-sendmail }: stdenv.mkDerivation rec { name = "mdadm-4.1"; @@ -8,26 +8,34 @@ stdenv.mkDerivation rec { sha256 = "0jjgjgqijpdp7ijh8slzzjjw690kydb1jjadf0x5ilq85628hxmb"; }; - # This is to avoid self-references, which causes the initrd to explode - # in size and in turn prevents mdraid systems from booting. - allowedReferences = [ stdenv.cc.libc.out system-sendmail ]; - patches = [ ./no-self-references.patch ]; makeFlags = [ - "NIXOS=1" "INSTALL=install" "INSTALL_BINDIR=$(out)/sbin" + "NIXOS=1" "INSTALL=install" "BINDIR=$(out)/sbin" "MANDIR=$(out)/share/man" "RUN_DIR=/dev/.mdadm" "STRIP=" ] ++ stdenv.lib.optionals (stdenv.hostPlatform != stdenv.buildPlatform) [ "CROSS_COMPILE=${stdenv.cc.targetPrefix}" ]; + enableParallelBuilding = true; + nativeBuildInputs = [ groff ]; - preConfigure = '' + postPatch = '' sed -e 's@/lib/udev@''${out}/lib/udev@' \ -e 's@ -Werror @ @' \ -e 's@/usr/sbin/sendmail@${system-sendmail}@' -i Makefile + sed -i \ + -e 's@/usr/bin/basename@${coreutils}/bin/basename@g' \ + -e 's@BINDIR/blkid@${utillinux}/bin/blkid@g' \ + *.rules + ''; + + # This is to avoid self-references, which causes the initrd to explode + # in size and in turn prevents mdraid systems from booting. + postFixup = '' + grep -r $out $out/bin && false || true ''; meta = with stdenv.lib; { From b4581211058eb27bbfbc8738ac44b718fc743400 Mon Sep 17 00:00:00 2001 From: Nikolay Amiantov Date: Thu, 25 Jul 2019 16:32:08 +0300 Subject: [PATCH 6/9] stage-1 initrd: replace absolute paths for mdadm We don't patch basename and readlink now too as they were added for mdadm in 8ecd3a5e1db4. --- nixos/modules/system/boot/stage-1.nix | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/nixos/modules/system/boot/stage-1.nix b/nixos/modules/system/boot/stage-1.nix index 788e3f4a2ab..4c2d130d5a5 100644 --- a/nixos/modules/system/boot/stage-1.nix +++ b/nixos/modules/system/boot/stage-1.nix @@ -217,13 +217,11 @@ let --replace ata_id ${extraUtils}/bin/ata_id \ --replace scsi_id ${extraUtils}/bin/scsi_id \ --replace cdrom_id ${extraUtils}/bin/cdrom_id \ - --replace ${pkgs.utillinux}/sbin/blkid ${extraUtils}/bin/blkid \ - --replace /sbin/blkid ${extraUtils}/bin/blkid \ + --replace ${pkgs.coreutils}/bin/basename ${extraUtils}/bin/basename \ + --replace ${pkgs.utillinux}/bin/blkid ${extraUtils}/bin/blkid \ --replace ${pkgs.lvm2}/sbin ${extraUtils}/bin \ - --replace /sbin/mdadm ${extraUtils}/bin/mdadm \ + --replace ${pkgs.mdadm}/sbin ${extraUtils}/sbin \ --replace ${pkgs.bash}/bin/sh ${extraUtils}/bin/sh \ - --replace /usr/bin/readlink ${extraUtils}/bin/readlink \ - --replace /usr/bin/basename ${extraUtils}/bin/basename \ --replace ${udev}/bin/udevadm ${extraUtils}/bin/udevadm done From b9b27912ce1c7afb0e99ebee46ad9805abddaaf7 Mon Sep 17 00:00:00 2001 From: Nikolay Amiantov Date: Thu, 25 Jul 2019 16:36:05 +0300 Subject: [PATCH 7/9] mdadm: install systemd units --- pkgs/os-specific/linux/mdadm/default.nix | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkgs/os-specific/linux/mdadm/default.nix b/pkgs/os-specific/linux/mdadm/default.nix index 764d43f6c2c..5aa9e9b43da 100644 --- a/pkgs/os-specific/linux/mdadm/default.nix +++ b/pkgs/os-specific/linux/mdadm/default.nix @@ -12,12 +12,15 @@ stdenv.mkDerivation rec { makeFlags = [ "NIXOS=1" "INSTALL=install" "BINDIR=$(out)/sbin" + "SYSTEMD_DIR=$(out)/lib/systemd/system" "MANDIR=$(out)/share/man" "RUN_DIR=/dev/.mdadm" "STRIP=" ] ++ stdenv.lib.optionals (stdenv.hostPlatform != stdenv.buildPlatform) [ "CROSS_COMPILE=${stdenv.cc.targetPrefix}" ]; + installFlags = [ "install-systemd" ]; + enableParallelBuilding = true; nativeBuildInputs = [ groff ]; From ca780f4a186ab27425bd57737fe7b9638c2b27a8 Mon Sep 17 00:00:00 2001 From: Nikolay Amiantov Date: Thu, 25 Jul 2019 16:36:16 +0300 Subject: [PATCH 8/9] swraid service: use upstream units This fixes a serious bug on NixOS with swraid where mdadm arrays weren't properly stopped on shutdown. Rather than fixing the unit by adding `Before=final.target` we completely move to upstream units, which uses systemd shutdown hooks instead. This also drives down maintenance costs for us. --- nixos/modules/tasks/swraid.nix | 44 +++------------------------------- 1 file changed, 3 insertions(+), 41 deletions(-) diff --git a/nixos/modules/tasks/swraid.nix b/nixos/modules/tasks/swraid.nix index 93e03c44c86..7f07684651c 100644 --- a/nixos/modules/tasks/swraid.nix +++ b/nixos/modules/tasks/swraid.nix @@ -6,51 +6,13 @@ services.udev.packages = [ pkgs.mdadm ]; + systemd.packages = [ pkgs.mdadm ]; + systemd.shutdownPackages = [ pkgs.mdadm ]; + boot.initrd.availableKernelModules = [ "md_mod" "raid0" "raid1" "raid10" "raid456" ]; boot.initrd.extraUdevRulesCommands = '' cp -v ${pkgs.mdadm}/lib/udev/rules.d/*.rules $out/ ''; - systemd.services.mdadm-shutdown = { - wantedBy = [ "final.target"]; - after = [ "umount.target" ]; - - unitConfig = { - DefaultDependencies = false; - }; - - serviceConfig = { - Type = "oneshot"; - ExecStart = ''${pkgs.mdadm}/bin/mdadm --wait-clean --scan''; - }; - }; - - systemd.services."mdmon@" = { - description = "MD Metadata Monitor on /dev/%I"; - - unitConfig.DefaultDependencies = false; - - serviceConfig = { - Type = "forking"; - Environment = "IMSM_NO_PLATFORM=1"; - ExecStart = ''${pkgs.mdadm}/bin/mdmon --offroot --takeover %I''; - KillMode = "none"; - }; - }; - - systemd.services."mdadm-grow-continue@" = { - description = "Manage MD Reshape on /dev/%I"; - - unitConfig.DefaultDependencies = false; - - serviceConfig = { - ExecStart = ''${pkgs.mdadm}/bin/mdadm --grow --continue /dev/%I''; - StandardInput = "null"; - StandardOutput = "null"; - StandardError = "null"; - KillMode = "none"; - }; - }; - } From 717b8b3219e0a207e16e28f828f88060b0477d0b Mon Sep 17 00:00:00 2001 From: Nikolay Amiantov Date: Fri, 26 Jul 2019 12:11:44 +0300 Subject: [PATCH 9/9] systemd service: remove generator-packages option Use systemd.packages instead, it's less error prone and more in line with what's expected. --- nixos/modules/rename.nix | 4 +--- nixos/modules/system/boot/systemd.nix | 25 ++++++------------------- nixos/modules/tasks/filesystems/nfs.nix | 1 - nixos/modules/tasks/swraid.nix | 1 - 4 files changed, 7 insertions(+), 24 deletions(-) diff --git a/nixos/modules/rename.nix b/nixos/modules/rename.nix index b01febe551f..e0d64914ef4 100644 --- a/nixos/modules/rename.nix +++ b/nixos/modules/rename.nix @@ -221,6 +221,7 @@ with lib; (mkRemovedOptionModule [ "services" "mysql" "pidDir" ] "Don't wait for pidfiles, describe dependencies through systemd") (mkRemovedOptionModule [ "services" "mysql" "rootPassword" ] "Use socket authentication or set the password outside of the nix store.") (mkRemovedOptionModule [ "services" "zabbixServer" "dbPassword" ] "Use services.zabbixServer.database.passwordFile instead.") + (mkRemovedOptionModule [ "systemd" "generator-packages" ] "Use systemd.packages instead.") # ZSH (mkRenamedOptionModule [ "programs" "zsh" "enableSyntaxHighlighting" ] [ "programs" "zsh" "syntaxHighlighting" "enable" ]) @@ -257,9 +258,6 @@ with lib; (mkRenamedOptionModule [ "networking" "extraResolvconfConf" ] [ "networking" "resolvconf" "extraConfig" ]) (mkRenamedOptionModule [ "networking" "resolvconfOptions" ] [ "networking" "resolvconf" "extraOptions" ]) - # systemd - (mkRenamedOptionModule [ "systemd" "generator-packages" ] [ "systemd" "generatorPackages" ]) - ] ++ (flip map [ "blackboxExporter" "collectdExporter" "fritzboxExporter" "jsonExporter" "minioExporter" "nginxExporter" "nodeExporter" "snmpExporter" "unifiExporter" "varnishExporter" ] diff --git a/nixos/modules/system/boot/systemd.nix b/nixos/modules/system/boot/systemd.nix index 96c4ee30584..1025a038c4b 100644 --- a/nixos/modules/system/boot/systemd.nix +++ b/nixos/modules/system/boot/systemd.nix @@ -427,7 +427,8 @@ in systemd.packages = mkOption { default = []; type = types.listOf types.package; - description = "Packages providing systemd units."; + example = literalExample "[ pkgs.systemd-cryptsetup-generator ]"; + description = "Packages providing systemd units and hooks."; }; systemd.targets = mkOption { @@ -497,13 +498,6 @@ in ''; }; - systemd.generatorPackages = mkOption { - default = []; - type = types.listOf types.package; - example = literalExample "[ pkgs.systemd-cryptsetup-generator ]"; - description = "Packages providing systemd generators."; - }; - systemd.shutdown = mkOption { type = types.attrsOf types.path; default = {}; @@ -514,13 +508,6 @@ in ''; }; - systemd.shutdownPackages = mkOption { - default = []; - type = types.listOf types.package; - example = literalExample "[ pkgs.mdadm ]"; - description = "Packages providing systemd shutdown executables."; - }; - systemd.defaultUnit = mkOption { default = "multi-user.target"; type = types.str; @@ -779,9 +766,9 @@ in environment.etc = let # generate contents for /etc/systemd/system-${type} from attrset of links and packages - hooks = type: links: packages: pkgs.runCommand "system-${type}" { + hooks = type: links: pkgs.runCommand "system-${type}" { preferLocalBuild = true; - packages = packages; + packages = cfg.packages; } '' set -e mkdir -p $out @@ -854,8 +841,8 @@ in ${concatStringsSep "\n" cfg.tmpfiles.rules} ''; - "systemd/system-generators" = { source = hooks "generators" cfg.generators cfg.generatorPackages; }; - "systemd/system-shutdown" = { source = hooks "shutdown" cfg.shutdown cfg.shutdownPackages; }; + "systemd/system-generators" = { source = hooks "generators" cfg.generators; }; + "systemd/system-shutdown" = { source = hooks "shutdown" cfg.shutdown; }; }); services.dbus.enable = true; diff --git a/nixos/modules/tasks/filesystems/nfs.nix b/nixos/modules/tasks/filesystems/nfs.nix index c17cf804950..e0e8bb1f03d 100644 --- a/nixos/modules/tasks/filesystems/nfs.nix +++ b/nixos/modules/tasks/filesystems/nfs.nix @@ -56,7 +56,6 @@ in boot.initrd.kernelModules = mkIf inInitrd [ "nfs" ]; systemd.packages = [ pkgs.nfs-utils ]; - systemd.generatorPackages = [ pkgs.nfs-utils ]; environment.etc = { "idmapd.conf".source = idmapdConfFile; diff --git a/nixos/modules/tasks/swraid.nix b/nixos/modules/tasks/swraid.nix index 7f07684651c..8fa19194bed 100644 --- a/nixos/modules/tasks/swraid.nix +++ b/nixos/modules/tasks/swraid.nix @@ -7,7 +7,6 @@ services.udev.packages = [ pkgs.mdadm ]; systemd.packages = [ pkgs.mdadm ]; - systemd.shutdownPackages = [ pkgs.mdadm ]; boot.initrd.availableKernelModules = [ "md_mod" "raid0" "raid1" "raid10" "raid456" ];