Merge lp:~brian-murray/update-manager/phased-update-source-packages into lp:update-manager

[Brian Murray]

As discussed we should be handling the phased update percentage for binary packages in their source package groups so that we don't end up in a situations where binary dependent packages are not installed or uninstallable due to the clients phasing luck.

[Steve Langasek (vorlon)]

> self.random.seed("%s-%s-%s" % (
> pkg.candidate.source_name, pkg.candidate.version,
> self.machine_uniq_id))

There's a buglet here; in some cases the binary package versions for packages from the same source package may not be the same, causing the binaries to be phased differently still. (Example: util-linux vs. bsdutils). I think this is probably ignorable for now, though.

+ if not is_security_update:
+ if src_name not in ignored_phased_updates:
+ if self._is_ignored_phased_update(pkg):
+ ignored_phased_updates.append(src_name)
+ continue
+ elif src_name in ignored_phased_updates:
+ continue

So if the source package has already been marked as to-be-ignored, we don't check again; but if a previous binary from the source package has been seen and is *not* to be ignored, we do check again. Fortunately we'll get the same result either way, so checking again doesn't hurt us, but it seems inconsistent. We could simply do this as:

+ if not is_security_update:
+ if self._is_ignored_phased_update(pkg):
+ continue

and that should have the same effect.

It would, however, cause your test case to fail since your test case returns '50' as the random value for all inputs, and your test data has two different phased values for zsh vs. zsh-dev. I guess a more 'realistic' reproducible test case would check that the same random value is returned for both binary packages.

[lp:~brian-murray/update-manager/phased-update-source-packages]

2623. By Brian Murray on 2013-05-17

modifications based upon reviewer feedback

[Brian Murray (brian-murray)]

I've updated my branch based upon your feedback and created a new test case.

=== modified file 'UpdateManager/Core/UpdateList.py'
--- UpdateManager/Core/UpdateList.py	2013-05-01 05:35:33 +0000
+++ UpdateManager/Core/UpdateList.py	2013-05-17 23:20:31 +0000
@@ -205,6 +205,7 @@
         self.security_groups = []
         self.num_updates = 0
         self.random = random.Random()
+        self.ignored_phased_updates = []
         # a stable machine uniq id
         with open(self.UNIQ_MACHINE_ID_FILE) as f:
             self.machine_uniq_id = f.read()
@@ -357,7 +358,7 @@
             # multiple runs of the update-manager, so we need to
             # feed a seed that is a combination of the pkg/ver/machine
             self.random.seed("%s-%s-%s" % (
-                pkg.name, pkg.candidate.version,
+                pkg.candidate.source_name, pkg.candidate.version,
                 self.machine_uniq_id))
             threshold = pkg.candidate.record[self.PHASED_UPDATES_KEY]
             percentage = self.random.randint(0, 100)
@@ -456,10 +457,14 @@
                     continue
 
                 # see if its a phased update and *not* a security update
+                # keep track of packages for which the update percentage was
+                # not met for testing
                 is_security_update = self._is_security_update(pkg)
-                if (not is_security_update and
-                        self._is_ignored_phased_update(pkg)):
-                    continue
+                src_name = pkg.candidate.source_name
+                if not is_security_update:
+                    if self._is_ignored_phased_update(pkg):
+                        self.ignored_phased_updates.append(pkg)
+                        continue
 
                 if is_security_update:
                     security_pkgs.append(pkg)

=== modified file 'debian/changelog'
--- debian/changelog	2013-05-13 10:04:53 +0000
+++ debian/changelog	2013-05-17 23:20:31 +0000
@@ -3,10 +3,17 @@
   [ Sami Jaktholm ]
   * Greatly speed up update calculation (LP: #1167277)
 
+<<<<<<< TREE
   [ Martin Pitt ]
   * Drop unnecessary ubuntu-drivers-common build dependency, to ease
     backporting.
 
+=======
+  [ Brian Murray ]
+  * Modify phased update percentage to use source packages and not binary
+    packages, additionally add a test for this.
+
+>>>>>>> MERGE-SOURCE
  -- Michael Terry <[e-mail]>  Wed, 01 May 2013 14:32:43 -0700
 
 update-manager (1:0.186) raring; urgency=low

=== modified file 'tests/aptroot-update-list-test/var/lib/apt/lists/archive.ubuntu.com_ubuntu_dists_lucid_main_binary-amd64_Packages'
--- tests/aptroot-update-list-test/var/lib/apt/lists/archive.ubuntu.com_ubuntu_dists_lucid_main_binary-amd64_Packages	2012-08-20 15:42:16 +0000
+++ tests/aptroot-update-list-test/var/lib/apt/lists/archive.ubuntu.com_ubuntu_dists_lucid_main_binary-amd64_Packages	2013-05-17 23:20:31 +0000
@@ -60,3 +60,33 @@
 Supported: 5y
 Phased-Update-Percentage: 10
 
+Package: zsh-dev
+Priority: optional
+Section: libdevel
+Installed-Size: 348
+Maintainer: Ubuntu Developers <[e-mail]>
+Original-Maintainer: Clint Adams <[e-mail]>
+Architecture: amd64
+Source: zsh
+Version: 4.3.10-5ubuntu3
+Filename: pool/main/z/zsh/zsh-dev_4.3.10-5ubuntu3_amd64.deb
+Size: 79682
+MD5sum: 307bcd1994a2eb0d01cc06856a75fb52
+SHA1: 9e8d41855b62ba018341882099e3849e550aa55c
+SHA256: 5c615269c6a41ed44923edc696ab77c5c4e4b9c0ece17c0e3d767db80bfb65cc
+Description: A shell with lots of features (development files)
+ Zsh is a UNIX command interpreter (shell) usable as an
+ interactive login shell and as a shell script command
+ processor. Of the standard shells, zsh most closely resembles
+ ksh but includes many enhancements. Zsh has command-line editing,
+ built-in spelling correction, programmable command completion,
+ shell functions (with autoloading), a history mechanism, and a
+ host of other features.
+ .
+ This package contains headers and scripts necessary to compile
+ third-party modules.
+Homepage: http://www.zsh.org/
+Bugs: https://bugs.launchpad.net/ubuntu/+filebug
+Origin: Ubuntu
+Supported: 18m
+Phased-Update-Percentage: 10

=== modified file 'tests/aptroot-update-list-test/var/lib/dpkg/status'
--- tests/aptroot-update-list-test/var/lib/dpkg/status	2013-01-16 12:35:39 +0000
+++ tests/aptroot-update-list-test/var/lib/dpkg/status	2013-05-17 23:20:31 +0000
@@ -19,3 +19,14 @@
 Phased-Update-Percentage: 10
 Description: a example package with a phased updates percentage of 10%
 
+Package: zsh-dev
+Status: install ok installed
+Priority: important
+Section: admin
+Installed-Size: 3005
+Maintainer: Ubuntu Developers <[e-mail]>
+Architecture: all
+Version: 0.1
+Phased-Update-Percentage: 10
+Description: a second binary example package with a phased updates percentage of 10%
+

=== modified file 'tests/test_update_list.py'
--- tests/test_update_list.py	2013-02-22 05:47:27 +0000
+++ tests/test_update_list.py	2013-05-17 23:20:31 +0000
@@ -46,7 +46,16 @@
         with patch.object(self.updates_list.random, "randint") as mock_randint:
             mock_randint.return_value = 1
             self.updates_list.update(self.cache)
-            self.assertUpdatesListLen(2)
+            self.assertUpdatesListLen(3)
+
+    def test_second_phased_binary_not_included(self):
+        """ Test that there is no overlap between the source packages of the
+            packages being ignored and installed """
+        self.updates_list.update(self.cache)
+        ignored_srcs = set([pkg.candidate.source_name for pkg in self.updates_list.ignored_phased_updates])
+        group = self.updates_list.update_groups[0]
+        install_srcs = set([x.pkg.candidate.source_name for x in group.items])
+        self.assertTrue(len(ignored_srcs & install_srcs) == 0)
 
     def test_phased_percentage_included_via_force(self):
         """ Test that the "always" override config works """
@@ -59,7 +68,7 @@
         with patch.object(self.updates_list.random, "randint") as mock_randint:
             mock_randint.return_value = 100
             self.updates_list.update(self.cache)
-            self.assertUpdatesListLen(2)
+            self.assertUpdatesListLen(3)
 
     def test_phased_percentage_excluded_via_force(self):
         """ Test that the "never" override config works """
@@ -82,7 +91,7 @@
         with patch.object(self.updates_list.random, "randint") as mock_randint:
             mock_randint.return_value = 100
             self.updates_list.update(self.cache)
-            self.assertUpdatesListLen(2)
+            self.assertUpdatesListLen(3)
 
 
 class GroupingTestCase(unittest.TestCase):