Merge ~waveform/ubuntu/+source/open-iscsi:merge-2.1.5-jammy into ubuntu/+source/open-iscsi:debian/sid

Proposed by Dave Jones
Status: Needs review
Proposed branch: ~waveform/ubuntu/+source/open-iscsi:merge-2.1.5-jammy
Merge into: ubuntu/+source/open-iscsi:debian/sid
Diff against target: 5527 lines (+5262/-16)
16 files modified
debian/README.Debian (+2/-2)
debian/changelog (+1337/-0)
debian/control (+13/-2)
debian/extra/startup-checks.sh (+1/-1)
debian/open-iscsi-udeb.start (+1/-1)
debian/open-iscsi.postinst (+1/-1)
debian/rules (+4/-9)
debian/tests/README-boot-test.md (+139/-0)
debian/tests/control (+4/-0)
debian/tests/get-image (+227/-0)
debian/tests/patch-image (+374/-0)
debian/tests/test-open-iscsi.py (+427/-0)
debian/tests/testlib.py (+1488/-0)
debian/tests/testsuite (+6/-0)
debian/tests/tgt-boot-test (+534/-0)
debian/tests/xkvm (+704/-0)
Reviewer Review Type Date Requested Status
Christian Ehrhardt  (community) Approve
git-ubuntu import Pending
Review via email: mp+414361@code.qastaging.launchpad.net
To post a comment you must log in.
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

open-iscsi | 2.1.5-1 | testing | source, amd64, arm64, armel, armhf, i386, mips64el, mipsel, ppc64el, s390x

open-iscsi | 2.1.4-0ubuntu3 | jammy | source, amd64, arm64, armhf, ppc64el, riscv64, s390x

This is after a massive pick-up of our Delta by Debian in 2.1.4-1 so you had a lot to drop :-)

I found no auxiliary tags for the merge like old logical and such, I'll try to review as-is without them.

* Changelog:
  - [+] changelog entry correct version and targeted codename
  - [+] changelog entries correct
  - [+] bug references correct
  - [+] update-maintainer has been run

* Merge - Indirect Changes:
  - [+] no upstream changes that need adaptation
  - [+] no further upstream version to consider
  - [+] debian changes look safe

* Merge - Old Delta:
  - [+] dropped changes are ok to be dropped
  - [x] nothing else to drop
        Why does debian not need "d/t/test-open-iscsi.py: adopt to resolvectl (systemd v249
        compat)" they are >=249 too. If the check reveals it can be dropped with 250 please
        mention that.
  - [x] changes forwarded upstream/debian
        I've not seen it, did you forward "d/rules: remove duplicated dh_installsystemd section"
        as that seems to affect Debian as well?

* New Delta:
  - [+] no new patches added

* Git/Maintenance
  - [+] testcases added or not needed for this (has enough)
  - [+] commits are properly split (more important on -dev than on SRUs)

* Build/Test:
  - [+] build on all arch look ok
  - [+] verified PPA package installs/uninstalls
  - [x] autopkgtest against the PPA package passes
     Did you run autopkgtest against this PPA yet?
     I've seen none, remember you can check and trigger results on the PPA via
     $ lp-test-ppa ppa:waveform/open-iscsi --release jammy --showpass
     (from git+ssh://<email address hidden>/~ubuntu-server/ubuntu-helpers)

I had no time for sanity checks as I have no iscsi env atm.
But once the autopkgtests have run that should be fine.

Need-Info for the few questions above, but looks like 98% done and great :-)

review: Needs Information
Revision history for this message
Dave Jones (waveform) wrote :

> I found no auxiliary tags for the merge like old logical and such, I'll try to
> review as-is without them.

Yes, I didn't directly use git-ubuntu as there's (still) no arm builds for it. So
I just did a rebase of the old split plus the subsequent versions on top of it.

> Why does debian not need "d/t/test-open-iscsi.py: adopt to resolvectl (systemd v249
> compat)" they are >=249 too. If the check reveals it can be dropped with 250 please
> mention that.

Because the entire autopkgtest suite is part of our delta and not upstream either.
I'm guessing there are good reasons for that, possibly hinted at by the note in
the git commit "this suite should be replaced by LIO that replaced TGT in main"? In
other words, presumably the test suite is intended to be replaced by something better
(LIO?) at some point (but what's TGT ... and which main? I'm just guessing at
stuff here).

> I've not seen it, did you forward "d/rules: remove duplicated dh_installsystemd
> section" as that seems to affect Debian as well?

I've *reported* it upstream (https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1004014),
but not sent our patch as we only activate the socket, whilst Debian activates both
socket *and* service in d/rules, so our patch likely isn't what they want.

