Merge ~smoser/ubuntu/+source/open-iscsi:fix/1802354-disco-iscsid-run-if-rootfs into ubuntu/+source/open-iscsi:ubuntu/devel

[Scott Moser]

Add udev rule for iscsi disks to ensure iscsid is running.

This change makes open-iscsi install a udev rule that includes
systemd_wants [1] to ensure that if there are iscsi disks attached
then iscsid.service will be running.

 [1] https://www.freedesktop.org/software/systemd/man/systemd.device.html

This fixes a regression caused when iscsid was changed to be socket
activated under bug 1755858. The problematic case was when the
only iscsi mount was performed by the initramfs.

LP: #1802354

see commit message

[Steve Langasek (vorlon)]

LGTM.

review: Approve

diff --git a/debian/changelog b/debian/changelog
index e242fa5..135aac7 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,14 @@
+open-iscsi (2.0.874-5ubuntu10) UNRELEASED; urgency=medium
+
+  * d/iscsi-disk.rules, d/tests: Add a udev rule so that iscsid.service
+    will be run when udev disks are attached.  (LP: #1802354)
+  * debian/tests: improve tgt boot test
+     - disable snapd and snap.seeded services to avoid unnecessary
+       resource consumption during tests.
+     - save artifacts from the test run.
+
+ -- Scott Moser <[e-mail]>  Tue, 13 Nov 2018 20:00:31 -0500
+
 open-iscsi (2.0.874-5ubuntu9) cosmic; urgency=medium
 
   * d/net-interface-handler: replace 'domainsearch' with the correct
diff --git a/debian/iscsi-disk.rules b/debian/iscsi-disk.rules
new file mode 100644
index 0000000..e0ef836
--- /dev/null
+++ b/debian/iscsi-disk.rules
@@ -0,0 +1,3 @@
+# When iscsi disks are present, iscsid.service should be running. LP: #1802354
+# ID_PATH looks like ip-<ipv4-dotted-quad>:<port>-iscsi-<target>-lun-<lun>
+SUBSYSTEM=="block", ACTION=="add", ENV{ID_PATH}=="*-iscsi-*", ENV{SYSTEMD_WANTS}="iscsid.service"
diff --git a/debian/open-iscsi.install b/debian/open-iscsi.install
index 1e09755..14cddd4 100755
--- a/debian/open-iscsi.install
+++ b/debian/open-iscsi.install
@@ -15,4 +15,5 @@ debian/extra/activate-storage.sh       /lib/open-iscsi
 debian/open-iscsi.kmod              => /lib/modules-load.d/open-iscsi.conf
 debian/net-interface-handler           /lib/open-iscsi
 debian/iscsi-network-interface.rules => /lib/udev/rules.d/70-iscsi-network-interface.rules
+debian/iscsi-disk.rules => /lib/udev/rules.d/70-iscsi-disk.rules
 debian/open-iscsi.finalrd              /usr/share/finalrd
diff --git a/debian/tests/patch-image b/debian/tests/patch-image
index 95cacc5..0688912 100755
--- a/debian/tests/patch-image
+++ b/debian/tests/patch-image
@@ -251,6 +251,10 @@ main() {
             [ -f "${pkg}" ] && debs[${#debs[@]}]="$pkg" ||
                 packages[${packages[@]}]="$pkg"
         done
+        if [ "${#packages[@]}" -eq 0 -a "${#debs[@]}" -eq 0 ]; then
+            packages=( open-iscsi )
+            debug 1 "no packages or debs given, using packages=${packages[*]}"
+        fi
     fi
 
     # if open-iscsi is not in the packages list above, we handle it specifically
@@ -309,6 +313,7 @@ main() {
     mount -o bind /dev "$target"/dev || :
     mount -o bind /dev/pts "$target"/dev/pts || :
 
+    debug 1 "${#packages[@]} packages: ${packages[*]}"
     if [ "${#packages[@]}" != "0" ]; then
         DEBIAN_FRONTEND=noninteractive chroot "$target" sh -exc '
             mypkg=$1
@@ -322,6 +327,7 @@ main() {
             fail "failed to install ${packages[*]} in $target"
     fi
 
+    debug 1 "${#debs[@]} debs: ${debs[*]}"
     if [ "${#debs[@]}" != "0" ]; then
         local tmpd=""
         tmpd=$(mktemp -d "${target}/tmp/${0##*/}.XXXXXX")
diff --git a/debian/tests/test-open-iscsi.py b/debian/tests/test-open-iscsi.py
index 27a5e0b..deff8fc 100644
--- a/debian/tests/test-open-iscsi.py
+++ b/debian/tests/test-open-iscsi.py
@@ -75,6 +75,13 @@ bukket:
        exit
    fi
    systemd-resolve --status --no-pager
+
+ - &get_iscsid_status |
+   [ "${1:--}" != "-" ] && exec >"$1" 2>&1
+   udevadm settle
+   systemctl is-active iscsid.service
+   systemctl status --no-pager --full iscsid.service
+
  - &add_and_remove_tuntap |
    #!/bin/sh
    # LP: #1785108 would break dns when any device was removed.
@@ -86,11 +93,20 @@ bukket:
    ip tuntap del mode tap $tapdev
    udevadm settle
 
+ - &collect_debug_mounts |
+   [ "${1:--}" != "-" ] && exec >"$1" 2>&1
+   [ -x /usr/local/bin/debug-mounts ] || exit 0
+   /usr/local/bin/debug-mounts
+
 runcmd:
  - [ sh, -c, *add_and_remove_tuntap ]
  - [ mkdir, -p, /output ]
- - [ cp, /etc/resolv.conf, /output]
- - [ sh, -c, *get_resolved_status, --, /output/systemd-resolve-status.txt]
+ - [ cp, /etc/resolv.conf, /output ]
+ - [ sh, -c, *get_resolved_status, --, /output/systemd-resolve-status.txt ]
+ - [ sh, -c, *get_iscsid_status, --, /output/iscsid-status.txt ]
+ - [ sh, -c, *collect_debug_mounts, --, /output/debug-mounts.txt ]
+ - [ sh, -c, 'journalctl --boot=0 --output=short-monotonic > /output/journal.txt' ]
+ - [ sh, -c, 'dpkg-query --show > /output/manifest.txt' ]
  - [ tar, -C, /output, -cf, /dev/disk/by-id/virtio-output-disk, . ]
 
 power_state:
@@ -243,7 +259,7 @@ class CloudImageTest(testlib.TestlibCase, PrivateOpenIscsiTest):
         path = os.path.join(self.tmpdir, 'output-disk.img')
         subprocess.check_call([
             'qemu-img', 'create', '-f',  'raw', path, '10M'])
-	return path
+        return path
 
     def extract_files(self, path):
         # get contents in a dictionary of tarball at 'path'
@@ -273,6 +289,10 @@ class CloudImageTest(testlib.TestlibCase, PrivateOpenIscsiTest):
         netdev = ("--netdev=user,net={network},host={host},dns={dns},"
                   "dnssearch={dnssearch}").format(**info)
 
+        artifacts_dir = os.environ.get('AUTOPKGTEST_ARTIFACTS')
+        if artifacts_dir and not os.path.isdir(artifacts_dir):
+            os.makedirs(artifacts_dir)
+
         output_disk = self.create_output_disk()
         cmd = [
             tgt_boot_cmd, '-v', netdev,
@@ -286,8 +306,14 @@ class CloudImageTest(testlib.TestlibCase, PrivateOpenIscsiTest):
         env['BOOT_TIMEOUT'] = env.get('BOOT_TIMEOUT', '60m')
         subprocess.check_call(cmd, env=env)
 
+        if artifacts_dir:
+            tgz = os.path.join(artifacts_dir, "tgt-collected.tar.gz")
+            shutil.copy(output_disk, tgz)
+            print("Copied output_disk '%s' to artifacts dir '%s'" %
+                  (output_disk, tgz))
+
         files = self.extract_files(output_disk)
-        print("files: %s" % files)
+        print("collected files: %s" % files.keys())
         resolvconf = files.get('resolv.conf', "NO_RESOLVCONF_FOUND")
         resolve_status = files.get('systemd-resolve-status.txt')
 
@@ -322,6 +348,15 @@ class CloudImageTest(testlib.TestlibCase, PrivateOpenIscsiTest):
                 msg=("search addr '%s' not in systemd-resolve status: %s" %
                      (dns_search, resolve_status)))
 
+        # iscsid-status.txt has first line output from
+        # 'systemctl is-active iscsid.service' and then 'systemcl status'
+        iscsid_status = files.get('iscsid-status.txt')
+        is_active = iscsid_status.splitlines()[0]
+        self.assertEqual(
+            "active", is_active,
+            msg=("Expected iscsid.service active, found '%s'.\n%s\n" %
+                 (is_active, iscsid_status)))
+
 
 def get_image(top_d, release):
     cmd = ['get-image', top_d, 'cloud-daily', release]
diff --git a/debian/tests/tgt-boot-test b/debian/tests/tgt-boot-test
index 6ed7a9b..65a38c7 100755
--- a/debian/tests/tgt-boot-test
+++ b/debian/tests/tgt-boot-test
@@ -16,6 +16,8 @@ _DEF_CMDLINE_TMPL=(
     net.ifnames=0 BOOTIF_DEFAULT=eth0
     root={root} {overlay_drive}
     console=ttyS0 {cmdline_ds}
+    # LP: #1796137
+    systemd.mask=snapd.seeded.service systemd.mask=snapd.service
 )
 DEF_CMDLINE_TMPL="${_DEF_CMDLINE_TMPL[*]}"