0.7.10 (patches applied)applied/0.7.10applied/ubuntu/vivid-proposedapplied/ubuntu/vivid-develapplied/ubuntu/vivid

Imported using git-ubuntu import.

4 files changed, 228 insertions, 32 deletions

diff --git a/debian/changelog b/debian/changelog
index 8d1091c..dade448 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,21 @@
+ubuntu-core-upgrader (0.7.10) vivid; urgency=medium
+
+ * ubuntucoreupgrader/upgrader.py: _cmd_mount(): Don't sync partitions
+ for a full (non-delta) update (LP: #1444500).
+ * ubuntucoreupgrader/tests/test_upgrader.py:
+ - touch_file(): Fix to make file of size 0, not 1.
+ - test_marker_file(): Removed.
+ - test_other_considered_empty(): New test.
+ * ubuntucoreupgrader/tests/utils.py:
+ - append_file(): Create zero-length file if contents are None.
+ * ubuntucoreupgrader/upgrader.py:
+ - Replaced marker file logic by tighter scoped strategy where the s-i
+ config file is pre-created with zero size prior to performing an s-i
+ update operation. This strategy allows a failed update to be identified
+ from either a missing s-i config file, or a zero-length one.
+
+ -- James Hunt <james.hunt@ubuntu.com> Mon, 13 Apr 2015 20:28:33 +0100
+
 ubuntu-core-upgrader (0.7.9) vivid; urgency=medium
 
 * mkfs(): Fixed mkfs command line (missing comma). 
diff --git a/ubuntucoreupgrader/tests/test_upgrader.py b/ubuntucoreupgrader/tests/test_upgrader.py
index da1a364..55da157 100644
--- a/ubuntucoreupgrader/tests/test_upgrader.py
+++ b/ubuntucoreupgrader/tests/test_upgrader.py
@@ -169,9 +169,9 @@ class UpgradeTestCase(unittest.TestCase):
 def touch_file(path):
 '''
 Create an empty file (creating any necessary intermediate
- directories in @path.
+ directories in @path).
 '''
- create_file(path, '')
+ create_file(path, None)
 
 
 def make_commands(update_list):
@@ -275,9 +275,70 @@ class UbuntuCoreUpgraderObectTestCase(UbuntuCoreUpgraderTestCase):
 upgrader.TIMESTAMP_FILE = '/dev/null'
 upgrader.MOUNTPOINT_CMD = "true"
 upgrader.run()
+ self.assertTrue(upgrader.other_has_been_formatted)
 
 mock_mkfs.assert_called_with(*MOCK_FS_TUPLE)
 
+ @patch.object(Upgrader, 'sync_partitions')
+ @patch('ubuntucoreupgrader.upgrader.get_mount_details')
+ @patch('ubuntucoreupgrader.upgrader.remount')
+ @patch('ubuntucoreupgrader.upgrader.mount')
+ @patch('ubuntucoreupgrader.upgrader.fsck')
+ @patch('ubuntucoreupgrader.upgrader.mkfs')
+ @patch('ubuntucoreupgrader.upgrader.unmount')
+ def test_mount_unmount_no_format(self, mock_umount, mock_mkfs, mock_fsck,
+ mock_mount, mock_remount,
+ mock_mount_details, mock_sync_partitions):
+ 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.
+ args = ['cmdfile']
+ options = parse_args(args=args)
+ commands = make_commands([self.TARFILE])
+
+ commands.insert(0, 'mount system')
+ commands.append('unmount system')
+
+ upgrader = Upgrader(options, commands, [])
+ upgrader.TIMESTAMP_FILE = '/dev/null'
+ upgrader.MOUNTPOINT_CMD = "true"
+ upgrader.run()
+
+ self.assertTrue(mock_sync_partitions.called)
+
+ @patch.object(Upgrader, 'sync_partitions')
+ @patch('ubuntucoreupgrader.upgrader.get_mount_details')
+ @patch('ubuntucoreupgrader.upgrader.remount')
+ @patch('ubuntucoreupgrader.upgrader.mount')
+ @patch('ubuntucoreupgrader.upgrader.fsck')
+ @patch('ubuntucoreupgrader.upgrader.mkfs')
+ @patch('ubuntucoreupgrader.upgrader.unmount')
+ def test_mount_unmount_with_format(self, mock_umount, mock_mkfs, mock_fsck,
+ mock_mount, mock_remount,
+ mock_mount_details,
+ mock_sync_partitions):
+ 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.
+ args = ['cmdfile']
+ options = parse_args(args=args)
+ commands = make_commands([self.TARFILE])
+
+ commands.insert(0, 'format system')
+ commands.insert(1, 'mount system')
+ commands.append('unmount system')
+
+ upgrader = Upgrader(options, commands, [])
+ upgrader.TIMESTAMP_FILE = '/dev/null'
+ upgrader.MOUNTPOINT_CMD = "true"
+ upgrader.run()
+
+ self.assertFalse(mock_sync_partitions.called)
+
 def test_empty_removed_file(self):
 root_dir = make_tmp_dir()
 
@@ -325,6 +386,44 @@ class UbuntuCoreUpgraderObectTestCase(UbuntuCoreUpgraderTestCase):
 
 shutil.rmtree(self.cache_dir)
 
+ def test_other_considered_empty(self):
+
+ si_config_file = '/etc/system-image/channel.ini'
+
+ cache_dir = make_tmp_dir()
+ 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.cache_dir = cache_dir
+
+ self.assertTrue(upgrader.other_considered_empty())
+
+ target = upgrader.get_mount_target()
+ channel_ini = os.path.normpath('{}/{}'.format(target,
+ si_config_file))
+ si_dir = os.path.dirname(channel_ini)
+
+ os.makedirs(si_dir)
+
+ self.assertTrue(upgrader.other_considered_empty())
+
+ touch_file(channel_ini)
+
+ self.assertTrue(upgrader.other_considered_empty())
+
+ create_file(channel_ini, "my size is >0")
+
+ self.assertFalse(upgrader.other_considered_empty())
+
+ os.remove(channel_ini)
+
+ self.assertTrue(upgrader.other_considered_empty())
+
+ shutil.rmtree(cache_dir)
 
 if __name__ == "__main__":
 unittest.main()
diff --git a/ubuntucoreupgrader/tests/utils.py b/ubuntucoreupgrader/tests/utils.py
index 31d3729..9297f32 100644
--- a/ubuntucoreupgrader/tests/utils.py
+++ b/ubuntucoreupgrader/tests/utils.py
@@ -50,10 +50,11 @@ def append_file(path, contents):
 os.makedirs(dirname, mode=TEST_DIR_MODE, exist_ok=True)
 
 with open(path, 'a') as fh:
- fh.writelines(contents)
+ if contents:
+ fh.writelines(contents)
 
- if not contents.endswith('\n'):
- fh.write('\n')
+ if not contents.endswith('\n'):
+ fh.write('\n')
 
 
 def create_file(path, contents):
diff --git a/ubuntucoreupgrader/upgrader.py b/ubuntucoreupgrader/upgrader.py
index 8e7ec23..2bf5681 100644
--- a/ubuntucoreupgrader/upgrader.py
+++ b/ubuntucoreupgrader/upgrader.py
@@ -52,19 +52,6 @@ log = logging.getLogger()
 
 DEFAULT_ROOT = '/'
 
-# Name of the writable user data partition label as created by
-# ubuntu-device-flash(1).
-WRITABLE_DATA_LABEL = 'writable'
-
-# name of primary root filesystem partition label as created by
-# ubuntu-device-flash(1).
-SYSTEM_DATA_A_LABEL = 'system-a'
-
-# name of primary root filesystem partition label as created by
-# ubuntu-device-flash(1). Note that this partition will
-# only be present if this is an A/B upgrade system.
-SYSTEM_DATA_B_LABEL = 'system-b'
-
 # The tar file contains a 'system/' directory with the new files
 # to apply to the real system. It may also contain a top-level
 # file called 'removed' that lists files relative to the system
@@ -237,6 +224,13 @@ def mkfs(device, fs_type, label):
 sys.exit(1)
 
 
+def touch_file(path):
+ '''
+ Create an empty file specified by path.
+ '''
+ open(path, 'w').close()
+
+
 def get_mount_details(target):
 '''
 Returns a list comprising device, filesystem type and filesystem
@@ -536,7 +530,7 @@ class Upgrader():
 completed successfully.
 '''
 file = os.path.join(self.cache_dir, self.TIMESTAMP_FILE)
- open(file, 'w').close()
+ touch_file(file)
 
 def get_mount_target(self):
 '''
@@ -665,6 +659,10 @@ class Upgrader():
 .format(target_type))
 return
 
+ if self.other_considered_empty():
+ log.debug('other root partition is empty')
+ self.other_is_empty = True
+
 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
@@ -680,7 +678,7 @@ class Upgrader():
 
 self.remount_rootfs(writable=True)
 
- if self.other_is_empty:
+ if self.other_is_empty and not self.full_image:
 # Copy current rootfs data to the other rootfs's blank
 # partition.
 log.debug('syncing root partitions')
@@ -708,12 +706,39 @@ class Upgrader():
 
 self.remount_rootfs(writable=False)
 
+ def other_considered_empty(self):
+ '''
+ Returns True if the other rootfs should be considered empty.
+
+ See sync_partitions() for further details.
+
+ '''
+ target = self.get_mount_target()
+ channel_ini = os.path.normpath('{}/{}'.format(target,
+ SYSTEM_IMAGE_CHANNEL_CONFIG))
+
+ try:
+ st = os.stat(channel_ini)
+ except OSError:
+ # ENOENT
+ return True
+
+ if st.st_size == 0:
+ # The config file is zero bytes long. This is used as a
+ # marker to denote that the initial "sync system-a to
+ # system-b" was started, but was interrupted, either in the
+ # copy phase or in the s-i unpack phase. Either way, the
+ # partition must be considered empty until the config file
+ # has a size > 0.
+ return True
+
+ return False
+
 def remount_rootfs(self, writable=False):
 target = self.get_mount_target()
 
 if writable:
- root = subprocess.check_output(
- ["findmnt", "-o", "SOURCE", "-n", target]).strip()
+ root, _, _ = get_mount_details(target)
 
 # ro->rw so need to fsck first.
 unmount(target)
@@ -722,14 +747,6 @@ class Upgrader():
 fsck(root)
 
 mount(root, target, "rw")
-
- channel_ini = '{}/{}'.format(target,
- SYSTEM_IMAGE_CHANNEL_CONFIG)
- channel_ini = os.path.normpath(channel_ini)
-
- if not os.path.exists(channel_ini):
- log.debug('root partition on {} is empty'.format(root))
- self.other_is_empty = True
 else:
 # rw->ro so no fsck required.
 remount(target, "ro")
@@ -886,6 +903,62 @@ class Upgrader():
 
 XXX: Assumes that the other rootfs is already mounted r/w to
 mountpoint get_mount_target().
+
+ Strategy: The partition sync is only called once (when the "other"
+ rootfs is empty), to make the empty partition identical to the
+ current rootfs partition, before unpacking the latest s-i updates
+ onto it.
+
+ Since there are 2 operations to perform for the initial update
+ (copy and unpack) and since the upgrader must be resilient to
+ power outages at any point when either of the operations are
+ running, we need a way for the upgrader to detect if a previous
+ invocation failed. This is achieved by treating the s-i
+ configuration file (SYSTEM_IMAGE_CHANNEL_CONFIG) in a special
+ way.
+
+ = Background =
+
+ A rootfs is not considered complete until the file
+ SYSTEM_IMAGE_CHANNEL_CONFIG has been unpacked onto it. However,
+ that implies we cannot copy the original
+ SYSTEM_IMAGE_CHANNEL_CONFIG from the current rootfs to "other"
+ as the copy is only the first operation and we don't want that
+ "partial update" to be considered as valid.
+
+ Ideally, we would perform the copy, excluding
+ just SYSTEM_IMAGE_CHANNEL_CONFIG. However, rsync (which has
+ excellent support for copying files with exclusions) is not
+ available.
+
+ = Approach =
+
+ The current approach is:
+
+ 1) Create the directory part of SYSTEM_IMAGE_CHANNEL_CONFIG
+ on "other".
+
+ 2) Touch SYSTEM_IMAGE_CHANNEL_CONFIG as an empty file on
+ "other".
+
+ 3) Perform an "update copy" from current to "other". Note
+ that this will not modify the pre-existing
+ SYSTEM_IMAGE_CHANNEL_CONFIG, but will conveniently correct
+ the permissions on /etc/system-image/ to match those on
+ "current" (in the case that the original permissions used
+ by this function do not match those on "current").
+
+ The second part of the operation is to unpack the s-i update onto
+ "other" (the final part of which is to unpack
+ SYSTEM_IMAGE_CHANNEL_CONFIG (which will be >0 bytes), thus
+ making that rootfs complete and valid).
+
+ = Special treatment of SYSTEM_IMAGE_CHANNEL_CONFIG =
+
+ Using this approach makes it easy to determine if the initial
+ update failed since if either SYSTEM_IMAGE_CHANNEL_CONFIG does
+ not exist or is only zero bytes, the s-i unpack must have failed
+ meaning the rootfs must be considered "empty".
 '''
 target = self.get_mount_target()
 bindmount_rootfs_dir = tempfile.mkdtemp(prefix=script_name,
@@ -895,8 +968,13 @@ class Upgrader():
 cwd = os.getcwd()
 os.chdir(bindmount_rootfs_dir)
 
- cmd = '/bin/cp'
- args = [cmd, '-a', '.', target]
+ channel_ini = os.path.normpath('{}/{}'.format(target,
+ SYSTEM_IMAGE_CHANNEL_CONFIG))
+ si_dir = os.path.dirname(channel_ini)
+ os.makedirs(si_dir)
+ touch_file(channel_ini)
+
+ args = ['/bin/cp', '-au', '.', target]
 
 log.debug('running (from directory {}): {}'
 .format(bindmount_rootfs_dir, args))