Merge lp:~tyler-baker/lava-dispatcher/consolidate-busybox-httpd into lp:lava-dispatcher

[Tyler Baker]

Consolidation of the busybox httpd logic. The ipmi_pxe and bootloader class both use these functions to allow the exchange of files to and from the target. It makes sense to reuse a single implementation, rather than maintaining both.

* Moved all the functionality into target.py for reuse.
* start_http_server -> start_busybox_http_server
* stop_http_server -> stop_busybox_http_server

Tested these changes on a bootloader device.

http://community.validation.linaro.org/scheduler/job/1065

However, I cannot test these changes on a highbank node until merged into staging.

[Michael Hudson-Doyle (mwhudson)]

I know you're just moving code around so this is a side comment but does the httpd's pid really need to come to the host? Seems we could just execute

$ busybox httpd &
$ echo $! > /tmp/httpd.pid

to start and

$ kill `cat /tmp/httpd.pid`

to stop it.

[Antonio Terceiro (terceiro)]

Looks good to me.

I also agree with Michael's comments - doing that will simplify the code even more.

 review approve

review: Approve

[Tyler Baker (tyler-baker)]

I'll update the branch per Michael's comments after I retest. Thanks for the suggestion!

[lp:~tyler-baker/lava-dispatcher/consolidate-busybox-httpd]

676. By Tyler Baker on 2013-09-10

Remove httpd pid

677. By Tyler Baker on 2013-09-10

Remove the dead code

[Tyler Baker (tyler-baker)]

Branch updated to reflect comments.

Tested the changes on a bootloader device:

http://community.validation.linaro.org/scheduler/job/1138

Merging to test on the highbank in staging.

[Tyler Baker (tyler-baker)]

Confirmed logic works correctly on highbank in staging.

https://staging.validation.linaro.org/scheduler/job/2285

=== modified file 'lava_dispatcher/device/bootloader.py'
--- lava_dispatcher/device/bootloader.py	2013-08-30 22:15:05 +0000
+++ lava_dispatcher/device/bootloader.py	2013-09-10 16:31:28 +0000
@@ -51,7 +51,6 @@
         self._boot_cmds = None
         self._lava_cmds = None
         self._uboot_boot = False
-        self._http_pid = None
         # This is the offset into the path, used to reference bootfiles
         self._offset = self.scratch_dir.index('images')
 
@@ -115,11 +114,13 @@
 
     def deploy_linaro(self, hwpack, rfs, bootloadertype):
         self._uboot_boot = False
-        super(BootloaderTarget, self).deploy_linaro(hwpack, rfs, bootloadertype)
+        super(BootloaderTarget, self).deploy_linaro(hwpack, rfs,
+                                                            bootloadertype)
 
     def deploy_linaro_prebuilt(self, image, bootloadertype):
         self._uboot_boot = False
-        super(BootloaderTarget, self).deploy_linaro_prebuilt(image, bootloadertype)
+        super(BootloaderTarget, self).deploy_linaro_prebuilt(image,
+                                                             bootloadertype)
 
     def _inject_boot_cmds(self):
         if self._is_job_defined_boot_cmds(self.config.boot_cmds):
@@ -164,28 +165,6 @@
         else:
             super(BootloaderTarget, self)._boot_linaro_image()
 
-    def start_http_server(self, runner, ip):
-        if self._http_pid is not None:
-            raise OperationFailed("busybox httpd already running with pid %d"
-                                  % self._http_pid)
-        # busybox produces no output to parse for,
-        # so run it in the bg and get its pid
-        runner.run('busybox httpd -f &')
-        runner.run('echo pid:$!:pid', response="pid:(\d+):pid", timeout=10)
-        if runner.match_id != 0:
-            raise OperationFailed("busybox httpd did not start")
-        else:
-            self._http_pid = runner.match.group(1)
-        url_base = "http://%s" % ip
-        return url_base
-
-    def stop_http_server(self, runner):
-        if self._http_pid is None:
-            raise OperationFailed("busybox httpd not running, \
-                                  but stop_http_server called.")
-        runner.run('kill %s' % self._http_pid)
-        self._http_pid = None
-
     @contextlib.contextmanager
     def file_system(self, partition, directory):
         if self._uboot_boot:
@@ -202,7 +181,7 @@
                 runner.run('cd /tmp')  # need to be in same dir as fs.tgz
 
                 ip = runner.get_target_ip()
-                url_base = self.start_http_server(runner, ip)
+                url_base = self._start_busybox_http_server(runner, ip)
 
                 url = url_base + '/fs.tgz'
                 logging.info("Fetching url: %s" % url)
@@ -226,32 +205,10 @@
                     runner.run('rm -rf %s' % targetdir)
                     self._target_extract(runner, tf, parent_dir)
             finally:
-                self.stop_http_server(runner)
+                self._stop_busybox_http_server(runner)
         else:
             with super(BootloaderTarget, self).file_system(
                                                 partition, directory) as path:
                 yield path
 
-    def _target_extract(self, runner, tar_file, dest, timeout=-1):
-        tmpdir = self.context.config.lava_image_tmpdir
-        url = self.context.config.lava_image_url
-        tar_file = tar_file.replace(tmpdir, '')
-        tar_url = '/'.join(u.strip('/') for u in [url, tar_file])
-        self._target_extract_url(runner, tar_url, dest, timeout=timeout)
-
-    def _target_extract_url(self, runner, tar_url, dest, timeout=-1):
-        decompression_cmd = ''
-        if tar_url.endswith('.gz') or tar_url.endswith('.tgz'):
-            decompression_cmd = '| /bin/gzip -dc'
-        elif tar_url.endswith('.bz2'):
-            decompression_cmd = '| /bin/bzip2 -dc'
-        elif tar_url.endswith('.tar'):
-            decompression_cmd = ''
-        else:
-            raise RuntimeError('bad file extension: %s' % tar_url)
-
-        runner.run('wget -O - %s %s | /bin/tar -C %s -xmf -'
-                   % (tar_url, decompression_cmd, dest),
-                   timeout=timeout)
-
 target_class = BootloaderTarget
 
=== modified file 'lava_dispatcher/device/ipmi_pxe.py'
--- lava_dispatcher/device/ipmi_pxe.py	2013-08-30 22:15:05 +0000
+++ lava_dispatcher/device/ipmi_pxe.py	2013-09-10 16:31:28 +0000
@@ -32,7 +32,6 @@
 )
 from lava_dispatcher.errors import (
     CriticalError,
-    OperationFailed,
 )
 from lava_dispatcher.downloader import (
     download_image,
@@ -176,7 +175,8 @@
                 runner.run('/bin/tar -cmzf /tmp/fs.tgz -C %s %s' % (parent_dir, target_name))
                 runner.run('cd /tmp')  # need to be in same dir as fs.tgz
 
-                url_base = runner.start_http_server()
+                ip = runner.get_target_ip()
+                url_base = self._start_busybox_http_server(runner, ip)
 
                 url = url_base + '/fs.tgz'
                 logging.info("Fetching url: %s" % url)
@@ -200,38 +200,16 @@
                     self._target_extract(runner, tf, parent_dir)
 
             finally:
-                    runner.stop_http_server()
+                    self._stop_busybox_http_server()
                     runner.run('umount /mnt')
 
-    def _target_extract(self, runner, tar_file, dest, timeout=-1):
-        tmpdir = self.context.config.lava_image_tmpdir
-        url = self.context.config.lava_image_url
-        tar_file = tar_file.replace(tmpdir, '')
-        tar_url = '/'.join(u.strip('/') for u in [url, tar_file])
-        self._target_extract_url(runner, tar_url, dest, timeout=timeout)
-
-    def _target_extract_url(self, runner, tar_url, dest, timeout=-1):
-        decompression_cmd = ''
-        if tar_url.endswith('.gz') or tar_url.endswith('.tgz'):
-            decompression_cmd = '| /bin/gzip -dc'
-        elif tar_url.endswith('.bz2'):
-            decompression_cmd = '| /bin/bzip2 -dc'
-        elif tar_url.endswith('.tar'):
-            decompression_cmd = ''
-        else:
-            raise RuntimeError('bad file extension: %s' % tar_url)
-
-        runner.run('wget -O - %s %s | /bin/tar -C %s -xmf -'
-                   % (tar_url, decompression_cmd, dest),
-                   timeout=timeout)
-
     @contextlib.contextmanager
     def _as_master(self):
         self.bootcontrol.power_on_boot_master()
         self.proc.expect("\(initramfs\)")
         self.proc.sendline('export PS1="%s"' % self.MASTER_PS1)
         self.proc.expect(self.MASTER_PS1_PATTERN, timeout=180, lava_no_logging=1)
-        runner = BusyboxHttpdMasterCommandRunner(self)
+        runner = MasterCommandRunner(self)
 
         runner.run(". /scripts/functions")
         runner.run("DEVICE=%s configure_networking" %
@@ -257,32 +235,3 @@
 
 
 target_class = IpmiPxeTarget
-
-
-class BusyboxHttpdMasterCommandRunner(MasterCommandRunner):
-    """A CommandRunner to use when the target is booted into the master image.
-    """
-    http_pid = None
-
-    def __init__(self, target):
-        super(BusyboxHttpdMasterCommandRunner, self).__init__(target)
-
-    def start_http_server(self):
-        master_ip = self.get_master_ip()
-        if self.http_pid is not None:
-            raise OperationFailed("busybox httpd already running with pid %d" % self.http_pid)
-        # busybox produces no output to parse for, so run it in the bg and get its pid
-        self.run('busybox httpd -f &')
-        self.run('echo pid:$!:pid', response="pid:(\d+):pid", timeout=10)
-        if self.match_id != 0:
-            raise OperationFailed("busybox httpd did not start")
-        else:
-            self.http_pid = self.match.group(1)
-        url_base = "http://%s" % master_ip
-        return url_base
-
-    def stop_http_server(self):
-        if self.http_pid is None:
-            raise OperationFailed("busybox httpd not running, but stop_http_server called.")
-        self.run('kill %s' % self.http_pid)
-        self.http_pid = None
 
=== modified file 'lava_dispatcher/device/target.py'
--- lava_dispatcher/device/target.py	2013-08-30 22:15:05 +0000
+++ lava_dispatcher/device/target.py	2013-09-10 16:31:28 +0000
@@ -191,13 +191,15 @@
 
     def _customize_bootloader(self, connection, boot_cmds):
         for line in boot_cmds:
-            parts = re.match('^(?P<action>sendline|expect)\s*(?P<command>.*)', line)
+            parts = re.match('^(?P<action>sendline|expect)\s*(?P<command>.*)',
+                             line)
             if parts:
                 try:
                     action = parts.group('action')
                     command = parts.group('command')
                 except AttributeError as e:
-                    raise Exception("Badly formatted command in boot_cmds %s" % e)
+                    raise Exception("Badly formatted command in \
+                                      boot_cmds %s" % e)
                 if action == "sendline":
                     connection.send(command)
                     connection.sendline('')
@@ -205,8 +207,41 @@
                     command = re.escape(command)
                     connection.expect(command, timeout=300)
             else:
-                self._wait_for_prompt(connection, self.config.bootloader_prompt, timeout=300)
-                connection.sendline(line)        
+                self._wait_for_prompt(connection,
+                                      self.config.bootloader_prompt,
+                                      timeout=300)
+                connection.sendline(line)
+
+    def _target_extract(self, runner, tar_file, dest, timeout=-1):
+        tmpdir = self.context.config.lava_image_tmpdir
+        url = self.context.config.lava_image_url
+        tar_file = tar_file.replace(tmpdir, '')
+        tar_url = '/'.join(u.strip('/') for u in [url, tar_file])
+        self._target_extract_url(runner, tar_url, dest, timeout=timeout)
+
+    def _target_extract_url(self, runner, tar_url, dest, timeout=-1):
+        decompression_cmd = ''
+        if tar_url.endswith('.gz') or tar_url.endswith('.tgz'):
+            decompression_cmd = '| /bin/gzip -dc'
+        elif tar_url.endswith('.bz2'):
+            decompression_cmd = '| /bin/bzip2 -dc'
+        elif tar_url.endswith('.tar'):
+            decompression_cmd = ''
+        else:
+            raise RuntimeError('bad file extension: %s' % tar_url)
+
+        runner.run('wget -O - %s %s | /bin/tar -C %s -xmf -'
+                   % (tar_url, decompression_cmd, dest),
+                   timeout=timeout)
+
+    def _start_busybox_http_server(self, runner, ip):
+        runner.run('busybox httpd -f &')
+        runner.run('echo $! > /tmp/httpd.pid')
+        url_base = "http://%s" % ip
+        return url_base
+
+    def _stop_busybox_http_server(self, runner):
+        runner.run('kill `cat /tmp/httpd.pid`')
 
     def _customize_ubuntu(self, rootdir):
         self.deployment_data = Target.ubuntu_deployment_data