Merge ~chad.smith/ubuntu/+source/needrestart:lp-2059337-dont-autorestart-cloud-final into ubuntu/+source/needrestart:ubuntu/devel

[Chad Smith]

    d/p/ubuntu-avoid-restart-cloud-final.patch

    Ensure needrestart skips automatic restart of cloud-init systemd
    services representing the cloud-init boot stages:
    - cloud-init-local.service
    - cloud-init.service
    - cloud-config.service
    - cloud-final.service

    A restart of these oneshot services may cause cloud-init to be unable to
    setup apt repositories or install packages requested by cloud-init user-data.

    Version 3.6-7ubuntu1 introduced a downstream change in behavior for Ubuntu
    where needrestart will automatically restart services across
    non-interactive apt-get dist-upgrade.

    When cloud-init user-data provides both package_upgrade: true and requests
    for either package installs or PPA setup, cloud-final.service will
    call apt-get dist-upgrade in non-interactive mode and follow up with any
    number of add-apt-repository or apt-get install commands. If cloud-init
    is also a package being upgraded by dist-upgrade, needrestart to suggests that
    the running cloud-final.service is a candidate for automatic restart.

    This results in a SIGTERM to cloud-final.service that prevents the rest
    of cloud-init's APT configuration from completing.

    Given that /etc/cloud/cloud.cfg allows for the package_update_upgrade_install
    module to be configured to run in other boot stages, we need to skip
    restart on any of the 4 cloud-init boot stage services mentioned above.

    LP: #2059337

[Brett Holman (holmanb)]

We need to skip the other cloud-init services as well.

review: Needs Fixing

[Chad Smith (chad.smith)]

Thanks @holmanb, I just force-pushed these changes after testing:

Test procedure:

1. Create user-data that tells cloud-init to run apt-get dist-upgrade and install the sl package

cat > lp-2059337.yaml <<EOF
#cloud-config
# trigger needrestart SIGTERM against cloud-final due to apt-get dist-upgrade call and cloud-init pkg upgrade
package_update: true
package_upgrade: true
packages: [sl]
EOF

2. init a noble lxc container with cloud-config to trigger apt-get dist-upgrade and package installs
lxc init ubuntu-daily:noble nn-test -c cloud-init.user-data="$(cat lp-2059337.yaml)"

3. inject this autoinstall.conf to /etc/needrestart/needrestart.conf
lxc file push ex/needrestart.conf nn-test/etc/needrestart/

4. start the lxd instance, letting cloud-init run.
lxc start nn-test; sleep 2; lxc exec nn-test -- cloud-init status --wait --format=yaml

5. grep need restart behavior in cloud-init-output.log asserting cloud-final.service is skipped
lxc exec nn-test -- grep -A 8 "Service restarts" /var/log/cloud-init-output.log

--- Test run output shows deferred cloud-final.service
/var/log/cloud-init-output.log:Service restarts being deferred:
/var/log/cloud-init-output.log- systemctl restart cloud-final.service
/var/log/cloud-init-output.log- /etc/needrestart/restart.d/dbus.service
/var/log/cloud-init-output.log- systemctl restart systemd-logind.service
/var/log/cloud-init-output.log- systemctl restart unattended-upgrades.service
/var/log/cloud-init-output.log-
/var/log/cloud-init-output.log-No containers need to be restarted.
/var/log/cloud-init-output.log-
/var/log/cloud-init-output.log-No user sessions are running outdated binaries.
--
/var/log/cloud-init-output.log:Service restarts being deferred:
/var/log/cloud-init-output.log- systemctl restart cloud-final.service
/var/log/cloud-init-output.log- /etc/needrestart/restart.d/dbus.service
/var/log/cloud-init-output.log- systemctl restart systemd-logind.service
/var/log/cloud-init-output.log- systemctl restart unattended-upgrades.service
/var/log/cloud-init-output.log-
/var/log/cloud-init-output.log-No containers need to be restarted.
/var/log/cloud-init-output.log-
/var/log/cloud-init-output.log-No user sessions are running outdated binaries.

[Brett Holman (holmanb)]

Thanks for fixing that Chad!

review: Approve

[Athos Ribeiro (athos-ribeiro)]

