Merge ~smoser/ubuntu/+source/open-iscsi:bug/1785108-net-interface-handler-runs-always into ubuntu/+source/open-iscsi:ubuntu/devel

[Scott Moser]

net-interface-handler: Apply changes only for the iscsi-root interface.

net-interface-handler was effectively bringing "up" the iscsi-root
interface for all network device adds and it "down" for all the removes.
The quotes are used above because the design point of the script is to
basically fake ifupdown into believing the event already occurred.

The change here is to only operate when the INTERFACE provided by
udev is the iscsi root device.

LP: #1785108

see commit message

f5d9539... by Scott Moser on 2018-08-02

debian/tests/test-open-iscsi.py: attempt to trigger 1785108

Without the fix applied, this would cause test failure.

[Christian Ehrhardt  (paelzer)]

I know these env vars are defined by the caller, but shouldn't we do a minimal bit of
[ -n $INTERFACE ] || return
before using it?

After wondering about this I found that it is done at least for some variables like DOMAINSEARCH below. I know that those are optional and INTERFACE should be mandatory, but I'd feel better if it was checked before being used.

Other than that the change LGTM - up to you if you'd want to add a check for some extra safety +1

I have not had a setup to test this on iscsi-root, but I assume you did so already on oracles cloud right?

review: Approve

8b3d475... by Scott Moser on 2018-08-07

only run 'timeout' from test harness making interactive more useful.

The test harness needs/wants the timeout set, but when running
interactively you do not get console output and cannot interact
with qemu. So have the harness set BOOT_TIMEOUT and only use
timeout if it is set.

5bf4f57... by Scott Moser on 2018-08-07

update doc debian/tests/README-boot-test.md

d3fa82a... by Scott Moser on 2018-08-07

update changelog for BOOT_TIMEOUT

d0d00c2... by Scott Moser on 2018-08-07

update changelog: README-boot-test.md and whitespace

406ab8f... by Scott Moser on 2018-08-07

update changelog fix 1785108

3554b6c... by Scott Moser on 2018-08-07

releasing package open-iscsi version 2.0.874-5ubuntu7

diff --git a/debian/changelog b/debian/changelog
index ebdada2..41024ba 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,13 @@
+open-iscsi (2.0.874-5ubuntu7) UNRELEASED; urgency=medium
+
+  * d/tests: make interactive use of tgt-boot-test more usable by
+    only using 'timeout' from the test harness.
+  * debian/tests/README-boot-test.md: minor doc fixes and whitespace.
+  * d/net-interface-handler: Apply changes only for the iscsi-root interface.
+    (LP: #1785108)
+
+ -- Scott Moser <[e-mail]>  Tue, 07 Aug 2018 15:46:41 -0400
+
 open-iscsi (2.0.874-5ubuntu6) cosmic; urgency=medium
 
   * Ship finalrd logout hook.
diff --git a/debian/net-interface-handler b/debian/net-interface-handler
index 7717f61..15d35c6 100755
--- a/debian/net-interface-handler
+++ b/debian/net-interface-handler
@@ -10,12 +10,23 @@
 # ifupdown appears to have no way to do this without also running
 # /etc/network/*.d/ scripts.
 
+assert_interface() {
+    # udev sets INTERFACE to the name of the currently-processed nic.
+    [ -n "$INTERFACE" ] && return 0
+    echo "environment variable INTERFACE not set." 1>&2;
+    return 1
+}
+
 start() {
     CR="
 "
+    assert_interface || return
     ifile=/run/initramfs/open-iscsi.interface
-    if [ -f "$ifile" ] && read iface < "$ifile" &&
-       ! grep -qs "^$iface=" /run/network/ifstate; then
+
+    [ -f "$ifile" ] && read iface < "$ifile" || return 0
+    [ "$INTERFACE" = "$iface" ] || return
+
+    if ! grep -qs "^$iface=" /run/network/ifstate; then
         mkdir -p /run/network
         echo "$iface=$iface" >>/run/network/ifstate
 
@@ -51,9 +62,12 @@ EOF
 }
 
 stop() {
+    assert_interface || return
     ifile=/run/initramfs/open-iscsi.interface
-    if [ -f "$ifile" ] && read iface < "$ifile" &&
-       grep -qs "^$iface=" /run/network/ifstate; then
+    [ -f "$ifile" ] && read iface < "$ifile" || return 0
+    [ "$INTERFACE" = "$iface" ] || return
+
+    if grep -qs "^$iface=" /run/network/ifstate; then
         grep -v "^$iface=" /run/network/ifstate >/run/network/.ifstate.tmp || true
         mv /run/network/.ifstate.tmp /run/network/ifstate
 
diff --git a/debian/tests/README-boot-test.md b/debian/tests/README-boot-test.md
index 4d6033f..e45bbf0 100644
--- a/debian/tests/README-boot-test.md
+++ b/debian/tests/README-boot-test.md
@@ -44,7 +44,7 @@ The test case in `debian/tests/test-open-iscsi.py` uses some helper tools.
      using the updated initramfs we wouldn't really be testing the new
      open-iscsi.
 
-     It will upgrade any packages inside that are mentioned in 
+     It will upgrade any packages inside that are mentioned in
      ADT_TEST_TRIGGERS environment.  It will also install open-iscsi if
      it is not in that list.
 
@@ -85,14 +85,16 @@ Testing manually looks like this:
     ## Get the image you want.  This creates out.d/disk.img and disk.img.dist
     $ get-image xenial.d xenial
 
-    ## patch the image with an open-iscsi, which creates xenial/kernel
-    ## and xenial/initrd from the kernel and initramfs inside the image.
+    ## patch the image with an open-iscsi, which creates xenial.d/kernel
+    ## and xenial.d/initrd from the kernel and initramfs inside the image.
     $ apt-get download open-iscsi
     $ deb=$(ls open-iscsi_*.deb | tail -n 1)
-    $ patch-image xenial/disk.img "$deb" --kernel=xenial/kernel --initrd=xenial/initrd
+    $ sudo ./debian/tests/patch-image \
+         --kernel=xenial.d/kernel --initrd=xenial.d/initrd
+         xenial.d/disk.img "$deb"
 
     ## Boot the system, log in, look around.
-    $ tgt-boot-test -v xenial/disk.img xenial/kernel xenial/initrd
+    $ tgt-boot-test -v xenial.d/disk.img xenial.d/kernel xenial.d/initrd
 
 
 ### Features of tgt-boot-test ###
diff --git a/debian/tests/test-open-iscsi.py b/debian/tests/test-open-iscsi.py
index e7e37a8..27a5e0b 100644
--- a/debian/tests/test-open-iscsi.py
+++ b/debian/tests/test-open-iscsi.py
@@ -75,8 +75,19 @@ bukket:
        exit
    fi
    systemd-resolve --status --no-pager
+ - &add_and_remove_tuntap |
+   #!/bin/sh
+   # LP: #1785108 would break dns when any device was removed.
+   tapdev="mytap0"
+   echo ==== Adding $tapdev ====
+   ip tuntap add mode tap user root $tapdev
+   udevadm settle
+   echo ==== Removing $tapdev ====
+   ip tuntap del mode tap $tapdev
+   udevadm settle
 
 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]
@@ -271,7 +282,10 @@ class CloudImageTest(testlib.TestlibCase, PrivateOpenIscsiTest):
             self.info['initrd']]
         sys.stderr.write(' '.join(cmd) + "\n")
 
