[Merge] ~matsubara/ubuntu/+source/open-iscsi:ubuntu/devel into ~ubuntu-server-dev/ubuntu/+source/open-iscsi:ubuntu/devel
Scott Moser
smoser at ubuntu.com
Mon Mar 21 14:24:17 UTC 2016
inline comments. over alll, looks great this will really be nice.
Diff comments:
> diff --git a/.gitignore b/.gitignore
> index 5761abc..49187ae 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -1 +1,5 @@
> *.o
> +*.pyc
> +debian/tests/xenial.d
> +debian/tests/trusty.d
> +*.swp
i dont think we'd want to carry this as delta.
> diff --git a/debian/tests/control b/debian/tests/control
> index c07342a..4752a91 100644
> --- a/debian/tests/control
> +++ b/debian/tests/control
> @@ -1,3 +1,3 @@
> Tests: install sysvinit-install daemon testsuite
> Restrictions: needs-root isolation-machine breaks-testbed
> -Depends: open-iscsi, python
> +Depends: open-iscsi, python, tgt, qemu-system-x86, ubuntu-cloudimage-keyring, simplestreams
add 'ubuntu-distro-info' just for kicks here (used below for getting available rleases). Also, where do we declare what arches this runs on ? as qemu-system-x86 is not desireable as the qemu on ppc64. Easiest thing right now is to just say this runs on amd64 and i386 only.
> diff --git a/debian/tests/test-open-iscsi.py b/debian/tests/test-open-iscsi.py
> index f39c23a..8ed8289 100644
> --- a/debian/tests/test-open-iscsi.py
> +++ b/debian/tests/test-open-iscsi.py
> @@ -151,6 +152,88 @@ discovery.sendtargets.auth.password_in = %s
> result = "Could not find '%s' in report:\n" % i
> self.assertTrue(i in report, result + report)
>
> + def test_net_interface_handler_execute_bit(self):
> + '''Test /lib/open-iscsi/net-interface-handler is executable.'''
> + nih_path = '/lib/open-iscsi/net-interface-handler'
> + self.assertTrue(os.access(nih_path, os.X_OK))
> +
> +
> +class MAASEphemeralTest(testlib.TestlibCase, PrivateOpenIscsiTest):
> + '''Test MAAS ephemeral image can boot.
> +
> + Downloads MAAS ephemeral image, patches the image to use dep8's built
> + open-iscsi package, sets the image to be served by tgt and then boot that
> + image under (nested) kvm. It runs cloud-init in the booted VM to collect
> + some data and verify the image worked as expected.
> + '''
> +
> + def setUp(self):
> + self.here = os.path.dirname(os.path.abspath(__file__))
> + # Download MAAS ephemeral image.
> + self.get_maas_ephemeral()
> + # Patch the image to use the dep8 built open-iscsi.
> + self.patch_image()
> + self.output_disk_path = self.create_output_disk(self.here)
> + self.release = testlib.ubuntu_release()
> +
> + def create_output_disk(self, base_path):
> + output_disk_path = os.path.join(base_path, 'output_disk.img')
> + subprocess.call([
> + 'qemu-img', 'create', '-f', 'raw', output_disk_path, '10M'])
> + return output_disk_path
> +
> + def get_maas_ephemeral(self):
> + get_eph_cmd = os.path.join(self.here, 'get-maas-eph')
> + subprocess.call([
> + get_eph_cmd, os.path.join(self.here, '{}.d'.format(self.release)),
> + self.release, 'hwe-x'])
'hwe-x' there should be based on the release. ie, for Y, it will need 'hwe-y'. It looks like get-maas-eph will do the right thing if you just leave it off.
> +
> + def patch_image(self):
> + '''Patch root-image with dep8 built open-iscsi package.'''
> + root_image_path = os.path.join(
> + self.here, '{}.d'.format(self.release), 'root-image')
> + open_iscsi_deb_path = os.path.join(
> + # XXX: matsubara fix this path hackery.
> + # os.environ['ADTTMP'], 'binaries', 'open-iscsi.deb')
> + self.here, '..', '..', '..', '..', 'binaries', 'open-iscsi.deb')
> + open_iscsi_deb = open(open_iscsi_deb_path)
> + install_cmd = (
> + 'cat >/tmp/my.deb && dpkg -i /tmp/my.deb; ret=$?;'
> + 'rm -f /tmp/my.deb; exit $ret')
> + subprocess.check_output([
> + 'mount-image-callback', '--system-mounts',
> + root_image_path, '--', 'chroot', '_MOUNTPOINT_', '/bin/sh', '-c',
> + install_cmd], stdin=open_iscsi_deb)
> +
> + def extract_files(self, path):
> + tmpdir = mkdtemp()
> + subprocess.call(['tar', '-C', tmpdir, '-xf', path])
> + return [os.path.join(tmpdir, f) for f in os.listdir(tmpdir)]
> +
> + def test_tgt_boot(self):
> + tgt_boot_cmd = os.path.join(self.here, 'tgt-boot-test')
> + env = os.environ
> + env['HOST_IP'] = '10.0.3.1'
is this guaranteed? i doubt it. probably need some better way to get the right address. i dont know.
> + # Add self.here to PATH so xkvm will be available to tgt-boot-test
> + env['PATH'] = env['PATH'] + ":%s" % self.here
> + env['OUTPUT_DISK'] = self.output_disk_path
> + rel_dir = '{}.d'.format(self.release)
> + subprocess.call([
> + tgt_boot_cmd,
> + os.path.join(self.here, rel_dir, 'root-image'),
> + os.path.join(self.here, rel_dir, 'hwe-x/boot-kernel'),
hm... 'hwe-x' coded here too. so maybe we *do* need to know it here rather than letting get-maas-eph just do the right thing. So, if you know the release, then the default hwe-X string is: "hwe-" + release[0]
> + os.path.join(self.here, rel_dir, 'hwe-x/boot-initrd')],
> + env=env)
> + files = self.extract_files(self.output_disk_path)
> + for f in files:
> + if f.endswith('resolv.conf'):
> + resolvconf_contents = open(f).read()
> + self.assertIn(
> + '10.0.2.3', resolvconf_contents,
> + msg="10.0.2.3 not in resolvconf contents: \n{}".format(
> + resolvconf_contents))
> +
> +
> if __name__ == '__main__':
> import optparse
> parser = optparse.OptionParser()
--
https://code.launchpad.net/~matsubara/ubuntu/+source/open-iscsi/+git/open-iscsi/+merge/289642
Your team Ubuntu Server Developers is subscribed to branch ~ubuntu-server-dev/ubuntu/+source/open-iscsi:ubuntu/devel.
More information about the Ubuntu-reviews
mailing list