That said, I'm not entirely sure what the intent is in Debian for this, as they did
merge other socket-activation-only bits but kept d/rules largely unchanged? Anyway,
not being certain what the intent is, I warned about the duplication but will leave
it to their discretion how they wish to fix it.

> Did you run autopkgtest against this PPA yet?
> I've seen none, remember you can check and trigger results on the PPA via
> $ lp-test-ppa ppa:waveform/open-iscsi --release jammy --showpass
> (from git+ssh://<email address hidden>/~ubuntu-server/ubuntu-helpers)

Erm, not sure I can -- I don't think I have the authority to launch autopkgtests
(your otherwise fine script runs with no error exit code, but doesn't appear to
actually launch for me).
anything for me)

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Thanks for the explanation and filing that Debian bug.

So the new delta can next time be folded with our autopkgtest delta.

Tests are good as well (as discussed in chat I started them for you):
  open-iscsi @ amd64:
    20.01.22 13:05:09 Log 🗒️ ✅ Triggers: ['open-iscsi/2.1.5-1ubuntu1']
      install PASS ✅
      testsuite PASS ✅
      nested PASS ✅
  open-iscsi @ arm64:
    20.01.22 13:04:24 Log 🗒️ ✅ Triggers: ['open-iscsi/2.1.5-1ubuntu1']
      install PASS ✅
      testsuite PASS ✅
      nested PASS ✅
  open-iscsi @ armhf:
    20.01.22 12:50:30 Log 🗒️ ✅ Triggers: ['open-iscsi/2.1.5-1ubuntu1']
  open-iscsi @ ppc64el:
    20.01.22 12:58:44 Log 🗒️ ✅ Triggers: ['open-iscsi/2.1.5-1ubuntu1']
      install PASS ✅
      testsuite PASS ✅
      nested PASS ✅
  open-iscsi @ s390x:
    20.01.22 12:55:20 Log 🗒️ ✅ Triggers: ['open-iscsi/2.1.5-1ubuntu1']
      install PASS ✅
      testsuite PASS ✅
      nested PASS ✅

Armhf skips (ok) as they have no isolation-machine

Therefore +1 then, LGTM

P.S. yes LIO is the new and better userspace compared to TGT

review: Approve
Revision history for this message
Dave Jones (waveform) wrote :

Do I need to do anything else to have this merged (preferably preserving the branch history for future users?), or is that an automatic process at this point?

Revision history for this message
Dave Jones (waveform) wrote :

Hmmm, just ran into something rather worrying while taking a look at the dbus merge: apparently recent versions of debhelper (including our version in jammy) have broken --no-stop-on-upgrade (or its deprecated alias --no-restart-on-upgrade): https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=994204

That would imply that open-iscsi *will* restart on upgrade when this merge gets built by this version of debhelper. In dbus, Debian have worked around this (https://salsa.debian.org/utopia-team/dbus/-/commit/4c5195a13c69364dce50063afac368930ec75c91), but I don't really want to start playing whack-a-mole patching things that shouldn't be restarted (I'd rather get this fixed in debhelper itself). However, if we do fix this in debhelper, we should hold off on merging this until that fix lands (so that the resulting package gets built "correctly").

Revision history for this message
Lucas Kanashiro (lucaskanashiro) wrote (last edit ):

The debhelper bug mentioned in the comment above was already fixed in Ubuntu (more info LP: #1959054). Since the changes submitted here were already reviewed and approved by Christian, I'll be moving forward and uploading this version of open-iscsi before we reach feature freeze.

Revision history for this message
Lucas Kanashiro (lucaskanashiro) wrote :

Package uploaded:

$ dput ubuntu ../open-iscsi_2.1.5-1ubuntu1_source.changes
Checking signature on .changes
gpg: ../open-iscsi_2.1.5-1ubuntu1_source.changes: Valid signature from F823A2729883C97C
Checking signature on .dsc
gpg: ../open-iscsi_2.1.5-1ubuntu1.dsc: Valid signature from F823A2729883C97C
Package includes an .orig.tar.gz file although the debian revision suggests
that it might not be required. Multiple uploads of the .orig.tar.gz may be
rejected by the upload queue management software.
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading open-iscsi_2.1.5-1ubuntu1.dsc: done.
  Uploading open-iscsi_2.1.5.orig.tar.gz: done.
  Uploading open-iscsi_2.1.5-1ubuntu1.debian.tar.xz: done.
  Uploading open-iscsi_2.1.5-1ubuntu1_source.buildinfo: done.
  Uploading open-iscsi_2.1.5-1ubuntu1_source.changes: done.
Successfully uploaded packages.

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches