diff -Nru ubuntu-image-0.12+16.04ubuntu1/debian/changelog ubuntu-image-0.14+16.04ubuntu2/debian/changelog --- ubuntu-image-0.12+16.04ubuntu1/debian/changelog 2016-11-08 22:31:21.000000000 +0000 +++ ubuntu-image-0.14+16.04ubuntu2/debian/changelog 2017-01-20 14:59:20.000000000 +0000 @@ -1,3 +1,45 @@ +ubuntu-image (0.14+16.04ubuntu2) xenial; urgency=medium + + * SRU tracking bug LP: #1655133 + * d/changelog: Clean up bug tags so as not to confuse SRU linkage. + + -- Barry Warsaw Fri, 20 Jan 2017 09:59:20 -0500 + +ubuntu-image (0.14+16.04ubuntu1) xenial; urgency=medium + + * Add CI for Python 3.6. (LP:1650402) + * Fix the test suite for sparse files on ZFS. (LP:1656371) + * d/t/control: Add Restriction: isolation-machine for the mount test + since devmapper is not namespaced and thus can interfere with other + containers. This will prevent the test from running in schroot and + lxd/lxc backends, but will continue to run in qemu backends. + (LP:1656391) + + -- Barry Warsaw Fri, 13 Jan 2017 18:20:39 -0500 + +ubuntu-image (0.13+16.04ubuntu2) xenial; urgency=medium + + * d/tests/mount: Fix a typo and handle a case where the root file system + isn't found. + + -- Barry Warsaw Tue, 10 Jan 2017 17:33:58 -0500 + +ubuntu-image (0.13+16.04ubuntu1) xenial; urgency=medium + + * Refuse to write images to /tmp when running ubuntu-image as a snap, + since the snap's /tmp is not accessible to the user. (LP:1646968) + * When sparse copying the resulting disk image, don't try to preserve + mode,ownership,timestamps. Over remote file systems (e.g. afp, hgfs) + this can fail. Over local file systems, they'll be preserved anyway. + (LP:1637554) + * Use the `role` attribute of the gadget.yaml when creating images, + especially for `role:mbr` and `role:system-data` (LP:1642914 and + LP:1643086) + * d/tests/mount: Switch to the stable channel for snaps. + * Make the test suite immune to the locale. (LP:1638570) + + -- Barry Warsaw Mon, 09 Jan 2017 15:38:40 -0500 + ubuntu-image (0.12+16.04ubuntu1) xenial; urgency=medium * SRU tracking bug LP: #1646608 diff -Nru ubuntu-image-0.12+16.04ubuntu1/debian/tests/control ubuntu-image-0.14+16.04ubuntu2/debian/tests/control --- ubuntu-image-0.12+16.04ubuntu1/debian/tests/control 2016-11-08 22:31:21.000000000 +0000 +++ ubuntu-image-0.14+16.04ubuntu2/debian/tests/control 2017-01-20 14:59:20.000000000 +0000 @@ -3,27 +3,32 @@ Test-Command: ubuntu-image --help Depends: ubuntu-image +Features: test-name=help Test-Command: ubuntu-image --version Depends: ubuntu-image +Features: test-name=version Test-Command: man ubuntu-image Depends: ubuntu-image, man-db +Features: test-name=manpage # Something in the tox/pip/setuptools stack requires git. -Test-Command: tox -e py35 +Test-Command: tox -e py35-nocov,py36-nocov Depends: @builddeps@, git Restrictions: needs-root +Features: test-name=unittests Test-Command: tox -e qa Depends: @builddeps@, git Restrictions: needs-root +Features: test-name=qa Tests: coverage.sh Depends: @builddeps@, git, lsb-release Restrictions: needs-root Tests: mount -Restrictions: needs-root, allow-stderr +Restrictions: needs-root, allow-stderr, isolation-machine Depends: @, kpartx diff -Nru ubuntu-image-0.12+16.04ubuntu1/debian/tests/coverage.sh ubuntu-image-0.14+16.04ubuntu2/debian/tests/coverage.sh --- ubuntu-image-0.12+16.04ubuntu1/debian/tests/coverage.sh 2016-11-08 22:31:21.000000000 +0000 +++ ubuntu-image-0.14+16.04ubuntu2/debian/tests/coverage.sh 2017-01-20 14:59:20.000000000 +0000 @@ -9,4 +9,4 @@ export UBUNTU_IMAGE_CODENAME="devel" fi -tox -e coverage +tox -e py35-cov,py36-cov diff -Nru ubuntu-image-0.12+16.04ubuntu1/debian/tests/mount ubuntu-image-0.14+16.04ubuntu2/debian/tests/mount --- ubuntu-image-0.12+16.04ubuntu1/debian/tests/mount 2016-11-08 22:31:21.000000000 +0000 +++ ubuntu-image-0.14+16.04ubuntu2/debian/tests/mount 2017-01-20 14:59:20.000000000 +0000 @@ -17,13 +17,13 @@ from contextlib import ExitStack from subprocess import PIPE, run -from ubuntu_image.parser import FileSystemType, parse +from ubuntu_image.parser import FileSystemType, StructureRole, parse TMP = os.environ['AUTOPKGTEST_TMP'] DIR = os.path.abspath(os.path.join('debian', 'tests', 'models')) STATUS = {} -CHANNEL = 'beta' +CHANNEL = 'stable' for model_file in os.listdir(DIR): @@ -50,7 +50,7 @@ # kpartx output; 2) there is a magical last partition not defined in # the gadget.yaml which is the writable partition and is always # mountable. - if part.type == 'mbr': + if part.role is StructureRole.mbr: continue mountable.append( # True if the partition is mountable, False otherwise. @@ -65,7 +65,7 @@ proc = run(['kpartx', '-avs', disk_img], universal_newlines=True, stdout=PIPE) - resources.callback(run, ['kpartx', '-sdv', disk_img]) + resources.callback(run, ['kpartx', '-dvs', disk_img]) # Parse the kpartx output to figure out what the device names are. # E.g. here is the current output of kpartx -avs as an example. It's # the third column we care about. @@ -79,6 +79,12 @@ # add map loop5p7 (252:7): 0 2048 linear 7:5 16384 # add map loop5p8 (252:8): 0 262144 linear 7:5 18432 # add map loop5p9 (252:9): 0 1051446 linear 7:5 280576 + # + # In order to produce a bootable image, the root file system partition + # *must* be labeled 'writable'. While we're looping through the + # kpartx output, keep track of the last partition we see. That'll be + # the root file system. + root_fs = None for i, line in enumerate(proc.stdout.splitlines()): if not mountable[i]: continue @@ -87,6 +93,7 @@ device = parts[2] src = '/dev/mapper/{}'.format(device) dst = os.path.join(TMP, '{}.mnt{}'.format(base, i)) + root_fs = src os.mkdir(dst) resources.callback(os.rmdir, dst) proc = run(['mount', src, dst], @@ -99,6 +106,16 @@ else: # Failed. model_status[device] = (False, proc.stdout, proc.stderr) + # Now ensure the file system label for root. + if root_fs is None: + model_status['no rootfs found!'] = (False, '', '') + else: + proc = run(['/sbin/e2label', root_fs], + universal_newlines=True, + stdout=PIPE, stderr=PIPE) + if proc.stdout.strip() != 'writable': + model_status['rootfs label != writable'] = ( + False, proc.stdout, proc.stderr) # Collate results. diff -Nru ubuntu-image-0.12+16.04ubuntu1/devel-coverage.ini ubuntu-image-0.14+16.04ubuntu2/devel-coverage.ini --- ubuntu-image-0.12+16.04ubuntu1/devel-coverage.ini 2016-11-08 22:31:21.000000000 +0000 +++ ubuntu-image-0.14+16.04ubuntu2/devel-coverage.ini 2017-01-20 14:59:20.000000000 +0000 @@ -3,7 +3,7 @@ parallel = true omit = setup* - .tox/coverage/lib/python*/site-packages/* + .tox/*-cov/lib/python*/site-packages/* ubuntu_image/tests/* ubuntu_image/testing/* /usr/lib/python3/dist-packages/* @@ -11,7 +11,7 @@ [paths] source = ubuntu_image - .tox/coverage/lib/python*/site-packages/ubuntu_image + .tox/*-cov/lib/python*/site-packages/ubuntu_image [report] #fail_under = 100 diff -Nru ubuntu-image-0.12+16.04ubuntu1/.gitignore ubuntu-image-0.14+16.04ubuntu2/.gitignore --- ubuntu-image-0.12+16.04ubuntu1/.gitignore 2016-11-08 22:31:21.000000000 +0000 +++ ubuntu-image-0.14+16.04ubuntu2/.gitignore 2017-01-20 14:59:20.000000000 +0000 @@ -9,3 +9,5 @@ prime stage *.snap +coverage.xml +diffcov.html diff -Nru ubuntu-image-0.12+16.04ubuntu1/snapcraft.yaml ubuntu-image-0.14+16.04ubuntu2/snapcraft.yaml --- ubuntu-image-0.12+16.04ubuntu1/snapcraft.yaml 2016-11-08 22:31:21.000000000 +0000 +++ ubuntu-image-0.14+16.04ubuntu2/snapcraft.yaml 2017-01-20 14:59:20.000000000 +0000 @@ -2,7 +2,7 @@ summary: Create Ubuntu images description: | Use this tool to create Ubuntu images. -version: 0.12+real1 +version: 0.13+real1 confinement: devmode apps: diff -Nru ubuntu-image-0.12+16.04ubuntu1/tox.ini ubuntu-image-0.14+16.04ubuntu2/tox.ini --- ubuntu-image-0.12+16.04ubuntu1/tox.ini 2016-11-08 22:31:21.000000000 +0000 +++ ubuntu-image-0.14+16.04ubuntu2/tox.ini 2017-01-20 14:59:20.000000000 +0000 @@ -1,51 +1,42 @@ [tox] -envlist = py35, coverage, qa +envlist = {py35,py36}-{nocov,cov,diffcov},qa +#recreate = True skip_missing_interpreters = True [testenv] -commands = python -m nose2 -v {posargs} -deps = - nose2 - responses +commands = + nocov: python -m nose2 -v {posargs} + cov,diffcov: python -m coverage run {[coverage]rc} -m nose2 + cov,diffcov: python -m coverage combine {[coverage]rc} + #cov: python -m coverage html {[coverage]rc} + cov: python -m coverage report -m {[coverage]rc} --fail-under=100 + diffcov: python -m coverage xml {[coverage]rc} + diffcov: diff-cover coverage.xml --html-report diffcov.html + diffcov: diff-cover coverage.xml --fail-under=100 usedevelop = True sitepackages = True +deps = + nose2 + cov,diffcov: coverage + diffcov: diff_cover passenv = PYTHON* UBUNTU_IMAGE_* *_proxy +setenv = + cov: COVERAGE_PROCESS_START={[coverage]rcfile} + cov: COVERAGE_OPTIONS="-p" + cov: COVERAGE_FILE={toxinidir}/.coverage [coverage] rcfile = {toxinidir}/{env:UBUNTU_IMAGE_CODENAME:devel}-coverage.ini rc = --rcfile={[coverage]rcfile} -[testenv:coverage] -basepython = python3 -commands = - python -m coverage run {[coverage]rc} -m nose2 - python -m coverage combine {[coverage]rc} - #python -m coverage html {[coverage]rc} - python -m coverage report -m {[coverage]rc} --fail-under=100 -usedevelop = True -deps = - nose2 - coverage -setenv = - COVERAGE_PROCESS_START={[coverage]rcfile} - COVERAGE_OPTIONS="-p" - COVERAGE_FILE={toxinidir}/.coverage - [testenv:qa] basepython = python3 commands = python -m flake8 ubuntu_image -[testenv:docs] -basepython = python3 -commands = - python setup.py build_sphinx -deps: - sphinx - [flake8] max-line-length = 79 jobs = 1 diff -Nru ubuntu-image-0.12+16.04ubuntu1/ubuntu_image/builder.py ubuntu-image-0.14+16.04ubuntu2/ubuntu_image/builder.py --- ubuntu-image-0.12+16.04ubuntu1/ubuntu_image/builder.py 2016-11-08 22:31:21.000000000 +0000 +++ ubuntu-image-0.14+16.04ubuntu2/ubuntu_image/builder.py 2017-01-20 14:59:20.000000000 +0000 @@ -5,12 +5,14 @@ import logging from math import ceil +from pathlib import Path from subprocess import CalledProcessError from tempfile import TemporaryDirectory from ubuntu_image.helpers import MiB, mkfs_ext4, run, snap, sparse_copy from ubuntu_image.image import Image, MBRImage from ubuntu_image.parser import ( - BootLoader, FileSystemType, VolumeSchema, parse as parse_yaml) + BootLoader, FileSystemType, StructureRole, VolumeSchema, + parse as parse_yaml) from ubuntu_image.state import State @@ -18,6 +20,10 @@ _logger = logging.getLogger('ubuntu-image') +class TMPNotReadableFromOutsideSnap(Exception): + """ubuntu-image snap cannot write images to /tmp""" + + class ModelAssertionBuilder(State): def __init__(self, args): super().__init__() @@ -34,21 +40,26 @@ self.resources.enter_context(TemporaryDirectory()) if args.workdir is None else args.workdir) - # Where the disk.img file ends up. + # Where the disk.img file ends up. /tmp to a snap is not the same + # /tmp outside of the snap. When running as a snap, don't allow the + # user to output a disk image to a location that won't exist for them. self.output = ( os.path.join(self.workdir, 'disk.img') if args.output is None else args.output) + in_snap = any(key.startswith('SNAP') for key in os.environ) + path = os.sep.join(self.output.split(os.sep)[:2]) + if in_snap and path == '/tmp': + raise TMPNotReadableFromOutsideSnap # Information passed between states. self.rootfs = None self.rootfs_size = 0 - self.rootfs_offset = 0 + self.part_images = None self.image_size = 0 self.bootfs = None self.bootfs_sizes = None self.images = None - self.boot_images = None - self.root_img = None + self.entry = None self.disk_img = None self.gadget = None self.args = args @@ -61,7 +72,6 @@ state = super().__getstate__() state.update( args=self.args, - boot_images=self.boot_images, bootfs=self.bootfs, bootfs_sizes=self.bootfs_sizes, cloud_init=self.cloud_init, @@ -71,10 +81,9 @@ image_size=self.image_size, images=self.images, output=self.output, - root_img=self.root_img, + part_images=self.part_images, rootfs=self.rootfs, rootfs_size=self.rootfs_size, - rootfs_offset=self.rootfs_offset, unpackdir=self.unpackdir, ) return state @@ -82,7 +91,6 @@ def __setstate__(self, state): super().__setstate__(state) self.args = state['args'] - self.boot_images = state['boot_images'] self.bootfs = state['bootfs'] self.bootfs_sizes = state['bootfs_sizes'] self.cloud_init = state['cloud_init'] @@ -92,10 +100,9 @@ self.image_size = state['image_size'] self.images = state['images'] self.output = state['output'] - self.root_img = state['root_img'] + self.part_images = state['part_images'] self.rootfs = state['rootfs'] self.rootfs_size = state['rootfs_size'] - self.rootfs_offset = state['rootfs_offset'] self.unpackdir = state['unpackdir'] def _log_exception(self, name): @@ -196,9 +203,7 @@ # At least one structure is required. for partnum, part in enumerate(volume.structures): target_dir = os.path.join(self.workdir, 'part{}'.format(partnum)) - # XXX: Use file system label for the moment, until we get a proper - # way to identify the boot partition. - if part.filesystem_label == 'system-boot': + if part.role is StructureRole.system_boot: self.bootfs = target_dir if volume.bootloader is BootLoader.uboot: boot = os.path.join( @@ -279,36 +284,47 @@ self.images = os.path.join(self.workdir, '.images') os.makedirs(self.images) # The image for the boot partition. - self.boot_images = [] + self.part_images = [] volumes = self.gadget.volumes.values() assert len(volumes) == 1, 'For now, only one volume is allowed' volume = list(volumes)[0] + farthest_offset = 0 for partnum, part in enumerate(volume.structures): - # The root file system implicitly lives right after the farthest - # out structure, which may not be the last named structure in the - # gadget.yaml. - self.rootfs_offset = max( - self.rootfs_offset, (part.offset + part.size)) part_img = os.path.join(self.images, 'part{}.img'.format(partnum)) - self.boot_images.append(part_img) - run('dd if=/dev/zero of={} count=0 bs={} seek=1'.format( - part_img, part.size)) - if part.filesystem is FileSystemType.vfat: - label_option = ( - '-n {}'.format(part.filesystem_label) - # XXX I think this could be None or the empty string, but - # this needs verification. - if part.filesystem_label - else '') - # XXX: hard-coding of sector size - run('mkfs.vfat -s 1 -S 512 -F 32 {} {}'.format( - label_option, part_img)) - # Sanity check whether everything will fit on the disk if an explicit - # size was given on the command line. XXX: This hard-codes last 34 - # 512-byte sectors for backup GPT, empirically derived from sgdisk - # behavior. - calculated = ceil( - (self.rootfs_offset + self.rootfs_size) / 1024 + 17) * 1024 + if part.role is StructureRole.system_data: + # The image for the root partition. + if part.size is None: + part.size = self.rootfs_size + elif part.size < self.rootfs_size: + _logger.warning('rootfs partition size ({}) smaller than ' + 'actual rootfs contents {}'.format( + part.size, self.rootfs_size)) + part.size = self.rootfs_size + # We defer creating the root file system image because we have + # to populate it at the same time. See mkfs.ext4(8) for + # details. + Path(part_img).touch() + os.truncate(part_img, self.rootfs_size) + else: + run('dd if=/dev/zero of={} count=0 bs={} seek=1'.format( + part_img, part.size)) + if part.filesystem is FileSystemType.vfat: + label_option = ( + '-n {}'.format(part.filesystem_label) + # XXX I think this could be None or the empty string, + # but this needs verification. + if part.filesystem_label + else '') + # XXX: hard-coding of sector size. + run('mkfs.vfat -s 1 -S 512 -F 32 {} {}'.format( + label_option, part_img)) + self.part_images.append(part_img) + farthest_offset = max(farthest_offset, (part.offset + part.size)) + # Calculate or check the final image size. + # + # XXX: Hard-codes last 34 512-byte sectors for backup GPT, + # empirically derived from sgdisk behavior. + calculated = ceil(farthest_offset / 1024 + 17) * 1024 if self.args.image_size is None: self.image_size = calculated else: @@ -319,14 +335,6 @@ self.image_size = calculated else: self.image_size = self.args.image_size - # The image for the root partition. - self.root_img = os.path.join(self.images, 'root.img') - # Create empty file with holes. - with open(self.root_img, 'w'): - pass - os.truncate(self.root_img, self.rootfs_size) - # We defer creating the root file system image because we have to - # populate it at the same time. See mkfs.ext4(8) for details. self._next.append(self.populate_filesystems) def populate_filesystems(self): @@ -334,9 +342,14 @@ assert len(volumes) == 1, 'For now, only one volume is allowed' volume = list(volumes)[0] for partnum, part in enumerate(volume.structures): - part_img = self.boot_images[partnum] + part_img = self.part_images[partnum] part_dir = os.path.join(self.workdir, 'part{}'.format(partnum)) - if part.filesystem is FileSystemType.none: + if part.role is StructureRole.system_data: + # The root partition needs to be ext4, which may or may not be + # populated at creation time, depending on the version of + # e2fsprogs. + mkfs_ext4(part_img, self.rootfs, part.filesystem_label) + elif part.filesystem is FileSystemType.none: image = Image(part_img, part.size) offset = 0 for content in part.content: @@ -367,9 +380,6 @@ else: raise AssertionError('Invalid part filesystem type: {}'.format( part.filesystem)) - # The root partition needs to be ext4, which may or may not be - # populated at creation time, depending on the version of e2fsprogs. - mkfs_ext4(self.root_img, self.rootfs) self._next.append(self.make_disk) def make_disk(self): @@ -393,53 +403,37 @@ image = Image(self.disk_img, self.image_size) offset_writes = [] part_offsets = {} - root_offset = 0 for i, part in enumerate(volume.structures): if part.name is not None: part_offsets[part.name] = part.offset if part.offset_write is not None: offset_writes.append((part.offset, part.offset_write)) - image.copy_blob(self.boot_images[i], + image.copy_blob(self.part_images[i], bs='1M', seek=part.offset // MiB(1), count=ceil(part.size / MiB(1)), conv='notrunc') - if part.type in ('bare', 'mbr'): + if part.role is StructureRole.mbr or part.type == 'bare': continue # sgdisk takes either a sector or a KiB/MiB argument; assume # that the offset and size are always multiples of 1MiB. # # XXX Size must not be zero, which will happen if part.size < 1MiB partition_args = dict( - new='{}M:+{}M'.format( - part.offset // MiB(1), part.size // MiB(1)), + new='{}M:+{}K'.format( + part.offset // MiB(1), ceil(part.size / 1024)), typecode=part.type, ) # XXX: special-casing. if (volume.schema is VolumeSchema.mbr and - part.filesystem_label == 'system-boot'): + part.role is StructureRole.system_boot): partition_args['activate'] = True + elif (volume.schema is VolumeSchema.gpt and + part.role is StructureRole.system_data): + partition_args['change_name'] = 'writable' if part.name is not None: partition_args['change_name'] = part.name image.partition(part_id, **partition_args) part_id += 1 - # Put the rootfs after the farthest out structure, which may not - # be the last named structure in the gadget.yaml. - root_offset = max(root_offset, (part.offset + part.size)) - # Create and write the main snappy writable partition (i.e. rootfs) as - # the last partition. - root_offset_mib = self.rootfs_offset // MiB(1) - root_size_kib = ceil(self.rootfs_size / 1024) - image.partition( - part_id, - new='{}M:+{}K'.format(root_offset_mib, root_size_kib), - typecode=('83', '0FC63DAF-8483-4772-8E79-3D69D8477DE4')) - if volume.schema is VolumeSchema.gpt: - image.partition(part_id, change_name='writable') - image.copy_blob( - self.root_img, - bs='1M', seek=root_offset_mib, - count=ceil(self.rootfs_size / MiB(1)), - conv='notrunc') for value, dest in offset_writes: # Decipher non-numeric offset_write values. if isinstance(dest, tuple): diff -Nru ubuntu-image-0.12+16.04ubuntu1/ubuntu_image/helpers.py ubuntu-image-0.14+16.04ubuntu2/ubuntu_image/helpers.py --- ubuntu-image-0.12+16.04ubuntu1/ubuntu_image/helpers.py 2016-11-08 22:31:21.000000000 +0000 +++ ubuntu-image-0.14+16.04ubuntu2/ubuntu_image/helpers.py 2017-01-20 14:59:20.000000000 +0000 @@ -117,7 +117,7 @@ def sparse_copy(src, dst, *, follow_symlinks=True): - args = ['cp', '-p', '--sparse=always', src, dst] + args = ['cp', '--sparse=always', src, dst] if not follow_symlinks: args.append('-P') run(args) diff -Nru ubuntu-image-0.12+16.04ubuntu1/ubuntu_image/__main__.py ubuntu-image-0.14+16.04ubuntu2/ubuntu_image/__main__.py --- ubuntu-image-0.12+16.04ubuntu1/ubuntu_image/__main__.py 2016-11-08 22:31:21.000000000 +0000 +++ ubuntu-image-0.14+16.04ubuntu2/ubuntu_image/__main__.py 2017-01-20 14:59:20.000000000 +0000 @@ -8,7 +8,8 @@ from contextlib import suppress from pickle import dump, load from ubuntu_image import __version__ -from ubuntu_image.builder import ModelAssertionBuilder +from ubuntu_image.builder import ( + ModelAssertionBuilder, TMPNotReadableFromOutsideSnap) from ubuntu_image.helpers import as_size from ubuntu_image.i18n import _ from ubuntu_image.parser import GadgetSpecificationError @@ -147,7 +148,11 @@ state_machine = load(fp) state_machine.workdir = args.workdir else: - state_machine = ModelAssertionBuilder(args) + try: + state_machine = ModelAssertionBuilder(args) + except TMPNotReadableFromOutsideSnap as error: + print(error.__doc__, file=sys.stderr) + return 1 # Run the state machine, either to the end or thru/until the named state. try: if args.thru: diff -Nru ubuntu-image-0.12+16.04ubuntu1/ubuntu_image/parser.py ubuntu-image-0.14+16.04ubuntu2/ubuntu_image/parser.py --- ubuntu-image-0.12+16.04ubuntu1/ubuntu_image/parser.py 2016-11-08 22:31:21.000000000 +0000 +++ ubuntu-image-0.14+16.04ubuntu2/ubuntu_image/parser.py 2017-01-20 14:59:20.000000000 +0000 @@ -326,6 +326,8 @@ image_id = image_spec.get('id') structures = [] last_offset = 0 + farthest_offset = 0 + rootfs_seen = False for structure in image_spec['structure']: name = structure.get('name') offset = structure.get('offset') @@ -369,7 +371,7 @@ # which is exactly equivalent to the preferred role:mbr field, # however a deprecation warning is issued in the former case. if structure_type == 'mbr': - if structure_role: + if structure_role is not None: raise GadgetSpecificationError( 'Type mbr and role fields assigned at the same time, ' 'please use the mbr role instead') @@ -398,12 +400,15 @@ 'GUID structure type with non-GPT schema') # Check for implicit vs. explicit partition offset. if offset is None: + # XXX: Ensure the special case of the mbr role doesn't + # extend beyond the confines of the mbr. if (structure_role is not StructureRole.mbr and last_offset < MiB(1)): offset = MiB(1) else: offset = last_offset last_offset = offset + size + farthest_offset = max(farthest_offset, last_offset) # Extract the rest of the structure data. structure_id = structure.get('id') filesystem = structure['filesystem'] @@ -421,6 +426,22 @@ raise GadgetSpecificationError( 'mbr structures must not specify a file system') filesystem_label = structure.get('filesystem-label', name) + # Support the legacy mode setting of partition roles through + # filesystem labels. + if structure_role is None: + if filesystem_label == 'system-boot': + structure_role = StructureRole.system_boot + warn('volumes::structure::filesystem_label' + ' used for defining partition roles; use role ' + 'instead.', DeprecationWarning) + elif structure_role is StructureRole.system_data: + rootfs_seen = True + # For images to work the system-data (rootfs) partition needs + # to have the 'writable' filesystem label set. + if filesystem_label not in (None, 'writable'): + raise GadgetSpecificationError( + '`role: system-data` structure must have an implicit ' + "label, or 'writable': {}".format(filesystem_label)) # The content will be one of two formats, and no mixing is # allowed. I.e. even though multiple content sections are allowed # in a single structure, they must all be of type A or type B. If @@ -466,6 +487,28 @@ part.type if part.name is None else part.name, part.offset, last_end)) last_end = part.offset + part.size + if not rootfs_seen: + # We still need to handle the case of unspecified system-data + # partition where we simply attach the rootfs at the end of the + # partition list. + # + # Since so far we have no knowledge of the rootfs contents, the + # size is set to 0, knowing that the builder code will resize it + # to fit all the contents. + warn('No role: system-data partition found, a implicit rootfs ' + 'partition will be appended at the end of the partition ' + 'list. An explicit system-data partition is now required.', + DeprecationWarning) + structures.append(StructureSpec( + None, # name + farthest_offset, None, # offset, offset_write + None, # size; None == calculate + ( # type; hybrid mbr/gpt + '83', '0FC63DAF-8483-4772-8E79-3D69D8477DE4'), + None, StructureRole.system_data, # id, role + FileSystemType.ext4, # file system type + 'writable', # file system label + [])) # contents if not bootloader_seen: raise GadgetSpecificationError('No bootloader structure named') return GadgetSpec(device_tree_origin, device_tree, volume_specs, diff -Nru ubuntu-image-0.12+16.04ubuntu1/ubuntu_image/testing/helpers.py ubuntu-image-0.14+16.04ubuntu2/ubuntu_image/testing/helpers.py --- ubuntu-image-0.12+16.04ubuntu1/ubuntu_image/testing/helpers.py 2016-11-08 22:31:21.000000000 +0000 +++ ubuntu-image-0.14+16.04ubuntu2/ubuntu_image/testing/helpers.py 2017-01-20 14:59:20.000000000 +0000 @@ -4,7 +4,7 @@ import shutil import logging -from contextlib import ExitStack +from contextlib import ExitStack, contextmanager from pkg_resources import resource_filename from ubuntu_image.builder import ModelAssertionBuilder from unittest.mock import patch @@ -87,3 +87,18 @@ self._resources.close() # Don't suppress any exceptions. return False + + +@contextmanager +def envar(key, value): + missing = object() + # Temporarily set an environment variable. + old_value = os.environ.get(key, missing) + os.environ[key] = value + try: + yield + finally: + if old_value is missing: + del os.environ[key] + else: + os.environ[key] = old_value diff -Nru ubuntu-image-0.12+16.04ubuntu1/ubuntu_image/tests/test_builder.py ubuntu-image-0.14+16.04ubuntu2/ubuntu_image/tests/test_builder.py --- ubuntu-image-0.12+16.04ubuntu1/ubuntu_image/tests/test_builder.py 2016-11-08 22:31:21.000000000 +0000 +++ ubuntu-image-0.14+16.04ubuntu2/ubuntu_image/tests/test_builder.py 2017-01-20 14:59:20.000000000 +0000 @@ -13,7 +13,8 @@ from tempfile import NamedTemporaryFile, TemporaryDirectory from types import SimpleNamespace from ubuntu_image.helpers import MiB, run -from ubuntu_image.parser import BootLoader, FileSystemType, VolumeSchema +from ubuntu_image.parser import ( + BootLoader, FileSystemType, StructureRole, VolumeSchema) from ubuntu_image.testing.helpers import LogCapture, XXXModelAssertionBuilder from ubuntu_image.testing.nose import NosePlugin from unittest import TestCase, skipIf @@ -61,7 +62,7 @@ debug=False, extra_snaps=None, model_assertion=self.model_assertion, - output=output, + output=output.name, workdir=None, ) state = self._resources.enter_context(XXXModelAssertionBuilder(args)) @@ -249,6 +250,7 @@ # Now we have to craft enough of gadget definition to drive the # method under test. part = SimpleNamespace( + role=StructureRole.system_boot, filesystem_label='system-boot', filesystem=FileSystemType.none, ) @@ -303,6 +305,7 @@ # Now we have to craft enough of gadget definition to drive the # method under test. part = SimpleNamespace( + role=StructureRole.system_boot, filesystem_label='system-boot', filesystem=FileSystemType.none, ) @@ -354,6 +357,7 @@ target='bt/', ) part = SimpleNamespace( + role=None, filesystem_label='not a boot', filesystem=FileSystemType.ext4, content=[contents1, contents2], @@ -423,6 +427,7 @@ target='bt', ) part = SimpleNamespace( + role=None, filesystem_label='not a boot', filesystem=FileSystemType.ext4, content=[contents], @@ -460,6 +465,7 @@ state._next.append(state.calculate_bootfs_size) # Craft a gadget specification. part = SimpleNamespace( + role=None, filesystem=FileSystemType.none, ) volume = SimpleNamespace( @@ -493,7 +499,7 @@ state.images = os.path.join(workdir, '.images') os.makedirs(state.images) part0_img = os.path.join(state.images, 'part0.img') - state.boot_images = [part0_img] + state.part_images = [part0_img] # Craft a gadget specification. contents1 = SimpleNamespace( image='image1.img', @@ -516,6 +522,7 @@ offset=127, ) part = SimpleNamespace( + role=None, filesystem=FileSystemType.none, content=[contents1, contents2, contents3, contents4], size=150, @@ -578,7 +585,7 @@ state.images = os.path.join(workdir, '.images') os.makedirs(state.images) part0_img = os.path.join(state.images, 'part0.img') - state.boot_images = [part0_img] + state.part_images = [part0_img] # Craft a gadget specification. contents1 = SimpleNamespace( image='image1.img', @@ -586,6 +593,7 @@ offset=None, ) part = SimpleNamespace( + role=None, filesystem=FileSystemType.ext4, filesystem_label='hold the door', content=[contents1], @@ -611,7 +619,7 @@ # actually got called twice, but it's only the first call # (i.e. the one creating the part, not the image) that we care # about. - self.assertEqual(len(mock.call_args_list), 2) + self.assertEqual(len(mock.call_args_list), 1) posargs, kwargs = mock.call_args_list[0] self.assertEqual( posargs, @@ -640,7 +648,7 @@ state.images = os.path.join(workdir, '.images') os.makedirs(state.images) part0_img = os.path.join(state.images, 'part0.img') - state.boot_images = [part0_img] + state.part_images = [part0_img] # Craft a gadget specification. contents1 = SimpleNamespace( image='image1.img', @@ -648,6 +656,7 @@ offset=None, ) part = SimpleNamespace( + role=None, filesystem=801, filesystem_label='hold the door', content=[contents1], @@ -737,6 +746,7 @@ state.images = os.path.join(workdir, '.images') state.disk_img = os.path.join(workdir, 'disk.img') state.image_size = MiB(10) + state.rootfs_size = MiB(1) os.makedirs(state.images) # Craft a gadget schema. part0 = SimpleNamespace( @@ -758,14 +768,22 @@ part2 = SimpleNamespace( name='gamma', type='mbr', - role=None, + role=StructureRole.mbr, size=MiB(1), offset=0, offset_write=None, ) + part3 = SimpleNamespace( + name=None, + type=('83', '0FC63DAF-8483-4772-8E79-3D69D8477DE4'), + role=StructureRole.system_data, + size=state.rootfs_size, + offset=MiB(5), + offset_write=None, + ) volume = SimpleNamespace( # gadget.yaml appearance order. - structures=[part2, part0, part1], + structures=[part2, part0, part1, part3], schema=VolumeSchema.gpt, ) state.gadget = SimpleNamespace( @@ -783,12 +801,10 @@ part2_img = os.path.join(state.images, 'part2.img') with open(part2_img, 'wb') as fp: fp.write(b'\3' * 13) - state.root_img = os.path.join(state.images, 'root.img') - state.rootfs_size = MiB(1) - state.rootfs_offset = MiB(5) - with open(state.root_img, 'wb') as fp: + root_img = os.path.join(state.images, 'part3.img') + with open(root_img, 'wb') as fp: fp.write(b'\4' * 14) - state.boot_images = [part2_img, part0_img, part1_img] + state.part_images = [part2_img, part0_img, part1_img, root_img] # Create the disk. next(state) # Verify some parts of the disk.img's content. First, that we've @@ -852,18 +868,28 @@ state.images = os.path.join(workdir, '.images') state.disk_img = os.path.join(workdir, 'disk.img') state.image_size = MiB(10) + state.rootfs_size = MiB(1) os.makedirs(state.images) # Craft a gadget schema. part0 = SimpleNamespace( name='assets', + role=None, size=MiB(1), offset=0, offset_write=None, type='bare', ) + part1 = SimpleNamespace( + name=None, + type=('83', '0FC63DAF-8483-4772-8E79-3D69D8477DE4'), + role=StructureRole.system_data, + size=state.rootfs_size, + offset=MiB(5), + offset_write=None, + ) volume = SimpleNamespace( # gadget.yaml appearance order. - structures=[part0], + structures=[part0, part1], schema=VolumeSchema.gpt, ) state.gadget = SimpleNamespace( @@ -875,12 +901,10 @@ part0_img = os.path.join(state.images, 'part0.img') with open(part0_img, 'wb') as fp: fp.write(b'\1' * 11) - state.root_img = os.path.join(state.images, 'root.img') - state.rootfs_size = MiB(1) - state.rootfs_offset = MiB(5) - with open(state.root_img, 'wb') as fp: + root_img = os.path.join(state.images, 'root.img') + with open(root_img, 'wb') as fp: fp.write(b'\4' * 14) - state.boot_images = [part0_img] + state.part_images = [part0_img, root_img] # Create the disk. next(state) # Verify some parts of the disk.img's content. First, that we've @@ -926,6 +950,7 @@ state.images = os.path.join(workdir, '.images') state.disk_img = os.path.join(workdir, 'disk.img') state.image_size = MiB(10) + state.rootfs_size = MiB(1) os.makedirs(state.images) # Craft a gadget schema. part0 = SimpleNamespace( @@ -947,15 +972,23 @@ part2 = SimpleNamespace( name='gamma', type='mbr', - role=None, + role=StructureRole.mbr, size=MiB(1), offset=0, offset_write=None, ) + part3 = SimpleNamespace( + name=None, + type=('83', '0FC63DAF-8483-4772-8E79-3D69D8477DE4'), + role=StructureRole.system_data, + size=state.rootfs_size, + offset=MiB(5), + offset_write=None, + ) volume = SimpleNamespace( # gadget.yaml appearance order which does not match the disk # offset order. LP: #1642999 - structures=[part1, part0, part2], + structures=[part1, part0, part2, part3], schema=VolumeSchema.gpt, ) state.gadget = SimpleNamespace( @@ -973,12 +1006,10 @@ part2_img = os.path.join(state.images, 'part2.img') with open(part2_img, 'wb') as fp: fp.write(b'\3' * 13) - state.root_img = os.path.join(state.images, 'root.img') - state.rootfs_size = MiB(1) - state.rootfs_offset = MiB(5) - with open(state.root_img, 'wb') as fp: + root_img = os.path.join(state.images, 'root.img') + with open(root_img, 'wb') as fp: fp.write(b'\4' * 14) - state.boot_images = [part1_img, part0_img, part2_img] + state.part_images = [part1_img, part0_img, part2_img, root_img] # Create the disk. next(state) # Verify some parts of the disk.img's content. First, that we've @@ -1023,8 +1054,8 @@ self.assertEqual(partitions[2], ('writable', 10240)) def test_make_disk_with_parts_system_boot(self): - # For MBR-style volumes, a part with a filesystem-label of - # 'system-boot' gets the boot flag turned on in the partition table. + # For MBR-style volumes, a part with a role of 'system-boot' + # gets the boot flag turned on in the partition table. with ExitStack() as resources: workdir = resources.enter_context(TemporaryDirectory()) unpackdir = resources.enter_context(TemporaryDirectory()) @@ -1044,19 +1075,28 @@ state.images = os.path.join(workdir, '.images') state.disk_img = os.path.join(workdir, 'disk.img') state.image_size = MiB(10) + state.rootfs_size = MiB(1) os.makedirs(state.images) # Craft a gadget schema. part0 = SimpleNamespace( name=None, + role=StructureRole.system_boot, filesystem_label='system-boot', type='da', - role=None, size=MiB(1), offset=MiB(1), offset_write=None, ) + part1 = SimpleNamespace( + name=None, + type=('83', '0FC63DAF-8483-4772-8E79-3D69D8477DE4'), + role=StructureRole.system_data, + size=state.rootfs_size, + offset=MiB(2), + offset_write=None, + ) volume = SimpleNamespace( - structures=[part0], + structures=[part0, part1], schema=VolumeSchema.mbr, ) state.gadget = SimpleNamespace( @@ -1068,12 +1108,10 @@ part0_img = os.path.join(state.images, 'part0.img') with open(part0_img, 'wb') as fp: fp.write(b'\1' * 11) - state.root_img = os.path.join(state.images, 'root.img') - state.rootfs_size = MiB(1) - state.rootfs_offset = MiB(2) - with open(state.root_img, 'wb') as fp: + root_img = os.path.join(state.images, 'root.img') + with open(root_img, 'wb') as fp: fp.write(b'\4' * 14) - state.boot_images = [part0_img] + state.part_images = [part0_img, root_img] # Create the disk. next(state) # Verify the disk image's partition table. @@ -1103,6 +1141,7 @@ state.images = os.path.join(workdir, '.images') state.disk_img = os.path.join(workdir, 'disk.img') state.image_size = MiB(10) + state.rootfs_size = MiB(1) os.makedirs(state.images) # Craft a gadget schema. part0 = SimpleNamespace( @@ -1121,8 +1160,16 @@ offset=MiB(4), offset_write=('alpha', 200), ) + part2 = SimpleNamespace( + name=None, + type=('83', '0FC63DAF-8483-4772-8E79-3D69D8477DE4'), + role=StructureRole.system_data, + size=state.rootfs_size, + offset=MiB(5), + offset_write=None, + ) volume = SimpleNamespace( - structures=[part0, part1], + structures=[part0, part1, part2], schema=VolumeSchema.gpt, ) state.gadget = SimpleNamespace( @@ -1137,11 +1184,10 @@ part1_img = os.path.join(state.images, 'part1.img') with open(part1_img, 'wb') as fp: fp.write(b'\2' * 12) - state.root_img = os.path.join(state.images, 'root.img') - state.rootfs_size = MiB(1) - with open(state.root_img, 'wb') as fp: + root_img = os.path.join(state.images, 'root.img') + with open(root_img, 'wb') as fp: fp.write(b'\4' * 14) - state.boot_images = [part0_img, part1_img] + state.part_images = [part0_img, part1_img, root_img] # Create the disk. next(state) # Verify that beta's offset was written 200 bytes past the start @@ -1167,22 +1213,32 @@ state._next.pop() state._next.append(state.prepare_filesystems) # Craft a gadget schema. + state.rootfs_size = MiB(1) part0 = SimpleNamespace( name='alpha', type='da', + role=None, filesystem=FileSystemType.none, size=MiB(1), offset=0, offset_write=None, ) + part1 = SimpleNamespace( + name=None, + type=('83', '0FC63DAF-8483-4772-8E79-3D69D8477DE4'), + role=StructureRole.system_data, + filesystem=FileSystemType.ext4, + size=state.rootfs_size, + offset=MiB(1), + offset_write=None, + ) volume = SimpleNamespace( - structures=[part0], + structures=[part0, part1], schema=VolumeSchema.gpt, ) state.gadget = SimpleNamespace( volumes=dict(volume1=volume), ) - state.rootfs_size = MiB(1) # Mock the run() call to prove that we never call dd. mock = resources.enter_context(patch('ubuntu_image.builder.run')) next(state) @@ -1210,24 +1266,32 @@ state._next.pop() state._next.append(state.prepare_filesystems) # Craft a gadget schema. + state.rootfs_size = MiB(1) part0 = SimpleNamespace( name='alpha', type='da', + role=None, filesystem=FileSystemType.none, size=MiB(1), offset=0, offset_write=None, ) + part1 = SimpleNamespace( + name=None, + type=('83', '0FC63DAF-8483-4772-8E79-3D69D8477DE4'), + role=StructureRole.system_data, + filesystem=FileSystemType.ext4, + size=state.rootfs_size, + offset=MiB(1), + offset_write=None, + ) volume = SimpleNamespace( - structures=[part0], + structures=[part0, part1], schema=VolumeSchema.gpt, ) state.gadget = SimpleNamespace( volumes=dict(volume1=volume), ) - state.rootfs_size = MiB(1) - # Mock the run() call to prove that we never call dd. - resources.enter_context(patch('ubuntu_image.builder.run')) next(state) self.assertEqual(state.image_size, 2114560) @@ -1248,24 +1312,32 @@ state._next.pop() state._next.append(state.prepare_filesystems) # Craft a gadget schema. + state.rootfs_size = MiB(1) part0 = SimpleNamespace( name='alpha', type='da', + role=None, filesystem=FileSystemType.none, size=MiB(1), offset=0, offset_write=None, ) + part1 = SimpleNamespace( + name=None, + type=('83', '0FC63DAF-8483-4772-8E79-3D69D8477DE4'), + role=StructureRole.system_data, + filesystem=FileSystemType.ext4, + size=state.rootfs_size, + offset=MiB(1), + offset_write=None, + ) volume = SimpleNamespace( - structures=[part0], + structures=[part0, part1], schema=VolumeSchema.gpt, ) state.gadget = SimpleNamespace( volumes=dict(volume1=volume), ) - state.rootfs_size = MiB(1) - # Mock the run() call to prove that we never call dd. - resources.enter_context(patch('ubuntu_image.builder.run')) next(state) self.assertEqual(state.image_size, MiB(5)) @@ -1288,24 +1360,32 @@ state._next.pop() state._next.append(state.prepare_filesystems) # Craft a gadget schema. + state.rootfs_size = MiB(1) part0 = SimpleNamespace( name='alpha', type='da', + role=None, filesystem=FileSystemType.none, size=MiB(1), offset=0, offset_write=None, ) + part1 = SimpleNamespace( + name=None, + type=('83', '0FC63DAF-8483-4772-8E79-3D69D8477DE4'), + role=StructureRole.system_data, + filesystem=FileSystemType.ext4, + size=state.rootfs_size, + offset=MiB(1), + offset_write=None, + ) volume = SimpleNamespace( - structures=[part0], + structures=[part0, part1], schema=VolumeSchema.gpt, ) state.gadget = SimpleNamespace( volumes=dict(volume1=volume), ) - state.rootfs_size = MiB(1) - # Mock the run() call to prove that we never call dd. - resources.enter_context(patch('ubuntu_image.builder.run')) mock = resources.enter_context( patch('ubuntu_image.builder._logger.warning')) next(state) @@ -1340,9 +1420,11 @@ state = resources.enter_context(XXXModelAssertionBuilder(args)) state._next.pop() state._next.append(state.prepare_filesystems) + state.rootfs_size = MiB(1) # Craft a gadget schema. part0 = SimpleNamespace( name='alpha', + role=None, type='aa', size=MiB(1), offset=MiB(2), @@ -1352,6 +1434,7 @@ ) part1 = SimpleNamespace( name='beta', + role=None, type='bb', size=MiB(1), offset=MiB(4), @@ -1361,6 +1444,7 @@ ) part2 = SimpleNamespace( name='gamma', + role=StructureRole.mbr, type='mbr', size=MiB(1), offset=0, @@ -1368,18 +1452,23 @@ filesystem=FileSystemType.ext4, filesystem_label='gamma', ) + part3 = SimpleNamespace( + name=None, + type=('83', '0FC63DAF-8483-4772-8E79-3D69D8477DE4'), + role=StructureRole.system_data, + size=state.rootfs_size, + offset=MiB(5), + offset_write=None, + ) volume = SimpleNamespace( # gadget.yaml appearance order which does not match the disk # offset order. LP: #1642999 - structures=[part1, part0, part2], + structures=[part1, part0, part2, part3], schema=VolumeSchema.gpt, ) state.gadget = SimpleNamespace( volumes=dict(volume1=volume), ) - state.rootfs_size = MiB(1) - # Mock the run() call to prove that we never call dd. - resources.enter_context(patch('ubuntu_image.builder.run')) mock = resources.enter_context( patch('ubuntu_image.builder._logger.warning')) next(state) @@ -1425,25 +1514,34 @@ os.makedirs(state.images) # Craft a gadget schema. This is based on the official pi3-kernel # gadget.yaml file. + state.rootfs_size = 947980 contents0 = SimpleNamespace( source='boot-assets/', target='/', ) part0 = SimpleNamespace( name=None, + role=StructureRole.system_boot, filesystem_label='system-boot', filesystem='vfat', type='0C', - role=None, size=MiB(128), offset=MiB(1), offset_write=None, contents=[contents0], ) + part1 = SimpleNamespace( + name=None, + type=('83', '0FC63DAF-8483-4772-8E79-3D69D8477DE4'), + role=StructureRole.system_data, + size=state.rootfs_size, + offset=MiB(129), + offset_write=None, + ) volume0 = SimpleNamespace( schema=VolumeSchema.mbr, bootloader=BootLoader.uboot, - structures=[part0], + structures=[part0, part1], ) state.gadget = SimpleNamespace( volumes=dict(pi3=volume0), @@ -1453,12 +1551,10 @@ part0_img = os.path.join(state.images, 'part0.img') with open(part0_img, 'wb') as fp: fp.write(b'\1' * 11) - state.root_img = os.path.join(state.images, 'root.img') - state.rootfs_size = 947980 - state.rootfs_offset = MiB(129) - with open(state.root_img, 'wb') as fp: - fp.write(b'\2' * 947980) - state.boot_images = [part0_img] + root_img = os.path.join(state.images, 'part1.img') + with open(root_img, 'wb') as fp: + fp.write(b'\2' * state.rootfs_size) + state.part_images = [part0_img, root_img] # Create the disk. next(state) # The root file system must be at least 947980 bytes. @@ -1469,6 +1565,60 @@ # byte sectors. self.assertGreaterEqual(partition1['size'], 1852) + def test_explicit_rootfs_too_small(self): + # The root file system (i.e. 'system-data' label) is explicitly given + # as a structure in the gadget.yaml, but the given size is too small. + with ExitStack() as resources: + workdir = resources.enter_context(TemporaryDirectory()) + # Fast forward a state machine to the method under test. + args = SimpleNamespace( + cloud_init=None, + given_image_size='1M', + image_size=MiB(1), + output=None, + unpackdir=None, + workdir=workdir, + ) + # Jump right to the method under test. + state = resources.enter_context(XXXModelAssertionBuilder(args)) + state._next.pop() + state._next.append(state.prepare_filesystems) + # Craft a gadget schema. + state.rootfs_size = MiB(1) + part0 = SimpleNamespace( + name='alpha', + type='da', + role=None, + filesystem=FileSystemType.none, + size=MiB(1), + offset=0, + offset_write=None, + ) + part1 = SimpleNamespace( + name=None, + type=('83', '0FC63DAF-8483-4772-8E79-3D69D8477DE4'), + role=StructureRole.system_data, + filesystem=FileSystemType.ext4, + size=state.rootfs_size - 1111, + offset=MiB(1), + offset_write=None, + ) + volume = SimpleNamespace( + structures=[part0, part1], + schema=VolumeSchema.gpt, + ) + state.gadget = SimpleNamespace( + volumes=dict(volume1=volume), + ) + mock = resources.enter_context( + patch('ubuntu_image.builder._logger.warning')) + next(state) + posargs, kwargs = mock.call_args_list[0] + self.assertEqual( + posargs[0], + 'rootfs partition size (1047465) smaller than ' + 'actual rootfs contents 1048576') + def test_snap_command_fails(self): # LP: #1621445 - If the snap(1) command fails, don't print the full # traceback unless --debug is given. diff -Nru ubuntu-image-0.12+16.04ubuntu1/ubuntu_image/tests/test_helpers.py ubuntu-image-0.14+16.04ubuntu2/ubuntu_image/tests/test_helpers.py --- ubuntu-image-0.12+16.04ubuntu1/ubuntu_image/tests/test_helpers.py 2016-11-08 22:31:21.000000000 +0000 +++ ubuntu-image-0.14+16.04ubuntu2/ubuntu_image/tests/test_helpers.py 2017-01-20 14:59:20.000000000 +0000 @@ -1,6 +1,7 @@ """Test the helpers.""" import os +import errno import logging from contextlib import ExitStack @@ -34,6 +35,29 @@ pass +def is_sparse(path): + # LP: #1656371 - Looking at stat().st_blocks won't work on ZFS file + # systems, since that seems to return 1, whereas on EXT4 it returns 0. + # Rather than hard code the value based on file system type (which could + # be different even on other file systems), a more reliable way seems to + # be to use SEEK_DATA with an offset of 0 to find the first block of data + # after position 0. If there is no data, an ENXIO will get raised, at + # least on any modern Linux kernels we care about. See lseek(2) for + # details. + with open(path, 'r') as fp: + try: + os.lseek(fp.fileno(), 0, os.SEEK_DATA) + except OSError as error: + # There is no OSError subclass for ENXIO. + if error.errno != errno.ENXIO: + raise + # The expected exception occurred, meaning, there is no data in + # the file, so it's entirely sparse. + return True + # The expected exception did not occur, so there is data in the file. + return False + + class MountMocker: def __init__(self, results_dir): self.mountpoint = None @@ -140,10 +164,10 @@ fp = resources.enter_context(open(sparse_file, 'w')) os.truncate(fp.fileno(), 1000000) # This file is sparse. - self.assertEqual(os.stat(sparse_file).st_blocks, 0) + self.assertTrue(is_sparse(sparse_file)) copied_file = os.path.join(tmpdir, 'copied.dat') sparse_copy(sparse_file, copied_file) - self.assertEqual(os.stat(copied_file).st_blocks, 0) + self.assertTrue(is_sparse(copied_file)) def test_copy_symlink(self): with ExitStack() as resources: @@ -152,7 +176,7 @@ fp = resources.enter_context(open(sparse_file, 'w')) os.truncate(fp.fileno(), 1000000) # This file is sparse. - self.assertEqual(os.stat(sparse_file).st_blocks, 0) + self.assertTrue(is_sparse(sparse_file)) # Create a symlink to the sparse file. linked_file = os.path.join(tmpdir, 'linked.dat') os.symlink(sparse_file, linked_file) diff -Nru ubuntu-image-0.12+16.04ubuntu1/ubuntu_image/tests/test_image.py ubuntu-image-0.14+16.04ubuntu2/ubuntu_image/tests/test_image.py --- ubuntu-image-0.12+16.04ubuntu1/ubuntu_image/tests/test_image.py 2016-11-08 22:31:21.000000000 +0000 +++ ubuntu-image-0.14+16.04ubuntu2/ubuntu_image/tests/test_image.py 2017-01-20 14:59:20.000000000 +0000 @@ -124,11 +124,23 @@ self.assertFalse(image.initialized) image.partition(1, new='33:3000', activate=True, typecode='83') self.assertTrue(image.initialized) - proc = run('sfdisk --list {}'.format(self.img)) - info = proc.stdout.splitlines()[-1].split() - device = self.img + '1' - self.assertEqual( - info, [device, '*', '33', '3032', '3000', '1.5M', '83', 'Linux']) + proc = run('sfdisk --json {}'.format(self.img)) + disk_info = load_json(proc.stdout) + partitions = disk_info['partitiontable'] + # The device id is unpredictable. + partitions.pop('id') + self.assertEqual(partitions, { + 'device': self.img, + 'label': 'dos', + 'unit': 'sectors', + 'partitions': [{ + 'bootable': True, + 'node': '{}1'.format(self.img), + 'size': 3000, + 'start': 33, + 'type': '83', + }] + }) def test_mbr_image_partition_append(self): image = MBRImage(self.img, MiB(2)) @@ -138,12 +150,28 @@ self.assertTrue(image.initialized) # Append the next one. image.partition(2, new='3032:1000', typecode='dd') - proc = run('sfdisk --list {}'.format(self.img)) - info = proc.stdout.splitlines()[-1].split() - device = self.img + '2' - self.assertEqual( - # No boot-flag star. - info, [device, '3033', '4032', '1000', '500K', 'dd', 'unknown']) + proc = run('sfdisk --json {}'.format(self.img)) + disk_info = load_json(proc.stdout) + partitions = disk_info['partitiontable'] + # The device id is unpredictable. + partitions.pop('id') + self.assertEqual(partitions, { + 'label': 'dos', + 'device': self.img, + 'unit': 'sectors', + 'partitions': [{ + 'node': '{}1'.format(self.img), + 'start': 33, + 'size': 3000, + 'type': '83', + 'bootable': True, + }, { + 'node': '{}2'.format(self.img), + 'start': 3033, + 'size': 1000, + 'type': 'dd', + }], + }) def test_mbr_image_partition_tuple_typecode(self): # See the spec; type codes can by hybrid mbr/gpt style. @@ -153,11 +181,23 @@ 1, new='33:3000', activate=True, typecode=('83', '00000000-0000-0000-0000-0000deadbeef')) self.assertTrue(image.initialized) - proc = run('sfdisk --list {}'.format(self.img)) - info = proc.stdout.splitlines()[-1].split() - device = self.img + '1' - self.assertEqual( - info, [device, '*', '33', '3032', '3000', '1.5M', '83', 'Linux']) + proc = run('sfdisk --json {}'.format(self.img)) + disk_info = load_json(proc.stdout) + partitions = disk_info['partitiontable'] + # The device id is unpredictable. + partitions.pop('id') + self.assertEqual(partitions, { + 'device': self.img, + 'label': 'dos', + 'unit': 'sectors', + 'partitions': [{ + 'bootable': True, + 'node': '{}1'.format(self.img), + 'size': 3000, + 'start': 33, + 'type': '83', + }] + }) def test_mbr_image_partition_bad_keyword(self): image = MBRImage(self.img, MiB(2)) @@ -173,7 +213,7 @@ image.partition(1, new='33:3000', activate=True, typecode='83', change_name='first') self.assertTrue(image.initialized) - proc = run('sfdisk --list {} --json'.format(self.img)) + proc = run('sfdisk --json {}'.format(self.img)) disk_info = load_json(proc.stdout) partitions = disk_info['partitiontable'] # See? No name. However, the device id is unpredictable. diff -Nru ubuntu-image-0.12+16.04ubuntu1/ubuntu_image/tests/test_main.py ubuntu-image-0.14+16.04ubuntu2/ubuntu_image/tests/test_main.py --- ubuntu-image-0.12+16.04ubuntu1/ubuntu_image/tests/test_main.py 2016-11-08 22:31:21.000000000 +0000 +++ ubuntu-image-0.14+16.04ubuntu2/ubuntu_image/tests/test_main.py 2017-01-20 14:59:20.000000000 +0000 @@ -3,7 +3,7 @@ import os import logging -from contextlib import ExitStack +from contextlib import ExitStack, contextmanager from io import StringIO from pickle import load from pkg_resources import resource_filename @@ -15,7 +15,7 @@ from ubuntu_image.testing.helpers import ( CrashingModelAssertionBuilder, DoNothingBuilder, EarlyExitLeaveATraceAssertionBuilder, EarlyExitModelAssertionBuilder, - LogCapture, XXXModelAssertionBuilder) + LogCapture, XXXModelAssertionBuilder, envar) from ubuntu_image.testing.nose import NosePlugin from unittest import TestCase, skipIf from unittest.mock import call, patch @@ -26,6 +26,16 @@ raise CalledProcessError(1, 'failing command') +@contextmanager +def chdir(new_dir): + here = os.getcwd() + try: + os.chdir(new_dir) + yield + finally: + os.chdir(here) + + class BadGadgetModelAssertionBuilder(XXXModelAssertionBuilder): gadget_yaml = 'bad-gadget.yaml' @@ -176,6 +186,51 @@ main(('--output', imgfile, self.model_assertion)) self.assertTrue(os.path.exists(imgfile)) + def test_output_to_snaps_tmp(self): + # LP: 1646968 - Snappy maps /tmp to a private directory so when run as + # a snap you cannot use `-o /tmp/something`. + # + # For reference see: + # http://snapcraft.io/docs/reference/env + # http://www.zygoon.pl/2016/08/snap-execution-environment.html + self._resources.enter_context(envar('SNAP_NAME', 'crackle-pop')) + self._resources.enter_context(patch( + 'ubuntu_image.__main__.ModelAssertionBuilder', + DoNothingBuilder)) + tmpdir = self._resources.enter_context(TemporaryDirectory()) + imgfile = os.path.join(tmpdir, 'my-disk.img') + self.assertFalse(os.path.exists(imgfile)) + code = main(('--output', '/tmp/my.img', self.model_assertion)) + self.assertEqual(code, 1) + self.assertEqual( + self._stderr.getvalue(), + 'ubuntu-image snap cannot write images to /tmp\n') + + def test_no_output_as_snaps_pwd_tmp(self): + # LP: 1646968 - With no --output, if the current working directory is + # /tmp, refuse to run. + # + # For reference see: + # http://snapcraft.io/docs/reference/env + # http://www.zygoon.pl/2016/08/snap-execution-environment.html + self._resources.enter_context(envar('SNAP_NAME', 'crackle-pop')) + # XXX This could fail if /tmp doesn't exist for some reason. It might + # be better to actually mock os.getcwd() to return /tmp instead. I'm + # not doing that here though because I don't want to track down the + # mock location. + self._resources.enter_context(chdir('/tmp')) + self._resources.enter_context(patch( + 'ubuntu_image.__main__.ModelAssertionBuilder', + DoNothingBuilder)) + tmpdir = self._resources.enter_context(TemporaryDirectory()) + imgfile = os.path.join(tmpdir, 'my-disk.img') + self.assertFalse(os.path.exists(imgfile)) + code = main((self.model_assertion,)) + self.assertEqual(code, 1) + self.assertEqual( + self._stderr.getvalue(), + 'ubuntu-image snap cannot write images to /tmp\n') + def test_resume_and_model_assertion(self): with self.assertRaises(SystemExit) as cm: main(('--resume', self.model_assertion)) diff -Nru ubuntu-image-0.12+16.04ubuntu1/ubuntu_image/tests/test_parser.py ubuntu-image-0.14+16.04ubuntu2/ubuntu_image/tests/test_parser.py --- ubuntu-image-0.12+16.04ubuntu1/ubuntu_image/tests/test_parser.py 2016-11-08 22:31:21.000000000 +0000 +++ ubuntu-image-0.14+16.04ubuntu2/ubuntu_image/tests/test_parser.py 2017-01-20 14:59:20.000000000 +0000 @@ -18,6 +18,9 @@ structure: - type: 00000000-0000-0000-0000-0000deadbeef size: 400M + - type: 83,00000000-0000-0000-0000-0000feedface + role: system-data + size: 100M """) self.assertEqual(gadget_spec.device_tree_origin, 'gadget') self.assertIsNone(gadget_spec.device_tree) @@ -26,7 +29,7 @@ self.assertEqual(volume0.schema, VolumeSchema.gpt) self.assertEqual(volume0.bootloader, BootLoader.uboot) self.assertIsNone(volume0.id) - self.assertEqual(len(volume0.structures), 1) + self.assertEqual(len(volume0.structures), 2) structure0 = volume0.structures[0] self.assertIsNone(structure0.name) self.assertEqual(structure0.offset, MiB(1)) @@ -550,6 +553,25 @@ for key in gadget_spec.volumes} ) + def test_volume_structure_role_system_data_bad_label(self): + with ExitStack() as resources: + cm = resources.enter_context( + self.assertRaises(GadgetSpecificationError)) + parse("""\ +volumes: + first-image: + bootloader: u-boot + structure: + - type: 00000000-0000-0000-0000-0000feedface + size: 200 + role: system-data + filesystem-label: foobar +""") + self.assertEqual( + str(cm.exception), + ('`role: system-data` structure must have an implicit label, ' + "or 'writable': foobar")) + def test_volume_structure_type_none(self): gadget_spec = parse(""" volumes: @@ -636,10 +658,13 @@ - type: bare size: 100 role: mbr + - type: 83,00000000-0000-0000-0000-0000feedface + role: system-data + size: 100M """) self.assertEqual(len(gadget_spec.volumes), 1) volume = gadget_spec.volumes['first-image'] - self.assertEqual(len(volume.structures), 1) + self.assertEqual(len(volume.structures), 2) structure = volume.structures[0] self.assertEqual(structure.role, StructureRole.mbr) self.assertEqual(structure.type, 'bare') @@ -781,6 +806,21 @@ str(cm.exception), 'mbr structures must not specify a file system') + def test_volume_special_label_system_boot(self): + gadget_spec = parse("""\ +volumes: + first-image: + bootloader: u-boot + structure: + - type: 00000000-0000-0000-0000-0000feedface + size: 200 + filesystem-label: system-boot +""") + volume0 = gadget_spec.volumes['first-image'] + partition = volume0.structures[0] + self.assertEqual(partition.filesystem_label, 'system-boot') + self.assertEqual(partition.role, StructureRole.system_boot) + def test_content_spec_a(self): gadget_spec = parse("""\ volumes: @@ -1841,6 +1881,26 @@ self.assertEqual(str(cm.exception), 'mbr structure must start at offset 0') + def test_implicit_system_data_partition(self): + gadget_spec = parse("""\ +volumes: + first-image: + bootloader: u-boot + structure: + - type: 00000000-0000-0000-0000-0000deadbeef + size: 400M + - type: 00000000-0000-0000-0000-0000feedface + size: 100M +""") + volume0 = gadget_spec.volumes['first-image'] + self.assertEqual(len(volume0.structures), 3) + partition2 = volume0.structures[2] + self.assertEqual(partition2.role, StructureRole.system_data) + self.assertEqual(partition2.offset, MiB(501)) + self.assertEqual(partition2.size, None) + self.assertEqual(partition2.filesystem, FileSystemType.ext4) + self.assertEqual(partition2.filesystem_label, 'writable') + class TestPartOrder(TestCase): # LP: #1631423 @@ -1859,6 +1919,7 @@ - type: 00000000-0000-0000-0000-dd00deadbeef size: 400M - type: 00000000-0000-0000-0000-cc00deadbeef + role: system-data size: 500M - type: 00000000-0000-0000-0000-bb00deadbeef size: 100M @@ -1887,6 +1948,7 @@ size: 400M offset: 800M - type: 00000000-0000-0000-0000-cc00deadbeef + role: system-data size: 500M offset: 200M - type: 00000000-0000-0000-0000-bb00deadbeef @@ -1918,6 +1980,7 @@ size: 400M offset: 800M - type: 00000000-0000-0000-0000-cc00deadbeef + role: system-data size: 500M offset: 200M - type: 00000000-0000-0000-0000-bb00deadbeef @@ -1934,3 +1997,28 @@ (UUID('00000000-0000-0000-0000-bb00deadbeef'), MiB(700)), (UUID('00000000-0000-0000-0000-aa00deadbeef'), MiB(1)), ]) + + def test_mixed_offset_ordering_implicit_rootfs(self): + gadget_spec = parse("""\ +volumes: + first: + schema: gpt + bootloader: grub + structure: + - type: 00000000-0000-0000-0000-dd00deadbeef + size: 400M + offset: 800M + - type: 00000000-0000-0000-0000-cc00deadbeef + size: 500M + offset: 200M + - type: 00000000-0000-0000-0000-bb00deadbeef + size: 100M + - type: 00000000-0000-0000-0000-aa00deadbeef + size: 100M + offset: 1M +""") + volume0 = gadget_spec.volumes['first'] + self.assertEqual(len(volume0.structures), 5) + partition4 = volume0.structures[4] + self.assertEqual(partition4.role, StructureRole.system_data) + self.assertEqual(partition4.offset, MiB(1200)) diff -Nru ubuntu-image-0.12+16.04ubuntu1/ubuntu_image/version.txt ubuntu-image-0.14+16.04ubuntu2/ubuntu_image/version.txt --- ubuntu-image-0.12+16.04ubuntu1/ubuntu_image/version.txt 2016-11-08 22:31:21.000000000 +0000 +++ ubuntu-image-0.14+16.04ubuntu2/ubuntu_image/version.txt 2017-01-20 14:59:20.000000000 +0000 @@ -1 +1 @@ -0.12+16.04ubuntu1 +0.14+16.04ubuntu2 diff -Nru ubuntu-image-0.12+16.04ubuntu1/xenial-coverage.ini ubuntu-image-0.14+16.04ubuntu2/xenial-coverage.ini --- ubuntu-image-0.12+16.04ubuntu1/xenial-coverage.ini 2016-11-08 22:31:21.000000000 +0000 +++ ubuntu-image-0.14+16.04ubuntu2/xenial-coverage.ini 2017-01-20 14:59:20.000000000 +0000 @@ -3,7 +3,7 @@ parallel = true omit = setup* - .tox/coverage/lib/python*/site-packages/* + .tox/*-cov/lib/python*/site-packages/* ubuntu_image/tests/* ubuntu_image/testing/* /usr/lib/python3/dist-packages/* @@ -11,7 +11,7 @@ [paths] source = ubuntu_image - .tox/coverage/lib/python*/site-packages/ubuntu_image + .tox/*-cov/lib/python*/site-packages/ubuntu_image [report] #fail_under = 100