Binary package rename (SC-1623) #2887

orndorffgrant · 2023-12-11T18:17:28Z

Why is this needed?

This PR renames the binary packages created from ubuntu-advantage-tools as agreed. The new package names are "ubuntu-pro-client" and "ubuntu-pro-image-auto-attach".

This PR is packaging heavy and deserves extra scrutiny there.

This PR also adjusts our tests to use the new packages.

A subsequent PR will also include a manual test script for testing the transition

Test Steps

Test adding this ppa and upgrading: ppa:orndorffgrant/proclient-rename-test

A test that can only be done with debs and not with a proper apt source of the full ubuntu-advantage-tools set of binaries:

launch a lxd container
copy over just ubuntu-pro-client.deb
apt install ubuntu-pro-client.deb
That should fail saying that ubuntu-advantage-tools >=31 is also required

Checklist

I have updated or added any unit tests accordingly
I have updated or added any integration tests accordingly
Changes here need to be documented, and this was done in:

Does this PR require extra reviews?

Yes
No

github-actions · 2023-12-12T18:00:07Z

Jira: SC-1623

GitHub Issues: No GitHub issues are fixed by this PR. (No commits have Fixes: #### references)

Launchpad Bugs: No Launchpad bugs are fixed by this PR. (No commits have LP: #### references)

Documentation: The changes in this PR do not require documentation changes.

👍 this comment to confirm that this is correct.

github-actions · 2023-12-12T18:00:09Z

🌎 This PR changes translatable messages. 🌏

Please select which scenarios apply. For further explanation, please read our policy on message changes.

New messages are being added.
- We will ask translators to take a look and add translations if they have time, but it will not block this PR.
Existing messages are being changed.
- ⚠️ Please add a comment with justification of why messages are being altered.
- If the changes are trivial (e.g. a typo fix), then translations must be preserved.
- If the changes are substantial, then we will ask translators to take a look and update translations if they have time, but it will not block this PR.
Existing messages are being deleted.
- No special action needed.

orndorffgrant · 2023-12-12T18:02:18Z

Existing messages are being changed.

⚠️ Please add a comment with justification of why messages are being altered.

If the changes are trivial (e.g. a typo fix), then translations must be preserved.

If the changes are substantial, then we will ask translators to take a look and update translations if they have time, but it will not block this PR.

Only the package name inside a message is being updated. Any translated messages that include it will be updated in this PR.

paride · 2024-01-05T14:36:30Z

debian/control

@@ -32,6 +32,15 @@ Vcs-Browser: https://github.com/canonical/ubuntu-pro-client
 Rules-Requires-Root: no

 Package: ubuntu-advantage-tools
+Depends: ubuntu-pro-client, ${misc:Depends}


We probably don't need ${misc:Depends} here (but it's also not harmful in any way).

I removed it but then lintian gives debhelper-but-no-misc-depends as a warning so I'll add it back

paride · 2024-01-05T14:38:00Z

debian/control


 Package: ubuntu-pro-client-l10n
 Architecture: any
-Depends: ${misc:Depends}, ubuntu-advantage-tools (>=30~)
+Depends: ${misc:Depends}, ubuntu-pro-client (>=31~)


We should be able to automate these versioned dependencies by using ${binary:Version}, see the deb-substvars(5) manpage.

lucasmoura

As @paride mentioned, we still need to explore ${binary:Version}, but besides that point, I can confirm that this setup works and we don't detach when upgrading to the new binary package

renanrodrigo

LGTM overall, works on the local tests I did.
However, as I would not have the expertise for a final approval, I am binding this to Paride+SRU aproval of course :D

features/steps/ubuntu_advantage_tools.py

orndorffgrant · 2024-01-16T14:52:37Z

@paride @basak We are officially using this method for the binary package rename. Please review it here in this PR. When we are happy with it, I'll merge and add it on top of the release PR (#2913).

Also, please note that there is a product desire to totally remove the ubuntu-advantage-tools transition package in 26.04 onwards. Please take that future change into account when reviewing.

paride

On d/control:

The identical descriptions for the transitional packages make lintian slightly unhappy:

I: ubuntu-advantage-tools source: duplicate-long-description ubuntu-advantage-pro ubuntu-advantage-tools [debian/control]
I: ubuntu-advantage-tools source: duplicate-short-description ubuntu-advantage-pro ubuntu-advantage-tools [debian/control]

Consider overriding that lintian tag, or using slightly different descriptions (e.g. naming the package: transitional dummy package for <package>).

On lintian

This one way I run lintian. From the unpackage source tree (the proposed branch in this case), I build the package creating a .changes file that contains both the binary packages and source package, e.g.

DEB_BUILD_OPTIONS=nocheck dpkg-buildpackage -sa -us -uc

That nocheck is to skip tests, useful to make iterations quicker. Then I run lintian like this:

lintian -EvIL +pedantic

to get all possible warnings. It will automatically find the relevant ../*.changes file and check it both the binary packages and source package it refers.

Let's make sure that this branch does not introduce new warnings; or if it does and we are OK with those, let's override them.

paride · 2024-01-18T14:59:59Z

debian/control

+Architecture: all
+Priority: optional
+Section: oldlibs
+Description: transitional package


Nit: the best practice is to use the "transitional dummy package" string, see https://www.debian.org/doc/manuals/developers-reference/best-pkging-practices.en.html#best-practices-for-debian-control.

paride · 2024-01-18T15:14:24Z

debian/control

@@ -35,6 +35,15 @@ Vcs-Browser: https://github.com/canonical/ubuntu-pro-client
 Rules-Requires-Root: no

 Package: ubuntu-advantage-tools
+Depends: ubuntu-pro-client (= ${binary:Version}), ${misc:Depends}


This Depends together with Architecture: all (which is correct for a transitional package) causes the following lintian error:

E: ubuntu-advantage-tools source: not-binnmuable-all-depends-any ubuntu-advantage-tools -> ubuntu-pro-client

(Run lintian-explain-tags not-binnmuable-all-depends-any for more info on it.) Now, this is not an issue on Ubuntu, because in Ubuntu we don't name binNMUs, however I think we should still avoid in in any case.

The easiest way in my opinion is to drop the versioned dependency altogether, we don't need to worry about an ubuntu-pro-client package being "old" because we're just introducing it. It will be the "real" package to have versioned dependencies as needed.

paride · 2024-01-18T15:18:59Z

debian/control

+Breaks: ubuntu-advantage-tools (<<31~)
+Replaces: ubuntu-advantage-tools (<<31~)
+# IMPORTANT: ubuntu-pro-client does not "Provide" ubuntu-advantage-tools
+# TODO fill this in with rationale after testing.


I think it's fine not to have a Provides for the replaced package if not needed, but if you have an explanation on why we don't want it (so we avoid adding it in the future), +1 for adding a comment on it under the ubuntu-pro-client binary package stanza.

paride · 2024-01-18T15:33:43Z

@orndorffgrant added a few review comments, but it should be all small stuff, the general approach looks good.

orndorffgrant · 2024-01-19T21:56:35Z

Thank you @paride! I believe I have addressed all of your comments in the latest commits

paride

Thanks for addressing all the review comments, this LGTM!

basak

As discussed, this is very complex and so I fear that it will go wrong for some edge case. We will need to rely on good testing, and I understand that's already been discussed and there's a good plan for this. I have not been able to spot any significant issues. A couple of very minor comments are inline.

basak · 2024-02-06T13:45:54Z

debian/rules

@@ -39,50 +58,54 @@ endif
 endif

 override_dh_gencontrol:
-	echo extra:Depends=$(APT_PKG_DEPS) $(DISTRO_INFO_DEPS) >> debian/ubuntu-advantage-tools.substvars
+	echo extra:Depends=$(APT_PKG_DEPS) $(DISTRO_INFO_DEPS) $(SELF_DEPS) >> debian/ubuntu-pro-client.substvars


We've got > in $(SELF_DEPS) so does the echo need to be quoted? Presumably you've tested it and it works as-is so this is just a style thing, but it is making me twitchy :-)

Eg. I think this should work?

echo "extra:Depends=$(APT_PKG_DEPS) $(DISTRO_INFO_DEPS) $(SELF_DEPS)" >> debian/ubuntu-pro-client.substvars

Ah - I think this line might have been removed in a future commit anyway? But still I think we're still writing > to substvars so quoting is a good idea unless I've missed something.

Yep the end result of this PR is currently no change to that part of the line, but it should have quotes. I'll go ahead and add them in a new commit on top.

basak · 2024-02-06T15:30:49Z

debian/control

+# On those releases, ubuntu-minimal (and ubuntu-cloud-minimal) Depends on
+# ubuntu-advantage-tools, which prevents it from being removed without also
+# removing ubuntu-minimal or ubuntu-cloud-minimal. We consider that to be a
+# sufficient # warning to users that removing ubuntu-advantage-tools is not


(Very very minor) wrap error

Whoops! I'll fix it

We're renaming the following packages using transitional packages to align the package names with the product naming: * ubuntu-advantage-tools -> ubuntu-pro-client * ubuntu-advantage-pro -> ubuntu-pro-image-auto-attach Because old versions of ubuntu-advantage-tools will break ESM upon removal, and ubuntu-pro-client "Breaks" ubuntu-advantage-tools <<31~, we need to force existing instances to upgrade to ubuntu-advantage-tools 31 (the transitional package) in order to install the new ubuntu-pro-client package. Enforcing that is accomplished via a Depends: ubuntu-advantage-tools on existing Ubuntu releases. Future Ubuntu releases (Noble onward) will not have this Depends. Because of that, all previous package migrations (from versions older than 31) are left in ubuntu-advantage-tools.{preinst,postinst,postrm}. All future package migrations will happen in ubuntu-pro-client.{preinst,postinst,postrm}.

By removing "Provides: ubuntu-advantage-tools", we lower the chances of ubuntu-advantage-tools <31 being removed during an install of ubuntu-pro-client, becuase ubuntu-minimal (and ubuntu-cloud-minimal) Depend on ubuntu-advantage-tools. This removes the need for ubuntu-pro-client Depends: ubuntu-advantage-tools.

Add integration test that install the ubuntu-advantage-tools transition package and checks that it also install the new ubuntu-pro-client package. We are also performing the same check for the ubuntu-advantage-pro package

orndorffgrant force-pushed the binary-package-rename branch from 8ee5d9d to 97339fb Compare December 12, 2023 17:59

orndorffgrant added the next label Dec 13, 2023

orndorffgrant force-pushed the binary-package-rename branch 4 times, most recently from 9539e30 to 0f5d0a8 Compare December 13, 2023 20:24

paride reviewed Jan 5, 2024

View reviewed changes

lucasmoura approved these changes Jan 5, 2024

View reviewed changes

renanrodrigo approved these changes Jan 7, 2024

View reviewed changes

orndorffgrant force-pushed the binary-package-rename branch 3 times, most recently from b7af0f7 to e0e1fff Compare January 8, 2024 18:40

orndorffgrant marked this pull request as ready for review January 8, 2024 19:11

orndorffgrant mentioned this pull request Jan 9, 2024

Release 31 #2913

Merged

6 tasks

orndorffgrant force-pushed the binary-package-rename branch 2 times, most recently from ec64a2c to 23e58f7 Compare January 10, 2024 16:16

orndorffgrant commented Jan 10, 2024

View reviewed changes

features/steps/ubuntu_advantage_tools.py Show resolved Hide resolved

orndorffgrant mentioned this pull request Jan 12, 2024

New name metapackages #2915

Closed

5 tasks

orndorffgrant requested review from basak and paride January 16, 2024 14:52

lucasmoura force-pushed the binary-package-rename branch from 1817847 to 2733f3e Compare January 16, 2024 20:54

orndorffgrant force-pushed the binary-package-rename branch from 2733f3e to 289252e Compare January 16, 2024 21:06

paride requested changes Jan 18, 2024

View reviewed changes

orndorffgrant force-pushed the binary-package-rename branch from 3e4272d to a82c752 Compare January 19, 2024 21:56

orndorffgrant force-pushed the binary-package-rename branch from a82c752 to 1bc8515 Compare January 20, 2024 01:52

paride approved these changes Jan 23, 2024

View reviewed changes

This was referenced Jan 26, 2024

wip: Fix cloudinit systemd deadlock #2927

Closed

daemon: wait for cloud-config.service to complete in python #2937

Merged

basak approved these changes Feb 6, 2024

View reviewed changes

orndorffgrant and others added 11 commits February 6, 2024 14:59

d/rules: remove old ubuntu release logic

3db8455

d/control: homepage and description improvements

9720d89

tests: update behave testing utils to use new packages

d619b89

update-pos

1572d10

fix package name in translations

5ff96f3

tests: add test for transition package

5e28214

Add integration test that install the ubuntu-advantage-tools transition package and checks that it also install the new ubuntu-pro-client package. We are also performing the same check for the ubuntu-advantage-pro package

lintian: fix or override errors and warnings

b073874

d/control: add explanation for the lack of Provides

d331e06

rename: no need for 'image' in auto-attach package name

db62a10

orndorffgrant force-pushed the binary-package-rename branch from 0286348 to afc801d Compare February 6, 2024 19:59

d/rules: fix quotes in echo command

5fc24b0

orndorffgrant force-pushed the binary-package-rename branch from afc801d to 5fc24b0 Compare February 7, 2024 02:57

orndorffgrant merged commit 9e7d4a3 into main Feb 7, 2024
25 checks passed

orndorffgrant deleted the binary-package-rename branch February 7, 2024 14:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Binary package rename (SC-1623) #2887

Binary package rename (SC-1623) #2887

orndorffgrant commented Dec 11, 2023 •

edited

Loading

github-actions bot commented Dec 12, 2023

github-actions bot commented Dec 12, 2023 •

edited by orndorffgrant

Loading

orndorffgrant commented Dec 12, 2023 •

edited

Loading

paride Jan 5, 2024

orndorffgrant Jan 8, 2024

paride Jan 5, 2024

lucasmoura left a comment

renanrodrigo left a comment

orndorffgrant commented Jan 16, 2024

paride left a comment

paride Jan 18, 2024

paride Jan 18, 2024

paride Jan 18, 2024

paride commented Jan 18, 2024

orndorffgrant commented Jan 19, 2024

paride left a comment

basak left a comment

basak Feb 6, 2024

orndorffgrant Feb 6, 2024

basak Feb 6, 2024

orndorffgrant Feb 6, 2024

Binary package rename (SC-1623) #2887

Binary package rename (SC-1623) #2887

Conversation

orndorffgrant commented Dec 11, 2023 • edited Loading

Why is this needed?

Test Steps

Checklist

Does this PR require extra reviews?

github-actions bot commented Dec 12, 2023

github-actions bot commented Dec 12, 2023 • edited by orndorffgrant Loading

orndorffgrant commented Dec 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucasmoura left a comment

Choose a reason for hiding this comment

renanrodrigo left a comment

Choose a reason for hiding this comment

orndorffgrant commented Jan 16, 2024

paride left a comment

Choose a reason for hiding this comment

On d/control:

On lintian

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paride commented Jan 18, 2024

orndorffgrant commented Jan 19, 2024

paride left a comment

Choose a reason for hiding this comment

basak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

orndorffgrant commented Dec 11, 2023 •

edited

Loading

github-actions bot commented Dec 12, 2023 •

edited by orndorffgrant

Loading

orndorffgrant commented Dec 12, 2023 •

edited

Loading