[sru] sos upstream 4.7.0

[Arif Ali]

[ Impact ]

* A new sosreport version is available upstream, and following the https://wiki.ubuntu.com/SosreportUpdates policy for stable release exception we will now SRU the sosreport 4.7.0 upstream release.

* This release introduces a couple of new plugins as well as bugfixes done between the previous Ubuntu 4.5.6 sosreport version and the new 4.7.0.

[ Test Plan ]

 * The detailed test plan can be found on https://wiki.ubuntu.com/SosreportUpdates and should be tested by a couple of users.

[ Where problems could occur ]

* Some plugins might stop working, and this will show when running sosreport.

* Uploading to S3 needs the python3-boto3 dependency installed.

* If there is an issue with a plugin that is causing instability to the system, this can be disabled by running 'sosreport -n <plugin_name>'.

* Currently running sos collect with Juju 3 is -not- working, even for the current 4.5.6 Ubuntu version, we are working upstream on a fix here: https://github.com/sosreport/sos/pull/3422 this is not part of the core functionality of sos, and one can still run sos report manually on the nodes.

[ Other Info ]

* Useful plugins that are now part of 4.7.0 and could do some testing:

* runtime/lxd We have added an LXD runtime so that we can gather data directly from LXD, same with what one would do with k8s.
* plugins/canonical_livepatch_onprem.py will gather data from livepatch-server
* plugins/ceph.py add Reef release commands and gather microceph data
* plugins/coredump.py will capture coredump info from coredumpctl
* plugins/infinidat.py will gather data from servers that make use if the Infinidat storage solution
* plugins/kubernetes.py will now gather container information from microk8s
* plugins/mellanox_firmware.py will gather data from mellanox devices
* plugins/openstack_masakari.py will gather data from openstack masakari
* plugins/openstack_masakarimonitors.py will gather data from openstack masakari monitors
* plugins/vectordev.py will gather the config from vectordev (cos-proxy)

[Arif Ali (arif-ali)]

The above debdiffs have been added, and ready for verification

We are waiting for the 4.4 main patch for bionic in #2038648 to be uploaded before we crack on with the Pro update for bionic for 4.7.0

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "noble-sosreport.debdiff" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

[Lucas Kanashiro (lucaskanashiro)]

@Arif are the attached debdiffs ready to be uploaded? I do not know if I get it right, but the bug you mentioned is not yet fixed:

https://bugs.launchpad.net/ubuntu/+source/sosreport/+bug/2038648

If this is ready for ~ubuntu-sponsors, please state so. If not, please unsubscribe ~ubuntu-sponsors and subscribe again once it is ready.

[Arif Ali (arif-ali)]

@Lucas, it is ready.

We were planning to resolve that bug as part of the whole SRU of 4.7.0.

When n/m/j/f are done, we'll get the bionic one done too, and that will be 4.7.0 as well, I hope that makes sense.

Or, would you prefer us to fix the current version there first, and then do the SRU for 4.7.0?

EDIT: in-fact I already uploaded a debdiff on that LP, so that is ready for SRU for that fix too

[Arif Ali (arif-ali)]

new debdiff with new oracular being the version, and building on top of the 2 versions that were not there previously

[Andreas Hasenack (ahasenack)]

I pushed the oracular debdiff to a git repo at
https://code.launchpad.net/~ahasenack/ubuntu/+source/sosreport/+git/sosreport/+ref/oracular-patch-pilot-sosreport-update-2054395

I'm slightly concerned with the d/copyright changes. We seem to be removing attributions from others:

diff -Nru sosreport-4.5.6/debian/copyright sosreport-4.7.0/debian/copyright
--- sosreport-4.5.6/debian/copyright 2024-04-14 22:45:27.000000000 +0100
+++ sosreport-4.7.0/debian/copyright 2024-05-01 07:51:23.000000000 +0100
@@ -1,36 +1,8 @@
 Format: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
 Upstream-Name: sosreport
-Upstream-Contact: Bryn M. Reeves <email address hidden>
+Upstream-Contact: Jake Hunsaker <email address hidden>
 Source: https://github.com/sosreport/sos

