autopkgtests failures due to debhelper 13.6 changes

Hi Athos -- this is an interesting one.

Forgive my ignorance, but per Colin's comments in https://salsa.debian.org/pkg-debconf/debconf/-/merge_requests/10 "... dpkg-reconfigure is simulating the process of reinstalling a package without actually unpacking its files again" (which aligned with my understanding of dpkg-reconfigure). Surely if it's "simulating [...] reinstalling a package", then:

> dpkg-reconfigure --frontend=noninteractive postgresql-common

should:

> [result] in a call to
>
> deb-systemd-invoke stop 'postgresql.service',

After all, if it's simulating a reinstallation, shouldn't it restart the service? Surely the error is the test suite assuming that it shouldn't? You are certainly correct that:

> A possible solution to the issue would be to introduce the "-r" (or
> --no-stop-on-upgrade) option to dh_installsystemd in Ubuntu's
> postgresql-common d/rules. This should be enough to keep the
> expected (old) behavior in the package maintainer scripts.

(aside: I'd beg people to use the long option and never touch "-r" or "-R" given we wound up having to grep thru maintscripts to find instances of --no-{restart,stop}-{on,after}-upgrade when fixing the debhelper issue -- not to mention they're just more readable)

Still, it seems to me that the assumption that dpkg-reconfigure *won't* restart postgres is incorrect? Then again, if it's already commonly expected behaviour, maintaining it may well be the wisest course. It would mean that upgrades won't restart the service which may not be desirable, although needsrestart may provide a fallback there?

Interested to hear any thoughts you may have on this -- in tinkering with debhelper and dpkg-reconfigure, I'm painfully aware I'm messing with widely used stuff with a long history of assumed quirks!

Sorry, I'd mis-interpreted this bug! I'd gone digging through the history of debconf and couldn't understand how dpkg-reconfigure was only restarting things just *now*, but hadn't correctly understood that your bug was complaining the service was stopped, but *not started* again. So, my apologies: must read more carefully!

Anyway, to the problem as described: checking d/rules in postgresql-common we find:

# do not (re)start postgresql.service on postgresql-common install/upgrades
override_dh_installinit:
 dh_installinit -ppostgresql-common --name=postgresql -u'defaults 19 21' --no-start
override_dh_installsystemd:
 dh_installsystemd --no-start

This strongly suggests you *do* want to add --no-stop-on-upgrade because the comment indicates a mis-understanding of --no-start. The --no-start option tells dh_install{init,systemd} that you don't want the service automatically started upon installation but in the absence of --no-stop-on-upgrade it *will* still attempt to restart it on upgrades (this isn't terribly well documented in the dh_installsystemd manpage -- I should correct that).

There is one other way you could go about solving this, but it's horribly subtle. The reason preinst is stopping postgres is that --no-start implies --no-restart-after-upgrade, the (terribly named) option that specifies a service *should* be restarted but by stopping it before and upgrade and starting it again afterward (as opposed to --restart-after-upgrade, the usual default which simply runs a restart after an upgrade). Quite *why* --no-start implies --no-restart-after-upgrade I've no idea; I did note it at the time I was hacking on debhelper and it's one of those things that, given free reign, I'd excise. However, as mentioned before I tried to be extremely conservative with my changes to debhelper (and dpkg-reconfigure).

Anyway, the other way you could solve this would be by specifying --no-start and --restart-after-upgrade. This would mean preinst doesn't try to stop the service, but postinst will "try-restart" the service (which means restart if it's already running, but don't start it if it isn't). However, that would run contrary to the comment in d/rules so I suspect it's not what you want to do.

Hope that's not too confusing! Do let me know if there's anything that needs clarification above.

Hi Dave,

Thanks for the thoughtful reply!

> After all, if it's simulating a reinstallation, shouldn't it restart the service? [...]

Usually, yes, I agree it should restart the service. In this specific postgresql-common case, debian/rules was crafted so a restart would not happen at this point:

https://salsa.debian.org/postgresql/postgresql-common/-/blob/master/debian/rules#L27-31

Now, while the test suite regression is only present in Ubuntu (and not in Debian) due to the dpkg-reconfigure delta, a change of behavior was also introduced in the upgrade path due to the debhelper change at

https://salsa.debian.org/debian/debhelper/-/commit/742b0c5ac8f4a6cfcd699f08915a8109a5ebec35.

The service, which would be left untouched on postgresql-common upgrades, will now be stopped on upgrades.

Finally,

> You are certainly correct that [we should add --no-stop-on-upgrade option to dh_installsystemd?]

This would restore the previous, designed behavior of the package upgrade path. Still, the preinst script would differ from the Debian one. Therefore, I filed

https://code.launchpad.net/~athos-ribeiro/ubuntu/+source/postgresql-common/+git/postgresql-common/+merge/422669

and will also file a Debian bug to include the Debian maintainers in the discussion before we do introduce the delta here.

This bug was fixed in the package postgresql-common - 241ubuntu1

---------------
postgresql-common (241ubuntu1) kinetic; urgency=medium

  * d/rules: do not stop service on package upgrades. (LP: #1973382)

 -- Athos Ribeiro <email address hidden> Mon, 16 May 2022 09:58:44 -0300

It's unclear to me from a reading of this diff that the changes to debian/rules are correct. --no-start --no-stop-on-upgrade reads to me as meaning the service will not be restarted on upgrade. That means security updates will not be applied to the running service, which is the expected behavior by default.

The dh_installsystemd manpage appears to agree with me:

       -r, --no-stop-on-upgrade, --no-restart-on-upgrade
           Do not stop service on upgrade. This has the side-effect of not
           restarting the service as a part of the upgrade.

Why is --no-start there in the first place?

There is also the --restart-after-upgrade option to dh_installsystemd, which the manpage says "This is the default behaviour in compat 10." And we're using compat 13....

Of course, the postgresql-common package doesn't actually ship the daemon; that's shipped in the versioned 'postgresql' package. So perhaps the concerns about not restarting on upgrade of postgresql-common are moot, as long as the versioned package itself handles restarting?

Thanks for the review, Steve!

> Why is --no-start there in the first place?

This was introduced in [1]. It is there because the versioned package handles restarting, as stated next in

> So perhaps the concerns about not restarting on upgrade of postgresql-common are moot, as long as the versioned package itself handles restarting?

With the current jammy delta in debhelper/debconf, the next build of postgresql-common will result in the following snippet being added in the postinst.

+if [ -z "${DPKG_ROOT:-}" ] && [ "$1" = upgrade ] && [ -d /run/systemd/system ] ; then
+ deb-systemd-invoke stop 'postgresql.service' >/dev/null || true
+fi

This will stop the service and not re-start it. The desired outcome of applying the proposed fix is to maintain the postinst script as it is now, for the next time the package is re-built (preventing a regression).

[1] https://salsa.debian.org/postgresql/postgresql-common/-/commit/36a608c4f69b47d546192a686b4ad7f8e86f9493

Thanks, that explanation suffices.

Since the current package in jammy isn't broken and this only affects autopkgtests and rebuilds, this should like LP: #2007794 be block-proposed.

(which it already is)

Hello Athos, or anyone else affected,

Accepted postgresql-common into jammy-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/postgresql-common/238ubuntu0.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-jammy to verification-done-jammy. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-jammy. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

All autopkgtests for the newly accepted postgresql-common (238ubuntu0.1) for jammy have finished running.
The following regressions have been reported in tests triggered by the package:

resource-agents/1:4.7.0-1ubuntu7 (amd64)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/jammy/update_excuses.html#postgresql-common

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

The regressions above were not related to the proposed changes. re-triggering some tests fixed the issue.

Tests pass as designed in the test plan. Moreover, There is no logical changes in the new package control information files.