Update focal, jammy to landscape-client 23.02

Status changed to 'Confirmed' because the bug affects multiple users.

Hi Mitch,

Some thoughts:
* looks like Lunar already has 23.02, so I think that can be skipped. Also maybe confirm that you want to target kinetic.
* I don't have the button showing, but if you have the "target to series" button, please do so for the series you would like to target.
* Please consider a "Where problems could occur" section

In the meantime I have unsubscribed Ubuntu Sponsors, please re-subscribe Ubuntu Sponsors when this is ready.

Thanks!

Thanks Dan!

* I've removed Lunar and Kinetic from the targets
* I have the "target to series" button, but it only gives me options in the landscape-client project, not the ubuntu project, so I can't change it to target focal and jammy.
* I didn't originally have a "Where problems could occur" section because we don't anticipate any. I've added one essentially to that effect.

Hi,

I've changed the metadata to reflect your last update. However, would it be possible to give an explicit rationale for the Won't Fix in Kinetic? I'm guessing it's due to Kinetic being almost EOL combined with the server-centric nature of the package ?

Also, I'm a bit confused by the debian/changelog part of the debdiff, as it removes Jammy-related entries to add a Focal one. Would it be possible to generate one for each targetted release against the relevant versions currently in the archive to clarify things?

In addition, it's usually required to explicitly mark out all the fixed bugs in the changelog entry via a LP: #XXXXXXX stanza so that their status can be automatically updated as the package goes through the motions.

I unsubscribed ~ubuntu-sponsors. Please resubscribe ~ubuntu-sponsors once you addressed Simon's comment.

debdiff for focal.

Rationale for won't-fix for kinetic is as you say: kinetic will be unsupported shortly and Landscape does not have a need to support beyond that. We have PPAs for users who wish to install newer landscape-client versions on unsupported series.

I've attached debdiffs for both focal and jammy. Changelog should now list all of the bugs mentioned in the description here.

> In addition, it's usually required to explicitly mark out all the fixed bugs
> in the changelog entry via a LP: #XXXXXXX stanza so that their status can be
> automatically updated as the package goes through the motions.

I'm sorry, but this is upside-down advice for SRUs of packages with documented SRU exception processes. Linking all of the bugs in the changelog means that the SRU process will enforce verification of each of those bugs, instead of using this single bug report for the SRU as expected. There is an ongoing discussion about certain cases where additional launchpad bug refs naturally wind up in the changelog of packages with SRU exceptions and what to do there <https://lists.ubuntu.com/archives/ubuntu-release/2023-June/005656.html>, but this isn't one of those cases, you've explicitly gone back and added them.

I would just edit the changelog to remove the launchpad bug refs and upload, *except*, that there are changes in the packaging under debian/. There is some discussion on https://wiki.ubuntu.com/LandscapeUpdates about packaging, which is good, but it's my position as a member of the SRU team (a position I believe is widely shared by the SRU team despite not being explicitly documented) that packaging changes never fall under the SRU exception policy but require their own validation in the SRU process.

In particular:

 override_dh_installsystemd:
- dh_installsystemd --no-enable --no-start
+ dh_installsystemd --no-enable --no-start --no-stop-on-upgrade

Unfortunately dh_installsystemd is, in a word, a trainwreck. Most of the options have semantics that are unintuitive from their names, and many have inscrutable side effects. To sponsor this, I would need a separate bug that explains why you are making this change, what you expect the effect to be, and what testing will be done to confirm correct behavior across the various relevant scenarios (new install; upgrade with landscape client running; upgrade with landscape client not running).

At a glance, I would *infer* that the reason for this change is that the upgrade of landscape-client may be happening under landscape-client itself, therefore stopping it on upgrade could leave the system in an inconsistent state. And that makes sense, but also if we're not restarting the client on upgrade that means we don't get security updates applied to the running process... Ideally we would have something like openssh, where a service restart will restart the daemon but not any running sessions.

Then you also have:

- [ -f /var/run/landscape/landscape-client.pid ] && kill -s USR1 `cat /var/run/landscape/landscape-client.pid` > /dev/null 2>&1 || :
+ systemctl kill --signal=USR1 --kill-who=main landscape-client > /dev/null 2>&1 || :

That's straightforward and explained by the linked bug #1968189. However, 'systemctl kill' is a rather surprising interface to use. I see that SIGUSR1 is used to tell landscape-client to reopen its logs after rotation, so it makes sense that you don't want to use 'systemctl restart' to restart the whole service; but this is fairly non-obvious. Could you add a comment explaining this rationale for not using 'systemctl re...

>In particular:
>
> override_dh_installsystemd:
>- dh_installsystemd --no-enable --no-start
>+ dh_installsystemd --no-enable --no-start --no-stop-on-upgrade
>
>Unfortunately dh_installsystemd is, in a word, a trainwreck. Most of
>the options have semantics that are unintuitive from their names, and
>many have inscrutable side effects. To sponsor this, I would need a
>separate bug that explains why you are making this change, what you
>expect the effect to be, and what testing will be done to confirm
>correct behavior across the various relevant scenarios (new install;
>upgrade with landscape client running; upgrade with landscape client
>not running).

As one of those (in part) responsible for this trainwreck, I feel I
should jump in with some extra words of caution here.

*If* the intent of this change is to ensure that the landscape-client
service does not restart at all on upgrade, then this change is correct
and *potentially* safe on jammy (22.04), though it should be tested for
reasons that will become obvious below (it should also, as Steve has
noted, be documented).

On focal (20.04), it is absolutely not safe and can lead to the daemon
stopping and never restarting. When --no-start is used, the default
restart behaviour in *some* versions of debhelper is changed from
--restart-after-upgrade (the current default) to
--no-restart-after-upgrade (the older default). In versions of debhelper
prior to 13.6 in jammy, --no-restart-after-upgrade placed the "stop"
action in the prerm of the *old* version of the package, and the "start"
action in the postinst of the *new* version. This means that if you
transition from the --no-restart-after-upgrade behaviour to
--no-stop-on-upgrade, the "stop" action still takes place (because the
old version of the package on the system still has this in its prerm)
but your shiny new version won't attempt to start or restart the package
in its postinst, leaving the service dead.

If you need clarification on any of the above, please feel free to reach
out to me on IRC/MM but I would urge careful testing of any such change,
most especially on releases prior to jammy where this bug is still
extant in debhelper.

Thanks for looking at this Steve, and thanks for the additional context Dave.

Steve, I'm not sure what the takeaway is from the "individual bugs in changelog" part of this discussion. Should I remove the bug refs except for the ones that impact packaging changes under debian/ ? If so, I'll do that right away.

