Issue 30961 in chromium-os: Investigate moving the invocation of servod out of autotest code

[chrom...@googlecode.com]

Status: Assigned
Owner: jrbarne...@chromium.org
Labels: Type-Bug Pri-1 Area-Lab iteration-56

New issue 30961 by sco...@chromium.org: Investigate moving the invocation
of servod out of autotest code
http://code.google.com/p/chromium-os/issues/detail?id=30961

Servod is currently started within Autotest as a subprocess every time a
different test that utilizes servo is run. If the assumption that the only
time we need servod launched is when run_remote_tests is used is true, we
should investigate moving the launching of a long standing servod process
into run_remote_tests.

One unresolved issue with how we currently invoke servod is how we tell
Autotest to use servod on localhost or try and use the lab convention of
hostname-servo for the servod server.

There are a few options to solve this:
1) Pass arguments in to autoserv that the control file then uses to create
the host object pointing at the proper servod instance
2) Update the site code that invokes servod to check a particular
environment variable and use it with impunity if it is set.

Correct me if I am wrong but to implement 1 we would need to change the
control file to parse args using utils.args_to_dict and pass in the proper
option to hosts.create_host. Arg parsing at this level is less than desired
and becomes the onus of every control file author to remember what to add
for the test to function properly.

The second option is to create an environment variable that the servod site
code honors and uses if it is set. This to me seems like the cleanest
solution. It is a bit magical and that is bad but overall it isn't too far
and we can make it apparent what is happening with logging. You have
minimal code to add to run_remote_tests to set that variable and minimal
code to add to the site host implementation.

If something was missed here or does not make sense given incorrect
assumptions please add those below otherwise lets implement #2.

[chrom...@googlecode.com]

Updates:
Blockedon: 30980

Comment #1 on issue 30961 by jrbarne...@chromium.org: Investigate moving

the invocation of servod out of autotest code
http://code.google.com/p/chromium-os/issues/detail?id=30961

Regarding figuring out in autoserv whether to connect to a local servod,
or a lab servod: The problem is essentially solved. Control files for
tests
that need servo pass require_servo=True to hosts.create_host(). SiteHost
sees that setting in _initialize(), and applies this logic:
1) If the target has a servo in the lab, use it regardless of
require_servo.
2) If there's no lab servo, and require_servo is true, start servod
locally.
3) Otherwise, there's no servo.

To make it work if servod is started by run_remote_tests.sh, it's sufficient
to change "start servod locally" in step 2) to just "connect to a local
servod".
The actual code change to make this happen is more than just SiteHost, but
not very big.

Changing run_remote_tests.sh to start servod is little more than adding an
option to request it, provided we fix issue 30980. I note that some complex
servo configurations might need morework; I don't think such use cases
need to be addressed, because it would be easy for users with complex
requirements to start servod manually.

[chrom...@googlecode.com]

Updates:
Status: Started

Comment #2 on issue 30961 by jrbarne...@chromium.org: Investigate moving

the invocation of servod out of autotest code
http://code.google.com/p/chromium-os/issues/detail?id=30961

Changes will consist of the following pieces:
* Remove code to launch 'servod' from autotest.
* Add a new --servo option to run_remote_tests, and start servod
if the option is provided.

The automatic start of 'servod' will need provide a board type on
the command line, so it can't be complete without bug 30980.

As noted in the previous comment, users that need to customize
command line options can (must) start 'servod' manually, and then
use run_remote_tests without the --servo option.

[chrom...@googlecode.com]

Comment #3 on issue 30961 by tbr...@chromium.org: Investigate moving the

invocation of servod out of autotest code
http://code.google.com/p/chromium-os/issues/detail?id=30961

Without,

--servo_sid ( or equiv ) users with multiple servos connected to the host
won't be able to invoke servod via run_remote_tests

In addition, without
--servo_port ( or equiv ) users desiring to launch multiple servod's won't
be able to.

That port information will also need to be communicated to the autotest so
that its client communicates to the correct server.

Perhaps adding

--servo[='string'] functionality will provide the necessary functionality
w/o individual aruguments.

where 'string' is 'port=<num>,serial=<id>'

[chrom...@googlecode.com]

Comment #4 on issue 30961 by jrbarne...@chromium.org: Investigate moving

the invocation of servod out of autotest code
http://code.google.com/p/chromium-os/issues/detail?id=30961

We definitely need to solve the port designation issue.

How common is it for a user with two servo devices to want to
use run_remote_tests? If we solve the port designation problem,
those users can just start servo manually. However, if there are
enough users affected, that solution may not be good enough.

[chrom...@googlecode.com]

Updates:
Blockedon: -chromium-os:30980 30980

Comment #6 on issue 30961 by jrbarne...@chromium.org: Investigate moving

the invocation of servod out of autotest code
http://code.google.com/p/chromium-os/issues/detail?id=30961

(No comment was entered for this change.)

[chrom...@googlecode.com]

Comment #8 on issue 30961 by bugdro...@chromium.org: Investigate moving the

invocation of servod out of autotest code

http://code.google.com/p/chromium-os/issues/detail?id=30961#c8

Commit: 55fb80614936ddcec6d8c2d0c81c86db55e52b62
Email: jrbar...@chromium.org

Remove _launch_servod() from servo.py

If 'servod' is to run locally, it is now launched from run_remote_tests,
rather than from inside autoserv.

BUG=chromium-os:30961
TEST=platform_ServoPyAuto, with a locally attached servo for an Alex

Change-Id: I93372498dc494309a134ad2eddf14f542b95e4a6
Reviewed-on: https://gerrit.chromium.org/gerrit/23565
Tested-by: Richard Barnette <jrbar...@chromium.org>
Reviewed-by: Todd Broch <tbr...@chromium.org>
Commit-Ready: Richard Barnette <jrbar...@chromium.org>

M server/cros/servo.py
M server/hosts/site_host.py
M server/site_tests/platform_CloseOpenLid/control
M server/site_tests/platform_InstallRecoveryImage/control
M server/site_tests/platform_LidStress/control
M server/site_tests/platform_LongPressPower/control
M server/site_tests/platform_ServoPyAuto/control

[chrom...@googlecode.com]

Comment #9 on issue 30961 by bugdro...@chromium.org: Investigate moving the

invocation of servod out of autotest code

http://code.google.com/p/chromium-os/issues/detail?id=30961#c9

Commit: 40fd03d54244fa2dec7a3974c5fb5f8a624ad937
Email: jrbar...@chromium.org

Add new --servo option to run_remote_tests.sh

When the new option is used, the script will start servod
automatically at the start of testing, and stop it when the
tests are complete.

CQ-DEPEND=I93372498dc494309a134ad2eddf14f542b95e4a6

BUG=chromium-os:30961
TEST=platform_ServoPyAuto

Change-Id: I0e6d64e004ea603fa71e6aa6dd93c12aa8b222a1
Reviewed-on: https://gerrit.chromium.org/gerrit/23566

Tested-by: Richard Barnette <jrbar...@chromium.org>
Reviewed-by: Todd Broch <tbr...@chromium.org>

Reviewed-by: Chris Sosa <so...@chromium.org>
Commit-Ready: Richard Barnette <jrbar...@chromium.org>

M run_remote_tests.sh

[chrom...@googlecode.com]

Comment #10 on issue 30961 by bugdro...@chromium.org: Investigate moving

the invocation of servod out of autotest code

http://code.google.com/p/chromium-os/issues/detail?id=30961#c10

Commit: 0edf7e1cc0d2130f039cf78608e1f962bc890db9
Email: tbr...@chromium.org

autotest: servo: remove remnants of servod.

BUG=chromium-os:30961
TEST=(rchenna) runs FAFT successfully

Change-Id: Id4c73d65b0106a400862b0ab01fe9c096b6d3275
Reviewed-on: https://gerrit.chromium.org/gerrit/25297
Reviewed-by: Rajesh Chenna <rch...@chromium.org>
Tested-by: Rajesh Chenna <rch...@chromium.org>
Commit-Ready: Todd Broch <tbr...@chromium.org>

M server/cros/servo.py
M server/cros/servo_test.py

[chrom...@googlecode.com]

Updates:
Status: Fixed
Blockedon: -chromium-os:30980 chromium-os:30980

Comment #13 on issue 30961 by jrbarne...@chromium.org: Investigate moving

the invocation of servod out of autotest code
http://code.google.com/p/chromium-os/issues/detail?id=30961

[chrom...@googlecode.com]

Comment #15 on issue 30961 by bugdro...@chromium.org: Investigate moving

the invocation of servod out of autotest code

http://code.google.com/p/chromium-os/issues/detail?id=30961#c15

Project: chromiumos/third_party/autotest
Branch : master
Author : Vadim Bendebury <vbe...@chromium.org>
Commit : bb7319569e0d6ed185f14a709dd1eb0d7f1b6467

Code Review +2: Todd Broch, Vadim Bendebury
Verified +1: Vadim Bendebury
Change-Id : I9692c8fae9e013cb1a3072b849df41228b63899f
Reviewed-at : https://gerrit.chromium.org/gerrit/39288

Complete servo_test cleanup

An earlier change (https://gerrit.chromium.org/gerrit/#/c/25297)
simplified the way servod is initialized, inadvertently leaving behind
some now unused code.

This change removes the dead code and further simplifies the interface.

BUG=chromium-os:30961
TEST=manual
ran

~/trunk/src/scripts/run_remote_tests.sh --board=link --servo \
--remote=192.168.1.4 suite:faft_lv1

observed it succeed.

M server/cros/servo_test.py