From 886f925b13a48a89507918bc96d88979357dc291 Mon Sep 17 00:00:00 2001 From: Ian Wienand Date: Thu, 1 Jun 2017 15:32:11 +1000 Subject: [PATCH] Use global state to check for duplicate fs labels As we add file-systems, add them to global state and check the labels are uniqiue. Add a unit test and remove the old global value. Bonus fixup to the length check, and a test for that too. Change-Id: I0f5a96f687c92e000afc9c98a26c49c4b1d3f28d --- diskimage_builder/block_device/level2/mkfs.py | 21 +++--- .../tests/config/duplicate_fs_labels.yaml | 68 +++++++++++++++++++ .../tests/config/too_long_fs_label.yaml | 5 ++ .../block_device/tests/test_config.py | 2 - .../block_device/tests/test_mkfs.py | 43 ++++++++++++ 5 files changed, 127 insertions(+), 12 deletions(-) create mode 100644 diskimage_builder/block_device/tests/config/duplicate_fs_labels.yaml create mode 100644 diskimage_builder/block_device/tests/config/too_long_fs_label.yaml create mode 100644 diskimage_builder/block_device/tests/test_mkfs.py diff --git a/diskimage_builder/block_device/level2/mkfs.py b/diskimage_builder/block_device/level2/mkfs.py index 20593254..8a2c18be 100644 --- a/diskimage_builder/block_device/level2/mkfs.py +++ b/diskimage_builder/block_device/level2/mkfs.py @@ -25,10 +25,6 @@ from diskimage_builder.block_device.utils import exec_sudo logger = logging.getLogger(__name__) -# There is the need that filesystem labels are unique: -# if not the boot and / or mount (with LABEL=) might fail. -file_system_labels = set() - # There is the need to check the length of the label of # the filesystem. The maximum length depends on the used filesystem. # This map provides information about the maximum label length. @@ -69,20 +65,25 @@ class FilesystemNode(NodeBase): "file system - using [img-rootfs] instead") self.label = "img-rootfs" - if self.label in file_system_labels: - raise BlockDeviceSetupException( - "File system label [%s] used more than once" % self.label) - file_system_labels.add(self.label) + # ensure we don't already have a fs with this label ... they + # all must be unique. + if 'fs_labels' in self.state: + if self.label in self.state['fs_labels']: + raise BlockDeviceSetupException( + "File system label [%s] used more than once" % self.label) + self.state['fs_labels'].append(self.label) + else: + self.state['fs_labels'] = [self.label] if self.type in file_system_max_label_length: if file_system_max_label_length[self.type] < len(self.label): raise BlockDeviceSetupException( "Label [{label}] too long for filesystem [{type}]: " - " [{len}] > [{max_len}]".format({ + "{len} > {max_len}".format(**{ 'label': self.label, 'type': self.type, 'len': len(self.label), - 'max': file_system_max_label_length[self.type]})) + 'max_len': file_system_max_label_length[self.type]})) else: logger.warning("Length of label [%s] cannot be checked for " "filesystem [%s]: unknown max length", diff --git a/diskimage_builder/block_device/tests/config/duplicate_fs_labels.yaml b/diskimage_builder/block_device/tests/config/duplicate_fs_labels.yaml new file mode 100644 index 00000000..b6f7d2f1 --- /dev/null +++ b/diskimage_builder/block_device/tests/config/duplicate_fs_labels.yaml @@ -0,0 +1,68 @@ +- local_loop: + name: image0 + +- partitioning: + base: image0 + name: mbr + label: mbr + partitions: + - name: root + base: image0 + flags: [ boot, primary ] + size: 55% + - name: var + base: image0 + size: 40% + - name: var_log + base: image0 + size: 5% + +- mkfs: + base: root + name: mkfs_root + label: duplicate + type: xfs + +- mount: + base: mkfs_root + name: mount_mkfs_root + mount_point: / + +- fstab: + base: mount_mkfs_root + name: fstab_mount_mkfs_root + fsck-passno: 1 + options: defaults + +- mkfs: + base: var + name: mkfs_var + label: duplicate + type: xfs + +- mount: + base: mkfs_var + name: mount_mkfs_var + mount_point: /var + +- fstab: + base: mount_mkfs_var + name: fstab_mount_mkfs_var + fsck-passno: 1 + options: defaults + +- mkfs: + base: var_log + name: mkfs_var_log + type: xfs + +- mount: + base: mkfs_var_log + name: mount_mkfs_var_log + mount_point: /var/log + +- fstab: + base: mount_mkfs_var_log + name: fstab_mount_mkfs_var_log + fsck-passno: 1 + options: defaults diff --git a/diskimage_builder/block_device/tests/config/too_long_fs_label.yaml b/diskimage_builder/block_device/tests/config/too_long_fs_label.yaml new file mode 100644 index 00000000..ae70f47b --- /dev/null +++ b/diskimage_builder/block_device/tests/config/too_long_fs_label.yaml @@ -0,0 +1,5 @@ +- mkfs: + base: fake + name: mkfs_root + label: this_label_is_too_long_to_work_with_xfs + type: xfs diff --git a/diskimage_builder/block_device/tests/test_config.py b/diskimage_builder/block_device/tests/test_config.py index 1e59433c..f96a8630 100644 --- a/diskimage_builder/block_device/tests/test_config.py +++ b/diskimage_builder/block_device/tests/test_config.py @@ -29,8 +29,6 @@ class TestConfig(TestBase): # reset all globals for each test. # XXX: remove globals :/ - import diskimage_builder.block_device.level2.mkfs - diskimage_builder.block_device.level2.mkfs.file_system_labels = set() import diskimage_builder.block_device.level3.mount diskimage_builder.block_device.level3.mount.sorted_mount_points = [] diff --git a/diskimage_builder/block_device/tests/test_mkfs.py b/diskimage_builder/block_device/tests/test_mkfs.py new file mode 100644 index 00000000..f8601630 --- /dev/null +++ b/diskimage_builder/block_device/tests/test_mkfs.py @@ -0,0 +1,43 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import logging + +import diskimage_builder.block_device.tests.test_config as tc + +from diskimage_builder.block_device.config import create_graph +from diskimage_builder.block_device.exception import \ + BlockDeviceSetupException + + +logger = logging.getLogger(__name__) + + +class TestMkfs(tc.TestGraphGeneration): + + def test_duplicate_labels(self): + + config = self.load_config_file('duplicate_fs_labels.yaml') + + self.assertRaisesRegex(BlockDeviceSetupException, + "used more than once", + create_graph, config, + self.fake_default_config, {}) + + def test_too_long_labels(self): + + config = self.load_config_file('too_long_fs_label.yaml') + + self.assertRaisesRegex(BlockDeviceSetupException, + "too long for filesystem", + create_graph, config, + self.fake_default_config, {})