Merge ~philroche/ubuntu/+source/google-guest-agent:feature/ubuntu/mantic-devel-no-start-services-on-upgrade into ubuntu/+source/google-guest-agent:ubuntu/mantic-devel

[Philip Roche]

fix: Add --no-stop-on-upgrade for upgrade path

* debian/rules: Add --no-stop-on-upgrade for upgrade path for
  google-startup-scripts.service google-shutdown-scripts.service
  to enforce no stop of these service on package upgrade.
  This has the desired side-effect of not stopping, starting or
  restarting the services as a part of the upgrade.
  Previously an upgrade of the google-guest-agent package would
  restart google-startup-scripts.service and
  google-shutdown-scripts.service which was unexpected and
  if google-shutdown-scripts was performing destructive actions
  then this could cause issues.

  This also removes the confusingly named `--no-restart-on-upgrade`
  option which actually does result in a service restart via the
  service to being stopped in the prerm script and started again in
  the postinst script (LP: #2019089)

[Philip Roche (philroche)]

PPA https://launchpad.net/~philroche/+archive/ubuntu/google-guest-agent/

Reproducer and test script @ https://pastebin.ubuntu.com/p/VgCyNzwNkn/

[Christian Ehrhardt  (paelzer)]

The history of these is:
20200617.00-0ubuntu1
   dh_installsystemd --no-restart-after-upgrade --no-restart-on-upgrade google-startup-scripts.service google-shutdown-scripts.service
20200617.00-0ubuntu5
      * debian/rules: Don't stop or start google-startup-scripts and
        google-shutdown-scripts services
- dh_installsystemd --no-restart-after-upgrade --no-restart-on-upgrade google-startup-scripts.service google-shutdown-scripts.service
+ dh_installsystemd --no-start --no-restart-after-upgrade google-startup-scripts.service google-shutdown-scripts.service
And now due to the bug changing it (for the same reason again of not starting).
- dh_installsystemd --no-start --no-restart-after-upgrade google-startup-scripts.service google-shutdown-scripts.service
+ dh_installsystemd --no-start --no-stop-on-upgrade google-startup-scripts.service google-shutdown-scripts.service

That looks reasonable.

With that the commit and changelog message is a bit odd, you are not "restoring" --no-stop-on-upgrade.
You are restoring the behavior of not stopping, but not the argument, that was never there so far.
And only due to changes in the defaults it now causes issues.

The whole second part of the changelog "Previously an upgrade of the google-guest-agent package would restart google-startup-scripts.service and google-shutdown-scripts.service which was unexpected and if google-shutdown-scripts was performing destructive actions then this could cause issues." seems like a detailed description of the bug. It is to some extent style and you might want to keep it, but TBH I'd remove that part completely and only reference the bug which describes it in even more detail already.

Furthermore I think we need to track down and explain why this is a problem in Jammy but not in focal. The only reason you'd usually have is a change in the compat level, but we have:
$ git show pkg/ubuntu/jammy-devel:debian/control | grep debhelper-compat
Build-Depends: debhelper-compat (= 12),
$ git show pkg/ubuntu/focal-devel:debian/control | grep debhelper-compat
Build-Depends: debhelper-compat (= 12),

Is it really not also affecting focal?
Before doing a hurried change, you should test building this on focal and check the resulting maintainer scripts. How do they behave and why?

Furthermore,
almost always when people use "--no-start", they should also think about "--no-enable".
Because as proposed it would install (and not start the service).
But once rebooted the service would be active.
Is that the behavior you want, or should we - while touching it - also add that flag?
And if you want it started after boot, why would you not want to start it on install?
Could it be that --no-start is only there because of past tries to get a no-restart behavior?
If so, could it be that you might want to start one but not the other?
You see, there are questions that should be answered to make this good in this upload and not cause another few to be needed :-)

