Merge pull request #568 from nix-community/idempotent-create

types *: make create idempotent

disk-deactivate/disk-deactivate

@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
 set -efux -o pipefail
-# dependencies: bash jq util-linux lvm2 mdadm zfs
+# dependencies: bash jq util-linux lvm2 mdadm zfs gnugrep
 disk=$(realpath "$1")
 
 lsblk -a -f >&2

example/zfs.nix

@@ -52,7 +52,7 @@
           "com.sun:auto-snapshot" = "false";
         };
         mountpoint = "/";
-        postCreateHook = "zfs snapshot zroot@blank";
+        postCreateHook = "zfs list -t snapshot -H -o name | grep -E '^zroot@blank$' || zfs snapshot zroot@blank";
 
         datasets = {
           zfs_fs = {

lib/tests.nix

@@ -243,15 +243,23 @@ let
             # running direct mode
             machine.succeed("${tsp-format}")
             machine.succeed("${tsp-mount}")
-            machine.succeed("${tsp-mount}") # verify that the command is idempotent
+            machine.succeed("${tsp-mount}") # verify that mount is idempotent
             machine.succeed("${tsp-disko}") # verify that we can destroy and recreate
+            machine.succeed("mkdir -p /mnt/home")
+            machine.succeed("touch /mnt/home/testfile")
+            machine.succeed("${tsp-format}") # verify that format is idempotent
+            machine.succeed("test -e /mnt/home/testfile")
           ''}
           ${lib.optionalString (testMode == "module") ''
             #  running module mode
             machine.succeed("${nodes.machine.system.build.formatScript}")
             machine.succeed("${nodes.machine.system.build.mountScript}")
-            machine.succeed("${nodes.machine.system.build.mountScript}") # verify that the command is idempotent
+            machine.succeed("${nodes.machine.system.build.mountScript}") # verify that mount is idempotent
             machine.succeed("${nodes.machine.system.build.diskoScript}") # verify that we can destroy and recreate again
+            machine.succeed("mkdir -p /mnt/home")
+            machine.succeed("touch /mnt/home/testfile")
+            machine.succeed("${nodes.machine.system.build.formatScript}") # verify that format is idempotent
+            machine.succeed("test -e /mnt/home/testfile")
           ''}
 
           ${postDisko}

lib/types/btrfs.nix

@@ -31,7 +31,11 @@ let
   swapCreate = mountpoint: swap:
     lib.concatMapStringsSep
       "\n"
-      (file: ''btrfs filesystem mkswapfile --size ${file.size} "${mountpoint}/${file.path}"'')
+      (file: ''
+      if ! test -e "${mountpoint}/${file.path}"; then
+        btrfs filesystem mkswapfile --size ${file.size} "${mountpoint}/${file.path}"
+      fi
+      '')
       (lib.attrValues swap);
 
 in
@@ -112,26 +116,33 @@ in
     _create = diskoLib.mkCreateOption {
       inherit config options;
       default = ''
-        mkfs.btrfs ${config.device} ${toString config.extraArgs}
+        # create the filesystem only if the device seems empty
-        ${lib.optionalString (config.swap != {}) ''
+        if ! (blkid '${config.device}' -o export | grep -q '^TYPE='); then
-          (
+          mkfs.btrfs "${config.device}" ${toString config.extraArgs}
-            MNTPOINT=$(mktemp -d)
+        fi
-            mount ${device} "$MNTPOINT" -o subvol=/
+        if (blkid "${config.device}" -o export | grep -q '^TYPE=btrfs$'); then
-            trap 'umount $MNTPOINT; rm -rf $MNTPOINT' EXIT
+          ${lib.optionalString (config.swap != {}) ''
-            ${swapCreate "$MNTPOINT" config.swap}
+            (
-          )
+              MNTPOINT=$(mktemp -d)
-        ''}
+              mount ${device} "$MNTPOINT" -o subvol=/
-        ${lib.concatMapStrings (subvol: ''
+              trap 'umount $MNTPOINT; rm -rf $MNTPOINT' EXIT
-          (
+              ${swapCreate "$MNTPOINT" config.swap}
-            MNTPOINT=$(mktemp -d)
+            )
-            mount ${config.device} "$MNTPOINT" -o subvol=/
+          ''}
-            trap 'umount $MNTPOINT; rm -rf $MNTPOINT' EXIT
+          ${lib.concatMapStrings (subvol: ''
-            SUBVOL_ABS_PATH="$MNTPOINT/${subvol.name}"
+            (
-            mkdir -p "$(dirname "$SUBVOL_ABS_PATH")"
+              MNTPOINT=$(mktemp -d)
-            btrfs subvolume create "$SUBVOL_ABS_PATH" ${toString subvol.extraArgs}
+              mount ${config.device} "$MNTPOINT" -o subvol=/
-            ${swapCreate "$SUBVOL_ABS_PATH" subvol.swap}
+              trap 'umount $MNTPOINT; rm -rf $MNTPOINT' EXIT
-          )
+              SUBVOL_ABS_PATH="$MNTPOINT/${subvol.name}"
-        '') (lib.attrValues config.subvolumes)}
+              mkdir -p "$(dirname "$SUBVOL_ABS_PATH")"
+              if ! btrfs subvolume show "$SUBVOL_ABS_PATH" > /dev/null 2>&1; then
+                btrfs subvolume create "$SUBVOL_ABS_PATH" ${toString subvol.extraArgs}
+              fi
+              ${swapCreate "$SUBVOL_ABS_PATH" subvol.swap}
+            )
+          '') (lib.attrValues config.subvolumes)}
+        fi
       '';
     };
     _mount = diskoLib.mkMountOption {
@@ -206,7 +217,7 @@ in
       readOnly = true;
       type = lib.types.functionTo (lib.types.listOf lib.types.package);
       default = pkgs:
-        [ pkgs.btrfs-progs pkgs.coreutils ];
+        [ pkgs.btrfs-progs pkgs.coreutils pkgs.gnugrep ];
       description = "Packages";
     };
   };