Your inference is correct, this change was made to accommodate cases where landscape-client is upgrading itself (see https://github.com/CanonicalLtd/landscape-client/commit/7448c4924e111493f214be9c4f06779653756ecf). I'll create a bug to address this change. I've already performed some testing, as detailed in https://wiki.canonical.com/Landscape/ClientSRUTests/23.02, but noticeably missing from those testcases is "upgrade with landscape client not running", so I'll get that done and update the testing report.

I've added a comment to LP:1968189 that I hope adequately explains the use of systemctl kill in that change.

I've created LP:2027613 to document the change to override_dh_installsystemd, as mentioned above. I've also completed manual testing of upgrading landscape client when the service is not running, and the service remained down, as expected. As mentioned in prev comment, I have previously done other upgrade testing on both focal and jammy.

I've also removed all of the bug references from the new changelog entry, as advised by vorlon.

When I sponsored this for lunar way back, I guess I didn't think about all the consequences of adding --no-stop-on-upgrade to the systemd unit installation, together with the already existing --no-start.

I decided to do a quick test, and registered a jammy lxd container with landscape with the jammy landscape-client (19.12). I then built 23.02 in a ppa, added that to the container, and asked landscape to upgrade landscape-client. The upgrade happened just fine, but the landscape-client processes stayed the same, i.e., no restart:

   3256 ? Ss 0:03 /usr/bin/python3 /usr/bin/landscape-client
   3257 ? Sl 0:03 \_ /usr/bin/python3 /usr/bin/landscape-broker --ignore-sigint
   3258 ? Sl 0:03 \_ /usr/bin/python3 /usr/bin/landscape-monitor --ignore-sigint
   3259 ? S 0:01 \_ /usr/bin/python3 /usr/bin/landscape-manager --ignore-sigint

(before and after)

With 19.12, if there were a point release for it without the systemd change, what exactly would happen? It would upgrade the landscape-client package, but not start the new version, and the machine would remain without a running landscape-client?

And now, I can see it will keep the old landscape-client running, is there a plan to address this? https://bugs.launchpad.net/ubuntu/+source/landscape-client/+bug/2027613 mentions the rationale, but not how to address restarting landscape-client after a package upgrade.

FWIW, this is the PPA I used to test: https://launchpad.net/~ahasenack/+archive/ubuntu/landscape-client-sru-23.02/

I see that the jammy debdiff uses version 23.02-0ubuntu1 in the changelog. This is the version number of the package in lunar. We need to be using 23.02-0ubuntu1~22.04.1 here.

> Steve, I'm not sure what the takeaway is from the "individual bugs in changelog"
> part of this discussion. Should I remove the bug refs except for the ones that
> impact packaging changes under debian/ ? If so, I'll do that right away.

Sorry that this was unclear.

The changelog should include references to LP: #2006402, which is the process bug for the landscape-client SRU exception; and LP: #2027613 which covers the maintainer script behavior changes that need special attention.

Currently debian/changelog includes neither of these...

Mitch, as these changes are not strictly mechanical, I'd ask you to make these changes and resubmit for sponsorship.

Steve, I've updated the changelog to reference both this bug and LP: #2027613. I've also updated the version numbers to include ~22.04.1 for jammy and ~20.04.1 for focal.

Andreas, I'm not sure what the best approach to address this is. I'll admit ignorance to the subtleties of these kinds of systemd changes. Is an explicit --restart-after-upgrade feasible?

I found this in sponsoring queue, the changelogs look better now in regard to what was asked above.
But this isn't ready yet as the discussion on the service handling is still ongoing, I'll comment to those in LP: #2027613

Updated diffs in light of discussion on #2027613 and the decided changes to landscape-client.service that acceptably resolve the systemd issues related with restarting the service on upgrade.

Great discussion on LP #2027613 . I believe that the solution proposed there seems to be what we need here regarding the systemd service.

The packaging changes look good to me now. I'd have sponsored the uploads if LP: #2027613 had the SRU template in the bug description, since this bug is also addressed in those uploads. Please, fix up the description in the mentioned bug.

I also recommend to revisit the SRU exception for the landscape-client [1], there are some steps in the process that I believe that are outdated. For instance, it explicitly requires a bzr branch for fix + tests but AFAICS the upstream project is now using git. It also requires approval of 2 landscape developers for each of those changes, and from the GitHub PRs it seems that just one is required to get anything merged. I'd write an email with proposed changes and send to the SRU team (via ubuntu-release mailing list).

[1] https://wiki.ubuntu.com/LandscapeUpdates

I've updated the description of #2027613 using the SRU template. We have it as an action item to review the SRU exception for landscape-client and propose changes.

The versioning implies a straight backport of the Ubuntu packaging in mantic (23.02-0ubuntu1~20.04.1), rather than a change to the Ubuntu package in focal (23.02-0ubuntu0.20.04.1), but they apply to the focal tree and miss the changelog entries from later releases.

I'd propose merging changelog with dpkg-mergechangelog such that all intermediate releases are included. Probably not necessarily building them with -v as that adds all the other bugs to the SRU.

Or alternatively, to change the versioning and explain all the changes in the SRU.

I've updated the changelogs using dpkg-mergechangelogs so the history is more fitting for a straight backport. Hope everything looks alright there.

I usually don't simply "bump," but it's been sitting in the sponsorship queue for a week and would be nice to get (a more qualified) review.

The debdiffs and changeslogs basically LGTM, I think this is ready for sponsoring. I'm not doing it right away as I noticed that now we have landscape-client 23.08-0ubuntu1 in mantic. This makes this SRU a bit odd (we're updating Ubuntu LTS releases to a new upstream version that is not packaged in anywhere else), and should we want to SRU 23.08-0ubuntu1 you'll need to perform the SRU verification twice.

