[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