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

Proposed by Tyler Baker on 2013-09-06
Status: Merged
Merged at revision: 677
Proposed branch: lp:~tyler-baker/lava-dispatcher/consolidate-busybox-httpd
Merge into: lp:lava-dispatcher
Diff against target: 261 lines (+49/-108)
3 files modified
lava_dispatcher/device/bootloader.py (+6/-49)
lava_dispatcher/device/ipmi_pxe.py (+4/-55)
lava_dispatcher/device/target.py (+39/-4)
To merge this branch: bzr merge lp:~tyler-baker/lava-dispatcher/consolidate-busybox-httpd
Reviewer Review Type Date Requested Status
Antonio Terceiro 2013-09-06 Approve on 2013-09-09
Review via email: mp+184403@code.launchpad.net

Description of the change

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.

To post a comment you must log in.
Michael Hudson-Doyle (mwhudson) wrote :

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) wrote :

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) wrote :

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

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) wrote :

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) wrote :

Confirmed logic works correctly on highbank in staging.

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lava_dispatcher/device/bootloader.py'
2--- lava_dispatcher/device/bootloader.py 2013-08-30 22:15:05 +0000
3+++ lava_dispatcher/device/bootloader.py 2013-09-10 16:31:28 +0000
4@@ -51,7 +51,6 @@
5 self._boot_cmds = None
6 self._lava_cmds = None
7 self._uboot_boot = False
8- self._http_pid = None
9 # This is the offset into the path, used to reference bootfiles
10 self._offset = self.scratch_dir.index('images')
11
12@@ -115,11 +114,13 @@
13
14 def deploy_linaro(self, hwpack, rfs, bootloadertype):
15 self._uboot_boot = False
16- super(BootloaderTarget, self).deploy_linaro(hwpack, rfs, bootloadertype)
17+ super(BootloaderTarget, self).deploy_linaro(hwpack, rfs,
18+ bootloadertype)
19
20 def deploy_linaro_prebuilt(self, image, bootloadertype):
21 self._uboot_boot = False
22- super(BootloaderTarget, self).deploy_linaro_prebuilt(image, bootloadertype)
23+ super(BootloaderTarget, self).deploy_linaro_prebuilt(image,
24+ bootloadertype)
25
26 def _inject_boot_cmds(self):
27 if self._is_job_defined_boot_cmds(self.config.boot_cmds):
28@@ -164,28 +165,6 @@
29 else:
30 super(BootloaderTarget, self)._boot_linaro_image()
31
32- def start_http_server(self, runner, ip):
33- if self._http_pid is not None:
34- raise OperationFailed("busybox httpd already running with pid %d"
35- % self._http_pid)
36- # busybox produces no output to parse for,
37- # so run it in the bg and get its pid
38- runner.run('busybox httpd -f &')
39- runner.run('echo pid:$!:pid', response="pid:(\d+):pid", timeout=10)
40- if runner.match_id != 0:
41- raise OperationFailed("busybox httpd did not start")
42- else:
43- self._http_pid = runner.match.group(1)
44- url_base = "http://%s" % ip
45- return url_base
46-
47- def stop_http_server(self, runner):
48- if self._http_pid is None:
49- raise OperationFailed("busybox httpd not running, \
50- but stop_http_server called.")
51- runner.run('kill %s' % self._http_pid)
52- self._http_pid = None
53-
54 @contextlib.contextmanager
55 def file_system(self, partition, directory):
56 if self._uboot_boot:
57@@ -202,7 +181,7 @@
58 runner.run('cd /tmp') # need to be in same dir as fs.tgz
59
60 ip = runner.get_target_ip()
61- url_base = self.start_http_server(runner, ip)
62+ url_base = self._start_busybox_http_server(runner, ip)
63
64 url = url_base + '/fs.tgz'
65 logging.info("Fetching url: %s" % url)
66@@ -226,32 +205,10 @@
67 runner.run('rm -rf %s' % targetdir)
68 self._target_extract(runner, tf, parent_dir)
69 finally:
70- self.stop_http_server(runner)
71+ self._stop_busybox_http_server(runner)
72 else:
73 with super(BootloaderTarget, self).file_system(
74 partition, directory) as path:
75 yield path
76
77- def _target_extract(self, runner, tar_file, dest, timeout=-1):
78- tmpdir = self.context.config.lava_image_tmpdir
79- url = self.context.config.lava_image_url
80- tar_file = tar_file.replace(tmpdir, '')
81- tar_url = '/'.join(u.strip('/') for u in [url, tar_file])
82- self._target_extract_url(runner, tar_url, dest, timeout=timeout)
83-
84- def _target_extract_url(self, runner, tar_url, dest, timeout=-1):
85- decompression_cmd = ''
86- if tar_url.endswith('.gz') or tar_url.endswith('.tgz'):
87- decompression_cmd = '| /bin/gzip -dc'
88- elif tar_url.endswith('.bz2'):
89- decompression_cmd = '| /bin/bzip2 -dc'
90- elif tar_url.endswith('.tar'):
91- decompression_cmd = ''
92- else:
93- raise RuntimeError('bad file extension: %s' % tar_url)
94-
95- runner.run('wget -O - %s %s | /bin/tar -C %s -xmf -'
96- % (tar_url, decompression_cmd, dest),
97- timeout=timeout)
98-
99 target_class = BootloaderTarget
100
101=== modified file 'lava_dispatcher/device/ipmi_pxe.py'
102--- lava_dispatcher/device/ipmi_pxe.py 2013-08-30 22:15:05 +0000
103+++ lava_dispatcher/device/ipmi_pxe.py 2013-09-10 16:31:28 +0000
104@@ -32,7 +32,6 @@
105 )
106 from lava_dispatcher.errors import (
107 CriticalError,
108- OperationFailed,
109 )
110 from lava_dispatcher.downloader import (
111 download_image,
112@@ -176,7 +175,8 @@
113 runner.run('/bin/tar -cmzf /tmp/fs.tgz -C %s %s' % (parent_dir, target_name))
114 runner.run('cd /tmp') # need to be in same dir as fs.tgz
115
116- url_base = runner.start_http_server()
117+ ip = runner.get_target_ip()
118+ url_base = self._start_busybox_http_server(runner, ip)
119
120 url = url_base + '/fs.tgz'
121 logging.info("Fetching url: %s" % url)
122@@ -200,38 +200,16 @@
123 self._target_extract(runner, tf, parent_dir)
124
125 finally:
126- runner.stop_http_server()
127+ self._stop_busybox_http_server()
128 runner.run('umount /mnt')
129
130- def _target_extract(self, runner, tar_file, dest, timeout=-1):
131- tmpdir = self.context.config.lava_image_tmpdir
132- url = self.context.config.lava_image_url
133- tar_file = tar_file.replace(tmpdir, '')
134- tar_url = '/'.join(u.strip('/') for u in [url, tar_file])
135- self._target_extract_url(runner, tar_url, dest, timeout=timeout)
136-
137- def _target_extract_url(self, runner, tar_url, dest, timeout=-1):
138- decompression_cmd = ''
139- if tar_url.endswith('.gz') or tar_url.endswith('.tgz'):
140- decompression_cmd = '| /bin/gzip -dc'
141- elif tar_url.endswith('.bz2'):
142- decompression_cmd = '| /bin/bzip2 -dc'
143- elif tar_url.endswith('.tar'):
144- decompression_cmd = ''
145- else:
146- raise RuntimeError('bad file extension: %s' % tar_url)
147-
148- runner.run('wget -O - %s %s | /bin/tar -C %s -xmf -'
149- % (tar_url, decompression_cmd, dest),
150- timeout=timeout)
151-
152 @contextlib.contextmanager
153 def _as_master(self):
154 self.bootcontrol.power_on_boot_master()
155 self.proc.expect("\(initramfs\)")
156 self.proc.sendline('export PS1="%s"' % self.MASTER_PS1)
157 self.proc.expect(self.MASTER_PS1_PATTERN, timeout=180, lava_no_logging=1)
158- runner = BusyboxHttpdMasterCommandRunner(self)
159+ runner = MasterCommandRunner(self)
160
161 runner.run(". /scripts/functions")
162 runner.run("DEVICE=%s configure_networking" %
163@@ -257,32 +235,3 @@
164
165
166 target_class = IpmiPxeTarget
167-
168-
169-class BusyboxHttpdMasterCommandRunner(MasterCommandRunner):
170- """A CommandRunner to use when the target is booted into the master image.
171- """
172- http_pid = None
173-
174- def __init__(self, target):
175- super(BusyboxHttpdMasterCommandRunner, self).__init__(target)
176-
177- def start_http_server(self):
178- master_ip = self.get_master_ip()
179- if self.http_pid is not None:
180- raise OperationFailed("busybox httpd already running with pid %d" % self.http_pid)
181- # busybox produces no output to parse for, so run it in the bg and get its pid
182- self.run('busybox httpd -f &')
183- self.run('echo pid:$!:pid', response="pid:(\d+):pid", timeout=10)
184- if self.match_id != 0:
185- raise OperationFailed("busybox httpd did not start")
186- else:
187- self.http_pid = self.match.group(1)
188- url_base = "http://%s" % master_ip
189- return url_base
190-
191- def stop_http_server(self):
192- if self.http_pid is None:
193- raise OperationFailed("busybox httpd not running, but stop_http_server called.")
194- self.run('kill %s' % self.http_pid)
195- self.http_pid = None
196
197=== modified file 'lava_dispatcher/device/target.py'
198--- lava_dispatcher/device/target.py 2013-08-30 22:15:05 +0000
199+++ lava_dispatcher/device/target.py 2013-09-10 16:31:28 +0000
200@@ -191,13 +191,15 @@
201
202 def _customize_bootloader(self, connection, boot_cmds):
203 for line in boot_cmds:
204- parts = re.match('^(?P<action>sendline|expect)\s*(?P<command>.*)', line)
205+ parts = re.match('^(?P<action>sendline|expect)\s*(?P<command>.*)',
206+ line)
207 if parts:
208 try:
209 action = parts.group('action')
210 command = parts.group('command')
211 except AttributeError as e:
212- raise Exception("Badly formatted command in boot_cmds %s" % e)
213+ raise Exception("Badly formatted command in \
214+ boot_cmds %s" % e)
215 if action == "sendline":
216 connection.send(command)
217 connection.sendline('')
218@@ -205,8 +207,41 @@
219 command = re.escape(command)
220 connection.expect(command, timeout=300)
221 else:
222- self._wait_for_prompt(connection, self.config.bootloader_prompt, timeout=300)
223- connection.sendline(line)
224+ self._wait_for_prompt(connection,
225+ self.config.bootloader_prompt,
226+ timeout=300)
227+ connection.sendline(line)
228+
229+ def _target_extract(self, runner, tar_file, dest, timeout=-1):
230+ tmpdir = self.context.config.lava_image_tmpdir
231+ url = self.context.config.lava_image_url
232+ tar_file = tar_file.replace(tmpdir, '')
233+ tar_url = '/'.join(u.strip('/') for u in [url, tar_file])
234+ self._target_extract_url(runner, tar_url, dest, timeout=timeout)
235+
236+ def _target_extract_url(self, runner, tar_url, dest, timeout=-1):
237+ decompression_cmd = ''
238+ if tar_url.endswith('.gz') or tar_url.endswith('.tgz'):
239+ decompression_cmd = '| /bin/gzip -dc'
240+ elif tar_url.endswith('.bz2'):
241+ decompression_cmd = '| /bin/bzip2 -dc'
242+ elif tar_url.endswith('.tar'):
243+ decompression_cmd = ''
244+ else:
245+ raise RuntimeError('bad file extension: %s' % tar_url)
246+
247+ runner.run('wget -O - %s %s | /bin/tar -C %s -xmf -'
248+ % (tar_url, decompression_cmd, dest),
249+ timeout=timeout)
250+
251+ def _start_busybox_http_server(self, runner, ip):
252+ runner.run('busybox httpd -f &')
253+ runner.run('echo $! > /tmp/httpd.pid')
254+ url_base = "http://%s" % ip
255+ return url_base
256+
257+ def _stop_busybox_http_server(self, runner):
258+ runner.run('kill `cat /tmp/httpd.pid`')
259
260 def _customize_ubuntu(self, rootdir):
261 self.deployment_data = Target.ubuntu_deployment_data

Subscribers

People subscribed via source and target branches