lib/types/filesystem.nix

@@ -44,9 +44,11 @@
     _create = diskoLib.mkCreateOption {
       inherit config options;
       default = ''
-        mkfs.${config.format} \
+        if ! (blkid '${config.device}' | grep -q 'TYPE='); then
-          ${toString config.extraArgs} \
+          mkfs.${config.format} \
-          ${config.device}
+            ${toString config.extraArgs} \
+            ${config.device}
+        fi
       '';
     };
     _mount = diskoLib.mkMountOption {
@@ -79,7 +81,7 @@
       readOnly = true;
       # type = lib.types.functionTo (lib.types.listOf lib.types.package);
       default = pkgs:
-        [ pkgs.util-linux ] ++ (
+        [ pkgs.util-linux pkgs.gnugrep ] ++ (
           # TODO add many more
           if (config.format == "xfs") then [ pkgs.xfsprogs ]
           else if (config.format == "btrfs") then [ pkgs.btrfs-progs ]

lib/types/gpt.nix

@@ -153,34 +153,44 @@ in
     _create = diskoLib.mkCreateOption {
       inherit config options;
       default = ''
+        if ! blkid "${config.device}" >/dev/null; then
+          ${lib.concatStrings (map (partition: ''
+            if sgdisk \
+              --info=${toString partition._index} \
+              ${config.device} > /dev/null 2>&1
+            then
+              sgdisk \
+                --align-end \
+                --new=${toString partition._index}:${partition.start}:${partition.end} \
+                --change-name=${toString partition._index}:${partition.label} \
+                --typecode=${toString partition._index}:${partition.type} \
+                ${config.device}
+              # ensure /dev/disk/by-path/..-partN exists before continuing
+              partprobe ${config.device}
+              udevadm trigger --subsystem-match=block
+              udevadm settle
+            fi
+          '') sortedPartitions)}
+
+          ${
+            lib.optionalString (sortedHybridPartitions != [])
+            ("sgdisk -h "
+              + (lib.concatStringsSep ":" (map (p: (toString p._index)) sortedHybridPartitions))
+              + (
+                lib.optionalString (!config.efiGptPartitionFirst) ":EE "
+              )
+              + parent.device)
+          }
+          ${lib.concatMapStrings (p:
+              p.hybrid._create
+            )
+            sortedHybridPartitions
+          }
+        fi
+
         ${lib.concatStrings (map (partition: ''
-          sgdisk \
-            --align-end \
-            --new=${toString partition._index}:${partition.start}:${partition.end} \
-            --change-name=${toString partition._index}:${partition.label} \
-            --typecode=${toString partition._index}:${partition.type} \
-            ${config.device}
-          # ensure /dev/disk/by-path/..-partN exists before continuing
-          partprobe ${config.device}
-          udevadm trigger --subsystem-match=block
-          udevadm settle
           ${lib.optionalString (partition.content != null) partition.content._create}
         '') sortedPartitions)}
-
-        ${
-          lib.optionalString (sortedHybridPartitions != [])
-          ("sgdisk -h "
-            + (lib.concatStringsSep ":" (map (p: (toString p._index)) sortedHybridPartitions))
-            + (
-              lib.optionalString (!config.efiGptPartitionFirst) ":EE "
-            )
-            + parent.device)
-        }
-        ${lib.concatMapStrings (p:
-            p.hybrid._create
-          )
-          sortedHybridPartitions}
-
       '';
     };
     _mount = diskoLib.mkMountOption {

lib/types/luks.nix

@@ -112,26 +112,28 @@ in
     _create = diskoLib.mkCreateOption {
       inherit config options;
       default = ''
-        ${lib.optionalString config.askPassword ''
+        if ! blkid "${config.device}" >/dev/null || ! (blkid "${config.device}" -o export | grep -q '^TYPE='); then
-          set +x
+          ${lib.optionalString config.askPassword ''
-          askPassword() {
+            set +x
-            echo "Enter password for ${config.device}: "
+            askPassword() {
-            IFS= read -r -s password
+              echo "Enter password for ${config.device}: "
-            echo "Enter password for ${config.device} again to be safe: "
+              IFS= read -r -s password
-            IFS= read -r -s password_check
+              echo "Enter password for ${config.device} again to be safe: "
-            export password
+              IFS= read -r -s password_check
-            [ "$password" = "$password_check" ]
+              export password
-          }
+              [ "$password" = "$password_check" ]
-          until askPassword; do
+            }
-            echo "Passwords did not match, please try again."
+            until askPassword; do
-          done
+              echo "Passwords did not match, please try again."
-          set -x
+            done
-        ''}
+            set -x
-        cryptsetup -q luksFormat ${config.device} ${toString config.extraFormatArgs} ${keyFileArgs}
+          ''}
-        ${cryptsetupOpen} --persistent
+          cryptsetup -q luksFormat ${config.device} ${toString config.extraFormatArgs} ${keyFileArgs}
-        ${toString (lib.forEach config.additionalKeyFiles (keyFile: ''
+          ${cryptsetupOpen} --persistent
-          cryptsetup luksAddKey ${config.device} ${keyFile} ${keyFileArgs}
+          ${toString (lib.forEach config.additionalKeyFiles (keyFile: ''
-        ''))}
+            cryptsetup luksAddKey ${config.device} ${keyFile} ${keyFileArgs}
+          ''))}
+        fi
         ${lib.optionalString (config.content != null) config.content._create}
       '';
     };
@@ -176,7 +178,7 @@ in
       internal = true;
       readOnly = true;
       type = lib.types.functionTo (lib.types.listOf lib.types.package);
-      default = pkgs: [ pkgs.cryptsetup ] ++ (lib.optionals (config.content != null) (config.content._pkgs pkgs));
+      default = pkgs: [ pkgs.gnugrep pkgs.cryptsetup ] ++ (lib.optionals (config.content != null) (config.content._pkgs pkgs));
       description = "Packages";
     };
   };

lib/types/lvm_pv.nix

@@ -31,7 +31,9 @@
     _create = diskoLib.mkCreateOption {
       inherit config options;
       default = ''
-        pvcreate ${config.device}
+        if ! (blkid '${config.device}' | grep -q 'TYPE='); then
+          pvcreate ${config.device}
+        fi
         echo "${config.device}" >>"$disko_devices_dir"/lvm_${config.vg}
       '';
     };
@@ -49,7 +51,7 @@
       internal = true;
       readOnly = true;
       type = lib.types.functionTo (lib.types.listOf lib.types.package);
-      default = pkgs: [ pkgs.lvm2 ];
+      default = pkgs: [ pkgs.gnugrep pkgs.lvm2 ];
       description = "Packages";
     };
   };

