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

Cloud specific binary packages #5655

Merged

Conversation

aciba90
Copy link
Contributor

@aciba90 aciba90 commented Sep 2, 2024

Do not squash

See individual commits for more context and:

Additional Context

Test Steps

Merge type

  • Squash merge using "Proposed Commit Message"
  • Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)

@aciba90 aciba90 added the packaging Supplemental package review requested label Sep 2, 2024
@aciba90
Copy link
Contributor Author

aciba90 commented Sep 3, 2024

This PR requires #5637 to be merged and upstream released to really get rid of python3-serial as a dependency of cloud-init-base. Marking this as Draft until then.

@aciba90 aciba90 marked this pull request as draft September 3, 2024 06:48
@aciba90 aciba90 force-pushed the cloud-specific-binary-packages branch 3 times, most recently from 65ba099 to 5ff63c9 Compare September 6, 2024 14:04
@aciba90
Copy link
Contributor Author

aciba90 commented Sep 6, 2024

I have updated this PR performing a new upstream release based on tip of main to include #5637.

@aciba90 aciba90 marked this pull request as ready for review September 6, 2024 14:07
@aciba90
Copy link
Contributor Author

aciba90 commented Sep 6, 2024

The ci check check-patches fails due to #5675.

Copy link
Member

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

Nice work! In general LGTM, though I think @blackboxsw and/or somebody from distro should review this too.

@aciba90
Copy link
Contributor Author

aciba90 commented Sep 10, 2024

Many thanks for the feedback, @TheRealFalcon.

Nice work! In general LGTM, though I think @blackboxsw and/or somebody from distro should review this too.

Could you, @basak, please take a look on this one? SC-1728 and US120 for context.

@blackboxsw blackboxsw self-assigned this Sep 11, 2024
@aciba90 aciba90 force-pushed the cloud-specific-binary-packages branch from 5ff63c9 to 2252c62 Compare September 17, 2024 09:59
@aciba90
Copy link
Contributor Author

aciba90 commented Sep 17, 2024

Force-pushed to add a debian/NEWS file documenting this feature.

@aciba90
Copy link
Contributor Author

aciba90 commented Sep 24, 2024

This PR is blocked by #5567, as we need to handle the debconf values related to MAAS handling. Marking this as draft.

^ This PR was in a wrong status. The MAAS handling was included in the ubuntu/oracular branch but a sync to ubuntu/devel from those changes is still missing.

@aciba90 aciba90 added the wip Work in progress, do not land label Sep 24, 2024
@aciba90 aciba90 added this to the cloud-init-24.4 milestone Sep 24, 2024
@aciba90 aciba90 force-pushed the cloud-specific-binary-packages branch from a494644 to 34700ec Compare September 24, 2024 13:12
@aciba90 aciba90 changed the base branch from ubuntu/devel to ubuntu/oracular September 24, 2024 13:14
@aciba90 aciba90 changed the base branch from ubuntu/oracular to ubuntu/devel September 25, 2024 09:25
Copy link
Collaborator

@blackboxsw blackboxsw left a comment

Choose a reason for hiding this comment

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

I'm seeing package upgrade issuse in postinst which point to inconsistencies in cloud-init w.r.t. a full package replacement of cloud-init-base from the original cloud-init package. I'm wondering if this is a case where we need to employ dpkg-divert here to avoid those file and directory conflicts.

Unpacking cloud-init-base (24.4~3+really24.3.1-0ubuntu5~ppa1) ...
dpkg: error processing archive /var/cache/apt/archives/cloud-init-base_24.4~3+really24.3.1-0ubuntu5~ppa1_all.deb (--unpack):
 trying to overwrite '/etc/cloud/cloud.cfg', which is also in package cloud-init 24.4~3+really24.3.1-0ubuntu4
dmesg: read kernel buffer failed: Operation not permitted
                                                         dpkg-deb: error: paste subprocess was killed by signal (Broken pipe)
