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/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", [])
