Merge lp://staging/~qzhang/lava-dispatcher/remove-hardcode into lp://staging/lava-dispatcher

On Wed, Mar 02, 2011, Spring Zhang wrote:
> Move hard code like board_type, lava_server_ip to a module lava.config
> 1. add two default config json files in cfg/, and it can be specified by other json files, which can extend board_type and board uboot cmd.
> 2. fix some issues, see r12
> 3. pass unit test
>
> Initially I plan to use json, Paul suggests to use module, consider the solution,
> 1. for there are some user-defined parameters like master server IP, unless LAVA is used only in Linaro validation team, we need to allow user to define such things.
> 2. for some fields which need to extend, like UBOOT configuration for different type of board. Or for item2, we can move to database?

 I think you're very right to want to go to .cfg files, yet because the
 config layout and parameters are still in lp:lava (which is a fairly
 new project), I would think it's a tad too early to go into a config
 file. Having the data in .py files is going to make our lives easier
 in this early stage, and it will also make it easier to test.

> - cmd.get('parameters', {})
> + params = cmd.get('parameters', {})

 Hmm, right, this still ain't fixed in tip; I've just committed the fix
 in r12 of lp:lava now; thanks

 Your changes looked good; thanks! For now, I would just put everything
 in .py instead of .cfg/.json; I don't have a strong view on this
 though, so I prefer not approving or disapproving this mp.

> +if __name__ == "__main__":
> + """
> + unit test by executing:
> + python -m lava.config
> + in lp:lava root directory
> + """
> + #load lava server cfg
> + LAVA_SERVER_CFG().load()
> + print "Board type map"
> + print "%s %s" % (LAVA_SERVER_CFG.IP, LAVA_SERVER_CFG.HOSTNAME)
> +
> + BOARD_CFG().load()
> + print "\nUboot cmd map"
> + for name, type in BOARD_CFG.BOARD_TYPE.items():
> + print "%s: %s" %(name, type)
> +
> + for type, cmd in BOARD_CFG.BOARDS_UBOOT.items():
> + print "%s: %s" %(type, cmd)

 Ah it seems we need proper unit tests

--
Loïc Minier

Yeah, as mentioned previously in email, I also felt like a python module was a better way to go than json config.

CC Zygmunt and Mirsad

On 3 March 2011 15:40, Spring Zhang <email address hidden> wrote:

> Spring Zhang has proposed merging lp:~qzhang/lava/remove-hardcode into
> lp:lava.
>
> Requested reviews:
> Paul Larson (pwlars)
> Loïc Minier (lool)
>
> For more details, see:
> https://code.launchpad.net/~qzhang/lava/remove-hardcode/+merge/52013<https://code.launchpad.net/%7Eqzhang/lava/remove-hardcode/+merge/52013>
>
> Move all configurations to module to lava.config due to comments, now it's
> one module to import
> --
> https://code.launchpad.net/~qzhang/lava/remove-hardcode/+merge/52013<https://code.launchpad.net/%7Eqzhang/lava/remove-hardcode/+merge/52013>
> You are the owner of lp:~qzhang/lava/remove-hardcode.
>
> === modified file 'lava/actions/deploy.py'
> --- lava/actions/deploy.py 2011-03-02 21:11:21 +0000
> +++ lava/actions/deploy.py 2011-03-03 07:39:59 +0000
> @@ -63,5 +63,6 @@
> self.client.run_shell_command(
> 'umount /mnt/boot',
> response = master_str)
> +
> class TimeoutError(Exception):
> pass
>
> === modified file 'lava/client.py'
> --- lava/client.py 2011-03-02 21:11:21 +0000
> +++ lava/client.py 2011-03-03 07:39:59 +0000
> @@ -1,37 +1,7 @@
> import pexpect
> import sys
> import time
> -
> -"""
> -This is an ugly hack, the uboot commands for a given board type and the
> board
> -type of a test machine need to come from the device registry. This is an
> -easy way to look it up for now though, just to show the rest of the code
> -around it
> -"""
> -BOARDS = {
> - "beagle":["mmc init",
> - "setenv bootcmd 'fatload mmc 0:3 0x80000000 uImage; fatload mmc "
> \
> - "0:3 0x81600000 uInitrd; bootm 0x80000000 0x81600000'",
> - "setenv bootargs ' console=tty0 console=ttyO2,115200n8 " \
> - "root=LABEL=testrootfs rootwait ro earlyprintk fixrtc nocompcache
> " \
> - "vram=12M omapfb.debug=y omapfb.mode=dvi:1280x720MR-16@60'",
> - "boot"],
> - "panda":["mmc init",
> - "setenv bootcmd 'fatload mmc 0:5 0x80200000 uImage; fatload mmc "
> \
> - "0:5 0x81600000 uInitrd; bootm 0x80200000 0x81600000'",
> - "setenv bootargs ' console=tty0 console=ttyO2,115200n8 " \
> - "root=LABEL=testrootfs rootwait ro earlyprintk fixrtc nocompcache
> " \
> - "vram=32M omapfb.vram=0:8M mem=463M ip=none'",
> - "boot"]
> -}
> -
> -BOARD_TYPE = {
> - "panda01": "panda",
> - "panda02": "panda",
> - "beaglexm01": "beagle",
> - "vexpress01": "vexpress",
> - "vexpress02": "vexpress"
> - }
> +from lava.config import *
>
> class LavaClient:
> def __init__(self, hostname):
> @@ -40,7 +10,8 @@
> #serial can be slow, races do funny things if you don't increase
> delay
> self.proc.delaybeforesend=1
> #This is temporary, eventually this should come from the db
> - self.board_type = BOARD_TYPE[hostname]
> + self.board_type = BOARD_CFG.BOARD_TYPE[hostname]
> + self.target = hostname
>
> def in_master_shell(self):
> """ Check that we are in a shell on the master image
> @@ -81,7 +52,7 @@
> except:
> self.hard_reboot()
> ...

On Thu, Mar 03, 2011, Spring Zhang wrote:
> - self.proc.sendline("LC_ALL=C ping -W4 -c1 192.168.1.10")
> + self.proc.sendline("LC_ALL=C ping -W4 -c1 %s" % LAVA_SERVER_CFG.IP)

 Maybe GATEWAY_IP? You could import GATEWAY_IP globally to not repeat
 LAVA_SERVER_CFG.

> +class BOARD_CFG:
> + BOARDS_UBOOT = {
> + "beagle":["mmc init",
> + "setenv bootcmd 'fatload mmc 0:3 0x80000000 uImage; fatload mmc " \
> + "0:3 0x81600000 uInitrd; bootm 0x80000000 0x81600000'",
> + "setenv bootargs ' console=tty0 console=ttyO2,115200n8 " \
> + "root=LABEL=testrootfs rootwait ro earlyprintk fixrtc nocompcache "\
> + "vram=12M omapfb.debug=y omapfb.mode=dvi:1280x720MR-16@60'",
> + "boot"],
[...]
> +
> + BOARD_TYPE = {
> + "panda01": "panda",
> + "panda02": "panda",
> + "beaglexm01": "beagle",
> + "vexpress01": "vexpress",
> + "vexpress02": "vexpress",
> + "bbg01": "mx51evk",
> + }

 Rather than using "beagle", "vexpress" etc. as indices, it might be
 more elegant to use classes and instantiate them. We would have:
 class Board(object):
     pass

 class PandaBoard(Board):
     u_boot_cmd = ("mmc init; "
         "setenv [...]")

     def __init__(self, name):
        self.name = name

 boards = [
     PandaBoard("panda01"),
     PandaBoard("panda02")]

> +class LAVA_SERVER_CFG:
> + IP = "192.168.1.10"
> + HOSTNAME = ""

 I would just put:
 # IP address of the main LAVA server on the LAN where the boards live
 SERVER_IP = "192.168.1.10"

 HOSTNAME is unused; just drop it

> +if __name__ == "__main__":
> + """
> + unit test by executing:
> + python -m lava.config
> + in lp:lava root directory
> + """

 Should actually be an unittest; will try to propose an example

--
Loïc Minier

 See lp:~lool/lava/add-unittest-for-config for a branch adding
 unittests.

 I wouldn't actually want unittests for config data, but only for
 code/logic. But this gives you an example layout and configs to
 implement tests in your future code submissions.

--
Loïc Minier