So overall, would you mind:
1. reworking the changelog entry to state more correctly what we do (e.g. not restoring)
2. once proven stating why it is a problem in jammy now, but not in e.g. Focal (can be...

review: Needs Information

[Philip Roche (philroche)]

Thank you for the detailed feedback. Moving back to WIP

[Philip Roche (philroche)]

> 1. reworking the changelog entry to state more correctly what we do (e.g. not restoring)

ack

> 2. once proven stating why it is a problem in jammy now, but not in e.g. Focal (can be done very shortly or not in changelog, but in details in the SRU template on the bug)

I have rebuilt the unpatched focal packages and published to https://launchpad.net/~philroche/+archive/ubuntu/google-guest-agent/ but this does not result in the troublesome `preinst` section like we had in jammy+

```
# Automatically added by dh_installsystemd/13.6ubuntu1
if [ -z "${DPKG_ROOT:-}" ] && [ "$1" = upgrade ] && [ -d /run/systemd/system ] ; then
 deb-systemd-invoke stop 'google-shutdown-scripts.service' 'google-startup-scripts.service' >/dev/null || true
fi
# End automatically added section
```

Could this be as a result of `dh_installdeb/12.10ubuntu1` being used on focal vs `dh_installdeb/13.6ubuntu1` on jammy and the default changing between these versions?

> 3. add --no-enable or state in the bug in detail why you do not want/need it

Will do. We do want the services to be enabled on initial installation and to be started on instance start but they should not be restarted/stopped during the lifetime of the instance.

[Philip Roche (philroche)]

@paelzer

> 1. reworking the changelog entry to state more correctly what we do (e.g. not restoring)

done

> 2. once proven stating why it is a problem in jammy now, but not in e.g. Focal (can be done very shortly or not in changelog, but in details in the SRU template on the bug)

I am still debugging why there is a diff here.

> 3. add --no-enable or state in the bug in detail why you do not want/need it

Done and https://code.launchpad.net/bugs/2019089 description updated

Unmerged commits

b91792d... by Philip Roche on 2023-05-11

fix: Add --no-stop-on-upgrade for upgrade path

* debian/rules: Add --no-stop-on-upgrade for upgrade path for
  google-startup-scripts.service google-shutdown-scripts.service
  to enforce no stop of these service on package upgrade.
  This has the desired side-effect of not stopping, starting or
  restarting the services as a part of the upgrade.
  Previously an upgrade of the google-guest-agent package would
  restart google-startup-scripts.service and
  google-shutdown-scripts.service which was unexpected and
  if google-shutdown-scripts was performing destructive actions
  then this could cause issues.

  This also removes the confusingly named `--no-restart-on-upgrade`
  option which actually does result in a service restart via the
  service to being stopped in the prerm script and started again in
  the postinst script (LP: #2019089)

diff --git a/debian/changelog b/debian/changelog
index f166009..6414aa6 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,13 @@
+google-guest-agent (20230426.00-0ubuntu2) mantic; urgency=medium
+
+    * debian/rules: Add --no-stop-on-upgrade for upgrade path for
+      google-startup-scripts.service google-shutdown-scripts.service
+      to enforce no stop of these service on package upgrade.
+      This has the desired side-effect of not stopping, starting or
+      restarting the services as a part of the upgrade (LP: #2019089)
+
+ -- Phil Roche <[e-mail]>  Fri, 12 May 2023 13:01:49 +0100
+
 google-guest-agent (20230426.00-0ubuntu1) mantic; urgency=medium
 
   * New upstream version 20230426.00. (LP: #2018272)
diff --git a/debian/rules b/debian/rules
index c455868..acde0ad 100755
--- a/debian/rules
+++ b/debian/rules
@@ -17,7 +17,7 @@ override_dh_auto_install:
 
 override_dh_installsystemd:
 	dh_installsystemd google-guest-agent.service
-	dh_installsystemd --no-start --no-restart-after-upgrade google-startup-scripts.service google-shutdown-scripts.service
+	dh_installsystemd --no-start --no-stop-on-upgrade google-startup-scripts.service google-shutdown-scripts.service
 
 override_dh_clean:
 	dh_clean vendor/