From 397d9a8f3c551d06a072fe2f9803483a7da87ead Mon Sep 17 00:00:00 2001 From: Sylvain Laperche Date: Fri, 3 Jul 2020 17:06:07 +0200 Subject: [PATCH] salt/volume: use sgdisk instead of parted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- docs/developer/architecture/volume.rst | 10 ++-- salt/_modules/metalk8s_volumes.py | 53 +++++++++++--------- salt/metalk8s/volumes/prepared/init.sls | 2 +- salt/metalk8s/volumes/prepared/installed.sls | 4 +- tests/post/steps/test_volume.py | 2 +- 5 files changed, 37 insertions(+), 34 deletions(-) diff --git a/docs/developer/architecture/volume.rst b/docs/developer/architecture/volume.rst index 140d718f78..f9461fa463 100644 --- a/docs/developer/architecture/volume.rst +++ b/docs/developer/architecture/volume.rst @@ -159,15 +159,15 @@ device. We use different strategies according to the **Volume** type: ``dev/disk/by-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/`` as device path. + setting the GUID of the partition to the **Volume** UUID. We can then use + ``/dev/disk/by-partuuid/`` 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/`` 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/`` as device path. * The **rawBlockDevice** is a LVM volume: we use the existing LVM UUID and use ``/dev/disk/by-id/dm-uuid-LVM-`` as device path. diff --git a/salt/_modules/metalk8s_volumes.py b/salt/_modules/metalk8s_volumes.py index c34bd755c3..bd7bd977ed 100644 --- a/salt/_modules/metalk8s_volumes.py +++ b/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,10 +482,10 @@ 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): @@ -494,14 +493,17 @@ def prepare(self, force=False): 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, ])) # }}} diff --git a/salt/metalk8s/volumes/prepared/init.sls b/salt/metalk8s/volumes/prepared/init.sls index 34f89a5613..f8a920c3e4 100644 --- a/salt/metalk8s/volumes/prepared/init.sls +++ b/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 }}: diff --git a/salt/metalk8s/volumes/prepared/installed.sls b/salt/metalk8s/volumes/prepared/installed.sls index b6a5f7cbb5..0517a7bf02 100644 --- a/salt/metalk8s/volumes/prepared/installed.sls +++ b/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 diff --git a/tests/post/steps/test_volume.py b/tests/post/steps/test_volume.py index 4b88fb2d60..20a01f9308 100644 --- a/tests/post/steps/test_volume.py +++ b/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"))