curtin-dev team mailing list archive

[Michael Hudson-Doyle <mp+409326@xxxxxxxxxxxxxxxxxx>]

Michael Hudson-Doyle has proposed merging ~mwhudson/curtin:image-action into curtin:master.

Commit message:
Add integration tests for some partitioning operations

This adds a new test directory, tests/integration, which sits
between the unittests and the vmtests in some sense. I've added a
way for curtin storage operations to operate on a file-backed
loop device rather than a real block device and written some
tests to cover the behaviour of partition_handler by actually
running block meta and then inspecting the resulting disk image
to check the results match expectations.

This is all motivated by the changes I want to make to support
editing partition tables.

Nothing runs these tests automatically yet, need to think about
that. Currently you can run them by hand with:

  sudo python3 -m pytest tests/integration/test_block_meta.py

Requested reviews:
  curtin developers (curtin-dev)

For more details, see:
https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/409326
-- 
Your team curtin developers is requested to review the proposed merge of ~mwhudson/curtin:image-action into curtin:master.

diff --git a/curtin/block/__init__.py b/curtin/block/__init__.py
index a33ccf2..c109d50 100644
--- a/curtin/block/__init__.py
+++ b/curtin/block/__init__.py
@@ -132,7 +132,7 @@ def partition_kname(disk_kname, partition_number):
                     os.path.realpath('%s-part%s' % (disk_link,
                                                     partition_number)))
 
-    for dev_type in ['bcache', 'nvme', 'mmcblk', 'cciss', 'mpath', 'md']:
+    for dev_type in ['bcache', 'nvme', 'mmcblk', 'cciss', 'mpath', 'md', 'loop']:
         if disk_kname.startswith(dev_type):
             partition_number = "p%s" % partition_number
             break
diff --git a/curtin/commands/block_meta.py b/curtin/commands/block_meta.py
index aae85c6..372fa86 100644
--- a/curtin/commands/block_meta.py
+++ b/curtin/commands/block_meta.py
@@ -72,6 +72,8 @@ CMD_ARGUMENTS = (
                         'choices': ['ext4', 'ext3'], 'default': None}),
      ('--umount', {'help': 'unmount any mounted filesystems before exit',
                    'action': 'store_true', 'default': False}),
+     ('--testmode', {'help': 'enable some test actions',
+                     'action': 'store_true', 'default': False}),
      ('mode', {'help': 'meta-mode to use',
                'choices': [CUSTOM, SIMPLE, SIMPLE_BOOT]}),
      )
@@ -81,7 +83,10 @@ CMD_ARGUMENTS = (
 @logged_time("BLOCK_META")
 def block_meta(args):
     # main entry point for the block-meta command.
-    state = util.load_command_environment(strict=True)
+    if not args.testmode:
+        state = util.load_command_environment(strict=True)
+    else:
+        state = {}
     cfg = config.load_command_config(args, state)
     dd_images = util.get_dd_images(cfg.get('sources', {}))
 
@@ -99,7 +104,8 @@ def block_meta(args):
         args.devices = devices
 
     LOG.debug('clearing devices=%s', devices)
-    meta_clear(devices, state.get('report_stack_prefix', ''))
+    if devices:
+        meta_clear(devices, state.get('report_stack_prefix', ''))
 
     # dd-images requires use of meta_simple
     if len(dd_images) > 0 and args.force_mode is False:
@@ -517,6 +523,9 @@ def get_path_to_storage_volume(volume, storage_config):
         volume_path = block.kname_to_path(bcache_kname)
         LOG.debug('got bcache volume path %s', volume_path)
 
+    elif vol.get('type') == 'image':
+        volume_path = vol['dev']
+
     else:
         raise NotImplementedError("cannot determine the path to storage \
             volume '%s' with type '%s'" % (volume, vol.get('type')))
@@ -530,6 +539,23 @@ def get_path_to_storage_volume(volume, storage_config):
     return volume_path
 
 
+DEVS = set()
+
+
+def image_handler(info, storage_config):
+    path = info['path']
+    if os.path.exists(path):
+        os.unlink(path)
+    with open(path, 'wb') as fp:
+        fp.truncate(int(util.human2bytes(info['size'])))
+    dev = util.subp([
+        'losetup', '--show', '--find', path],
+        capture=True)[0].strip()
+    info['dev'] = dev
+    DEVS.add(dev)
+    disk_handler(info, storage_config)
+
+
 def dasd_handler(info, storage_config):
     """ Prepare the specified dasd device per configuration
 
@@ -1967,7 +1993,11 @@ def meta_custom(args):
         'zpool': zpool_handler,
     }
 
-    state = util.load_command_environment(strict=True)
+    if args.testmode:
+        command_handlers['image'] = image_handler
+        state = {}
+    else:
+        state = util.load_command_environment(strict=True)
     cfg = config.load_command_config(args, state)
 
     storage_config_dict = extract_storage_ordered_dict(cfg)
@@ -1992,6 +2022,10 @@ def meta_custom(args):
                           (item_id, type(error).__name__, error))
                 raise
 
+    if args.testmode:
+        for dev in DEVS:
+            util.subp(['losetup', '--detach', dev])
+
     if args.umount:
         util.do_umount(state['target'], recursive=True)
     return 0
diff --git a/tests/integration/__init__.py b/tests/integration/__init__.py
new file mode 100644
index 0000000..5e43585
--- /dev/null
+++ b/tests/integration/__init__.py
@@ -0,0 +1,3 @@
+# This file is part of curtin. See LICENSE file for copyright and license info.
+
+# This directory contains tests that require root to run, essentially.
diff --git a/tests/integration/test_block_meta.py b/tests/integration/test_block_meta.py
new file mode 100644
index 0000000..bdf20e2
--- /dev/null
+++ b/tests/integration/test_block_meta.py
@@ -0,0 +1,188 @@
+# This file is part of curtin. See LICENSE file for copyright and license info.
+
+from collections import namedtuple
+import contextlib
+import os
+import shutil
+import subprocess
+import sys
+import tempfile
+import unittest
+import yaml
+
+from curtin import block, udev
+
+
+def r(*args, **kw):
+    kw['check'] = True
+    print("running *%s, **%s" % (args, kw))
+    return subprocess.run(*args, **kw)
+
+
+class IntegrationTestCase(unittest.TestCase):
+    """Common testing class which all curtin unit tests subclass."""
+
+    def tmp_dir(self, dir=None, cleanup=True):
+        """Return a full path to a temporary directory for the test run."""
+        if dir is None:
+            tmpd = tempfile.mkdtemp(
+                prefix="curtin-ci-%s." % self.__class__.__name__)
+        else:
+            tmpd = tempfile.mkdtemp(dir=dir)
+        self.addCleanup(shutil.rmtree, tmpd)
+        return tmpd
+
+    def tmp_path(self, path, _dir=None):
+        # return an absolute path to 'path' under dir.
+        # if dir is None, one will be created with tmp_dir()
+        # the file is not created or modified.
+        if _dir is None:
+            _dir = os.environ.get('INTTEST_DIR')
+            if _dir is None:
+                _dir = self.tmp_dir()
+
+        return os.path.normpath(
+            os.path.abspath(os.path.sep.join((_dir, path))))
+
+
+@contextlib.contextmanager
+def loop_dev(image):
+    cp = r(
+        ['losetup', '--show', '--find', '--partscan', image],
+        stdout=subprocess.PIPE, encoding='utf-8')
+    dev = cp.stdout.strip()
+    try:
+        udev.udevadm_trigger([dev])
+        yield dev
+    finally:
+        r(['losetup', '--detach', dev])
+
+
+PartData = namedtuple("PartData", ('number', 'offset', 'size'))
+
+
+def summarize_partitions(dev):
+    # We don't care about the kname
+    return sorted(
+        [PartData(*d[1:]) for d in block.sysfs_partition_data(dev)])
+
+
+class StorageConfigBuilder:
+
+    def __init__(self):
+        self.config = []
+        self.cur_image = None
+
+    def render(self):
+        return {
+            'storage': {
+                'config': self.config,
+                },
+            }
+
+    def add_image(self, *, path, size, **kw):
+        action = {
+            'type': 'image',
+            'id': 'id' + str(len(self.config)),
+            'path': path,
+            'size': size,
+            }
+        action.update(**kw)
+        self.cur_image = action['id']
+        self.config.append(action)
+
+    def add_part(self, *, size, **kw):
+        if self.cur_image is None:
+            raise Exception("no current image")
+        action = {
+            'type': 'partition',
+            'id': 'id' + str(len(self.config)),
+            'device': self.cur_image,
+            'size': size,
+            }
+        action.update(**kw)
+        self.config.append(action)
+
+
+class TestBlockMeta(IntegrationTestCase):
+
+    def run_bm(self, config):
+        config_path = self.tmp_path('config.yaml')
+        with open(config_path, 'w') as fp:
+            yaml.dump(config, fp)
+        cmd = [
+            sys.executable, '-m', 'curtin', '--showtrace', '-vv',
+            '-c', config_path, 'block-meta', '--testmode', 'custom',
+            ]
+        r(cmd)
+
+    def _test_default_offsets(self, ptable):
+        img = self.tmp_path('image.img')
+        config = StorageConfigBuilder()
+        config.add_image(path=img, size='100M', ptable=ptable)
+        psize = 40 << 20
+        config.add_part(size=psize, number=1)
+        config.add_part(size=psize, number=2)
+        self.run_bm(config.render())
+
+        with loop_dev(img) as dev:
+            self.assertEqual(
+                summarize_partitions(dev), [
+                    PartData(
+                        number=1, offset=1 << 20, size=psize),
+                    PartData(
+                        number=2, offset=(1 << 20) + psize, size=psize),
+                ])
+
+    def test_default_offsets_gpt(self):
+        self._test_default_offsets('gpt')
+
+    def test_default_offsets_msdos(self):
+        self._test_default_offsets('msdos')
+
+    def _test_non_default_numbering(self, ptable):
+        img = self.tmp_path('image.img')
+        config = StorageConfigBuilder()
+        config.add_image(path=img, size='100M', ptable=ptable)
+        psize = 40 << 20
+        config.add_part(size=psize, number=1)
+        config.add_part(size=psize, number=4)
+        self.run_bm(config.render())
+
+        with loop_dev(img) as dev:
+            self.assertEqual(
+                summarize_partitions(dev), [
+                    PartData(
+                        number=1, offset=1 << 20, size=psize),
+                    PartData(
+                        number=4, offset=(1 << 20) + psize, size=psize),
+                ])
+
+    def test_non_default_numbering_gpt(self):
+        self._test_non_default_numbering('gpt')
+
+    def BROKEN_test_non_default_numbering_msdos(self):
+        self._test_non_default_numbering('msdos')
+
+    def test_logical(self):
+        img = self.tmp_path('image.img')
+        config = StorageConfigBuilder()
+        config.add_image(path=img, size='100M', ptable='msdos')
+        config.add_part(size='50M', number=1, flag='extended')
+        config.add_part(size='10M', number=5, flag='logical')
+        config.add_part(size='10M', number=6, flag='logical')
+        self.run_bm(config.render())
+
+        with loop_dev(img) as dev:
+            self.assertEqual(
+                summarize_partitions(dev), [
+                    # extended partitions get a strange size in sysfs
+                    PartData(number=1, offset=1 << 20, size=1 << 10),
+                    PartData(number=5, offset=2 << 20, size=10 << 20),
+                    # part 5 takes us to 12 MiB offset, curtin leaves a 1 MiB
+                    # gap.
+                    PartData(number=6, offset=13 << 20, size=10 << 20),
+                ])
+
+            p1kname = block.partition_kname(block.path_to_kname(dev), 1)
+            self.assertTrue(block.is_extended_partition('/dev/' + p1kname))