Merge ~smoser/ubuntu/+source/open-iscsi:bug/1715468-test-support-systemd-resolved into ~usd-import-team/ubuntu/+source/open-iscsi:ubuntu/devel

[Scott Moser]

Change open-icsi's test to support checking systemd-resolved for dns info.

open-iscsi has a test case where it boots a system off a read-only iscsi
root device. It verifies the system starts up and shuts down successfully.
It also verifies that the dns 'search' and 'nameserver' entries provided
by the dhcp server are used by the system.

In the past, it did this just by reading asserting those values
were in /etc/resolv.conf. The change made was to also support reading
output of 'systemd-resolved --status'.

The text will still fail in current artful because of bug 1713803.

[Scott Moser (smoser)]

Note that this is still failing at the moment, as there is no solution for
 https://bugs.launchpad.net/ubuntu/+source/initramfs-tools/+bug/1713803

but that will be fixed separately.

we currently get a failure like this:

======================================================================
FAIL: test_tgt_boot (__main__.CloudImageTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "debian/tests/test-open-iscsi.py", line 287, in test_tgt_boot
    (dns_addr, resolve_status)))
AssertionError: 10.1.1.4 not in systemd-resolve status: Global
          DNSSEC NTA: 10.in-addr.arpa
                      16.172.in-addr.arpa
                      168.192.in-addr.arpa
                      17.172.in-addr.arpa
                      18.172.in-addr.arpa
                      19.172.in-addr.arpa
                      20.172.in-addr.arpa
                      21.172.in-addr.arpa
                      22.172.in-addr.arpa
                      23.172.in-addr.arpa
                      24.172.in-addr.arpa
                      25.172.in-addr.arpa
                      26.172.in-addr.arpa
                      27.172.in-addr.arpa
                      28.172.in-addr.arpa
                      29.172.in-addr.arpa
                      30.172.in-addr.arpa
                      31.172.in-addr.arpa
                      corp
                      d.f.ip6.arpa
                      home
                      internal
                      intranet
                      lan
                      local
                      private
                      test

Link 2 (eth0)
      Current Scopes: LLMNR/IPv4 LLMNR/IPv6
       LLMNR setting: yes
MulticastDNS setting: no
      DNSSEC setting: no
    DNSSEC supported: no

----------------------------------------------------------------------
Ran 3 tests in 74.603s

[ChristianEhrhardt (paelzer)]

Added the server team review slot to show up in the active reviews for all of us to be picked up as time permits.

[ChristianEhrhardt (paelzer)]

I know the actual test is Ubuntu Delta we added and not in Debian.
I reviewed the code and on that level agree to the changes.

I don't like that this is a "fix the test" change but the test is still failing (due to the other bug). As a workaround I ran the new .dsc through autopkgtest with a rebuild on zesty.
It passed the "testsuite" test which the MP is modifying.
I gave it a shot on Artful as well and it failed with the known issue of:
  File "/tmp/autopkgtest.VTSgHq/build.cjb/open-iscsi-2.0.874/debian/tests/test-open-iscsi.py", line 287, in test_tgt_boot
    (dns_addr, resolve_status)))
AssertionError: 10.1.1.4 not in systemd-resolve status: Global

Debian:
debian/tests/patch-image is in Debian, would you mind to report that and associate a Debian bug to the Ubuntu one to not carry that forever.

Other fixes:
Reading into 1713803 that will be fixed in an upload to another package. So on this migration you will again run into "always failed" and migrate.
That todo is already tracked in the bugs you have open, so that is ok I guess.

@Scott if the error in artful above is really expected then ack - is it?

be1911c... by Scott Moser on 2017-09-07

improve test to also look for search entries.

33de4ec... by Scott Moser on 2017-09-07

allow ISCSI_TEST_RELEASE to set the release for easier testing.

ad9e6fc... by Scott Moser on 2017-09-07

support resolvconf and resolved .. zesty fun.

[Scott Moser (smoser)]

