curtin-dev team mailing list archive

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

Thanks for the comments, pushed a commit which addresses most of them.

Diff comments:

> diff --git a/curtin/commands/block_meta.py b/curtin/commands/block_meta.py
> index aae85c6..4b5a56b 100644
> --- a/curtin/commands/block_meta.py
> +++ b/curtin/commands/block_meta.py
> @@ -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):

Added something but not sure it's quite what you meant?

> +        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
>  
> 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):

No, switched.

> +    kw['check'] = True
> +    print("running *%s, **%s" % (args, kw))
> +    return subprocess.run(*args, **kw)
> +
> +
> +class IntegrationTestCase(unittest.TestCase):

There were two reasons I wasn't using that class:

1. I didn't manage to import it from this location, although that was probably me being silly.
2. I thought it did some things that wouldn't be wanted in this context, although on looking it looks like it's only the allowed_subp stuff.

So yeah, updated this.

> +    """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):

Eh, maybe? Not feeling motivated to make this change right now.

> +        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

OK.

> +        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))


-- 
https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/409326
Your team curtin developers is subscribed to branch curtin:master.