salt/volume: use sgdisk instead of parted

By using sgdisk we can change the partition GUID to match the volume
UUID. Haven't found how to do it with parted…

Refs: #2421
Signed-off-by: Sylvain Laperche <[email protected]>

docs/developer/architecture/volume.rst

@@ -159,15 +159,15 @@ device. We use different strategies according to the **Volume** type:
   ``dev/disk/by-uuid/<volume-uuid>`` as device path.
 * **sparseLoopDevice** without filesystem: we create a GUID Partition Table on
   the sparse file and create a single partition encompassing the whole device,
-  setting the label of the partition to the **Volume** UUID. We can then use
-  ``/dev/disk/by-partlabel/<volume-name>`` as device path.
+  setting the GUID of the partition to the **Volume** UUID. We can then use
+  ``/dev/disk/by-partuuid/<volume-uuid>`` as device path.
 * **rawBlockDevice** without filesystem:
 
   * the **rawBlockDevice** is a disk (e.g. ``/dev/sdb``): we use the same
     strategy as above.
-  * the **rawBlockDevice** is a partition (e.g. ``/dev/sdb1``): we re-label the
-    partition using the **Volume** UUID and use
-    ``/dev/disk/by-partlabel/<volume-name>`` as device path.
+  * the **rawBlockDevice** is a partition (e.g. ``/dev/sdb1``): we change the
+    partition GUID using the **Volume** UUID and use
+    ``/dev/disk/by-partuuid/<volume-uuid>`` as device path.
   * The **rawBlockDevice** is a LVM volume: we use the existing LVM UUID and
     use ``/dev/disk/by-id/dm-uuid-LVM-<lvm-uuid>`` as device path.
 

salt/_modules/metalk8s_volumes.py

@@ -264,16 +264,19 @@ def path(self):
         """Path to the backing device."""
         return
 
+    @property
+    def uuid(self):
+        return self.get('metadata.uid').lower()
+
     @property
     def persistent_path(self):
         """Return a persistent path to the backing device."""
-        return '/dev/disk/by-uuid/{}'.format(self.get('metadata.uid'))
+        return '/dev/disk/by-uuid/{}'.format(self.uuid)
 
     @property
     def is_prepared(self):
         """Check if the volume is already prepared by us."""
-        uuid = self.get('metadata.uid').lower()
-        return _get_from_blkid(self.path).uuid == uuid
+        return _get_from_blkid(self.path).uuid == self.uuid
 
     def prepare(self, force=False):
         """Prepare the volume.
@@ -298,9 +301,7 @@ def prepare(self, force=False):
         # mkfs options, if any, are stored as JSON-encoded list.
         options = json.loads(params.get('mkfsOptions', '[]'))
         fs_type = params['fsType']
-        command = _mkfs(
-            self.path, fs_type, self.get('metadata.uid'), force, options
-        )
+        command = _mkfs(self.path, fs_type, self.uuid, force, options)
         _run_cmd(' '.join(command))
 
     def get(self, path):
@@ -319,9 +320,7 @@ class SparseLoopDevice(Volume):
 
     @property
     def path(self):
-        return '/var/lib/metalk8s/storage/sparse/{}'.format(
-            self.get('metadata.uid')
-        )
+        return '/var/lib/metalk8s/storage/sparse/{}'.format(self.uuid)
 
     @property
     def size(self):
@@ -389,14 +388,14 @@ class SparseLoopDeviceBlock(SparseLoopDevice):
 
     @property
     def persistent_path(self):
-        return '/dev/disk/by-partlabel/{}'.format(self.get('metadata.name'))
+        return '/dev/disk/by-partuuid/{}'.format(self.uuid)
 
     @property
     def is_prepared(self):
         """Check if the volume is already prepared."""
         # Partition 1 should be labelled with the volume UUID.
-        pattern = '1.+{}'.format(re.escape(self.get('metadata.name').lower()))
-        result  = _run_cmd(' '.join(['parted', self.path, 'print']))
+        pattern = '1.+{}$'.format(re.escape(self.get('metadata.name')))
+        result  = _run_cmd(' '.join(['sgdisk', '--print', self.path]))
         return re.search(pattern, result['stdout']) is not None
 
     def prepare(self, force=False):
@@ -468,7 +467,7 @@ def persistent_path(self):
                 realpath = os.path.realpath(symlink)
                 if os.path.basename(realpath) == name:
                     return symlink
-        return '/dev/disk/by-partlabel/{}'.format(self.get('metadata.name'))
+        return '/dev/disk/by-partuuid/{}'.format(self.uuid)
 
     @property
     def device_path(self):
@@ -483,25 +482,28 @@ def is_prepared(self):
         if self._kind == DeviceType.LVM:
             return True
         partition = self._partition or '1'
-        pattern = '{}.+{}'.format(
-            partition, re.escape(self.get('metadata.name').lower())
+        pattern = '{}.+{}$'.format(
+            partition, re.escape(self.get('metadata.name'))
         )
-        result  = _run_cmd(' '.join(['parted', self.device_path, 'print']))
+        result  = _run_cmd(' '.join(['sgdisk', '--print', self.device_path]))
         return re.search(pattern, result['stdout']) is not None
 
     def prepare(self, force=False):
         # Nothing to do in LVM case.
         if self._kind == DeviceType.LVM:
             return
         name = self.get('metadata.name')
-        # In case of partition, set the label to volume name.
+        # For partition, set the partition's label & UID to the volume's ones.
         if self._kind == DeviceType.PARTITION:
             _run_cmd(' '.join([
-                'parted', self.device_path, 'name', self._partition, name
+                'sgdisk',
+                '--partition-guid', '{}:{}'.format(self._partition, self.uuid),
+                '--change-name', '{}:{}'.format(self._partition, name),
+                self.device_path,
             ]))
         # Otherwise, create a GPT table and a unique partition.
         else:
-            prepare_block(self.path, name)
+            prepare_block(self.path, name, self.uuid)
 
 class DeviceType:
     DISK      = 1
@@ -659,17 +661,18 @@ def _mkfs_xfs(path, uuid, force=False, options=None):
     command.append(path)
     return command
 
-def prepare_block(path, name):
+def prepare_block(path, name, uuid):
     """Prepare a "Block" volume.
 
     We use a GPT table and a single partition to have a link between the volume
-    name and the paritition label.
+    name/uuid and the partition label/GUID.
     """
-    # Create a GPT partition table.
-    _run_cmd(' '.join(['parted', path, 'mktable', 'gpt']))
-    # Create a single partition labelled with the volume UUID.
     _run_cmd(' '.join([
-        'parted', '--align', 'optimal', path, 'mkpart', name, '0%', '100%',
+        'sgdisk',
+        '--largest-new', '1',
+        '--partition-guid', '1:{}'.format(uuid),
+        '--change-name', '1:{}'.format(name),
+        path,
     ]))
 
 # }}}

salt/metalk8s/volumes/prepared/init.sls

@@ -40,7 +40,7 @@ Prepare backing storage for {{ volume }}:
     - require:
       - metalk8s_package_manager: Install e2fsprogs
       - metalk8s_package_manager: Install xfsprogs
-      - metalk8s_package_manager: Install parted
+      - metalk8s_package_manager: Install gdisk
       - metalk8s_volumes: Create backing storage for {{ volume }}
 
 Provision backing storage for {{ volume }}:

salt/metalk8s/volumes/prepared/installed.sls

@@ -13,7 +13,7 @@ Install xfsprogs:
     - require:
       - test: Repositories configured
 
-Install parted:
-  {{ pkg_installed('parted') }}
+Install gdisk:
+  {{ pkg_installed('gdisk') }}
     - require:
       - test: Repositories configured

tests/post/steps/test_volume.py

@@ -437,7 +437,7 @@ def check_storage_is_created(context, host, name):
     if volume['spec'].get('mode', 'Filesystem') == 'Filesystem':
         host.run_test('test -b /dev/disk/by-uuid/{}'.format(uuid))
     else:
-        host.run_test('test -b /dev/disk/by-partlabel/{}'.format(name))
+        host.run_test('test -b /dev/disk/by-partuuid/{}'.format(uuid))
 
 
 @then(parsers.parse("the backing storage for Volume '{name}' is deleted"))