Hi Chad! Thanks for putting this up :)

I have a few nitpicks and questions below (inline).

review: Needs Information

[Chad Smith (chad.smith)]

Thanks Athos for the swift review here. I've fixed all comments you mentioned:

1. dep3 Description . on dep3 paragraph breaks
2. dep3 Author email format fix
3. dep3 dropped origin: other since we have author
4. dep3 Bug-Ubuntu instead of Bug:
5. quilt patch content: move the comment about bug LP: #2059337 closer to the cloud-init regex instead of adding the link to the generic upstream comment
6. dropped the stray .* from cloud-init regex. It was carried over from my earlier iteration of a more inclusive regex on cloud-.*.service. I decided on a 2nd pass that I needed to make it more strict to avoid matching any other unknown systemd services that started with the prefix `cloud-` and just limit it to cloud-init's 4 boot stage services: cloud-init-local.service, cloud-init.service, cloud-config.service and cloud-final.service.

[Athos Ribeiro (athos-ribeiro)]

Thanks, Chad. I am understanding this as a bug fix only change and therefore there is no need to file a freeze exception here.

Uploaded:

Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading needrestart_3.6-7ubuntu4.dsc: done.
  Uploading needrestart_3.6-7ubuntu4.debian.tar.xz: done.
  Uploading needrestart_3.6-7ubuntu4_source.buildinfo: done.
  Uploading needrestart_3.6-7ubuntu4_source.changes: done.
Successfully uploaded packages.

review: Approve

diff --git a/debian/changelog b/debian/changelog
index 1ff9ad5..9a6f036 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+needrestart (3.6-7ubuntu4) noble; urgency=medium
+
+  * d/p/ubuntu-avoid-restart-cloud-final.patch:
+    - avoid automatic restart of cloud-init systemd oneshot services when
+      cloud-init invokes apt-get dist-upgrade due to user-data (LP: #2059337)
+
+ -- Chad Smith <[e-mail]>  Wed, 27 Mar 2024 16:51:58 -0600
+
 needrestart (3.6-7ubuntu3) noble; urgency=medium
 
   * debian/tests:
diff --git a/debian/patches/series b/debian/patches/series
index 1a1f46c..e223724 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -7,3 +7,4 @@
 07-mark-unavailable-firmware-as-CURRENT.diff
 
 ubuntu-mode.patch
+ubuntu-avoid-restart-cloud-final.patch
diff --git a/debian/patches/ubuntu-avoid-restart-cloud-final.patch b/debian/patches/ubuntu-avoid-restart-cloud-final.patch
new file mode 100644
index 0000000..c1c42c5
--- /dev/null
+++ b/debian/patches/ubuntu-avoid-restart-cloud-final.patch
@@ -0,0 +1,28 @@
+Description: Avoid automated restart of cloud-*.service boot stages
+ Add a regex to skip needrestart on cloud-*.service units for Ubuntu.
+ In downstream Ubuntu, needrestart pkg automatically restarts services
+ during dist-upgrade when in non-interactive mode. This causes issues for
+ cloud-init when cloud-init deb is currently being upgraded and user-data
+ requests an apt-get dist-upgrade via package_upgrade: true and package
+ installs via packages: [sl]. In this case, needrestart  will SIGTERM
+ of cloud-final.service before it is able to complete other apt operations.
+ .
+ Because cloud-init's package update module can be configured to run during
+ any of cloud-init's boot stages, allow the regex to match any of these known
+ boot stages.
+Author: Chad Smith <[e-mail]>
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/needrestart/+bug/2059337
+Last-Update: 2024-04-04
+--- a/ex/needrestart.conf
++++ b/ex/needrestart.conf
+@@ -123,6 +123,10 @@
+     qr(^apt-daily\.service$) => 0,
+     qr(^apt-daily-upgrade\.service$) => 0,
+     qr(^unattended-upgrades\.service$) => 0,
++    # do not restart cloud-init services which may call apt dist-upgrade
++    # non-interactively. LP: #2059337
++    qr(^cloud-(init-local|init|config|final)\.service$) => 0,
++
+     # do not restart oneshot services from systemd-cron, see also #917073
+     qr(^cron-.*\.service$) => 0,
+