Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release 31.2 #2964

Merged
merged 6 commits into from
Mar 5, 2024
Merged

Release 31.2 #2964

merged 6 commits into from
Mar 5, 2024

Conversation

orndorffgrant
Copy link
Collaborator

@orndorffgrant orndorffgrant commented Feb 26, 2024

Why is this needed?

LP: #2055046

Test Steps

New behave test should pass

@orndorffgrant orndorffgrant force-pushed the release-31.2 branch 3 times, most recently from 89c9713 to 2463ff5 Compare February 26, 2024 20:34
Copy link
Contributor

@CalvoM CalvoM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The integration tests pass. LGTM. The failures are not related to the change.

@orndorffgrant orndorffgrant changed the title debian: rename logrotate file to new package name Release 31.2 Feb 27, 2024
@basak
Copy link
Contributor

basak commented Feb 27, 2024

Looks good. I guess this slipped through testing though? Is there anything else that we might have missed that this new wisdom might allow us to spot, or anything else that in hindsight we should test now that we are wiser?

$ grep-status -P ubuntu-advantage-tools -s Conffiles
Conffiles: 
 /etc/apt/apt.conf.d/20apt-esm-hook.conf 43c92b89ef81645065c6277ba90529c5
 /etc/apt/preferences.d/ubuntu-pro-esm-apps 30b368fbe91a2fcbefd509a5ca0dead4
 /etc/apt/preferences.d/ubuntu-pro-esm-infra 39f77c64b5c996572ccb8313f4ad8471
 /etc/logrotate.d/ubuntu-advantage-tools 852ec664fd0f1ecb26e726f5e801e9fd
 /etc/ubuntu-advantage/uaclient.conf f84a9c135c3d556856f9ab6d2751e365
 /etc/update-manager/release-upgrades.d/ubuntu-advantage-upgrades.cfg 82b5d8872cdcde8d3a0f68112de703fa
 /etc/update-motd.d/91-contract-ua-esm-status 6346b049bc070e0519c621de0f636bb5

There's nothing else that's being touched there as a result of this change, right?

@paride
Copy link
Contributor

paride commented Feb 27, 2024

This LGTM, pity that we didn't spot this. This could maybe have been easily spotted by checking what systemctl is-system-running returns. With a failing unit that should print degraded and return 1.

orndorffgrant and others added 5 commits February 27, 2024 13:37
This follows the python versions available in different Ubuntu releases.
This is a weak mapping though, and may need to be improved in the
future.

Signed-off-by: Renan Rodrigo <renanrodrigo@canonical.com>
@orndorffgrant
Copy link
Collaborator Author

Thank you for taking a look @basak and @paride!

Yes we missed this. It only occurs when logrotate runs, which is on a daily timer by default, so we wouldn't have noticed it with a generic test for system status. We do have a test that runs logrotate specifically, but it specifies our expected logrotate configuration file instead of using the system conf, so it didn't catch that the old conffile was left behind. I've updated our logrotate test in this PR to use the system logrotate conf, which reproduces this bug.

I'm looking at all our conffiles and not sure if any other changes are needed.
pro version 30:

root@test:~# dpkg-query --showformat='${Conffiles}' --show ubuntu-advantage-tools
 /etc/apt/apt.conf.d/20apt-esm-hook.conf 43c92b89ef81645065c6277ba90529c5
 /etc/apt/preferences.d/ubuntu-pro-esm-apps 30b368fbe91a2fcbefd509a5ca0dead4
 /etc/apt/preferences.d/ubuntu-pro-esm-infra 39f77c64b5c996572ccb8313f4ad8471
 /etc/logrotate.d/ubuntu-advantage-tools 852ec664fd0f1ecb26e726f5e801e9fd
 /etc/ubuntu-advantage/uaclient.conf f84a9c135c3d556856f9ab6d2751e365
 /etc/update-manager/release-upgrades.d/ubuntu-advantage-upgrades.cfg 82b5d8872cdcde8d3a0f68112de703fa
 /etc/update-motd.d/91-contract-ua-esm-status 6346b049bc070e0519c621de0f636bb5

After upgrade to 31.2 from this PR:

root@ua-jammy-3d241ba1bea5b37f0de162ecc4dec183:~# dpkg-query --showformat='${Conffiles}' --show ubuntu-advantage-tools
 /etc/update-motd.d/91-contract-ua-esm-status 6346b049bc070e0519c621de0f636bb5 obsolete
 /etc/update-manager/release-upgrades.d/ubuntu-advantage-upgrades.cfg 82b5d8872cdcde8d3a0f68112de703fa obsolete
 /etc/ubuntu-advantage/uaclient.conf f84a9c135c3d556856f9ab6d2751e365 obsolete
 /etc/apt/preferences.d/ubuntu-pro-esm-infra 39f77c64b5c996572ccb8313f4ad8471 obsolete
 /etc/apt/preferences.d/ubuntu-pro-esm-apps 30b368fbe91a2fcbefd509a5ca0dead4 obsolete
 /etc/apt/apt.conf.d/20apt-esm-hook.conf 43c92b89ef81645065c6277ba90529c5 obsolete

root@ua-jammy-3d241ba1bea5b37f0de162ecc4dec183:~# dpkg-query --showformat='${Conffiles}' --show ubuntu-pro-client
 /etc/apparmor.d/ubuntu_pro_apt_news bee709c28273d4723e6444d2d0374ce5
 /etc/apt/apt.conf.d/20apt-esm-hook.conf 43c92b89ef81645065c6277ba90529c5
 /etc/apt/preferences.d/ubuntu-pro-esm-apps 7e0d0e65835c9eda745c4a6d81e996e3
 /etc/apt/preferences.d/ubuntu-pro-esm-infra 4932ac7913fff2dadd94b9e832f636c8
 /etc/logrotate.d/ubuntu-pro-client 852ec664fd0f1ecb26e726f5e801e9fd
 /etc/ubuntu-advantage/uaclient.conf f84a9c135c3d556856f9ab6d2751e365
 /etc/update-manager/release-upgrades.d/ubuntu-advantage-upgrades.cfg 5e14da4979c6c68cef22c0c09f5d8793
 /etc/update-motd.d/91-contract-ua-esm-status 6346b049bc070e0519c621de0f636bb5

I'm not sure exactly what obsolete indicates: is it a problem or the expected correct state?

Notably, /etc/apt/preferences.d/ubuntu-pro-esm-apps, /etc/apt/preferences.d/ubuntu-pro-esm-infra, and /etc/update-manager/release-upgrades.d/ubuntu-advantage-upgrades.cfg have all changed. The release upgrades change is adding a new script, but the esm pinning changes are only in the comment (updating "ubuntu-advantage-tools" to "ubuntu-pro-client").

grant@lemur ~/c/w/ubuntu-pro-client (release-31.2)> git diff 30 preferences.d/ release-upgrades.d/
diff --git a/preferences.d/ubuntu-pro-esm-apps b/preferences.d/ubuntu-pro-esm-apps
index 5ded6f8f..ccdff4be 100644
--- a/preferences.d/ubuntu-pro-esm-apps
+++ b/preferences.d/ubuntu-pro-esm-apps
@@ -1,4 +1,4 @@
-# This file is used by Ubuntu Pro and supplied by the ubuntu-advantage-tools
+# This file is used by Ubuntu Pro and supplied by the ubuntu-pro-client
 # package. It has no effect if Ubuntu Pro services are not in use since no
 # other apt repositories are expected to match o=UbuntuESMApps.
 #
diff --git a/preferences.d/ubuntu-pro-esm-infra b/preferences.d/ubuntu-pro-esm-infra
index e956e0ae..125b31d0 100644
--- a/preferences.d/ubuntu-pro-esm-infra
+++ b/preferences.d/ubuntu-pro-esm-infra
@@ -1,4 +1,4 @@
-# This file is used by Ubuntu Pro and supplied by the ubuntu-advantage-tools
+# This file is used by Ubuntu Pro and supplied by the ubuntu-pro-client
 # package. It has no effect if Ubuntu Pro services are not in use since no
 # other apt repositories are expected to match o=UbuntuESM.
 #
diff --git a/release-upgrades.d/ubuntu-advantage-upgrades.cfg b/release-upgrades.d/ubuntu-advantage-upgrades.cfg
index c7da279a..c811ae03 100644
--- a/release-upgrades.d/ubuntu-advantage-upgrades.cfg
+++ b/release-upgrades.d/ubuntu-advantage-upgrades.cfg
@@ -1,4 +1,4 @@
 [Sources]
 Pockets=security,updates,proposed,backports,infra-security,infra-updates,apps-security,apps-updates
 [Distro]
-PostInstallScripts=./xorg_fix_proprietary.py, /usr/lib/ubuntu-advantage/upgrade_lts_contract.py
+PostInstallScripts=./xorg_fix_proprietary.py, /usr/lib/ubuntu-advantage/convert_list_to_deb822.py, /usr/lib/ubuntu-advantage/upgrade_lts_contract.py

I started over with v30 of u-a-t and I modified all of the conffiles in u-a-t v30 prior to upgrade to see what would happen. During upgrade, I got prompted for /etc/apt/preferences.d/ubuntu-pro-esm-apps, /etc/apt/preferences.d/ubuntu-pro-esm-infra, and /etc/update-manager/release-upgrades.d/ubuntu-advantage-upgrades.cfg. I think that may be appropriate and makes sense for ubuntu-advantage-upgrades.cfg, but undesirable for the esm pinning files since the change to the default file is trivial and inconsequential. I'm thinking about changing "ubuntu-pro-client" back to "ubuntu-advantage-tools" in the comments in the ESM pinning files to avoid this - do you agree @basak @paride ?

We could also potentially avoid changes to ubuntu-advantage-upgrades.cfg by moving the new python code into the existing script if we think it is worth avoiding changing the conffile (CC @renanrodrigo).

Changes were preserved for all other files. I also got the message:

Preserving user changes to /etc/logrotate.d/ubuntu-pro-client (renamed from /etc/logrotate.d/ubuntu-advantage-tools)...

And /etc/logrotate.d/ubuntu-pro-client.dpkg-new was left there which has the unchanged conffile installed by ubuntu-pro-client.

So if the obsolete status is expected, then I think the rest of the conffiles are being handled appropriately.

Aside: during upgrade we get these warnings. It is correct behavior to not delete these, but are the warnings avoidable?

Unpacking ubuntu-advantage-tools (31.1~22.04) over (30~22.04) ...
dpkg: warning: unable to delete old directory '/var/lib/ubuntu-advantage': Directory not empty
dpkg: warning: unable to delete old directory '/etc/ubuntu-advantage': Directory not empty

@orndorffgrant
Copy link
Collaborator Author

Following up here after an in person discussion and further experimentation.

I'm not sure exactly what obsolete indicates: is it a problem or the expected correct state?

We discussed this and decided it was unideal but not all that bad. I've added an additional commit that uses mv_conffile to remove this obsolete status in the happy case. Commit message copied here for convenience:

maintscripts: mv_conffile all conffiles in old package

This commit uses mv_conffile on all of the conffiles in the last version
of ubuntu-advantage-tools. It does not actually move them (the source and
destination are the same), but this removes the "obsolete" conffile
status from the ubuntu-advantage-tools package after upgrade. All of
these conffiles are listed correctly for ubuntu-pro-client after
upgrade.

The exception is if a user had modified one of these conffiles. In that
case, the changes are preserved, but the file is still listed as
"obsolete" for the ubuntu-advantage-tools package after upgrade.

I'm thinking about changing "ubuntu-pro-client" back to "ubuntu-advantage-tools" in the comments in the ESM pinning files to avoid this
...
We could also potentially avoid changes to ubuntu-advantage-upgrades.cfg by moving the new python code into the existing script if we think it is worth avoiding changing the conffile.

We decided these conffile changes were reasonable and prompts were okay if the user changed those files.

Aside: during upgrade we get these warnings.

We decided those are fine as well.

So in total, I think the latest commit I just pushed here helps a bit, and we are good to try another upload of 31.2 @paride and @basak - Let me know if you disagree or have additional findings.

This commit uses mv_conffile on all of the conffiles in the last version
of ubuntu-advantage-tools. It does not actually move them (the source and
destination are the same), but this removes the "obsolete" conffile
status from the ubuntu-advantage-tools package after upgrade. All of
these conffiles are listed correctly for ubuntu-pro-client after
upgrade.

The exception is if a user had modified one of these conffiles. In that
case, the changes are preserved, but the file is still listed as
"obsolete" for the ubuntu-advantage-tools package after upgrade.
@orndorffgrant
Copy link
Collaborator Author

orndorffgrant commented Feb 29, 2024

The tests that failed in the most recent CI run passed in previous runs and are only failing from timeouts from the security api

@paride
Copy link
Contributor

paride commented Feb 29, 2024

I uploaded to Noble:

Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading ubuntu-advantage-tools_31.2.dsc: done.
  Uploading ubuntu-advantage-tools_31.2.tar.xz: done.    
  Uploading ubuntu-advantage-tools_31.2_source.buildinfo: done.
  Uploading ubuntu-advantage-tools_31.2_source.changes: done.
Successfully uploaded packages.

however the SRU branches need fixing (they all target Xenial).

@orndorffgrant orndorffgrant merged commit 8f945f4 into release Mar 5, 2024
22 of 23 checks passed
@orndorffgrant orndorffgrant deleted the release-31.2 branch March 5, 2024 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants