[Merge] lp:~jamesodhunt/ubuntu/vivid/ubuntu-core-upgrader/add-more-tests into lp:ubuntu/ubuntu-core-upgrader
Michael Vogt
michael.vogt at canonical.com
Wed Mar 4 09:54:47 UTC 2015
Thanks for the update and for addressing the comments.
This branch looks good, two fixes please then it can be merged (see inline comments for the exact location):
- the debian/changelog is referring changes that are no longer there
- the docstring in mock_get_cache_dir() refers to the previous iteration of the function, please either update or remove
Once that is fixed I'm happy to merge the branch.
Diff comments:
> === modified file 'bin/ubuntu-core-upgrade'
> --- bin/ubuntu-core-upgrade 2015-02-20 17:00:34 +0000
> +++ bin/ubuntu-core-upgrade 2015-03-03 16:44:50 +0000
> @@ -34,20 +34,14 @@
> import atexit
> import shutil
> import subprocess
> -import argparse
>
> from ubuntucoreupgrader.unshare import unshare, UnshareFlags
> -from ubuntucoreupgrader.upgrader import Upgrader
> +from ubuntucoreupgrader.upgrader import Upgrader, parse_args
>
> # Extension added to the command-file to show the new image is in the
> # process of being applied.
> IN_PROGRESS_SUFFIX = 'applying'
>
> -# seconds to wait before a reboot (should one be required)
> -DEFAULT_REBOOT_DELAY = 2
> -
> -DEFAULT_ROOT = '/'
> -
> log = logging.getLogger()
>
>
> @@ -80,91 +74,6 @@
> log.addHandler(handler)
>
>
> -def parse_args():
> - '''
> - Handle command-line options.
> - '''
> -
> - parser = argparse.ArgumentParser(description='System image Upgrader')
> -
> - parser.add_argument(
> - '--check-reboot',
> - action='store_true',
> - help='''
> - Attempt to determine if a reboot may be required.
> -
> - This option is similar to --dry-run: no system changes are made.
> -
> - Note that the result of this command cannot be definitive since a
> - reboot would be triggered if a service failed to start (but this
> - option does not attempt to restart any services).
> - ''')
> -
> - parser.add_argument(
> - '--clean-only',
> - action='store_true',
> - help='Clean up from a previous upgrader run, but do not upgrade)')
> -
> - parser.add_argument(
> - '--debug',
> - nargs='?', const=1, default=0, type=int,
> - help='Dump debug info (specify numeric value to increase verbosity)')
> -
> - parser.add_argument(
> - '-n', '--dry-run',
> - action='store_true',
> - help='''
> - Simulate an update including showing processes that have locks
> - on files
> - ''')
> -
> - parser.add_argument(
> - '--force-inplace-upgrade',
> - action='store_true',
> - help='Apply an upgrade to current rootfs even if running " \
> - "on dual rootfs system')
> -
> - parser.add_argument(
> - '--leave-files',
> - action='store_true',
> - help='Do not remove the downloaded system image files after upgrade')
> -
> - parser.add_argument(
> - '--no-reboot',
> - action='store_true',
> - help='Do not reboot even if one would normally be required')
> -
> - parser.add_argument(
> - '--reboot-delay',
> - type=int, default=DEFAULT_REBOOT_DELAY,
> - help='''
> - Wait for specified number of seconds before rebooting
> - (default={})
> - '''.format(DEFAULT_REBOOT_DELAY))
> -
> - parser.add_argument(
> - '--root-dir',
> - default=DEFAULT_ROOT,
> - help='Specify an alternative root directory (for testing ONLY)')
> -
> - parser.add_argument(
> - '--show-other-details',
> - action='store_true',
> - help='Dump the details of the system-image vesion on the " \
> - "other root partition')
> -
> - parser.add_argument(
> - '-t', '--tmpdir',
> - help='Specify name for pre-existing temporary directory to use')
> -
> - parser.add_argument(
> - 'cmdfile', action="store",
> - nargs='?',
> - help='Name of file containing commands to execute')
> -
> - return parser.parse_args()
> -
> -
> def upgrade(options, commands, remove_list):
> upgrader = Upgrader(options, commands, remove_list)
> upgrader.run()
> @@ -194,14 +103,14 @@
>
>
> def main():
> - options = parse_args()
> + options = parse_args(sys.argv[1:])
>
> - if options.reboot_delay < 0:
> + if options.reboot_delay and options.reboot_delay < 0:
> sys.exit('ERROR: must specify a positive number')
>
> if not os.path.exists(options.root_dir):
> sys.exit('ERROR: root directory does not exist: {}'
> - .format(options.root))
> + .format(options.root_dir))
>
> if options.check_reboot:
> # check options must be inert
> @@ -219,9 +128,11 @@
> if not options.cmdfile:
> sys.exit('ERROR: need command file')
>
> - if not os.path.exists(options.cmdfile):
> + cmdfile = options.cmdfile
> +
> + if not os.path.exists(cmdfile):
> sys.exit('ERROR: command file does not exist: {}'
> - .format(options.cmdfile))
> + .format(cmdfile))
>
> if os.getuid() != 0:
> sys.exit("ERROR: need to be root")
>
> === modified file 'debian/changelog'
> --- debian/changelog 2015-03-03 15:22:49 +0000
> +++ debian/changelog 2015-03-03 16:44:50 +0000
> @@ -1,3 +1,21 @@
> +ubuntu-core-upgrader (0.7.5) UNRELEASED; urgency=medium
> +
> + * bin/ubuntu-core-upgrade:
> + - parse_args(): Return a dict for easier testing.
This change is no longer part of the diff
> + - Update for options dict.
> + * ubuntucoreupgrader/tests/test_upgrader.py:
> + - More tar_generator() tests.
> + - Added first Upgrader() object test.
> + * ubuntucoreupgrader/upgrader.py:
> + - Comments.
> + - tar_generator():
> + - Fix to allow '--root-dir' to work again.
> + - Fix bug where path not set (only affecting tests).
> + - Updates for accessing self.options dict.
> + - Assert root_dir option set.
> +
> + -- James Hunt <james.hunt at ubuntu.com> Mon, 02 Mar 2015 14:59:43 +0000
> +
> ubuntu-core-upgrader (0.7.4) vivid; urgency=low
>
> * ubuntucoreupgrader/tests/test_upgrader.py:
>
> === modified file 'ubuntucoreupgrader/tests/test_upgrader.py'
> --- ubuntucoreupgrader/tests/test_upgrader.py 2015-03-03 14:36:01 +0000
> +++ ubuntucoreupgrader/tests/test_upgrader.py 2015-03-03 16:44:50 +0000
> @@ -1,28 +1,426 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3
> +# -*- coding: utf-8 -*-
> +# --------------------------------------------------------------------
> +# Copyright © 2014-2015 Canonical Ltd.
> +#
> +# This program is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation, version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +# --------------------------------------------------------------------
> +
> +# --------------------------------------------------------------------
> +# Terminology:
> +#
> +# - "update archive": a fake system-image "ubuntu-hash.tar.xz" tar
> +# archive.
> +# --------------------------------------------------------------------
>
> import tarfile
> import tempfile
> import unittest
> +import os
> +import shutil
> +
> +from unittest.mock import patch
>
> from ubuntucoreupgrader.upgrader import (
> tar_generator,
> + Upgrader,
> + parse_args,
> )
>
> +script_name = os.path.basename(__file__)
> +
> +# file mode to use for creating test directories.
> +TEST_DIR_MODE = 0o750
> +
> +CMD_FILE = 'ubuntu_command'
> +
> +
> +def make_default_options():
> + return parse_args([])
> +
> +
> +def make_tmp_dir(tag=None):
> + '''
> + Create a temporary directory.
> + '''
> +
> + if tag:
> + prefix = '{}-{}-'.format(script_name, tag)
> + else:
> + prefix = script_name
> +
> + return tempfile.mkdtemp(prefix=prefix)
> +
>
> class UpgradeTestCase(unittest.TestCase):
>
> - def test_tar_generator_regression_unpack_assets(self):
> + def test_tar_generator_unpack_assets(self):
> tempf = tempfile.TemporaryFile()
> with tarfile.TarFile(fileobj=tempf, mode="w") as tar:
> +
> + # special top-level file that should not be unpacked
> + tar.add(__file__, "removed")
> +
> tar.add(__file__, "system/bin/true")
> - tar.add(__file__, "assets/kernel")
> + tar.add(__file__, "assets/vmlinuz")
> + tar.add(__file__, "assets/initrd.img")
> tar.add(__file__, "unreleated/something")
> tar.add(__file__, "hardware.yaml")
>
> result = [m.name for m in tar_generator(tar, "cache_dir", [], "/")]
> - self.assertEqual(
> - result, ["system/bin/true", "assets/kernel", "hardware.yaml"])
> -
> + self.assertEqual(result, ["system/bin/true", "assets/vmlinuz",
> + "assets/initrd.img", "hardware.yaml"])
> +
> + def test_tar_generator_system_files_unpack(self):
> + tempf = tempfile.TemporaryFile()
> + root_dir = make_tmp_dir()
> + cache_dir = make_tmp_dir()
> +
> + sys_dir = os.path.join(cache_dir, 'system')
> +
> + os.makedirs(os.path.join(root_dir, 'dev'))
> + os.makedirs(sys_dir)
> +
> + with tarfile.TarFile(fileobj=tempf, mode="w") as tar:
> + tar.add(__file__, "assets/vmlinuz")
> + tar.add(__file__, "assets/initrd.img")
> +
> + device_a = '/dev/null'
> + path = (os.path.normpath('{}/{}'.format(sys_dir, device_a)))
> + touch_file(path)
> +
> + self.assertTrue(os.path.exists(os.path.join(root_dir, device_a)))
> +
> + # should not be unpacked as already exists
> + tar.add(__file__, "system{}".format(device_a))
> +
> + device_b = '/dev/does-not-exist'
> + self.assertFalse(os.path.exists(os.path.join(root_dir, device_b)))
> +
> + # should be unpacked as does not exist
> + tar.add(__file__, "system{}".format(device_b))
> +
> + expected = ["assets/vmlinuz", "assets/initrd.img",
> + "dev/does-not-exist"]
> +
> + expected_results = [os.path.join(root_dir, file)
> + for file in expected]
> +
> + result = [m.name for m in
> + tar_generator(tar, cache_dir, [], root_dir)]
> +
> + self.assertEqual(result, expected_results)
> +
> + shutil.rmtree(root_dir)
> + shutil.rmtree(cache_dir)
> +
> + def test_tar_generator_system_files_remove_before_unpack(self):
> + """
> + Test that the upgrader removes certain files just prior to
> + overwriting them via the unpack.
> + """
> +
> + tempf = tempfile.TemporaryFile()
> + root_dir = make_tmp_dir()
> + cache_dir = make_tmp_dir()
> + sys_dir = os.path.join(cache_dir, 'system')
> +
> + os.makedirs(sys_dir)
> +
> + file = 'a/file'
> + dir = 'a/dir'
> +
> + with tarfile.TarFile(fileobj=tempf, mode="w") as tar:
> +
> + file_path = os.path.normpath('{}/{}'.format(root_dir, file))
> + touch_file(file_path)
> + self.assertTrue(os.path.exists(file_path))
> +
> + dir_path = os.path.normpath('{}/{}'.format(root_dir, dir))
> + os.makedirs(dir_path)
> + self.assertTrue(os.path.exists(dir_path))
> +
> + tar.add(__file__, "system/{}".format(file))
> +
> + expected = [file]
> + expected_results = [os.path.join(root_dir, f)
> + for f in expected]
> +
> + result = [m.name for m in
> + tar_generator(tar, cache_dir, [], root_dir)]
> +
> + self.assertEqual(result, expected_results)
> +
> + # file should be removed
> + self.assertFalse(os.path.exists(file_path))
> +
> + # directory should not be removed
> + self.assertTrue(os.path.exists(dir_path))
> +
> + shutil.rmtree(root_dir)
> + shutil.rmtree(cache_dir)
> +
> +
> +def append_file(path, contents):
> + '''
> + Append to a regular file (create it doesn't exist).
> + '''
> +
> + dirname = os.path.dirname(path)
> + os.makedirs(dirname, mode=TEST_DIR_MODE, exist_ok=True)
> +
> + with open(path, 'a') as fh:
> + fh.writelines(contents)
> +
> + if not contents.endswith('\n'):
> + fh.write('\n')
> +
> +
> +def create_file(path, contents):
> + '''
> + Create a regular file.
> + '''
> + append_file(path, contents)
> +
> +
> +def touch_file(path):
> + '''
> + Create an empty file (creating any necessary intermediate
> + directories in @path.
> + '''
> + create_file(path, '')
> +
> +
> +def make_commands(update_list):
> + """
> + Convert the specified list of update archives into a list of command
> + file commands.
> + """
> + l = []
> +
> + # FIXME: we don't currently add a mount verb (which would be more
> + # realistics) since we can't handle that in the tests.
> + # ##l.append('mount system')
> +
> + for file in update_list:
> + l.append('update {} {}.asc'.format(file, file))
> +
> + # FIXME: we don't currently add an unmount verb (which would be more
> + # realistics) since we can't handle that in the tests.
> + # ##l.append('unmount system')
> +
> + return l
> +
> +
> +class UpdateTree():
> + '''
> + Representation of a directory tree that will be converted into an
> + update archive.
> + '''
> + TEST_REMOVED_FILE = 'removed'
> + TEST_SYSTEM_DIR = 'system/'
> +
> + def __init__(self):
> +
> + # Directory tree used to construct the tar file from.
> + # Also used to hold the TEST_REMOVED_FILE file.
> + self.dir = make_tmp_dir(tag='UpdateTree-tar-source')
> +
> + self.removed_file = os.path.join(self.dir, self.TEST_REMOVED_FILE)
> +
> + # Directory to place create/modify files into.
> + self.system_dir = os.path.join(self.dir, self.TEST_SYSTEM_DIR)
> +
> + # Directory used to write the generated tarfile to.
> + # This directory should also be used to write the command file
> + # to.
> + self.tmp_dir = make_tmp_dir(tag='UpdateTree-cache')
> +
> + def destroy(self):
> + if os.path.exists(self.dir):
> + shutil.rmtree(self.dir)
> +
> + if os.path.exists(self.tmp_dir):
> + shutil.rmtree(self.tmp_dir)
> +
> + def tar_filter(self, member):
> + '''
> + Function to filter the tarinfo members before creating the
> + archive.
> + '''
> + # members are created with relative paths (no leading slash)
> + path = os.sep + member.name
> +
> + if member.name == '/.':
> + return None
> +
> + i = path.find(self.dir)
> + assert(i == 0)
> +
> + # remove the temporary directory elements
> + # (+1 for the os.sep we added above)
> + member.name = path[len(self.dir)+1:]
> +
> + return member
> +
> + def create_archive(self, name):
> + '''
> + Create an archive with the specified name from the UpdateTree
> + object. Also creates a fake signature file alongside the archive
> + file since this is currently required by the upgrader (although
> + it is not validated).
> +
> + :param name: name of tarfile.
> + :param name: full path to xz archive to create.
> + :return full path to tar file with name @name.
> + '''
> +
> + tar_path = os.path.join(self.tmp_dir, name)
> + tar = tarfile.open(tar_path, 'w:xz')
> +
> + # We can't just add recursively since that would attempt to add
> + # the parent directory. However, the real update tars don't
> + # include that, and attempting to ignore the parent directory
> + # results in an empty archive. So, walk the tree and add
> + # file-by-file.
> + for path, names, files in os.walk(self.dir):
> + for file in files:
> + full = os.path.join(path, file)
> + tar.add(full, recursive=False, filter=self.tar_filter)
> + if not files and not names:
> + # add (empty) directories
> + tar.add(path, recursive=False, filter=self.tar_filter)
> +
> + tar.close()
> +
> + signature = '{}.asc'.format(tar_path)
> +
> + with open(signature, 'w') as fh:
> + fh.write('fake signature file')
> +
> + return tar_path
> +
> +
> +class UbuntuCoreUpgraderTestCase(unittest.TestCase):
> + '''
> + Base class for Upgrader tests.
> + '''
> +
> + TARFILE = 'update.tar.xz'
> +
> + # result of last test run. Hack to deal with fact that even if a
> + # test fails, unittest still calls .tearDown() (whomever thought
> + # that was a good idea...?)
> + currentResult = None
> +
> + def setUp(self):
> + '''
> + Test setup.
> + '''
> + # Create an object to hold the tree that will be converted into
> + # an upgrade archive.
> + self.update = UpdateTree()
> +
> + # The directory which will have the update archive applied to
> + # it.
> + self.victim_dir = make_tmp_dir(tag='victim')
> +
> + def tearDown(self):
> + '''
> + Test cleanup.
> + '''
> +
> + if not self.currentResult.wasSuccessful():
> + # Do not clean up - the only sane option if a test fails.
> + return
> +
> + self.update.destroy()
> + self.update = None
> +
> + shutil.rmtree(self.victim_dir)
> + self.victim_dir = None
> +
> + def run(self, result=None):
> + self.currentResult = result
> + unittest.TestCase.run(self, result)
> +
> +
> +def mock_get_root_partitions_by_label():
> + matches = []
> +
> + matches.append(('system-a', '/dev/sda3', '/'))
> + matches.append(('system-b', '/dev/sda4', '/writable/cache/system'))
> +
> + return matches
> +
> +
> +def mock_make_mount_private(target):
> + pass
> +
> +
> +class UbuntuCoreUpgraderObectTestCase(UbuntuCoreUpgraderTestCase):
> +
> + def mock_get_cache_dir(self):
> + '''
> + This is rather nasty - returns the value of a global variable to
This comment does no longer apply, the variable is no longer global AFAICS.
> + allow the cache directory value to be known before the Upgrader
> + object is created.
> + '''
> + assert(self.cache_dir)
> + self.sys_dir = os.path.join(self.cache_dir, 'system')
> + os.makedirs(self.sys_dir, exist_ok=True)
> + return self.cache_dir
> +
> + @patch('ubuntucoreupgrader.upgrader.get_root_partitions_by_label',
> + mock_get_root_partitions_by_label)
> + @patch('ubuntucoreupgrader.upgrader.make_mount_private',
> + mock_make_mount_private)
> + def test_create_object(self):
> + root_dir = make_tmp_dir()
> +
> + file = 'created-regular-file'
> +
> + file_path = os.path.join(self.update.system_dir, file)
> + create_file(file_path, 'foo bar')
> +
> + self.cache_dir = self.update.tmp_dir
> +
> + archive = self.update.create_archive(self.TARFILE)
> + self.assertTrue(os.path.exists(archive))
> +
> + dest = os.path.join(self.cache_dir, os.path.basename(archive))
> + touch_file('{}.asc'.format(dest))
> +
> + commands = make_commands([self.TARFILE])
> +
> + options = make_default_options()
> +
> + # XXX: doesn't actually exist, but the option must be set since
> + # the upgrader looks for the update archives in the directory
> + # where this file is claimed to be.
> + options.cmdfile = os.path.join(self.cache_dir, 'ubuntu_command')
> +
> + options.root_dir = root_dir
> +
> + upgrader = Upgrader(options, commands, [])
> + upgrader.get_cache_dir = self.mock_get_cache_dir
> + upgrader.run()
> +
> + path = os.path.join(root_dir, file)
> + self.assertTrue(os.path.exists(path))
> +
> + shutil.rmtree(root_dir)
>
> if __name__ == "__main__":
> unittest.main()
>
> === modified file 'ubuntucoreupgrader/upgrader.py'
> --- ubuntucoreupgrader/upgrader.py 2015-03-03 14:36:01 +0000
> +++ ubuntucoreupgrader/upgrader.py 2015-03-03 16:44:50 +0000
> @@ -48,6 +48,7 @@
> import stat
> import errno
> import dbus
> +import argparse
>
> from enum import Enum
> from time import sleep
> @@ -56,6 +57,11 @@
>
> log = logging.getLogger()
>
> +# seconds to wait before a reboot (should one be required)
> +DEFAULT_REBOOT_DELAY = 2
> +
> +DEFAULT_ROOT = '/'
> +
> # Name of the writable user data partition label as created by
> # ubuntu-device-flash(1).
> WRITABLE_DATA_LABEL = 'writable'
> @@ -87,7 +93,6 @@
> TAR_FILE_SYSTEM_PREFIX = 'system/'
>
> # boot assets (kernel, initrd, .dtb's)
> -# (XXX: note no trailing slash to ensure we unpack the directory itself).
> TAR_FILE_ASSETS_PREFIX = 'assets/'
>
> # the file that describes the boot assets
> @@ -98,6 +103,93 @@
> SYSTEM_IMAGE_CHANNEL_CONFIG = '/etc/system-image/channel.ini'
>
>
> +def parse_args(args):
> + '''
> + Handle command-line options.
> +
> + Returns: dict of command-line options.
> + '''
> +
> + parser = argparse.ArgumentParser(description='System image Upgrader')
> +
> + parser.add_argument(
> + '--check-reboot',
> + action='store_true',
> + help='''
> + Attempt to determine if a reboot may be required.
> +
> + This option is similar to --dry-run: no system changes are made.
> +
> + Note that the result of this command cannot be definitive since a
> + reboot would be triggered if a service failed to start (but this
> + option does not attempt to restart any services).
> + ''')
> +
> + parser.add_argument(
> + '--clean-only',
> + action='store_true',
> + help='Clean up from a previous upgrader run, but do not upgrade)')
> +
> + parser.add_argument(
> + '--debug',
> + nargs='?', const=1, default=0, type=int,
> + help='Dump debug info (specify numeric value to increase verbosity)')
> +
> + parser.add_argument(
> + '-n', '--dry-run',
> + action='store_true',
> + help='''
> + Simulate an update including showing processes that have locks
> + on files
> + ''')
> +
> + parser.add_argument(
> + '--force-inplace-upgrade',
> + action='store_true',
> + help='Apply an upgrade to current rootfs even if running " \
> + "on dual rootfs system')
> +
> + parser.add_argument(
> + '--leave-files',
> + action='store_true',
> + help='Do not remove the downloaded system image files after upgrade')
> +
> + parser.add_argument(
> + '--no-reboot',
> + action='store_true',
> + help='Do not reboot even if one would normally be required')
> +
> + parser.add_argument(
> + '--reboot-delay',
> + type=int, default=DEFAULT_REBOOT_DELAY,
> + help='''
> + Wait for specified number of seconds before rebooting
> + (default={})
> + '''.format(DEFAULT_REBOOT_DELAY))
> +
> + parser.add_argument(
> + '--root-dir',
> + default=DEFAULT_ROOT,
> + help='Specify an alternative root directory (for testing ONLY)')
> +
> + parser.add_argument(
> + '--show-other-details',
> + action='store_true',
> + help='Dump the details of the system-image vesion on the " \
> + "other root partition')
> +
> + parser.add_argument(
> + '-t', '--tmpdir',
> + help='Specify name for pre-existing temporary directory to use')
> +
> + parser.add_argument(
> + 'cmdfile', action="store",
> + nargs='?',
> + help='Name of file containing commands to execute')
> +
> + return parser.parse_args(args=args)
> +
> +
> def remove_prefix(path, prefix=TAR_FILE_SYSTEM_PREFIX):
> '''
> Remove specified prefix from path and return the result.
> @@ -733,7 +825,7 @@
> def tar_generator(tar, cache_dir, removed_files, root_dir):
> '''
> Generator function to handle extracting members from the system
> - image tar file.
> + image tar files.
> '''
> for member in tar:
>
> @@ -749,6 +841,11 @@
> # extracted!
> device_prefix = '{}dev/'.format(TAR_FILE_SYSTEM_PREFIX)
>
> + # The partition to update is mounted at "cache_dir/system".
> + # However, the unpack occurs in directory cache_dir (since the
> + # tar files prefixes all system files with
> + # TAR_FILE_SYSTEM_PREFIX). For example, member.name may be
> + # something like "system/dev/null".
> mount_path = '{}/{}'.format(cache_dir, member.name)
>
> unpack = True
> @@ -762,56 +859,73 @@
> # device already exists
> unpack = False
>
> - if not (member.name == TAR_FILE_HARDWARE_YAML or
> - member.name.startswith(TAR_FILE_SYSTEM_PREFIX) or
> - member.name.startswith(TAR_FILE_ASSETS_PREFIX)):
> + member_prefix = None
> +
> + if member.name.startswith(TAR_FILE_SYSTEM_PREFIX):
> + member_prefix = TAR_FILE_SYSTEM_PREFIX
> + elif member.name.startswith(TAR_FILE_ASSETS_PREFIX):
> + member_prefix = TAR_FILE_ASSETS_PREFIX
> +
> + if not member_prefix:
> # unrecognised prefix directory
> unpack = False
>
> + if member.name == TAR_FILE_HARDWARE_YAML:
> + unpack = True
> +
> if not unpack:
> log.debug('not unpacking file {}'.format(member.name))
> + continue
> +
> + # A modified root directory requires convering
> + # absolute paths to be located below the modified root
> + # directory.
> + if root_dir != '/':
> + if member_prefix == TAR_FILE_SYSTEM_PREFIX:
> + base = remove_prefix(member.name, prefix=member_prefix)
> + member.name = os.path.normpath('{}/{}'.format(root_dir,
> + base))
> + else:
> + member.name = os.path.normpath('{}/{}'.format(root_dir,
> + member.name))
> +
> + if member.type in (tarfile.LNKTYPE, tarfile.SYMTYPE) \
> + and member.linkname.startswith('/'):
> + # Hard and symbolic links also need their
> + # 'source' updated to take account of the root
> + # directory.
> + #
> + # But rather than remove the prefix, we add the
> + # root directory as a prefix to contain the link
> + # within that root.
> + base = os.path.join(root_dir, member.linkname)
> +
> + member.linkname = '{}{}'.format(root_dir, base)
> +
> + path = member.name
> else:
> - # A modified root directory requires convering
> - # absolute paths to be located below the modified root
> - # directory.
> - if root_dir != '/':
> - base = remove_prefix(member.name)
> - member.name = '{}{}'.format(root_dir, base)
> -
> - if member.type in (tarfile.LNKTYPE, tarfile.SYMTYPE) \
> - and member.linkname.startswith('/'):
> - # Hard and symbolic links also need their
> - # 'source' updated to take account of the root
> - # directory.
> - #
> - # But rather than remove the prefix, we add the
> - # root directory as a prefix to contain the link
> - # within that root.
> - base = os.path.join(root_dir, member.linkname)
> -
> - member.linkname = '{}{}'.format(root_dir, base)
> - else:
> - path = mount_path
> -
> - log.debug('unpacking file {}'.format(member.name))
> -
> - # If the file is a binary and that binary is currently
> - # being executed by a process, attempting to unpack it
> - # will result in ETXTBSY (OSError: [Errno 26] Text file
> - # busy). The simplest way around this issue is to unlink
> - # the file just before unpacking it (ideally, we'd catch
> - # the exception and handle it separately). This allows
> - # the unpack to continue, and the running process to
> - # continue to use it's (old) version of the binary until
> - # it's corresponding service is restarted.
> - #
> - # Note that at this point, we already have another copy
> - # of the inode below /lost+found/.
> - if not member.isdir() and os.path.lexists(path):
> - log.debug('removing file {}'.format(path))
> - os.unlink(path)
> -
> - yield member
> + path = mount_path
> +
> + log.debug('unpacking file {} with path {}'
> + .format(member.name, path))
> +
> + # If the file is a binary and that binary is currently
> + # being executed by a process, attempting to unpack it
> + # will result in ETXTBSY (OSError: [Errno 26] Text file
> + # busy). The simplest way around this issue is to unlink
> + # the file just before unpacking it (ideally, we'd catch
> + # the exception and handle it separately). This allows
> + # the unpack to continue, and the running process to
> + # continue to use it's (old) version of the binary until
> + # it's corresponding service is restarted.
> + #
> + # Note that at this point, we already have another copy
> + # of the inode below /lost+found/.
> + if not member.isdir() and os.path.lexists(path):
> + log.debug('removing file {}'.format(path))
> + os.unlink(path)
> +
> + yield member
>
>
> class Upgrader():
> @@ -853,6 +967,14 @@
> return os.path.join(self.get_cache_dir(), self.MOUNT_TARGET)
>
> def __init__(self, options, commands, remove_list):
> + """
> + @options list of command-line options.
> + @commands: array of declarative commands to run to apply the new
> + s-i rootfs update.
> + @remove_list: list of files to remove before applying the new
> + image.
> +
> + """
> self.dispatcher = {
> 'format': self._cmd_format,
> 'load_keyring': self._cmd_load_keyring,
> @@ -882,6 +1004,8 @@
>
> self.options = options
>
> + assert('root_dir' in self.options)
> +
> # array of imperative commands to run, as generated by
> # system-image-cli(1).
> self.commands = commands
> @@ -909,8 +1033,6 @@
> if self.options.cmdfile:
> self.file_dir = os.path.dirname(self.options.cmdfile)
>
> - self.systemd = Systemd()
> -
> # list of systemd unit D-Bus paths (such as
> # '/org/freedesktop/systemd1/unit/cron_2eservice') that need to
> # be restarted since they have running processes that are
> @@ -924,12 +1046,6 @@
> # Note: Only used by UPGRADE_IN_PLACE.
> self.other_is_empty = False
>
> - target = self.get_mount_target()
> -
> - # Note that we need the stat of the mountpoint,
> - # *NOT* the device itself.
> - self.mountpoint_stat = os.stat(target)
> -
> def set_reboot_reason(self, reason):
> '''
> Set the reboot reason, if not already set. This ensures the
> @@ -959,11 +1075,14 @@
> Required setup.
> '''
>
> + target = self.get_mount_target()
> +
> + # Note that we need the stat of the mountpoint,
> + # *NOT* the device itself.
> + self.mountpoint_stat = os.stat(target)
> +
> self.determine_upgrade_type()
>
> - log.debug('System is using systemd version {}'
> - .format(self.systemd.version()))
> -
> log.debug('system upgrade type is {}'.format(self.upgrade_type))
> if self.options.debug > 1:
> log.debug('current rootfs device is {}'
> @@ -1080,10 +1199,11 @@
> log.warning('Not rebooting at user request')
> return
>
> + reboot_delay = self.options.reboot_delay
> # give the admin a chance to see the message
> log.debug('Waiting for {} seconds before rebooting'
> - .format(self.options.reboot_delay))
> - sleep(self.options.reboot_delay)
> + .format(reboot_delay))
> + sleep(reboot_delay)
> os.system('/sbin/reboot')
>
> def _cmd_format(self, args):
> @@ -1436,7 +1556,7 @@
> # It will either be a sym-link or a bind-mount
> # unrelated to those specified by writable-paths(5).
> if st.st_dev != self.mountpoint_stat.st_dev:
> - if self.options.debug > 1:
> + if self.options.debug and self.options.debug > 1:
> log.debug('Ignoring cross-FS file: {}'
> .format(file))
> continue
> @@ -1455,7 +1575,7 @@
> # - sym links to directories.
> #
> try:
> - if self.options.debug > 1:
> + if self.options.debug and self.options.debug > 1:
> log.debug('creating directory {}'
> .format(saved_link_dirname))
> os.makedirs(saved_link_dirname,
> @@ -1631,8 +1751,7 @@
> # os.path.join() refuses to work if the file
> # begins with a slash.
> base = remove_prefix(remove)
> - final = '{}{}'.format(self.options.root_dir,
> - base)
> + final = '{}{}'.format(self.options.root_dir, base)
>
> if not os.path.exists(final):
> # This scenario can only mean there is a bug
> @@ -1672,7 +1791,8 @@
> tar.extractall(path=self.get_cache_dir(),
> members=tar_generator(
> tar, self.get_cache_dir(),
> - self.removed_file, self.options.root_dir))
> + self.removed_file,
> + self.options.root_dir))
> tar.close()
>
> if self.upgrade_type == self.upgrade_types.UPGRADE_AB_PARTITIONS:
> @@ -1793,6 +1913,11 @@
> '''
> failed = False
>
> + self.systemd = Systemd()
> +
> + log.debug('System is using systemd version {}'
> + .format(self.systemd.version()))
> +
> for file in self.pid_file_map:
> for pid in self.pid_file_map[file]:
> ret = self.restart_associated_service(pid, file)
>
--
https://code.launchpad.net/~jamesodhunt/ubuntu/vivid/ubuntu-core-upgrader/add-more-tests/+merge/251571
Your team Ubuntu branches is subscribed to branch lp:ubuntu/ubuntu-core-upgrader.
More information about the Ubuntu-reviews
mailing list