Merge lp:~qzhang/lava-dispatcher/gen-test-tarball into lp:lava-dispatcher

[Spring Zhang]

Implement generate_tarballs interface based on new base
  1. refactor on generate_tarballs()
  2. use Popen() instead of system()
  3. add unit test, but haven't pass unit test yet
  4. Fix some error in lava-dispatch file

will fix errors later

[Loïc Minier (lool)]

On Tue, Mar 01, 2011, Spring Zhang wrote:
> --- lava-dispatch 2011-03-01 03:56:20 +0000
> +++ lava-dispatch 2011-03-01 11:48:59 +0000
> - cmd.get('parameters', {})
> + params = cmd.get('parameters', {})

 Wups, thanks!

> step = lava_commands[cmd['command']](target)
> step.run(**params)
>
> === modified file 'lava/actions/deploy.py'
> --- lava/actions/deploy.py 2011-02-26 14:41:09 +0000
> +++ lava/actions/deploy.py 2011-03-01 11:48:59 +0000
> @@ -1,33 +1,118 @@
> #!/usr/bin/python
> from lava.actions import BaseAction
> +from lava.client import LavaClient
> import time
> +from subprocess import PIPE
> +from subprocess import Popen
> +import shutil
> +import shlex
>
> class cmd_deploy_linaro_image(BaseAction):
> + #board type: [bootfs partition index, rootfs partition index]
> + BOARD_PARTS = {
> + "panda": [1, 2],
> + "beagle": [1, 2],
> + "vexpress": [1, 2],
> + "mx51evk": [2, 3],

 It's a bit sad to hardcode this

 The immediate thing I would have proposed would be to have a config
 file, but in reality I'm thinking we want to work towards not knowing
 the partition layout at all.

> + #use l-m-c to install hwpack and rootfs to qemu image
> + #assume root permission
> + hwpack_name = hwpack.rsplit("/", 1)[1]
> + rootfs_name = rootfs.rsplit("/", 1)[1]
> + qemuimg = "%s_sd.img" % boardtype
> + lmc_cmd = "linaro-media-create --rootfs ext3 --image_file %s" \
> + " --dev %s --binary %s --hwpack %s" % (qemuimg, boardtype,
> + rootfs_name, hwpack_name)

 Hmm why do you actually need to pass just the filename to l-m-c? Just
 pass hwpack_name and rootfs_name directly?

> + print "l-m-c command: %s" % lmc_cmd
> +
> + #TODO: should import l-m-c to use python way,
> + # and l-m-c supports no keyboard-input
> + """
> + proc = Popen(shlex.split(lmc_cmd), stdin=PIPE, stdout=PIPE,
> + stderr=PIPE)
> + output, rc = proc.communicate()

 You don't seem to use the output at all; use proc.wait() instead? if
 you don't want to see it on the console, open /dev/null and pass that
 as stdout / stderr.

> + if proc.returncode != 0:
> + print "l-m-c fails"
> + raise OperationFailed()

 Would be good to return a message in OperationFailed():
     if proc.returncode != 0:
         raise OperationFailed(
             "linaro-media-create failed with %s" % proc.returncode)

> + #package qemu image content to tarballs
> + #an ugly tar, need to find a way to create tar in relative path,
> + #maybe tarfile
> + qemu_img_cmds = [ "modprobe nbd max_part=8",
> + "qemu-nbd --connect=/dev/nbd0 %s" % qemuimg,

 Do we need to modprovbe nbd explicitly?

> + "mkdir -p /mnt/bootfs",
> + "sleep 2",

 a comment on why the sleep is needed would be nice

> + "mount /dev/nbd0p%s /mnt/bootfs" % self.BOARD_PARTS[boardtype][0],
> + "cd /mnt/bootfs",
> + "tar czf ~/%s ." % hwpack_name,

 tar -C to change directory; e.g. tar -C /mnt/bootfs czf foo.tgz .

 /mnt/bootfs sou...

[Spring Zhang (qzhang)]

Thanks for the review.

On Tue, 2011-03-01 at 12:19 +0000, Loïc Minier wrote:
> On Tue, Mar 01, 2011, Spring Zhang wrote:
> > --- lava-dispatch 2011-03-01 03:56:20 +0000
> > +++ lava-dispatch 2011-03-01 11:48:59 +0000
> > - cmd.get('parameters', {})
> > + params = cmd.get('parameters', {})
>
> Wups, thanks!
>
> > step = lava_commands[cmd['command']](target)
> > step.run(**params)
> >
> > === modified file 'lava/actions/deploy.py'
> > --- lava/actions/deploy.py 2011-02-26 14:41:09 +0000
> > +++ lava/actions/deploy.py 2011-03-01 11:48:59 +0000
> > @@ -1,33 +1,118 @@
> > #!/usr/bin/python
> > from lava.actions import BaseAction
> > +from lava.client import LavaClient
> > import time
> > +from subprocess import PIPE
> > +from subprocess import Popen
> > +import shutil
> > +import shlex
> >
> > class cmd_deploy_linaro_image(BaseAction):
> > + #board type: [bootfs partition index, rootfs partition index]
> > + BOARD_PARTS = {
> > + "panda": [1, 2],
> > + "beagle": [1, 2],
> > + "vexpress": [1, 2],
> > + "mx51evk": [2, 3],
>
> It's a bit sad to hardcode this
>
> The immediate thing I would have proposed would be to have a config
> file, but in reality I'm thinking we want to work towards not knowing
> the partition layout at all.
>
Agree.
> > + #use l-m-c to install hwpack and rootfs to qemu image
> > + #assume root permission
> > + hwpack_name = hwpack.rsplit("/", 1)[1]
> > + rootfs_name = rootfs.rsplit("/", 1)[1]
> > + qemuimg = "%s_sd.img" % boardtype
> > + lmc_cmd = "linaro-media-create --rootfs ext3 --image_file %s" \
> > + " --dev %s --binary %s --hwpack %s" % (qemuimg, boardtype,
> > + rootfs_name, hwpack_name)
>
> Hmm why do you actually need to pass just the filename to l-m-c? Just
> pass hwpack_name and rootfs_name directly?
>
Initial hwpack or rootfs is an URL, it will get by the previous function
to the local, and here l-m-c uses the hwpack/rootfs name.
> > + print "l-m-c command: %s" % lmc_cmd
> > +
> > + #TODO: should import l-m-c to use python way,
> > + # and l-m-c supports no keyboard-input
> > + """
> > + proc = Popen(shlex.split(lmc_cmd), stdin=PIPE, stdout=PIPE,
> > + stderr=PIPE)
> > + output, rc = proc.communicate()
>
> You don't seem to use the output at all; use proc.wait() instead? if
> you don't want to see it on the console, open /dev/null and pass that
> as stdout / stderr.
>
Do we need to log if there is some error to check?
> > + if proc.returncode != 0:
> > + print "l-m-c fails"
> > + raise OperationFailed()
>
> Would be good to return a message in OperationFailed():
> if proc.returncode != 0:
> raise OperationFailed(
> "linaro-media-create failed with %s" % proc.returncode)
>
Agree, some exceptions we need to define well.
> > + #package qemu image content to tarballs
> > + #an ugly tar, need to find a way to create tar in relative path,
> > + #maybe tarfile
> > + qemu_img_cmds = [ "modprobe nbd max_part=8",
> > + "qemu-...

[Loïc Minier (lool)]

On Tue, Mar 01, 2011, Spring Zhang wrote:
> > > + hwpack_name = hwpack.rsplit("/", 1)[1]
> > > + rootfs_name = rootfs.rsplit("/", 1)[1]
> > > + qemuimg = "%s_sd.img" % boardtype
> > > + lmc_cmd = "linaro-media-create --rootfs ext3 --image_file %s" \
> > > + " --dev %s --binary %s --hwpack %s" % (qemuimg, boardtype,
> > > + rootfs_name, hwpack_name)
> >
> > Hmm why do you actually need to pass just the filename to l-m-c? Just
> > pass hwpack_name and rootfs_name directly?
> >
> Initial hwpack or rootfs is an URL, it will get by the previous function
> to the local, and here l-m-c uses the hwpack/rootfs name.

 Hmm ok, then I think the vars could be named _url (hwpack_url,
 rootfs_url) and we could have rootfs_file vars when you've downloaded
 them locally.

 I am always scared to rely on the cwd or to use "cd"

 (Also, I think you were running "cd" in a subshell, so it probably
 didn't affect the current working directory)

> good, and we can include it in some config file you mentioned above
> > do you actually want to copy, or would moving the tarball be ok?

 Put the tarball directly in the right place? Unless it's some
 permanent mirror, in which case just refer to its full pathname

> > > +if __name__ == "__main__":
> > > + cmd = cmd_deploy_linaro_image("bbg01")
> > > + cmd.generate_tarballs("mx51evk", "http://pkgserver/original/hwpack_linaro-imx51_20110210-0_armel_unsupported.tar.gz", "http://pkgserver/original/linaro-natty-headless-tar-20110216-0.tar.gz")
> >
> > lots of hardcoding here
> It's for a test.

 Maybe this should be parsed from sys.argv instead so that others can
 use it; or just write a local test script, but don't commit it

   Thanks!
--
Loïc Minier

[Spring Zhang (qzhang)]

Your concern about "cd" is right, I met error when I did unit test.

[Spring Zhang (qzhang)]

Pending the merge request, I'll work on the comments and try to get it work before merge.

[Spring Zhang (qzhang)]

Also I want to clean the hard code of this part based from new remove-hardcode branch

[Paul Larson (pwlars)]

I think it would be a good idea to clean up the things mentioned here, and also re-sync with trunk since some of the recent changes there overlap or replace things that are done here. There are some whitespace/indentation things that should be cleaned up (indents are 4 spaces, not 8 or tab). Also, I'm not sure about using qemu-nbd. It adds an extra requirement, and a kernel module that's not usually loaded at that. The way I'm doing this in the lab right now is simply to save the offset of the necessary partitions for each type, and mount with -oloop,offset=... This doesn't solve the problem of having to store some kind of information, and if the image format ever changes, this will clearly break. One way I can think of around this might be to script parted sorta like:
parted qemu.img -s unit b p
and parse the output to get the offset of each of the partitions. If we can assume that we guess either 2 partitions means 1=boot 2=root and 3 means that 2=boot, 3=root, then we can use that without having to store any information about partition numbers or offsets. Seems like there should be a better way...

I submitted a wishlist item for l-m-c to see if they think it would be easy enough to teach l-m-c to output tarballs instead of a whole image. If it's not equally ugly to do something like that in l-m-c, that would make things really convenient for us.

review: Needs Fixing

[lp:~qzhang/lava-dispatcher/gen-test-tarball]

16. By Spring Zhang on 2011-03-14

pull from lp:lava r15

17. By Spring Zhang on 2011-03-15

Modification according to comments:
1. move hard code to config.py
2. remove sleep instructions
3. fix tar bug: ~ not recognized and -C
4. use /tmp as temp directory
5. remove self test code
6. add message in exception

Unmerged revisions

17. By Spring Zhang on 2011-03-15

Modification according to comments:
1. move hard code to config.py
2. remove sleep instructions
3. fix tar bug: ~ not recognized and -C
4. use /tmp as temp directory
5. remove self test code
6. add message in exception

16. By Spring Zhang on 2011-03-14

pull from lp:lava r15

15. By Spring Zhang on 2011-03-01

1. refactor on generate_tarballs()
2. use Popen() instead of system()
3. add unit test

14. By Spring Zhang on 2011-03-01

Fix no params error

13. By Spring Zhang on 2011-03-01

fix BOARD_TYPE reference

12. By Spring Zhang on 2011-03-01

implement generate_tarballs interface

11. By Spring Zhang on 2011-03-01

Merged from lp:lava r11
  Paul Larson 2011-02-28 [merge] Merge some quickstart documentation and add --help
    Loïc Minier 2011-02-26 Add quickstart with random notes to get started.
    Loïc Minier 2011-02-26 Add sample JSON file from the spec.
    Loïc Minier 2011-02-26 Test sys.argv's length and output usage information when it's too short or when
    Paul Larson 2011-02-28 [merge] Merge some misc cleanups
    Loïc Minier 2011-02-26 Small simplification of the handling of the default parameters value.
    Loïc Minier 2011-02-26 Drop unused identifier "id" as the expect() return value isn't checked there.
    Loïc Minier 2011-02-26 Drop useless try/except/raise construct.
    Loïc Minier 2011-02-26 Add missing definition of expected response (master_str).

10. By Spring Zhang on 2011-03-01

Add mx51evk test image uboot command, move board info to LavaClient for re-use

=== modified file 'lava/actions/deploy.py'
--- lava/actions/deploy.py	2011-03-11 10:58:57 +0000
+++ lava/actions/deploy.py	2011-03-15 11:15:58 +0000
@@ -1,25 +1,94 @@
 #!/usr/bin/python
 from lava.actions import BaseAction
+from lava.client import LavaClient
+from lava.config import TESTIMG_PARTS, BOARDS, WWW_DEPLOY_DIR
+from subprocess import PIPE
+from subprocess import Popen
+import shutil
+import shlex
 
 class cmd_deploy_linaro_image(BaseAction):
     def run(self, hwpack, rootfs):
         print "deploying on %s" % self.client.hostname
         print "  hwpack: %s" % hwpack
         print "  rootfs: %s" % rootfs
+        print "Generating test image hwpack and rootfs"
+        self.generate_tarballs(BOARDS[self.client.hostname], hwpack, rootfs)
+
         print "Booting master image"
         self.client.boot_master_image()
 
         print "Waiting for network to come up"
         self.client.wait_network_up()
 
-    def generate_tarballs(self):
+    def generate_tarballs(self, boardtype, hwpack, rootfs):
         """
         TODO: need to use linaro-media-create to generate an image and
         extract a tarball for bootfs and rootfs, then put them in a place
         where they can be retrived via http
 
         For reference, see magma-chamber branch, extract-image script
+
+        Make use of server ability to prepare the tarballs for hwpack 
+        and rootfs
         """
+        #TODO: check l-m-c tools installation
+        
+        #use l-m-c to install hwpack and rootfs to qemu image
+        #assume root permission
+        hwpack_name = hwpack.rsplit("/", 1)[1]
+        rootfs_name = rootfs.rsplit("/", 1)[1]
+        qemuimg = "%s_sd.img" % boardtype
+        lmc_cmd = "linaro-media-create --rootfs ext3 --image_file %s" \
+                " --dev %s --binary %s --hwpack %s" % (qemuimg, boardtype, 
+                 rootfs_name, hwpack_name)
+
+        print "l-m-c command: %s" % lmc_cmd
+
+        #TODO: should import l-m-c to use python way, 
+        #      and l-m-c supports no keyboard-input
+        proc = Popen(shlex.split(lmc_cmd), stdin=PIPE, stdout=PIPE, 
+                stderr=PIPE)
+        output, rc = proc.communicate()
+        if proc.returncode != 0:
+            raise OperationFailed("l-m-c failed")
+
+        #package qemu image content to tarballs
+        #an ugly tar, need to find a way to create tar in relative path, 
+        #maybe tarfile
+        qemu_img_cmds = [ "modprobe nbd max_part=8", 
+                "qemu-nbd --connect=/dev/nbd0 %s" % qemuimg,
+                "mkdir -p /mnt/bootfs",
+                "mount /dev/nbd0p%s /mnt/bootfs" % TESTIMG_PARTS[boardtype][0],
+                "tar czf /tmp/%s -C /mnt/bootfs ." % hwpack_name,
+                "umount /mnt/bootfs",
+                "mkdir -p /mnt/rootfs",
+                "mount /dev/nbd0p%s /mnt/rootfs" % TESTIMG_PARTS[boardtype][0],
+                "tar czf /tmp/%s -C /mnt/rootfs ." % rootfs_name,
+                "umount /mnt/rootfs",
+                "qemu-nbd -d %s" % qemuimg,
+                ]
+
+        for cmd in qemu_img_cmds:
+            print cmd
+            proc = Popen(shlex.split(cmd), stdin=PIPE, stdout=PIPE, 
+                    stderr=PIPE)
+            output, err = proc.communicate()
+            if proc.returncode != 0:
+                """
+                Do more operations
+                """
+                raise OperationFailed("Failed at command: %s" % cmd)
+
+        #move the tarball to one place of validation farm server
+        #now it's server:/var/www/testimg
+        shutil.copy("/tmp/%s" % hwpack_name, WWW_DEPLOY_DIR)
+        shutil.copy("/tmp/%s" % rootfs_name, WWW_DEPLOY_DIR)
+
+        #change hwpack and rootfs link to ready testimg,
+        #however, hwpack and rootfs are not class member now
+        hwpack = WWW_DEPLOY_DIR + hwpack_name
+        rootfs = WWW_DEPLOY_DIR + rootfs_name
 
     def deploy_linaro_rootfs(self, rootfs):
         print "Deploying linaro image"
@@ -66,3 +135,6 @@
 
 class TimeoutError(Exception):
     pass
+
+class NotImplementedError(Exception):
+    print "Not implemented now"

=== modified file 'lava/config.py'
--- lava/config.py	2011-03-11 15:19:29 +0000
+++ lava/config.py	2011-03-15 11:15:58 +0000
@@ -43,6 +43,16 @@
         "bbg01": Mx51evkBoard,
         }
 
+#board type: [bootfs partition index, rootfs partition index]
+TESTIMG_PARTS = {
+PandaBoard:  [1, 2],
+BeagleBoard: [1, 2],
+Mx51evkBoard: [2, 3],
+}
+
 #Main LAVA server IP in the boards farm
 LAVA_SERVER_IP = "192.168.1.10"
 
+WWW_DEPLOY_DIR = "/var/www/testimg"
+
+WWW_DEPLOY_URL = "http://pkgserver/testimg/"