@Mitch how do you prefer to proceed? Should we go ahead and sponsor your debdiffs, or do you prefer to to update them to SRU 23.08?

Hi Paride!

After conferring with the rest of the Landscape team, we've decided to go ahead with this SRU as-is, using the current debdiffs (for 23.03). I know it'll be a little odd and possibly a little awkward in the future, but we can deal with that as-needed.

Also, thank you @Simon for the "bump". I assumed such bumps from the requester are generally bad practice, and that the reviewers were just busy, otherwise I might've done something there myself.

Thanks!

Hello Mitch, or anyone else affected,

Accepted landscape-client into jammy-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/landscape-client/23.02-0ubuntu1~22.04.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.

Hello Mitch, or anyone else affected,

Accepted landscape-client into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/landscape-client/23.02-0ubuntu1~20.04.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-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. 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.

Thanks Steve!

In the process of testing, I discovered a regression introduced by the fix for LP: #2027613. This will need to be addressed before I will consider the package verified.

Added new debdiff addressing the recently discovered issue mentioned in previous comment. This includes a patch, 0001-start-service-during-config.patch that has been included upstream but not yet released.

I can also confirm that the version in -proposed (23.02-0ubuntu1~20.04.1) addresses the other referenced bugs. Testing details:

  - LP: #1995775 - generated a large error message from the package reporter and ensured it would run more frequently via slightly modifying installed package. Verified that the error reported to Landscape Server was truncated to a reasonable size.
  - LP: #2006396 - added tap interfaces to test machine, called landscape.lib.network.get_active_device_info and verified that no tap interfaces were included. Executed /usr/bin/landscape-sysinfo and verified that no tap interfaces were included in output.
  - LP: #1973202 - registered client with Landscape Server, set up a mirror pocket on Server and associated it to the client, verified that keys were stored in /etc/apt/trusted.gpg.d and not in legacy trusted.gpg file. Ensured no warning message about legacy keyring during apt-get update.
  - LP: #1827909 - created Parallels VM on MacOS machine, verified that VM type was properly reported as kvm after registration with Landscape Server.
  - LP: #1968189 - tailed landscape logfiles and executed logrotate --force /etc/logrotate.conf. Verified that output implied logfiles were rotated. Verified that new logfiles were created.
  - LP: #1947399 - registered client with Landscape Server, modified Server to respond to message exchanges with HTTP 500 responses, verified that client logged backoff related logs and delayed future message exchanges.
  - LP: #1917540 - registered client with Landscape Server, modified tags on the client side, but updated the next expected message sequence number Server-side before the message was sent. Verified that a valid message with the new sequence number was sent.
  - LP: #2027613 - see previous comment on this bug: new patch addresses issues discovered when testing this

  - LP: #1912516 - verified that landscape-config --is-registered had expected output and exit code when client was not registered and after registration.
  - LP: #2006401 - verified that Ubuntu Pro information was reported to Landscape Server during registration, and changes to Pro status was reported after registration.

All tests were done on both jammy and focal, but I'm not going to change the tag to verification-done-* until I hear back regarding the new patch.