Merge ~lamoura/ubuntu/+source/unattended-upgrades:md5sum-fix into ubuntu/+source/unattended-upgrades:ubuntu/xenial-devel

[Lucas Albuquerque Medeiros de Moura]

[Lucas Albuquerque Medeiros de Moura (lamoura)]

Currently, when we upgrade from trusty to xenial, we get a prompt stating that there is a diff on 50unattended-upgrades. The reason for that is because ucf has no visibility of the supported 50unattended-upgrades files. We are now adding the data/50unattended-upgrades.md5sum file to the xenial package. This file is already being used on later version of unattended-upgrades, but not on xenial.

Additionally, we are also adding the md5sum of the xenial 50unattended-upgrades file into that history file as well. Which we are also adding to the current package version as well:
https://github.com/mvo5/unattended-upgrades/pull/288

Also, we already have a ppa with that package built into it:
https://launchpad.net/~lamoura/+archive/ubuntu/uaclient-test/

[Chad Smith (chad.smith)]

+1 on this changest. Your PR adding the missing md5sum @ https://github.com/mvo5/unattended-upgrades/pull/288 already landed in master (and was already published into hirsute)

Also this changeset only officially affects Xenial as that md5sum only affected trusty -> xenial upgrade path (not xenial -> bionic or bionic -> focal). So, I think this changeset doesn't *need* to be in Bionic or Focal.

[Chad Smith (chad.smith)]

review: Approve

[Lucas Albuquerque Medeiros de Moura (lamoura)]

autopkgtest result:

I have used the following ppa to get the necessary files:
https://launchpad.net/~lamoura/+archive/ubuntu/unattended-upgrades-ppa

I have locally ran autopkgtest with the following command:
autopkgtest -o autopkgtest-unattended-upgrades -U unattended-upgrades_1.1ubuntu1.18.04.7~16.04.7.dsc -- qemu autopkgtest-xenial-amd64.img

[Chad Smith (chad.smith)]

Lucas passed me his unattended-upgrades autopkgtest run which took 2+ hours to run. It's too long to attach as a comment and too long to pastebin :(
https://private-fileshare.canonical.com/~csmith/unattended-upgrades-xenial-autopkgtest.log

[Bryce Harrington (bryce)]

Thanks for the autopkgtest results and PPA. I skimmed through the test results and verified there are no errors. Given this is going in as an SRU, there will be additional testing to come.

The version number and changelog entry look ok, but since this is going in as an SRU this needs to have a bug report associated with it for holding the SRU info. The bug # it currently references, LP: #1813346, is already closed and was used for an SRU of cloud-init. Sometimes it's ok to reuse an SRU for one package on an SRU of another package, but in this case it'll be easier for SRU reviewers (and thus more likely to make it through SRU quickly) if you create a new bug report for this.

If you've not filed SRUs before, https://wiki.ubuntu.com/StableReleaseUpdates is the authoritative procedure, however you can probably just cut and paste the comments from this MP since they cover 80% of what's needed in an SRU bug's description. The other bits that'll need added are a "[Test Plan]" section, and a "[Where problems could occur]" section.

Everything else looks ok to me, so once you've updated this branch with that bug #, I can go ahead and upload it.

review: Needs Fixing

[Lucas Albuquerque Medeiros de Moura (lamoura)]

Bryce, I am sorry for that. I have an SRU bug already filled for unattended-upgrades, but I was using the cloud-init one as base template and I used on the changelog by accident :(

Here is the unattended-upgrades SRU bug:
https://bugs.launchpad.net/ubuntu/+source/unattended-upgrades/+bug/1915547

And I have updated the code to use it. My mistake here :(

[Bryce Harrington (bryce)]

Cool, thanks. No apology necessary - this is why we do reviews, and was just glad to have found at least something. ;-)

I have some recommendations on the SRU text wording, but that's separate from the package upload so will give that feedback later. The package looks good, I've tagged and uploaded to xenial:

$ dput ubuntu *.changes
Checking signature on .changes
gpg: /home/bryce/pkg/UnattendedUpgrades/review-mp397867/unattended-upgrades_1.1ubuntu1.18.04.7~16.04.7_source.changes: Valid signature from E603B2578FB8F0FB
Checking signature on .dsc
gpg: /home/bryce/pkg/UnattendedUpgrades/review-mp397867/unattended-upgrades_1.1ubuntu1.18.04.7~16.04.7.dsc: Valid signature from E603B2578FB8F0FB
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading unattended-upgrades_1.1ubuntu1.18.04.7~16.04.7.dsc: done.
  Uploading unattended-upgrades_1.1ubuntu1.18.04.7~16.04.7.tar.xz: done.
  Uploading unattended-upgrades_1.1ubuntu1.18.04.7~16.04.7_source.changes: done.
Successfully uploaded packages.
$ git ubuntu tag --upload
$ git push pkg upload/1.1ubuntu1.18.04.7_16.04.7
Enumerating objects: 24, done.
Counting objects: 100% (24/24), done.
Delta compression using up to 12 threads
Compressing objects: 100% (17/17), done.
Writing objects: 100% (18/18), 4.52 KiB | 771.00 KiB/s, done.
Total 18 (delta 12), reused 2 (delta 1)
To ssh://git.launchpad.net/ubuntu/+source/unattended-upgrades
 * [new tag] upload/1.1ubuntu1.18.04.7_16.04.7 -> upload/1.1ubuntu1.18.04.7_16.04.7

review: Approve

[Bryce Harrington (bryce)]

The SRU bug you've written is good, and probably adequate to get this accepted into the release, however I have some tips in what to say to make the SRU sail through review more easily:

1. The [Impact] section you've written is a great analysis of the problem and its solution, but for impact I think what the SRU team is looking for is more like, "What is the pain end-users experience due to this issue, and how bad is it?" Often this section is just 2-3 sentences. I think in this case you could indicate that "unattended-upgrades by definition needs to work without user interaction, and prompting about the md5 sum mismatch can lead to failed upgrades that leave the apt system in an inconsistent state that can prevent future upgrades." Or something like that.

The rest of your test in Impact could be left there, or moved to Discussion. It's a great technical summary of the problem.

2. The [Test Case] section is less about you reporting how you've tested so far, and more about how someone reviewing the SRU can test and verify the fix themselves. So, like, imagine you're explaining to me (or someone with only cursory knowledge of unattended-upgrades) how to reproduce the problem, and then see how your package fixes it. E.g. launch an lxc container, install the current version of ..., then do whatever you do to kick unattended-upgrades to run, and then note in the apt log (or stdout or whereever) the prompt about the config change. The person validating the fix can then install the PPA and verify it does not exhibit the issue.

3. One would think the [Regression Potential] should be where to say, "This has a low potential of regression," but actually what the SRU team wants to see is more a discussion about, if there *was* a regression caused by this update, what might one look for? The SRU policy recently renamed this section to [Where problems could occur], and clarified what they're looking for; see
https://wiki.ubuntu.com/StableReleaseUpdates, '[Where problems could occur]'.

For this bug, I think you'd want to mention that the changes for this bug are limited to behavior during the upgrade process itself, and really only deals with the md5-checking functionality, so if there were a regression it'd likely be limited to that portion of apt's behavior.

For reference, here's a selection of past SRU's for unattended-upgrades that appear to have been accepted: LP: #1806487, LP: #1269177, LP: #1446552, LP: #1602536, LP: #1615381. That last one has a particularly good [Where problems could occur] section, and a good example of a succinct [Impact]. Some of the others have better examples of more detailed test cases.

Hope this helps!

[Lucas Albuquerque Medeiros de Moura (lamoura)]

Thank you for this context Bryce :)

It makes complete sense all the points that you made and I have updated the SRU with the information that was missing

[Lucas Albuquerque Medeiros de Moura (lamoura)]

Bryce, I have updated this MR following Robie comments on the SRU bug:
https://bugs.launchpad.net/ubuntu/+source/unattended-upgrades/+bug/1915547

We are now only shipping the md5sum for both Trusty and Xenial latest 50unattended-upgrades config.

I have uploaded the new package into this ppa:
https://launchpad.net/~lamoura/+archive/ubuntu/unattended-upgrades-ppa/

Please let me know If you think I should test this new change further

[Bryce Harrington (bryce)]

Changes look good, I've uploaded the updated branch:

$ dput -f ubuntu unattended-upgrades_1.1ubuntu1.18.04.7~16.04.7_source.changes
Checking signature on .changes
gpg: /home/bryce/pkg/UnattendedUpgrades/review-mp397867/unattended-upgrades_1.1ubuntu1.18.04.7~16.04.7_source.changes: Valid signature from E603B2578FB8F0FB
Checking signature on .dsc
gpg: /home/bryce/pkg/UnattendedUpgrades/review-mp397867/unattended-upgrades_1.1ubuntu1.18.04.7~16.04.7.dsc: Valid signature from E603B2578FB8F0FB
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading unattended-upgrades_1.1ubuntu1.18.04.7~16.04.7.dsc: done.
  Uploading unattended-upgrades_1.1ubuntu1.18.04.7~16.04.7.tar.xz: done.
  Uploading unattended-upgrades_1.1ubuntu1.18.04.7~16.04.7_source.changes: done.
Successfully uploaded packages.

$ git ubuntu tag --upload -f
Updated tag 'upload/1.1ubuntu1.18.04.7_16.04.7' (was 140665e)

$ git push -f pkg upload/1.1ubuntu1.18.04.7_16.04.7
Enumerating objects: 20, done.
Counting objects: 100% (20/20), done.
Delta compression using up to 12 threads
Compressing objects: 100% (14/14), done.
Writing objects: 100% (14/14), 2.06 KiB | 351.00 KiB/s, done.
Total 14 (delta 9), reused 0 (delta 0)
To ssh://git.launchpad.net/ubuntu/+source/unattended-upgrades
 + 140665e...e75398b upload/1.1ubuntu1.18.04.7_16.04.7 -> upload/1.1ubuntu1.18.04.7_16.04.7 (forced update)

diff --git a/data/50unattended-upgrades.md5sum b/data/50unattended-upgrades.md5sum
new file mode 100644
index 0000000..10b37ad
--- /dev/null
+++ b/data/50unattended-upgrades.md5sum
@@ -0,0 +1,2 @@
+3ad71443bccc1acc9cf4335668ef024f
+404d67afd60e7b8b11f08b1360da219b
diff --git a/debian/changelog b/debian/changelog
index ed7f777..c8dce5d 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,14 @@
+unattended-upgrades (1.1ubuntu1.18.04.7~16.04.7) xenial; urgency=medium
+
+  * data: add md5sum history file on the data folder (LP: #1915547)
+    - This file contains the md5sum of the current xenial and trusty
+      version of 50unattended-upgrades. This file will be used to avoid
+      prompting users about unattended-upgrade changes when upgrading
+      from trusty to xenial.
+  * debian/postint: make ucf command reference the md5sum history file
+
+ -- Lucas Moura <[e-mail]>  Wed, 10 Feb 2021 17:37:18 -0300
+
 unattended-upgrades (1.1ubuntu1.18.04.7~16.04.6) xenial; urgency=medium
 
   * data/50unattended-upgrades.Ubuntu: add new ESM repositories (LP: #1857051)
diff --git a/debian/postinst b/debian/postinst
index ba3e1ad..eac45f5 100644
--- a/debian/postinst
+++ b/debian/postinst
@@ -47,9 +47,10 @@ case "$1" in
 
 	CONFIG="/etc/apt/apt.conf.d/50unattended-upgrades"
 	NEWFILE="$CONFIG.ucftmp"
+	MD5SUM_FILE="/usr/share/unattended-upgrades/50unattended-upgrades.md5sum"
 	TEMPLATE="/usr/share/unattended-upgrades/50unattended-upgrades"
 	cp -a -f "$TEMPLATE" "$NEWFILE"
-	ucf --three-way --debconf-ok "$NEWFILE" "$CONFIG"
+	ucf --three-way --debconf-ok --sum-file "$MD5SUM_FILE" "$NEWFILE" "$CONFIG"
 	ucfr unattended-upgrades "$CONFIG"
 	rm -f "$NEWFILE"
 
diff --git a/setup.py b/setup.py
index 38a4b64..932bf01 100755
--- a/setup.py
+++ b/setup.py
@@ -21,6 +21,7 @@ if __name__ == "__main__":
                 ["data/20auto-upgrades",
                  "data/20auto-upgrades-disabled",
                  "data/50unattended-upgrades",
+                 "data/50unattended-upgrades.md5sum",
                  "unattended-upgrade-shutdown",
                  "data/update-motd-unattended-upgrades"]),
             ('../usr/share/man/man8/',