Code review comment for lp:~nick-schutt/lava-dispatcher/nicks-highbank-support

Revision history for this message
Nicholas Schutt (nick-schutt) wrote :

send_control('c') does not appear to work when running in the initrd master. Maybe that needs to be looked at and fixed, but to make things work I had to run in the background and use kill instead.

I modified to do this differently now:

class HBMasterCommandRunner(MasterCommandRunner):

    http_pid = None

    def start_http_server(self):
        master_ip = self.get_master_ip()
        if self.http_pid != None:
            raise OperationFailed("busybox httpd already running with pid %" % 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 == None:
            raise OperationFailed("busybox httpd not running, but stop_http_server called.")
        self.run('kill %s' % self.http_pid)
        self.http_pid = None

> > +
> > + def start_http_server(self, runner, port=80):
> > + # busybox produces no output to parse for, so let it run as a
> daemon
> > + runner.run('busybox httpd -v -p %s' % port)
> > + url_base = "http://%s:%s" % (self.master_ip, port)
> > + return url_base
>
> I don't think we should use the standard http port here, because most
> probably use cases will include installing a proper web server, and in
> that case trying to start busybox on the default port will break things.
>
> Try some high port that is unlikely to be used by another service ...
> like ... 50888 for now. :)
>
> > +
> > + def stop_http_server(self, runner):
> > + runner.run('killall busybox')
>
> Not safe. I think it's better to just start busybox httpd in the
> foreground and then send a control-C here.
>

« Back to merge proposal