-Files: *
-Copyright: 2012-2013 Bryn M. Reeves
- 2007-2013 Red Hat, Inc.
-License: GPL-2+
-
-Files: sos/report/plugins/kernelrt.py
-Copyright: 2012 Red Hat, Inc.
-License: GPL-2
-
-Files: debian/*
-Copyright: 2013 Adam Stokes <email address hidden>
-License: GPL-2+
-
-License: GPL-2
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License
- as published by the Free Software Foundation; version 2.
- .
- This application is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- General Public License for more details.
- .
- You should have received a copy of the GNU General Public License
- along with this program; if not, write to the Free Software
- Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301,
- USA.
-
 License: GPL-2+
  This program is free software; you can redistribute it and/or modify
  it under the terms of the GNU General Public License as published by

For example, the plugin sos/report/plugins/kernelrt.py is still shipped, and has the Red Hat Inc copyright text in it.

I know this change was done in the upstream git repo[1], but one of the authors is the same one pushing the change here. Previously all debian/* files were removed[2], but what was restored in [1] is not the same d/copyright.

1. https://github.com/sosreport/sos/commit/314cc502ac862494973712537bf7a7a0c34f96d8
2. https://github.com/sosreport/sos/commit/098d19684456eceb6bc2cea094b30effa19c37fc

[Arif Ali (arif-ali)]

Hi Andreas,

This was discussed in the PR [1], let me see if I can answer your questions

* All the files that need a copyright have the copyright at the top of the files, and hence not therefore require here. If we started to add all the copyright stuff, then this file would be considerably long
* The upstream core maintainer changed a few years ago, and Bryn is still one of the old maintainers, and he didn't have any issues with the change.
* I can change the debian/* back again, if that needs to change to have Adam the related copyright.

One of the main reasons of up-streaming the debian folder was to improve the CI testing, especially with some of the tests we had failed as part of SRU previously. The key thing was to be consistent upstream and downstream.

Happy to discuss the best course of action moving forward on this

[1] https://github.com/sosreport/sos/pull/3409#discussion_r1387216668

[Andreas Hasenack (ahasenack)]

Looking at [1], it states:

  Similarly, plain text files which include their own copyright information and are installed into
  the binary package unmodified need not have that copyright information copied into
  /usr/share/doc/PACKAGE/copyright

So in principle it's ok to drop the specific entry for the kernelrt.py plugin.

The debian/* entry I don't know, I see many packages have an entry like that, while others don't. The files in debian/* themselves don't have any copyright note on them, and not all are part of the final binary installation. On the other hand, removing this stanza from d/copyright means we also lose the information on who did the original packaging.

But another change is removing GPL-2+ for "Files: *". I'm not sure it was even correct before, as the LICENSE file in the source tree is clearly just "GPL-2" (no +). So we would have to track that one down.

See how this can get complicated very quickly? Changes to d/copyright need to be meticulously verified. I would prefer they are not done here, unless such careful verification and cross-checking with the debian policy is done, and also coordination with the same package in debian.

So if you want to change d/copyright, my recommendation would be to:
- do that in a branch of its own
- coordinate with debian, so both debian and ubuntu have the same file. Might need to update the version of the package in debian first, though (which would also benefit us both)

1. https://www.debian.org/doc/debian-policy/ch-archive.html#s-pkgcopyright

[Arif Ali (arif-ali)]

Sounds good to me, I'll revert the copyright change and upload new debdiffs for all. We can tackle the copyright file in a later SRU.

[Athos Ribeiro (athos-ribeiro)]

Thanks, Arif, the changes LGTM. I have a couple of questions:

- The maintainer field in d/control was reset. Was this on purpose?
- Version 4.7.1 is out since 2024-04-08; I understand this was first prepared prior that date. Still, I should ask: would you rather prepare 4.7.1 here to avoid SRUing a minor version shortly after this one lands?

[Athos Ribeiro (athos-ribeiro)]

Oh, there is one additional issue:

debdiff-apply failis with

File man/en/sosreport.1 is not a regular file -- refusing to patch
2 out of 2 hunks ignored

...

Traceback (most recent call last):
  File "/usr/bin/debdiff-apply", line 382, in <module>
    sys.exit(main(sys.argv[1:]))
             ^^^^^^^^^^^^^^^^^^
  File "/usr/bin/debdiff-apply", line 331, in main
    debdiff_apply(patch, patch_name, args)
  File "/usr/bin/debdiff-apply", line 207, in debdiff_apply
    call_patch(patch_str, "--dry-run", "-f")
  File "/usr/bin/debdiff-apply", line 139, in call_patch
    return subprocess.run(
           ^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/subprocess.py", line 571, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['patch', '-p1', '--dry-run', '-f']' returned non-zero exit status 1.

I can still apply it with `patch`, but it would be nice to check that (it seems it is trying to patch that symlink). How are you generating the debdiff?

[Erich Eickmeyer (eeickmeyer)]

@athos-ribeiro

A debdiff should most certainly not be trying to patch a symlink. I'd recommend trying it with `patch -p1 < {debdiff}` and build the source package to see if it works. If it complains about changes to the source that aren't reflected, then yes, there's a problem with the debdiff, but that's usually overcome by extracting the source.

[Arif Ali (arif-ali)]

@athos-ribeiro @eeickmeyer

I created the debdiff by comparing the 2 .dsc files, and indeed it include the file, as well it's symbolic linked file to be changed. I have re-ran the debdiff locally again this morning, and I can see the same thing. I can remove the offending section, and have been able to do a debdiff-apply; I can upload this debdiff if required.

The maintainer was originally changed by Julian on version 4.5.6-0ubuntu3 when a rebuild was done, not sure why, and this is for the case for only noble and oracular; the others are still the old maintainer. We are changing the maintainer soon, but we need to change a few things internally for that and hence were going to leave it as is.

The upstream project minor releases are created as a requirement for RedHat, and hence the primary maintainer that is RH does those releases. The core release cadence is 6 months, February and August. We plan to stick to that release process and potentially one intermediate. We have an internal testing criteria that can take some time to get our releases out, and hence the SRU was started a month after release, and you can see the release numbers in my PPA https://launchpad.net/~arif-ali/+archive/ubuntu/sosreport-dev.

Happy to amend anything that is required and get this moving forward.

[Athos Ribeiro (athos-ribeiro)]

Thanks for the reply, Arif.

Regarding the new upstream version: I understand it is not practical to go for 4.7.1 now given your explanation. Let's go with 4.7.0 then.

As for the bogus debdiffs, please, fix them and make them apply cleanly with debdiff-apply. This will at least slightly reduce the burden on the sponsors, given that in the current state, they will either need to make the changes themselves or pay attention to the patch reject file generated in the process.

Finally, for the maintainer field in d/control, please go through https://wiki.ubuntu.com/DebianMaintainerField so you understand why Julian did change that field in the past. However, dpkg also carries a change due to LP: #1951988, making dpkg-source also accept @canonical.com addresses. This is fair based on the reasoning discussed in the wikipage linked here. Hence, the change should be OK, and we should most likely update that wiki page.

[Athos Ribeiro (athos-ribeiro)]

Thanks, Arif.

Thanks for fixing the typo in the focal changelog.

For oracular, I see 2 new patches for cirrus (I see one of them is also present for noble). Why are we patching that file at all? AFAIU, it is a file parsed by the CI, and it is not even sipped in the package.

[Arif Ali (arif-ali)]

The new patch that was added at the end was about the msr module loading and was also present in the CI testing we found upstream and hence added it on.

In the original patch that is in noble it was == 24.04 and now it's >=24.04 this will ensure it covers any series that is noble and above, I hope that makes sense.

The second patch depends on the change of the first patch based on the git history. In essence the work was combined upstream to fix the issue when I added oracular to the testing. These patches have been directly imported from upstream and no extra modification is done. If you prefer that I remove the CI bits, I can do so, but that would mean dissecting the patch and won't be a direct import

When and if we ship 4.7.3 or 4.8.0 later in the year, these would be already included, including the CI bits

[Athos Ribeiro (athos-ribeiro)]

I see.

So, patch debian/patches/0006-cirrus-Update-images-add-noble-to-supported-list.patch is not needed here then, right?

And the changes in patch .cirrus.yml in patch debian/patches/0007-cirrus-Start-testing-on-new-devel-ubuntu-daily.patch are also not needed, right?

If my assumptions above are correct, you can

- remove patch 0006;
- remove the changes for the .cirrus.yml file from patch 0007, this way patch 0006 will no longer be needed;
- rename patch 0007 to a more meaningful name (this is not only about testing, right?).

[Arif Ali (arif-ali)]

Updated debdiff as suggested

[Athos Ribeiro (athos-ribeiro)]

Thanks, Arif.

The contents are correct as per the upstream release.

For the `X-Python3-Version: >= 3.6` entry in d/control, I kept wondering if it is this needed in anything other than bionic and I wonder how the SRU team will look at that. Unless we are supporting upgrade paths that would skip a release, e.g., upgrading directly from xenial to focal, jammy, or noble, this would have no effect at all in all but a bionic debdiff. No need to remove the entry though, I am just leaving this comment as a note for the SRU review later (let the SRU team decide on this one). Still, if you want to keep it, you will need to file the SRU template paperwork in the related bug (LP: #2038648).

In oracular, the last patch added is now a variation of an upstream commit so it applies correctly to the current version. In that case, you can keep the original patch file with just that hunk that did not apply removed. The Origin DEP3 header can be set to "backport, GITHUB_COMMIT_URL".

I also have a nitpick for d/patches:
on the "Origin" DEP3 header field entries, would you mind pointing to commits instead of PRs? this would make it clearer to whoever looks at the patch that the change was already accepted upstream without the need to open the URLs to verify if that is the case.
      e.g., instead of
      Origin: upstream, https://github.com/sosreport/sos/pull/3545
      use
      Origin: upstream, https://github.com/sosreport/sos/commit/7e81246fcfd35e17a69bc3d40400cd123e8ca7bf
      etc.

The mantic's debian/changelog file is missing entries for python3-packaging and python3-yaml. I did not review jammy and focal assuming they will also have similar instances of the cases listed above. Let's get oracular in a "ready to upload" shape, then we can go back fixing the SRUs, WDYT?

[Arif Ali (arif-ali)]

Thanks for your comments, and the amendments that you have asked for. The relevant Origin + Bugs for DEP3 have all been updated on all the debdiffs. I have also updated mantic changelog, and focal to be consistent.

To answer your question on the `X-Python3-Version: >= 3.6`, this is explicitly showing that this sosreport package supports 3.6 and above, and we therefore ensure it as part of the packaging process.

This ensures that when upstream decide to move on from 3.6, i.e. 3.8 later in the year, we'll bump this as part of one of the SRUs. This is explained in comment 35 [1].

I hope that makes sense, and let me know if there is anything else. I am hoping we can now get this over the line

[1] https://bugs.launchpad.net/ubuntu/+source/sosreport/+bug/2038648/comments/35