lib/types/lvm_vg.nix

@@ -68,16 +68,23 @@
         in
         ''
           readarray -t lvm_devices < <(cat "$disko_devices_dir"/lvm_${config.name})
-          vgcreate ${config.name} \
+          if ! vgdisplay "${config.name}" >/dev/null; then
-            "''${lvm_devices[@]}"
+            vgcreate ${config.name} \
+              "''${lvm_devices[@]}"
+          fi
+          ${lib.concatMapStrings (lv: ''
+            if ! lvdisplay '${config.name}/${lv.name}'; then
+              lvcreate \
+                --yes \
+                ${if lib.hasInfix "%" lv.size then "-l" else "-L"} ${lv.size} \
+                -n ${lv.name} \
+                ${lib.optionalString (lv.lvm_type != null) "--type=${lv.lvm_type}"} \
+                ${toString lv.extraArgs} \
+                ${config.name}
+            fi
+          '') sortedLvs}
+
           ${lib.concatMapStrings (lv: ''
-            lvcreate \
-              --yes \
-              ${if lib.hasInfix "%" lv.size then "-l" else "-L"} ${lv.size} \
-              -n ${lv.name} \
-              ${lib.optionalString (lv.lvm_type != null) "--type=${lv.lvm_type}"} \
-              ${toString lv.extraArgs} \
-              ${config.name}
             ${lib.optionalString (lv.content != null) lv.content._create}
           '') sortedLvs}
         '';

lib/types/mdadm.nix

@@ -34,19 +34,21 @@
     _create = diskoLib.mkCreateOption {
       inherit config options;
       default = ''
-        readarray -t disk_devices < <(cat "$disko_devices_dir"/raid_${config.name})
+        if ! test -e /dev/md/${config.name}; then
-        echo 'y' | mdadm --create /dev/md/${config.name} \
+          readarray -t disk_devices < <(cat "$disko_devices_dir"/raid_${config.name})
-          --level=${toString config.level} \
+          echo 'y' | mdadm --create /dev/md/${config.name} \
-          --raid-devices="$(wc -l "$disko_devices_dir"/raid_${config.name} | cut -f 1 -d " ")" \
+            --level=${toString config.level} \
-          --metadata=${config.metadata} \
+            --raid-devices="$(wc -l "$disko_devices_dir"/raid_${config.name} | cut -f 1 -d " ")" \
-          --force \
+            --metadata=${config.metadata} \
-          --homehost=any \
+            --force \
-          "''${disk_devices[@]}"
+            --homehost=any \
-        partprobe /dev/md/${config.name}
+            "''${disk_devices[@]}"
-        udevadm trigger --subsystem-match=block
+          partprobe /dev/md/${config.name}
-        udevadm settle
+          udevadm trigger --subsystem-match=block
-        # for some reason mdadm devices spawn with an existing partition table, so we need to wipe it
+          udevadm settle
-        sgdisk --zap-all /dev/md/${config.name}
+          # for some reason mdadm devices spawn with an existing partition table, so we need to wipe it
+          sgdisk --zap-all /dev/md/${config.name}
+        fi
         ${lib.optionalString (config.content != null) config.content._create}
       '';
     };

lib/types/swap.nix

@@ -40,9 +40,11 @@
     _create = diskoLib.mkCreateOption {
       inherit config options;
       default = ''
-        mkswap \
+        if ! blkid "${config.device}" -o export | grep -q '^TYPE='; then
-          ${toString config.extraArgs} \
+          mkswap \
-          ${config.device}
+            ${toString config.extraArgs} \
+            ${config.device}
+        fi
       '';
     };
     _mount = diskoLib.mkMountOption {

lib/types/table.nix

@@ -88,28 +88,33 @@
     _create = diskoLib.mkCreateOption {
       inherit config options;
       default = ''
-        parted -s ${config.device} -- mklabel ${config.format}
+        if ! blkid "${config.device}" >/dev/null; then
+          parted -s ${config.device} -- mklabel ${config.format}
+          ${lib.concatStrings (map (partition: ''
+            ${lib.optionalString (config.format == "gpt") ''
+              parted -s ${config.device} -- mkpart ${partition.name} ${diskoLib.maybeStr partition.fs-type} ${partition.start} ${partition.end}
+            ''}
+            ${lib.optionalString (config.format == "msdos") ''
+              parted -s ${config.device} -- mkpart ${partition.part-type} ${diskoLib.maybeStr partition.fs-type} ${partition.start} ${partition.end}
+            ''}
+            # ensure /dev/disk/by-path/..-partN exists before continuing
+            partprobe ${config.device}
+            udevadm trigger --subsystem-match=block
+            udevadm settle
+            ${lib.optionalString partition.bootable ''
+              parted -s ${config.device} -- set ${toString partition._index} boot on
+            ''}
+            ${lib.concatMapStringsSep "" (flag: ''
+              parted -s ${config.device} -- set ${toString partition._index} ${flag} on
+            '') partition.flags}
+            # ensure further operations can detect new partitions
+            partprobe ${config.device}
+            udevadm trigger --subsystem-match=block
+            udevadm settle
+            ${lib.optionalString (partition.content != null) partition.content._create}
+          '') config.partitions)}
+        fi
         ${lib.concatStrings (map (partition: ''
-          ${lib.optionalString (config.format == "gpt") ''
-            parted -s ${config.device} -- mkpart ${partition.name} ${diskoLib.maybeStr partition.fs-type} ${partition.start} ${partition.end}
-          ''}
-          ${lib.optionalString (config.format == "msdos") ''
-            parted -s ${config.device} -- mkpart ${partition.part-type} ${diskoLib.maybeStr partition.fs-type} ${partition.start} ${partition.end}
-          ''}
-          # ensure /dev/disk/by-path/..-partN exists before continuing
-          partprobe ${config.device}
-          udevadm trigger --subsystem-match=block
-          udevadm settle
-          ${lib.optionalString partition.bootable ''
-            parted -s ${config.device} -- set ${toString partition._index} boot on
-          ''}
-          ${lib.concatMapStringsSep "" (flag: ''
-            parted -s ${config.device} -- set ${toString partition._index} ${flag} on
-          '') partition.flags}
-          # ensure further operations can detect new partitions
-          partprobe ${config.device}
-          udevadm trigger --subsystem-match=block
-          udevadm settle
           ${lib.optionalString (partition.content != null) partition.content._create}
         '') config.partitions)}
       '';

lib/types/zfs_fs.nix

@@ -59,8 +59,10 @@
       # since (create order != mount order)
       # -p creates parents automatically
       default = ''
-        zfs create -up ${config._name} \
+        if ! zfs get type ${config._name} >/dev/null 2>&1; then
-          ${lib.concatStringsSep " " (lib.mapAttrsToList (n: v: "-o ${n}=${v}") config.options)}
+          zfs create -up ${config._name} \
+            ${lib.concatStringsSep " " (lib.mapAttrsToList (n: v: "-o ${n}=${v}") config.options)}
+        fi
       '';
     } // { readOnly = false; };
 

lib/types/zfs_volume.nix

@@ -47,13 +47,15 @@
     _create = diskoLib.mkCreateOption {
       inherit config options;
       default = ''
-        zfs create ${config._parent.name}/${config.name} \
+        if ! zfs get type ${config._parent.name}/${config.name} >/dev/null 2>&1; then
-          ${lib.concatStringsSep " " (lib.mapAttrsToList (n: v: "-o ${n}=${v}") config.options)} \
+          zfs create ${config._parent.name}/${config.name} \
-          -V ${config.size}
+            ${lib.concatStringsSep " " (lib.mapAttrsToList (n: v: "-o ${n}=${v}") config.options)} \
-        zvol_wait
+            -V ${config.size}
-        partprobe /dev/zvol/${config._parent.name}/${config.name}
+          zvol_wait
-        udevadm trigger --subsystem-match=block
+          partprobe /dev/zvol/${config._parent.name}/${config.name}
-        udevadm settle
+          udevadm trigger --subsystem-match=block
+          udevadm settle
+        fi
         ${lib.optionalString (config.content != null) config.content._create}
       '';
     };

lib/types/zpool.nix

@@ -62,13 +62,32 @@
       inherit config options;
       default = ''
         readarray -t zfs_devices < <(cat "$disko_devices_dir"/zfs_${config.name})
-        zpool create -f ${config.name} \
+        if zpool list '${config.name}'; then
-          -R ${rootMountPoint} ${config.mode} \
+          echo "not creating zpool ${config.name} as a pool with that name already exists" >&2
-          ${lib.concatStringsSep " " (lib.mapAttrsToList (n: v: "-o ${n}=${v}") config.options)} \
+        else
-          ${lib.concatStringsSep " " (lib.mapAttrsToList (n: v: "-O ${n}=${v}") config.rootFsOptions)} \
+          continue=1
-          "''${zfs_devices[@]}"
+          for dev in "''${zfs_devices[@]}"; do
-        if [[ $(zfs get -H mounted ${config.name} | cut -f3) == "yes" ]]; then
+            if ! blkid "$dev" >/dev/null; then
-          zfs unmount ${config.name}
+              # blkid fails, so device seems empty
+              :
+            elif (blkid "$dev" -o export | grep '^PTUUID='); then
+              echo "device $dev already has a partuuid, skipping creating zpool ${config.name}" >&2
+              continue=0
+            elif (blkid "$dev" -o export | grep '^TYPE='); then
+              echo "device $dev already has a partition, skipping creating zpool ${config.name}" >&2
+              continue=0
+            fi
+          done
+          if [ $continue -eq 1 ]; then
+            zpool create -f ${config.name} \
+              -R ${rootMountPoint} ${config.mode} \
+              ${lib.concatStringsSep " " (lib.mapAttrsToList (n: v: "-o ${n}=${v}") config.options)} \
+              ${lib.concatStringsSep " " (lib.mapAttrsToList (n: v: "-O ${n}=${v}") config.rootFsOptions)} \
+              "''${zfs_devices[@]}"
+            if [[ $(zfs get -H mounted ${config.name} | cut -f3) == "yes" ]]; then
+              zfs unmount ${config.name}
+            fi
+          fi
         fi
         ${lib.concatMapStrings (dataset: dataset._create) (lib.attrValues config.datasets)}
       '';
@@ -98,7 +117,7 @@
       internal = true;
       readOnly = true;
       type = lib.types.functionTo (lib.types.listOf lib.types.package);
-      default = pkgs: [ pkgs.util-linux ] ++ lib.flatten (map (dataset: dataset._pkgs pkgs) (lib.attrValues config.datasets));
+      default = pkgs: [ pkgs.gnugrep pkgs.util-linux ] ++ lib.flatten (map (dataset: dataset._pkgs pkgs) (lib.attrValues config.datasets));
       description = "Packages";
     };
   };