Christian,
I do not believe that 'debian/tests/patch-image' is in debian.
 $ git show pkg/debian/experimental:debian/tests/patch-image
 fatal: Path 'debian/tests/patch-image' exists on disk, but not in 'pkg/debian/experimental'.

It is really just part of the delta for that test we've added.

I've done some modifications to make the test easier to run for development. On our dev system 'diglett', which runs artful I've done:
  $ sudo ISCSI_TEST_RELEASE=zesty python debian/tests/test-open-iscsi.py
and that passes.
  $ sudo ISCSI_TEST_RELEASE=artful python debian/tests/test-open-iscsi.py

will fail as the dns is legitimately broken.

[ChristianEhrhardt (paelzer)]

Yeah as I mentioned in IRC I was doing the test on zesty as well but with the new package being built and it worked.

Sorry for asking on the patch-image it is clearly part of our Delta, was just not part of the same commit that added the rest. But that said:
1. test ok
2. fixing/improving the test
3. code review LGTM
=> Approve

And as you outline the "remaining" issue is already tracked in another bug.
So please go ahead and tag + upload or let me know if you want me to do so.

review: Approve

[ChristianEhrhardt (paelzer)]

Hi Scott,
I found your question on IRC.
Nish explained, but I still don't see an upload so following your request to do so.

I (git ubuntu lint) just found you still had set UNRELEASED.
But I can fix that up on tag/merge.

I tagged, pushed - verified the dsc debdiff matches the git one and sponsored it for you.
It is now at [1], please help tracking its build and proposed migration.

[1]: https://launchpad.net/ubuntu/+source/open-iscsi/2.0.874-4ubuntu2

