0.7.8 (patches applied)applied/0.7.8

Imported using git-ubuntu import.

5 files changed, 285 insertions, 63 deletions

diff --git a/debian/changelog b/debian/changelog
index dd47148..84c2727 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,14 @@
+ubuntu-core-upgrader (0.7.8) vivid; urgency=medium
+
+ * Implement s-i format command to handle full-image (non-delta) updates
+ correctly.
+ * ubuntucoreupgrader/upgrader.py: Tolerate an invalid 'removed' file
+ to avoid the upgrade failing attempting to remove '/writable/cache'
+ (see LP: #1437225).
+ * functional/test_upgrader.py: Add tests for an invalid removed file.
+
+ -- James Hunt <james.hunt@ubuntu.com> Tue, 07 Apr 2015 14:29:29 +0100
+
 ubuntu-core-upgrader (0.7.7) vivid; urgency=low
 
 * fix entry point for s-i 3.0
diff --git a/functional/test_upgrader.py b/functional/test_upgrader.py
index 8609376..27f76cf 100644
--- a/functional/test_upgrader.py
+++ b/functional/test_upgrader.py
@@ -25,7 +25,6 @@ import sys
 import os
 import logging
 import unittest
-import shutil
 
 from ubuntucoreupgrader.upgrader import (
 Upgrader,
@@ -37,7 +36,6 @@ module_dir = os.path.normpath(os.path.realpath(base_dir + os.sep + '..'))
 sys.path.append(base_dir)
 
 from ubuntucoreupgrader.tests.utils import (
- make_tmp_dir,
 append_file,
 TEST_DIR_MODE,
 create_file,
@@ -68,21 +66,11 @@ def call_upgrader(command_file, root_dir, update):
 args.append(command_file)
 commands = file_to_list(command_file)
 
- cache_dir = make_tmp_dir()
-
- def mock_get_cache_dir():
- cache_dir = update.tmp_dir
- sys_dir = os.path.join(cache_dir, 'system')
- os.makedirs(sys_dir, exist_ok=True)
- return cache_dir
-
 upgrader = Upgrader(parse_args(args), commands, [])
- upgrader.get_cache_dir = mock_get_cache_dir
+ upgrader.cache_dir = root_dir
 upgrader.MOUNTPOINT_CMD = "true"
 upgrader.run()
 
- shutil.rmtree(cache_dir)
-
 
 def create_device_file(path, type='c', major=-1, minor=-1):
 '''
@@ -216,7 +204,10 @@ class UpgraderFileRemovalTestCase(UbuntuCoreUpgraderTestCase):
 self.assertTrue(os.path.exists(file_path))
 self.assertTrue(os.path.isfile(file_path))
 
- call_upgrader(cmd_file, self.victim_dir, self.update)
+ # remove 'system' suffix that upgrader will add back on
+ vdir = os.path.split(self.victim_dir)[0]
+
+ call_upgrader(cmd_file, vdir, self.update)
 
 self.assertFalse(os.path.exists(file_path))
 
@@ -240,7 +231,8 @@ class UpgraderFileRemovalTestCase(UbuntuCoreUpgraderTestCase):
 self.assertTrue(os.path.exists(dir_path))
 self.assertTrue(os.path.isdir(dir_path))
 
- call_upgrader(cmd_file, self.victim_dir, self.update)
+ vdir = os.path.split(self.victim_dir)[0]
+ call_upgrader(cmd_file, vdir, self.update)
 
 self.assertFalse(os.path.exists(dir_path))
 
@@ -272,7 +264,8 @@ class UpgraderFileRemovalTestCase(UbuntuCoreUpgraderTestCase):
 self.assertTrue(os.path.exists(link_file_path))
 self.assertTrue(os.path.islink(link_file_path))
 
- call_upgrader(cmd_file, self.victim_dir, self.update)
+ vdir = os.path.split(self.victim_dir)[0]
+ call_upgrader(cmd_file, vdir, self.update)
 
 # original file should still be there
 self.assertTrue(os.path.exists(src_file_path))
@@ -310,7 +303,8 @@ class UpgraderFileRemovalTestCase(UbuntuCoreUpgraderTestCase):
 self.assertTrue(os.path.exists(link_file_path))
 self.assertTrue(os.path.islink(link_file_path))
 
- call_upgrader(cmd_file, self.victim_dir, self.update)
+ vdir = os.path.split(self.victim_dir)[0]
+ call_upgrader(cmd_file, vdir, self.update)
 
 # original directory should still be there
 self.assertTrue(os.path.exists(src_dir_path))
@@ -352,7 +346,8 @@ class UpgraderFileRemovalTestCase(UbuntuCoreUpgraderTestCase):
 
 self.assertTrue(src_inode == link_inode)
 
- call_upgrader(cmd_file, self.victim_dir, self.update)
+ vdir = os.path.split(self.victim_dir)[0]
+ call_upgrader(cmd_file, vdir, self.update)
 
 # original file should still be there
 self.assertTrue(os.path.exists(src_file_path))
@@ -392,9 +387,11 @@ class UpgraderFileRemovalTestCase(UbuntuCoreUpgraderTestCase):
 # device because it won't be :)
 self.assertTrue(os.path.isfile(file_path))
 
- call_upgrader(cmd_file, self.victim_dir, self.update)
+ vdir = os.path.split(self.victim_dir)[0]
+ call_upgrader(cmd_file, vdir, self.update)
 
- self.assertFalse(os.path.exists(file_path))
+ # upgrader doesn't currently remove device files
+ self.assertTrue(os.path.exists(file_path))
 
 
 class UpgraderFileAddTestCase(UbuntuCoreUpgraderTestCase):
@@ -595,6 +592,67 @@ class UpgraderFileAddTestCase(UbuntuCoreUpgraderTestCase):
 self.assertTrue(is_sym_link_broken(victim_link_file_path))
 
 
+class UpgraderRemoveFileTests(UbuntuCoreUpgraderTestCase):
+
+ def common_removed_file_test(self, contents):
+ '''
+ Common code to test for an invalid removed file.
+
+ The contents parameter specifies the contents of the removed
+ file.
+ '''
+ file = 'created-regular-file'
+
+ create_file(self.update.removed_file, contents)
+
+ file_path = os.path.join(self.update.system_dir, file)
+ create_file(file_path, 'foo bar')
+
+ archive = self.update.create_archive(self.TARFILE)
+ self.assertTrue(os.path.exists(archive))
+
+ cmd_file = os.path.join(self.update.tmp_dir, CMD_FILE)
+ make_command_file(cmd_file, [self.TARFILE])
+
+ vdir = os.path.split(self.victim_dir)[0]
+
+ file_path = os.path.join(vdir, file)
+ self.assertFalse(os.path.exists(file_path))
+
+ # XXX: There is an implicit test here since if the upgrader
+ # fails (as documented on LP: #1437225), this test will also
+ # fail.
+ call_upgrader(cmd_file, vdir, self.update)
+
+ self.assertTrue(os.path.exists(vdir))
+ self.assertTrue(os.path.exists(self.victim_dir))
+ self.assertTrue(os.path.exists(file_path))
+
+ # ensure the empty removed file hasn't removed the directory the
+ # unpack applies to.
+ self.assertTrue(self.victim_dir)
+
+ def test_removed_file_empty(self):
+ '''
+ Ensure the upgrader ignores an empty 'removed' file.
+ '''
+ self.common_removed_file_test('')
+
+ def test_removed_file_space(self):
+ '''
+ Ensure the upgrader handles a 'removed' file containing just a
+ space.
+ '''
+ self.common_removed_file_test(' ')
+
+ def test_removed_file_nl(self):
+ '''
+ Ensure the upgrader handles a 'removed' file containing just a
+ newline
+ '''
+ self.common_removed_file_test('\n')
+
+
 def main():
 kwargs = {}
 format = \
diff --git a/ubuntucoreupgrader/tests/test_upgrader.py b/ubuntucoreupgrader/tests/test_upgrader.py
index 9189a54..da1a364 100644
--- a/ubuntucoreupgrader/tests/test_upgrader.py
+++ b/ubuntucoreupgrader/tests/test_upgrader.py
@@ -27,6 +27,7 @@
 import tarfile
 import tempfile
 import unittest
+from unittest.mock import patch
 import os
 import shutil
 import sys
@@ -196,15 +197,6 @@ def make_commands(update_list):
 
 class UbuntuCoreUpgraderObectTestCase(UbuntuCoreUpgraderTestCase):
 
- def mock_get_cache_dir(self):
- '''
- Mock to allow the tests to control the cache directory location.
- '''
- 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
-
 def test_create_object(self):
 root_dir = make_tmp_dir()
 
@@ -233,7 +225,6 @@ class UbuntuCoreUpgraderObectTestCase(UbuntuCoreUpgraderTestCase):
 options.root_dir = root_dir
 
 upgrader = Upgrader(options, commands, [])
- upgrader.get_cache_dir = self.mock_get_cache_dir
 upgrader.MOUNTPOINT_CMD = "true"
 upgrader.run()
 
@@ -242,6 +233,98 @@ class UbuntuCoreUpgraderObectTestCase(UbuntuCoreUpgraderTestCase):
 
 shutil.rmtree(root_dir)
 
+ @patch('ubuntucoreupgrader.upgrader.get_mount_details')
+ @patch('ubuntucoreupgrader.upgrader.mount')
+ @patch('ubuntucoreupgrader.upgrader.unmount')
+ def test_no_format_in_cmd(self, mock_umount, mock_mount,
+ mock_mount_details):
+
+ # If the command file does not contain the format command, mkfs
+ # should not be called.
+ with patch('ubuntucoreupgrader.upgrader.mkfs') as mock_mkfs:
+ args = ['cmdfile']
+ options = parse_args(args=args)
+ commands = make_commands([self.TARFILE])
+ upgrader = Upgrader(options, commands, [])
+ upgrader.TIMESTAMP_FILE = '/dev/null'
+ upgrader.MOUNTPOINT_CMD = "true"
+ upgrader.run()
+
+ # No format command in command file, so should not have been
+ # called.
+ self.assertFalse(mock_mkfs.called)
+
+ @patch('ubuntucoreupgrader.upgrader.get_mount_details')
+ @patch('ubuntucoreupgrader.upgrader.mount')
+ @patch('ubuntucoreupgrader.upgrader.unmount')
+ def test_format(self, mock_umount, mock_mount, mock_mount_details):
+ MOCK_FS_TUPLE = ("device", "fstype", "label")
+ mock_mount_details.return_value = MOCK_FS_TUPLE
+
+ # mkfs should be called if the format command is specified in
+ # the command file.
+ with patch('ubuntucoreupgrader.upgrader.mkfs') as mock_mkfs:
+ args = ['cmdfile']
+ options = parse_args(args=args)
+ commands = make_commands([self.TARFILE])
+
+ # add format command to command file
+ commands.insert(0, 'format system')
+
+ upgrader = Upgrader(options, commands, [])
+ upgrader.TIMESTAMP_FILE = '/dev/null'
+ upgrader.MOUNTPOINT_CMD = "true"
+ upgrader.run()
+
+ mock_mkfs.assert_called_with(*MOCK_FS_TUPLE)
+
+ def test_empty_removed_file(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
+
+ removed_file = self.update.removed_file
+ # Create an empty removed file
+ create_file(removed_file, '')
+
+ 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.cache_dir = self.cache_dir
+ upgrader.MOUNTPOINT_CMD = "true"
+
+ si_file = self.TARFILE
+ si_signature = "{}.asc".format(self.TARFILE)
+ upgrader._cmd_update([si_file, si_signature])
+
+ # Ensure that the upgrader has not attempted to remove the
+ # cache_dir when the 'removed' file is empty.
+ #
+ # (Regression test for LP: #1437225).
+ self.assertTrue(os.path.exists(upgrader.cache_dir))
+
+ shutil.rmtree(self.cache_dir)
+
 
 if __name__ == "__main__":
 unittest.main()
diff --git a/ubuntucoreupgrader/tests/utils.py b/ubuntucoreupgrader/tests/utils.py
index dc4f463..31d3729 100644
--- a/ubuntucoreupgrader/tests/utils.py
+++ b/ubuntucoreupgrader/tests/utils.py
@@ -119,7 +119,8 @@ class UpdateTree():
 '''
 # all files listed in the removed list must be system files
 final = list(map(lambda a:
- '{}{}'.format(self.TEST_SYSTEM_DIR, a), removed_files))
+ os.path.normpath('{}{}'.format(self.TEST_SYSTEM_DIR,
+ a)), removed_files))
 
 contents = "".join(final)
 append_file(self.removed_file, contents)
@@ -194,7 +195,7 @@ class UbuntuCoreUpgraderTestCase(unittest.TestCase):
 
 This creates 2 temporary directories:
 
- - self.system dir: Used as to generate an update archive from.
+ - self.system_dir: Used to generate an update archive from.
 
 - self.tmp_dir: Used to write the generated archive file to. The
 intention is that this directory should also be used to hold
@@ -244,7 +245,9 @@ class UbuntuCoreUpgraderTestCase(unittest.TestCase):
 
 # The directory which will have the update archive applied to
 # it.
- self.victim_dir = make_tmp_dir(tag='victim')
+ self.victim_dir_base = make_tmp_dir(tag='victim')
+ self.victim_dir = os.path.join(self.victim_dir_base, 'system')
+ os.makedirs(self.victim_dir, mode=0o750)
 
 def tearDown(self):
 '''
diff --git a/ubuntucoreupgrader/upgrader.py b/ubuntucoreupgrader/upgrader.py
index 502fbc4..c744129 100644
--- a/ubuntucoreupgrader/upgrader.py
+++ b/ubuntucoreupgrader/upgrader.py
@@ -203,6 +203,56 @@ def fsck(device):
 sys.exit(1)
 
 
+def mkfs(device, fs_type, label):
+ '''
+ Run mkfs(8) on specified device.
+ '''
+ assert (os.path.exists(device))
+
+ args = ['/sbin/mkfs',
+ '-v'
+ '-t', fs_type,
+ '-L', label,
+ device]
+
+ log.debug('running: {}'.format(args))
+
+ proc = subprocess.Popen(args,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE,
+ universal_newlines=True)
+
+ ret = proc.wait()
+
+ if ret == 0:
+ return
+
+ stdout, stderr = proc.communicate()
+
+ log.error('{} returned {}: {}, {}'
+ .format(args,
+ ret,
+ stdout,
+ stderr))
+ sys.exit(1)
+
+
+def get_mount_details(target):
+ '''
+ Returns a list comprising device, filesystem type and filesystem
+ label for the mount target specified.
+ '''
+ args = ['findmnt', '-o', 'SOURCE,FSTYPE,LABEL', '-n', target]
+ output = subprocess.check_output(args, universal_newlines=True)
+ output = output.strip().split()
+
+ if len(output) != 3:
+ sys.exit('Failed to determine mount details for {}'
+ .format(target))
+
+ return output
+
+
 def remount(mountpoint, options):
 """
 Remount mountpoint using the specified options string (which is
@@ -468,33 +518,31 @@ class Upgrader():
 # If True, the other partition is considered empty. In this
 # scenario, prior to unpacking the latest image, the _current_
 # rootfs image is copied to the other partition.
- #
- # Note: Only used by UPGRADE_IN_PLACE.
 self.other_is_empty = False
 
+ # True if the upgrader has reformatted the "other" partition.
+ self.other_has_been_formatted = False
+
+ # cache_dir is used as a scratch pad, for downloading new images
+ # to and bind mounting the rootfs.
+ if self.options.tmpdir:
+ self.cache_dir = self.options.tmpdir
+ else:
+ self.cache_dir = self.DEFAULT_CACHE_DIR
+
 def update_timestamp(self):
 '''
 Update the timestamp file to record the time the last upgrade
 completed successfully.
 '''
- file = os.path.join(self.get_cache_dir(), self.TIMESTAMP_FILE)
+ file = os.path.join(self.cache_dir, self.TIMESTAMP_FILE)
 open(file, 'w').close()
 
- def get_cache_dir(self):
- '''
- Returns the full path to the cache directory, which is used as a
- scratch pad, for downloading new images to and bind mounting the
- rootfs.
- '''
- return self.options.tmpdir \
- if self.options.tmpdir \
- else self.DEFAULT_CACHE_DIR
-
 def get_mount_target(self):
 '''
 Get the full path to the mount target directory.
 '''
- return os.path.join(self.get_cache_dir(), self.MOUNT_TARGET)
+ return os.path.join(self.cache_dir, self.MOUNT_TARGET)
 
 def prepare(self):
 '''
@@ -566,9 +614,17 @@ class Upgrader():
 if self.options.dry_run or self.options.root_dir != '/':
 return
 
- log.warning('ingoring format target {} (unsupported operation)'
- .format(target))
- return
+ other = self.get_mount_target()
+
+ source, fstype, label = get_mount_details(other)
+
+ unmount(other)
+ mkfs(source, fstype, label)
+
+ # leave the mount as it was initially.
+ mount(source, other, "ro")
+
+ self.other_has_been_formatted = True
 
 def _cmd_load_keyring(self, args):
 try:
@@ -609,6 +665,19 @@ class Upgrader():
 .format(target_type))
 return
 
+ if self.other_is_empty and not self.other_has_been_formatted:
+ # We believe "other" is empty. However, it's possible that a
+ # previous attempt at upgrading failed due to a power
+ # outage. In that scenario, "other" may contain most of a
+ # rootfs image, but just be missing the files used to
+ # determine that the partition is empty. As such, if we
+ # believe the partition is empty, it should forcibly be made
+ # empty since it may contain detritus from a
+ # previously-failed unpack (possibly caused by a power
+ # outage).
+ log.warning('reformatting other (no system image)')
+ self._cmd_format("system")
+
 self.remount_rootfs(writable=True)
 
 if self.other_is_empty:
@@ -739,14 +808,19 @@ class Upgrader():
 else:
 to_remove = []
 
+ # by definition, full images don't have 'removed' files.
 if not self.full_image:
-
 if found_removed_file:
 log.debug('processing {} file'
 .format(self.removed_file))
 
 # process backwards to work around bug LP:#1381134.
 for remove in sorted(to_remove, reverse=True):
+
+ if not remove:
+ # handle invalid removed file entry (see LP:#1437225)
+ continue
+
 remove = remove.strip()
 
 # don't remove devices
@@ -762,13 +836,7 @@ class Upgrader():
 if '../' in remove:
 continue
 
- if self.options.root_dir == '/':
- final = os.path.join(self.get_cache_dir(), remove)
- else:
- # 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 = os.path.join(self.cache_dir, remove)
 
 if not os.path.exists(final):
 # This scenario can only mean there is a bug
@@ -804,9 +872,9 @@ class Upgrader():
 from unittest.mock import patch
 with patch("grp.getgrnam") as m:
 m.side_effect = KeyError()
- tar.extractall(path=self.get_cache_dir(),
+ tar.extractall(path=self.cache_dir,
 members=tar_generator(
- tar, self.get_cache_dir(),
+ tar, self.cache_dir,
 self.removed_file,
 self.options.root_dir))
 tar.close()
@@ -816,12 +884,12 @@ class Upgrader():
 Copy all rootfs data from the current partition to the other
 partition.
 
- XXX: Assumes that the other rootfs is already mounted to
+ XXX: Assumes that the other rootfs is already mounted r/w to
 mountpoint get_mount_target().
 '''
 target = self.get_mount_target()
 bindmount_rootfs_dir = tempfile.mkdtemp(prefix=script_name,
- dir=self.get_cache_dir())
+ dir=self.cache_dir)
 bind_mount("/", bindmount_rootfs_dir)
 
 cwd = os.getcwd()
@@ -846,4 +914,3 @@ class Upgrader():
 
 unmount(bindmount_rootfs_dir)
 os.rmdir(bindmount_rootfs_dir)
- self.other_is_empty = False