-        subprocess.check_call(cmd)
+        env = os.environ.copy()
+        env['BOOT_TIMEOUT'] = env.get('BOOT_TIMEOUT', '60m')
+        subprocess.check_call(cmd, env=env)
+
         files = self.extract_files(output_disk)
         print("files: %s" % files)
         resolvconf = files.get('resolv.conf', "NO_RESOLVCONF_FOUND")
diff --git a/debian/tests/tgt-boot-test b/debian/tests/tgt-boot-test
index f3708a0..6ed7a9b 100755
--- a/debian/tests/tgt-boot-test
+++ b/debian/tests/tgt-boot-test
@@ -514,7 +514,7 @@ main() {
     local mem="512" start="$SECONDS" ret=""
     local cmd=""
     cmd=(
-        timeout --kill-after=1m --signal=TERM ${BOOT_TIMEOUT:-60m}
+        ${BOOT_TIMEOUT:+timeout --kill-after=1m --signal=TERM $BOOT_TIMEOUT}
         xkvm -v -v "${netdev_args[@]}" ${overlay_drive_xkvm} "${pt[@]}" --
         -echr 0x5
         -m $mem ${serial_log:+-serial "file:$serial_log"} -nographic
diff --git a/debian/tests/xkvm b/debian/tests/xkvm
index 9015140..9513723 100755
--- a/debian/tests/xkvm
+++ b/debian/tests/xkvm
@@ -292,7 +292,7 @@ get_bios_opts() {
 
     if [ -n "$nvram" ]; then
         if [ ! -f "$nvram" ]; then
-            cp "$nvram_src" "$nvram" || 
+            cp "$nvram_src" "$nvram" ||
                 { error "failed copy $nvram_src to $nvram"; return 1; }
             debug 1 "copied $nvram_src to $nvram"
         fi
@@ -664,7 +664,7 @@ main() {
 
     local bus_devices
     bus_devices=( -device "$virtio_scsi_bus,id=virtio-scsi-xkvm" )
-    cmd=( "${kvmcmd[@]}" "${archopts[@]}" 
+    cmd=( "${kvmcmd[@]}" "${archopts[@]}"
           "${bios_opts[@]}"
           "${bus_devices[@]}"
           "${netargs[@]}"