Preparing to unpack .../cloud-init_24.4~3+really24.3.1-0ubuntu5~ppa1_all.deb ...
Unpacking cloud-init (24.4~3+really24.3.1-0ubuntu5~ppa1) over (24.4~3+really24.3.1-0ubuntu4) ...
dpkg: warning: unable to delete old directory '/etc/cloud/templates': Directory not empty
dpkg: warning: unable to delete old directory '/etc/cloud/cloud.cfg.d': Directory not empty
dpkg: warning: unable to delete old directory '/etc/cloud': Directory not empty
Errors were encountered while processing:
 /var/cache/apt/archives/cloud-init-base_24.4~3+really24.3.1-0ubuntu5~ppa1_all.deb
needrestart is being skipped since dpkg has failed

debian/NEWS Outdated Show resolved Hide resolved
debian/control Outdated Show resolved Hide resolved
debian/cloud-init-base.config Outdated Show resolved Hide resolved
debian/cloud-init-base.postinst Outdated Show resolved Hide resolved
@aciba90 aciba90 force-pushed the cloud-specific-binary-packages branch from 34700ec to 1c8e229 Compare September 27, 2024 08:07
Add infrastructure to be able to express cloud-specific cloud-init
binary packages.

Rename cloud-init to cloud-init-base. This one will represent cloud-init
plus the base dependency set.

Add `Breaks:` and `Replaces:` dependency with
cloud-init <= 24.4~2g2e4c39b7-0ubuntu1
to adopt and own files from cloud-init in cloud-init-base, see [1] for
more context.

Keep a cloud-init metapackage to install every possible dependency and
be able to operate on any platform.

SC-1854
US120

[1] https://www.debian.org/doc/debian-policy/ch-relationships.html#s-replaces
Add cloud-init-cloud-sigma and cloud-init-smart-os binary packages
depending on cloud-init-base and python3-serial.

Removes python3-serial from cloud-init-base and add it to cloud-init.

Fixes SC-1854
US120
  + Add warning about image portability to dpkg-reconfigure

Fixes SC-1855
@aciba90 aciba90 force-pushed the cloud-specific-binary-packages branch 2 times, most recently from 1c35ec1 to 5f75b35 Compare September 27, 2024 08:14
@aciba90
Copy link
Contributor Author

aciba90 commented Sep 27, 2024

Thanks, @blackboxsw, for the review.

This issue you were facing happened because the cloud-init version of the Breaks: and Repleaces: clauses did not match the one published in oracular, I have updated it and verified the error does not happen anymore. No further dpkg-divert should be needed in this case.

I have updated https://launchpad.net/~aciba/+archive/ubuntu/us-120-cloud-specific-cloud-init-packaging to contain the latest changes from this PR, feel free to use it while testing this.

I have addressed the rest of the concerns and this PR is ready to be reviewed again.

@aciba90 aciba90 removed the wip Work in progress, do not land label Sep 27, 2024
@aciba90 aciba90 force-pushed the cloud-specific-binary-packages branch from 5f75b35 to 03ce0a5 Compare September 27, 2024 09:10
@blackboxsw
Copy link
Collaborator

Thanks @aciba90 while I'm able to install from your PPA on Oracular and receive an exit 0 from the install, we still see warnings on the console during that process about directory conflicts:

lxc launch ubuntu-daily:oracular -c security.nesting=true test-ci-base
lxc exec test-ci-base
sudo add-apt-repository ppa:aciba/us-120-cloud-specific-cloud-init-packaging -y
...
apt install cloud-init -y
...
Unpacking cloud-init (24.4~4gc9dce94d-0ubuntu2~ppa1) over (24.4~3+really24.3.1-0
ubuntu4) ...
dpkg: warning: unable to delete old directory '/etc/cloud/templates': Directory 
not empty
dpkg: warning: unable to delete old directory '/etc/cloud/cloud.cfg.d': Director
y not empty
dpkg: warning: unable to delete old directory '/etc/cloud': Directory not empty
Selecting previously unselected package cloud-init-base.
Preparing to unpack .../cloud-init-base_24.4~4gc9dce94d-0ubuntu2~ppa1_all.deb ..
.
awk: fatal: cannot open file `/etc/fstab' for reading: No such file or directory
Unpacking cloud-init-base (24.4~4gc9dce94d-0ubuntu2~ppa1) ...
Setting up cloud-init-base (24.4~4gc9dce94d-0ubuntu2~ppa1) ...
Removing cloud-init/datasources in favor of cloud-init-base/datasources
Setting up cloud-init (24.4~4gc9dce94d-0ubuntu2~ppa1) ...
Processing triggers for man-db (2.12.1-3) ...
Processing triggers for rsyslog (8.2406.0-1ubuntu2) ...
Scanning processes...                                                           

No services need to be restarted.

No containers need to be restarted.

No user sessions are running outdated binaries.

No VM guests are running outdated hypervisor (qemu) binaries on this host.
root@test-bi-base:~# echo $?
0

@aciba90
Copy link
Contributor Author

aciba90 commented Sep 30, 2024

Thanks, @blackboxsw.

we still see warnings on the console during that process about directory conflicts

apt install cloud-init -y
...
Unpacking cloud-init (24.4~4gc9dce94d-0ubuntu2~ppa1) over (24.4~3+really24.3.1-0
ubuntu4) ...
dpkg: warning: unable to delete old directory '/etc/cloud/templates': Directory 
not empty
dpkg: warning: unable to delete old directory '/etc/cloud/cloud.cfg.d': Director
y not empty
dpkg: warning: unable to delete old directory '/etc/cloud': Directory not empty
Selecting previously unselected package cloud-init-base.
Preparing to unpack .../cloud-init-base_24.4~4gc9dce94d-0ubuntu2~ppa1_all.deb ..

We see similar directory conflicts while upgrading to 24.2-0ubuntu1~24.04.2 (in noble-proposed) in fresh noble lxd vm:

Preparing to unpack .../34-cloud-init_24.3.1-0ubuntu0~24.04.1_all.deb ...
Unpacking cloud-init (24.3.1-0ubuntu0~24.04.1) over (24.2-0ubuntu1~24.04.2) ...
dpkg: warning: unable to delete old directory '/etc/systemd/system/[email protected]': Directory not empty

The reason dpkg issues those warnings is that some of those folders contain some file that was not in the cloud-init deb package and dpkg tries to remove them before removing the previous version or before installing the new one.

Maybe there are other ways to avoid the warnings, but the one I can think of is the following:

Doing some kind of heavy lifting in the maintainer scripts to manually move the files to a temporary dir at preinst time and restoring them after the new version is installed.

I believe this is more a cosmetic (dpkg being overly explicit about what is happening) issue than a real concern, and it is not worth avoiding it because:

  1. Letting dpkg do its thing is less error-prone and safer than manually moving files in this case.
  2. As this upgrade will be rolled out to pp once the archive is open, way before pp is released, users hitting these warnings will be: users manually performing a do-release-upgrade from noble or oracular to pp. So the impact of the warnings is limited.

@blackboxsw
Copy link
Collaborator

I believe this is more a cosmetic (dpkg being overly explicit about what is happening) issue than a real concern, and it is not worth avoiding it because:

1. Letting dpkg do its thing is less error-prone and safer than manually moving files in this case.

2. As this upgrade will be rolled out to pp once the archive is open, way before pp is released, users hitting these warnings will be: users manually performing a `do-release-upgrade` from noble or oracular to pp. So the impact of the warnings is limited.

You are right, I was thinking maybe we maybe missed some supplemental rm_conffile or diversion above the Breaks/Replaces to ease this transition. Given that both the old cloud-init and the new cloud-init-base deliver the same conffiles/dirs etc I'm not worried about the transition being incomplete, just wondering if there was a way to quite that noise.

That said, it appears everything you are doing with Breaks/Replaces is correct for use-case #8 in https://wiki.debian.org/PackageTransition cheat sheet. So +1 from me on this changeset.

Copy link
Collaborator

@blackboxsw blackboxsw left a comment

Choose a reason for hiding this comment

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

Thank you for the detailed work here for Ubuntu downstream. This will help us reduce image size in some of our image offerings.

@aciba90
Copy link
Contributor Author

aciba90 commented Oct 3, 2024

Thanks for the deep review here!

@aciba90 aciba90 merged commit 81ee561 into canonical:ubuntu/devel Oct 3, 2024
19 of 20 checks passed
@aciba90 aciba90 deleted the cloud-specific-binary-packages branch October 3, 2024 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packaging Supplemental package review requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants