faft: Rework the xmlrpc server to allow quit and disconnect.
When shutting down, it's nice to have a way to cleanly quit and/or
disconnect from the RPC server, without the framework having to ssh
back in to kill the process. Disconnecting cleanly helps prevent
the RPC client from hanging on a dead connection, by forcing it to
make a new one instead.
In order to make quitting possible, the rpc server now uses the
autotest standard rpc_server module, so the RpcRouter class was
renamed to fit the XmlRpcDelegate.
This change also makes the RPC server itself terminate old instances,
to avoid encountering "address already in use" errors. This removes
the need for disconnect() to try running pkill on the dut.
The first tests to call disconnect() in the body are the Consecutive
Reboot tests, since they're simple tests that demonstrate a need to
disconnect for safety.
BUG=None
TEST=Run firmware_FAFTRPC.all and several FAFT levels: faft_lv1, faft_lv5
Change-Id: Ib9ba9852553da99453e2dec04944a68a0af2aa5e
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/autotest/+/1968389
Tested-by: Dana Goyette <[email protected]>
Commit-Queue: Dana Goyette <[email protected]>
Reviewed-by: Greg Edelston <[email protected]>
diff --git a/client/cros/faft/config.py b/client/cros/faft/config.py
index 2388af4..0299b30 100644
--- a/client/cros/faft/config.py
+++ b/client/cros/faft/config.py
@@ -9,7 +9,8 @@
# RPC server that runs on the DUT.
rpc_port = 9990
rpc_command = '/usr/local/autotest/cros/faft/rpc_server.py'
- rpc_command_short = 'rpc_server'
- rpc_ready_call = 'system.is_available'
+ rpc_command_short = 'rpc_server.py'
+ rpc_ready_call = 'ready'
+ rpc_quit_call = 'quit'
rpc_timeout = 20
rpc_logfile = '/var/log/faft_xmlrpc_server.log'
diff --git a/client/cros/faft/rpc_functions.py b/client/cros/faft/rpc_functions.py
index 3b8a7d6..14b80a8 100644
--- a/client/cros/faft/rpc_functions.py
+++ b/client/cros/faft/rpc_functions.py
@@ -8,12 +8,15 @@
@note: When adding categories, please also update server/cros/faft/rpc_proxy.pyi
"""
import httplib
+import logging
import os
+import signal
import sys
import tempfile
import traceback
import xmlrpclib
+from autotest_lib.client.cros import xmlrpc_server
from autotest_lib.client.common_lib.cros import cros_config
from autotest_lib.client.cros.faft.utils import (
cgpt_handler,
@@ -27,14 +30,14 @@
)
-class RPCRouter(object):
+class FaftXmlRpcDelegate(xmlrpc_server.XmlRpcDelegate):
"""
A class which routes RPC methods to the proper servicers.
Firmware tests are able to call an RPC method via:
- FAFTClient.[category].[method_name](params)
+ <FAFTClient>.[category].[method_name](params)
When XML-RPC is being used, the RPC server routes the called method to:
- RPCHandler._dispatch('[category].[method]', params)
+ <XmlRpcDelegate>._dispatch('[category].[method_name]', params)
The method is then dispatched to a Servicer class.
"""
@@ -43,6 +46,7 @@
@type os_if: os_interface.OSInterface
"""
+ self._ready = False
self.bios = BiosServicer(os_if)
self.cgpt = CgptServicer(os_if)
self.ec = EcServicer(os_if)
@@ -67,7 +71,37 @@
self._os_if = os_if
- def _report_error(self, fault_code, message, exc_info=None):
+ def __enter__(self):
+ """Enter the the delegate context (when XmlRpcServer.run() starts).
+
+ The server is marked ready here, rather than immediately when created.
+ """
+ logging.debug("%s: Serving FAFT functions", self.__class__.__name__)
+ self._ready = True
+
+ def __exit__(self, exception, value, traceback):
+ """Exit the delegate context (when XmlRpcServer.run() finishes).
+
+ The server is marked not ready, to prevent the client from using
+ the wrong server when quitting one instance and starting another.
+ """
+ self._ready = False
+ logging.debug("%s: Done.", self.__class__.__name__)
+
+ def quit(self):
+ """Exit the xmlrpc server."""
+ self._ready = False
+ os.kill(os.getpid(), signal.SIGINT)
+
+ def ready(self):
+ """Is the RPC server ready to serve calls in a useful manner?
+
+ The server is only marked ready during the XmlRpcServer.run() loop.
+ This method suppresses the extra logging of ready() from the superclass.
+ """
+ return self._ready
+
+ def _report_error(self, fault_code, message, exc_info=False):
"""Raise the given RPC error text, including information about last
exception from sys.exc_info(). The log file gets the traceback in text;
the raised exception keeps the old traceback (but not in text).
@@ -79,7 +113,7 @@
@param fault_code: the status code to use
@param message: the string message to include before exception text
- @param exc_info: the tuple from sys.exc_info()
+ @param exc_info: true to use the tuple from sys.exc_info()
@return the exception to raise
@type fault_code: int
@@ -96,10 +130,11 @@
traceback.format_exception(exc_class, exc, tb))
self._os_if.log('Error: %s.\n%s' % (message, tb_str.rstrip()))
- exc_str = ''.join(
- traceback.format_exception_only(exc_class, exc))
- exc = xmlrpclib.Fault(
- fault_code, '%s. %s' % (message, exc_str.rstrip()))
+ if not isinstance(exc, xmlrpclib.Fault):
+ exc_str = ''.join(
+ traceback.format_exception_only(exc_class, exc))
+ exc = xmlrpclib.Fault(
+ fault_code, '%s. %s' % (message, exc_str.rstrip()))
raise exc, None, tb
finally:
del exc_info
@@ -124,21 +159,13 @@
self._os_if.log('Called: %s%s' % (called_method, params))
name_pieces = called_method.split('.')
- num_pieces = len(name_pieces)
- if num_pieces < 1:
+ if not name_pieces:
raise self._report_error(
httplib.BAD_REQUEST,
'RPC request is invalid (completely empty): "%s"' %
called_method)
- if num_pieces < 2:
- # must be category.func (maybe with .__str__)
- raise self._report_error(
- httplib.BAD_REQUEST,
- 'RPC request is invalid (must have category.method format):'
- ' "%s"' % called_method)
-
method_name = name_pieces.pop()
category = '.'.join(name_pieces)
@@ -148,15 +175,15 @@
# Forbid early, to prevent seeing which methods exist.
raise self._report_error(
httplib.FORBIDDEN,
- 'RPC method name is private: %s.[%s]' %
- (category, method_name))
+ 'RPC method name is private: %s%s[%s]' %
+ (category, '.' if category else '', method_name))
- if not method_name:
+ elif not method_name:
# anything.()
raise self._report_error(
httplib.BAD_REQUEST,
- 'RPC method name is empty: %s.[%s]' %
- (category, method_name))
+ 'RPC method name is empty: %s%s[%s]' %
+ (category, '.' if category else '', method_name))
if category in self._rpc_servicers:
# system.func()
@@ -175,11 +202,12 @@
(category, method_name))
else:
- # .invalid()
- raise self._report_error(
- httplib.BAD_REQUEST,
- 'RPC request is invalid (empty category name): [%s].%s' %
- (category, method_name))
+ # .func() or .invalid()
+ holder = self
+ if not hasattr(holder, method_name):
+ raise self._report_error(
+ httplib.NOT_FOUND,
+ 'RPC method not found: [%s]' % method_name)
try:
method = getattr(holder, method_name)
diff --git a/client/cros/faft/rpc_server.py b/client/cros/faft/rpc_server.py
index 67c278a..2addfa1 100755
--- a/client/cros/faft/rpc_server.py
+++ b/client/cros/faft/rpc_server.py
@@ -4,16 +4,57 @@
# found in the LICENSE file.
"""Exposes the FAFTClient interface over XMLRPC.
-It launches a XMLRPC server and exposes the functions in RPCRouter().
+It launches an XMLRPC server and exposes the functions in RPCRouter().
"""
+
+import logging
import os
-from datetime import datetime
from optparse import OptionParser
-from SimpleXMLRPCServer import SimpleXMLRPCServer
+
+import psutil
import common
-from autotest_lib.client.cros.faft import rpc_functions
+from autotest_lib.client.cros import xmlrpc_server
from autotest_lib.client.cros.faft.utils import os_interface
+from autotest_lib.client.common_lib import logging_config
+
+
+def terminate_old():
+ """
+ Avoid "address already in use" errors by killing any leftover RPC server
+ processes, possibly from previous runs.
+
+ A process is a match if it has the same executable and command line:
+ /usr/local/bin/python2.7
+ ['/usr/bin/python2', '-u', '/usr/local/autotest/cros/faft/rpc_server.py']
+ """
+ me = psutil.Process()
+ for p in psutil.process_iter():
+ if p == me:
+ continue
+ try:
+ if p.exe() == me.exe() and p.cmdline() == me.cmdline():
+ logging.info('Terminating leftover process: %s', p)
+ try:
+ p.terminate()
+ p.wait(timeout=5)
+ logging.debug('Process exited')
+ except psutil.TimeoutExpired as e:
+ logging.warn('Process did not exit, '
+ 'falling back to SIGKILL: %s', e)
+ try:
+ p.kill()
+ p.wait(timeout=2.5)
+ logging.debug('Process exited')
+ except psutil.TimeoutExpired as e:
+ logging.exception('Process did not exit; '
+ 'address may remain in use.')
+ except psutil.AccessDenied:
+ logging.exception('Process could not be terminated; '
+ 'address may remain in use.')
+ except psutil.NoSuchProcess as e:
+ # process exited already
+ logging.debug('Process exited: %s', e)
def main():
@@ -27,22 +68,24 @@
help='port number of XMLRPC server')
(options, _) = parser.parse_args()
- print '[%s] XMLRPC Server: Spinning up FAFT server' % str(datetime.now())
+ config = logging_config.LoggingConfig()
+ config.configure_logging(use_console=True, verbose=True)
+
+ logging.debug('faft.rpc_server[%s] main...', os.getpid())
+ terminate_old()
+
+ # Import after terminate, so old process is killed even if import fails
+ from autotest_lib.client.cros.faft import rpc_functions
+
# Launch the XMLRPC server to provide FAFTClient commands.
os_if = os_interface.OSInterface()
os.chdir(os_if.state_dir)
- router = rpc_functions.RPCRouter(os_if)
-
- server = SimpleXMLRPCServer(('localhost', options.port),
- allow_none=True,
- logRequests=True)
- server.register_introspection_functions()
- server.register_instance(router)
- print '[%s] XMLRPC Server: Serving FAFT functions on port %s' % (str(
- datetime.now()), options.port)
-
- server.serve_forever()
+ server = xmlrpc_server.XmlRpcServer('localhost', options.port)
+ router = rpc_functions.FaftXmlRpcDelegate(os_if)
+ server.register_delegate(router)
+ server.run()
+ logging.debug('faft.rpc_server[%s] done.\n', os.getpid())
if __name__ == '__main__':
diff --git a/server/cros/faft/firmware_test.py b/server/cros/faft/firmware_test.py
index 61011b6..071bc62 100644
--- a/server/cros/faft/firmware_test.py
+++ b/server/cros/faft/firmware_test.py
@@ -236,6 +236,7 @@
self._remove_faft_lockfile()
self._remove_old_faft_lockfile()
self._record_faft_client_log()
+ self.faft_client.quit()
# Capture any new uart output, then discard log messages again.
self._cleanup_uart_capture()
@@ -1184,6 +1185,8 @@
def full_power_off_and_on(self):
"""Shutdown the device by pressing power button and power on again."""
boot_id = self.get_bootid()
+ self.faft_client.disconnect()
+
# Press power button to trigger Chrome OS normal shutdown process.
# We use a customized delay since the normal-press 1.2s is not enough.
self.servo.power_key(self.faft_config.hold_pwr_button_poweroff)
diff --git a/server/cros/faft/rpc_proxy.py b/server/cros/faft/rpc_proxy.py
index 744b20a..f72fb46 100644
--- a/server/cros/faft/rpc_proxy.py
+++ b/server/cros/faft/rpc_proxy.py
@@ -3,6 +3,7 @@
# found in the LICENSE file.
import httplib
+import logging
import socket
import time
import xmlrpclib
@@ -52,7 +53,7 @@
@ivar _client: the ssh host object
@type host: autotest_lib.server.hosts.abstract_ssh.AbstractSSHHost
@ivar _faft_client: the real serverproxy to use for calls
- @type _faft_client: xmlrpclib.ServerProxy
+ @type _faft_client: xmlrpclib.ServerProxy | None
"""
_client_config = ClientConfig()
@@ -109,7 +110,28 @@
def disconnect(self):
"""Disconnect the RPC server."""
- self._client.rpc_server_tracker.disconnect(self._client_config.rpc_port)
+ # The next start of the RPC server will terminate any leftovers,
+ # so no need to pkill upon disconnect.
+ if self._faft_client is not None:
+ logging.debug("Closing FAFT RPC server connection.")
+ self._client.rpc_server_tracker.disconnect(
+ self._client_config.rpc_port, pkill=False)
+ self._faft_client = None
+
+ def quit(self):
+ """Tell the RPC server to quit, then disconnect from it."""
+ if self._faft_client is None:
+ return
+ logging.debug("Telling FAFT RPC server to quit.")
+ try:
+ remote_quit = getattr(
+ self._faft_client, self._client_config.rpc_quit_call)
+ remote_quit()
+ except (StandardError, httplib.BadStatusLine, xmlrpclib.Error) as e:
+ logging.warn("Error while telling FAFT RPC server to quit: %s", e)
+
+ self._client.rpc_server_tracker.disconnect(
+ self._client_config.rpc_port, pkill=False)
self._faft_client = None
def __str__(self):
diff --git a/server/cros/faft/rpc_proxy.pyi b/server/cros/faft/rpc_proxy.pyi
index a09a746..e9e7a54 100644
--- a/server/cros/faft/rpc_proxy.pyi
+++ b/server/cros/faft/rpc_proxy.pyi
@@ -17,3 +17,8 @@
system: rpc_functions.SystemServicer
tpm: rpc_functions.TpmServicer
updater: rpc_functions.UpdaterServicer
+
+ connect: callable
+ disconnect: callable
+ quit: callable
+ ready: callable
diff --git a/server/hosts/rpc_server_tracker.py b/server/hosts/rpc_server_tracker.py
index 60f9bfa..9ff0f1a 100644
--- a/server/hosts/rpc_server_tracker.py
+++ b/server/hosts/rpc_server_tracker.py
@@ -247,8 +247,7 @@
logging.info('Established a jsonrpc connection through port %s.', port)
return proxy
-
- def disconnect(self, port):
+ def disconnect(self, port, pkill=True):
"""Disconnect from an RPC server on the host.
Terminates the remote RPC server previously started for
@@ -261,13 +260,13 @@
This function does nothing if requested to disconnect a port
that was not previously connected via _setup_rpc.
- @param port Port number passed to a previous call to
- `_setup_rpc()`.
+ @param port Port number passed to a previous call to `_setup_rpc()`.
+ @param pkill: if True, ssh in to the server and pkill the process.
"""
if port not in self._rpc_proxy_map:
return
remote_name, tunnel_proc, remote_pid = self._rpc_proxy_map[port]
- if remote_name:
+ if pkill and remote_name:
# We use 'pkill' to find our target process rather than
# a PID, because the host may have rebooted since
# connecting, and we don't want to kill an innocent
diff --git a/server/site_tests/firmware_ConsecutiveBoot/firmware_ConsecutiveBoot.py b/server/site_tests/firmware_ConsecutiveBoot/firmware_ConsecutiveBoot.py
index b327096..065fc67 100644
--- a/server/site_tests/firmware_ConsecutiveBoot/firmware_ConsecutiveBoot.py
+++ b/server/site_tests/firmware_ConsecutiveBoot/firmware_ConsecutiveBoot.py
@@ -71,6 +71,7 @@
else:
raise
+ self.faft_client.disconnect()
logging.info('Wait for client to go offline')
self.switcher.wait_for_client_offline(timeout=100, orig_boot_id=boot_id)
if self.check_ec_capability(['x86'], suppress_warning=True):
diff --git a/server/site_tests/firmware_FAFTRPC/config.py b/server/site_tests/firmware_FAFTRPC/config.py
index 3363dac..4a079dd 100644
--- a/server/site_tests/firmware_FAFTRPC/config.py
+++ b/server/site_tests/firmware_FAFTRPC/config.py
@@ -767,5 +767,23 @@
],
},
]
+ },
+ {
+ "category_name": '',
+ "test_cases": [
+ # explicit connect
+ {"method_name": "quit", "passing_args": [NO_ARGS]},
+ {"method_name": "connect", "passing_args": [NO_ARGS]},
+ {"method_name": "ready", "passing_args": [NO_ARGS]},
+ {"method_name": "disconnect", "passing_args": [NO_ARGS]},
+ {"method_name": "connect", "passing_args": [NO_ARGS]},
+ {"method_name": "ready", "passing_args": [NO_ARGS]},
+
+ # implicit connect
+ {"method_name": "quit", "passing_args": [NO_ARGS]},
+ {"method_name": "ready", "passing_args": [NO_ARGS]},
+ {"method_name": "disconnect", "passing_args": [NO_ARGS]},
+ {"method_name": "ready", "passing_args": [NO_ARGS]},
+ ]
}
]
diff --git a/server/site_tests/firmware_FAFTRPC/control.client b/server/site_tests/firmware_FAFTRPC/control.client
new file mode 100644
index 0000000..63a13bb
--- /dev/null
+++ b/server/site_tests/firmware_FAFTRPC/control.client
@@ -0,0 +1,37 @@
+# Copyright 2019 The Chromium OS Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+from autotest_lib.server import utils
+
+AUTHOR = "gredelston, kmshelton, dgoyette, waihong"
+NAME = "firmware_FAFTRPC.client"
+PURPOSE = "Verify that the faft rpc client side methods work as expected."
+CRITERIA = "This test will fail if the faft rpc client code is broken."
+ATTRIBUTES = "suite:faft_framework"
+TIME = "SHORT"
+TEST_CATEGORY = "Functional"
+TEST_CLASS = "firmware"
+TEST_TYPE = "server"
+JOB_RETRIES = 2
+
+DOC = """
+This test checks that the client-side methods function properly:
+ quit(): tells the remote end to quit, then disconnects.
+ disconnect(): just disconnects.
+
+"""
+
+args_dict = utils.args_to_dict(args)
+servo_args = hosts.CrosHost.get_servo_arguments(args_dict)
+
+def run_faftrpc(machine):
+ host = hosts.create_host(machine, servo_args=servo_args)
+ job.run_test("firmware_FAFTRPC",
+ host=host,
+ cmdline_args=args,
+ disable_sysinfo=True,
+ category_under_test=''
+ )
+
+parallel_simple(run_faftrpc, machines)
diff --git a/server/site_tests/firmware_FAFTRPC/firmware_FAFTRPC.py b/server/site_tests/firmware_FAFTRPC/firmware_FAFTRPC.py
index b996ad0..0a93c5f 100644
--- a/server/site_tests/firmware_FAFTRPC/firmware_FAFTRPC.py
+++ b/server/site_tests/firmware_FAFTRPC/firmware_FAFTRPC.py
@@ -145,7 +145,10 @@
"""
rpc_function = self.get_rpc_function(category, method)
- rpc_name = ".".join([category, method])
+ if category:
+ rpc_name = '%s.%s' % (category, method)
+ else:
+ rpc_name = method
try:
result = rpc_function(*params)
except xmlrpclib.Fault as e:
@@ -199,7 +202,10 @@
"""
rpc_function = self.get_rpc_function(category, method)
- rpc_name = ".".join([category, method])
+ if category:
+ rpc_name = '%s.%s' % (category, method)
+ else:
+ rpc_name = method
try:
result = rpc_function(*params)
except xmlrpclib.Fault as e:
@@ -254,7 +260,10 @@
@return: A callable method of the RPC proxy
"""
- rpc_function_handler = getattr(self.faft_client, category)
+ if category:
+ rpc_function_handler = getattr(self.faft_client, category)
+ else:
+ rpc_function_handler = self.faft_client
rpc_function = getattr(rpc_function_handler, method)
return rpc_function
@@ -285,9 +294,13 @@
if category_name == "ec" and not self.check_ec_capability():
logging.info("No EC found on DUT. Skipping EC category.")
continue
+
+ # Re-enable test mode, in case another category's tests disabled it.
+ self.faft_client.rpc_settings.enable_test_mode()
+
test_cases = rpc_category["test_cases"]
- logging.info("Testing %d cases for RPC category '%s'",
- len(test_cases), category_name)
+ logging.info("Testing %d cases for RPC category %s",
+ len(test_cases), repr(category_name))
for test_case in test_cases:
method_names = get_rpc_method_names_from_test_case(test_case)
passing_args = test_case.get("passing_args", [])