diff --git a/debian/changelog b/debian/changelog
index d407b86..e5e0a94 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+open-iscsi (2.0.874-4ubuntu2) UNRELEASED; urgency=medium
+
+  * debian/tests/test-open-iscsi.py: query systemd-resolved if present.
+    Ubuntu images now have systemd-resolved managing /etc/resolv.conf
+    so support test if that is the case (LP: #1715468).
+
+ -- Scott Moser <[e-mail]>  Wed, 06 Sep 2017 16:20:16 -0400
+
 open-iscsi (2.0.874-4ubuntu1) artful; urgency=medium
 
   * Merge with Debian unstable. Remaining changes:
diff --git a/debian/tests/README-boot-test.md b/debian/tests/README-boot-test.md
index fe93bc6..1c4250a 100644
--- a/debian/tests/README-boot-test.md
+++ b/debian/tests/README-boot-test.md
@@ -67,11 +67,11 @@ 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 grabs creates xenial/kernel
+    ## patch the image with an open-iscsi, which creates xenial/kernel
     ## and xenial/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" xenial/kernel xenial/initrd
+    $ patch-image xenial/disk.img "$deb" --kernel=xenial/kernel --initrd=xenial/initrd
 
     ## Boot the system, log in, look around.
     $ tgt-boot-test -v xenial/disk.img xenial/kernel xenial/initrd
diff --git a/debian/tests/patch-image b/debian/tests/patch-image
index e3df8d7..ccec1c7 100755
--- a/debian/tests/patch-image
+++ b/debian/tests/patch-image
@@ -121,15 +121,24 @@ main() {
     local packages=( )
     packages=( "$@" )
 
-    if [ "${#packages}" = "1" -a "${packages[0]}" = "none" ]; then
+    if [ "${#packages[@]}" = "1" -a "${packages[0]}" = "none" ]; then
         packages=( )
     elif [ "${#packages[@]}" -eq 0 -a -n "${ADT_TEST_TRIGGERS}" ]; then
         adt_test_triggers_to_bin_pkgs ${ADT_TEST_TRIGGERS} ||
-            fail "failed to find binary packages" \
+            fail "failed to find binary packages " \
                 "from ADT_TEST_TRIGGERS=$ADT_TEST_TRIGGERS"
         packages=( $_RET )
         debug 1 "read ADT_TEST_TRIGGERS=$ADT_TEST_TRIGGERS to set" \
             "packages=${packages[*]}"
+    else
+        local opackages="" debs="" pkg=""
+        opackages=( "${packages[@]}" )
+        packages=( )
+        debs=( )
+        for pkg in "${opackages[@]}"; do
+            [ -f "${pkg}" ] && debs[${#debs[@]}]="$pkg" ||
+                packages[${packages[@]}]="$pkg"
+        done
     fi
 
     # if open-iscsi is not in the packages list above, we handle it specifically
@@ -189,6 +198,15 @@ main() {
             fail "failed to install ${packages[*]} in $target"
     fi
 
+    if [ "${#debs[@]}" != "0" ]; then
+        local tmpd=""
+        tmpd=$(mktemp -d "${target}/tmp/${0##*/}.XXXXXX")
+        cp "${debs[@]}" "$tmpd"
+        DEBIAN_FRONTEND=noninteractive chroot "$target" sh -exc \
+            'cd "$1"; shift; dpkg -i *.deb' debinstalls "${tmpd#${target}}"
+        rm -Rf "${tmpd}"
+    fi
+
     local kpath="" ipath=""
     for f in "$target/boot/"*; do
         case "${f##*/}" in
@@ -208,7 +226,7 @@ main() {
 
     if [ -n "$initrd" ]; then
         [ -n "$ipath" ] || fail "unable to find initrd in $tsrc"
-        debug 1 "using initrd ${kernel#${ipath}}"
+        debug 1 "using initrd ${ipath#${target}}"
         cp "$ipath" "$initrd" ||
             fail "failed copy $ipath to $initrd"
         [ -z "$IMAGE" ] || chown "--reference=$IMAGE" "$initrd"
diff --git a/debian/tests/test-open-iscsi.py b/debian/tests/test-open-iscsi.py
index be5fbef..1c09925 100644
--- a/debian/tests/test-open-iscsi.py
+++ b/debian/tests/test-open-iscsi.py
@@ -59,6 +59,35 @@ username_in = 'ubuntu'
 password_in = 'ubuntupasswd'
 initiatorname = 'iqn.2009-10.com.example.hardy-multi:iscsi-01'
 
+COLLECT_USER_DATA = """\
+#cloud-config
+bukket:
+ - &get_resolved_status |
+   if [ "${1:--}" != "-" ]; then
+      exec >"$1" 2>&1
+   fi
+   if ! command -v systemd-resolve >/dev/null 2>&1; then
+       echo "systemd-resolve: not available."
+       exit
+   fi
+   if ! ( systemd-resolve --help | grep -q -- --status ); then
+       echo "systemd-resolve: no --status."
+       exit
+   fi
+   systemd-resolve --status --no-pager
+
+runcmd:
+ - [ mkdir, -p, /output ]
+ - [ cp, /etc/resolv.conf, /output]
+ - [ sh, -c, *get_resolved_status, --, /output/systemd-resolve-status.txt]
+ - [ tar, -C, /output, -cf, /dev/disk/by-id/virtio-output-disk, . ]
+
+power_state:
+ mode: poweroff
+ message: cloud-init finished. Shutting down.
+ timeout: 60
+"""
+
 try:
     from private.qrt.OpenIscsi import PrivateOpenIscsiTest
 except ImportError:
@@ -152,17 +181,6 @@ class CloudImageTest(testlib.TestlibCase, PrivateOpenIscsiTest):
     See README-boot-test.md for more information.
     '''
 
-    collect_user_data = textwrap.dedent("""\
-        runcmd:
-         - [ mkdir, -p, /output ]
-         - [ cp, /etc/resolv.conf, /output]
-         - [ tar, -C, /output, -cf, /dev/disk/by-id/virtio-output-disk, . ]
-        power_state:
-         mode: poweroff
-         message: cloud-init finished. Shutting down.
-         timeout: 60
-    """)
-
     @classmethod
     def setUpClass(cls):
         reason = (
@@ -175,7 +193,8 @@ class CloudImageTest(testlib.TestlibCase, PrivateOpenIscsiTest):
 
         here = os.path.dirname(os.path.abspath(__file__))
         os.environ['PATH'] = "%s:%s" % (here, os.environ['PATH'])
-        release = testlib.ubuntu_release()
+        release = os.environ.get(
+            "ISCSI_TEST_RELEASE", testlib.ubuntu_release())
         image_d = os.path.join(here, '{}.d'.format(release))
         # Download MAAS ephemeral image.
         info = {'release': release,
@@ -201,7 +220,7 @@ class CloudImageTest(testlib.TestlibCase, PrivateOpenIscsiTest):
         self.tmpdir = mkdtemp()
         udp = os.path.join(self.tmpdir, 'user-data')
         with open(udp, "w") as fp:
-            fp.write(self.collect_user_data)
+            fp.write(COLLECT_USER_DATA)
         self.info['user-data'] = udp
 
     def create_output_disk(self):
@@ -232,8 +251,9 @@ class CloudImageTest(testlib.TestlibCase, PrivateOpenIscsiTest):
         # Add self.here to PATH so xkvm will be available to tgt-boot-test
         dns_addr = '10.1.1.4'
         rel_dir = '{}.d'.format(self.info['release'])
+        dns_search = 'example.com'
         info = {'host': '10.1.1.2', 'dns': dns_addr,
-                'dnssearch': 'example.com', 'network': '10.1.1.0/24'}
+                'dnssearch': dns_search, 'network': '10.1.1.0/24'}
         netdev = ("--netdev=user,net={network},host={host},dns={dns},"
                   "dnssearch={dnssearch}").format(**info)
 
@@ -249,14 +269,39 @@ class CloudImageTest(testlib.TestlibCase, PrivateOpenIscsiTest):
         subprocess.check_call(cmd)
         files = self.extract_files(output_disk)
         print("files: %s" % files)
-        resolvconf_contents = "NO_RESOLVCONF_FOUND"
-        for name, contents in files.items():
-            if name.endswith('resolv.conf'):
-                resolvconf_contents = contents
-        self.assertIn(
-            dns_addr, resolvconf_contents,
-            msg = ("%s not in resolvconf contents: \n%s" %
-                   (dns_addr, resolvconf_contents)))
+        resolvconf = files.get('resolv.conf', "NO_RESOLVCONF_FOUND")
+        resolve_status = files.get('systemd-resolve-status.txt')
+
+        resolvconf_id = 'generated by resolvconf'
+        resolved_addr = "127.0.0.53"
+        if resolvconf_id in resolvconf:
+            print("resolvconf manages resolvconf.\n")
+            self.assertIn(
+                dns_addr, resolvconf,
+                msg = ("%s not in resolvconf contents: \n%s" %
+                       (dns_addr, resolvconf)))
+            if dns_search in resolvconf:
+                print("%s was found in resolv.conf." % dns_search)
+            elif resolved_addr in resolvconf and dns_search in resolve_status:
+                # zesty has resolvconf and systemd-resolved.
+                print("%s was in resolve_status and %s in resolv.conf" %
+                      (resolved_addr, dns_search))
+            else:
+                raise AssertionError(
+                    "%s domain is not being searched." % dns_search)
+
+        else:
+            print("systemd-resolved managing resolve.conf\n")
+            self.assertIn(
+                resolved_addr, resolvconf,
+                msg="%s not in resolved resolv.conf: %s" % (resolved_addr,
+                                                            resolvconf))
+            self.assertIn(dns_addr, resolve_status,
+                msg=("%s not in systemd-resolve status: %s" %
+                     (dns_addr, resolve_status)))
+            self.assertIn(dns_search, resolve_status,
+                msg=("search addr '%s' not in systemd-resolve status: %s" %
+                     (dns_search, resolve_status)))
 
 
 def get_image(top_d, release):