Simplify FuchsiaDevice config
Generate SSH configs for Fuchsia devices and remove `ssh_config` from
the FuchsiaDevice config. Provides a reproducible connection method.
Provide sensible defaults for `ssh_priv_key`, `authorized_file_loc`, and
`ffx_binary_path` by expanding the home directory and environmental
variables.
Test: python unit_tests/test_suite.py
Change-Id: Ibd1f6b2f06923da3bc8bd5aa310d1a45346f85e3
Reviewed-on: https://fuchsia-review.googlesource.com/c/antlion/+/769894
Reviewed-by: Marc Khouri <mnck@google.com>
Fuchsia-Auto-Submit: Sam Balana <sbalana@google.com>
Commit-Queue: Auto-Submit <auto-submit@fuchsia-infra.iam.gserviceaccount.com>
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 54f6914..9bc8ab7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,7 +10,17 @@
## [Unreleased]
-Nothing yet!
+### Changed
+
+- All path config options in `FuchsiaDevice` expand the home directory (`~`) and
+environmental variables
+ - Used by `ssh_priv_key`, `authorized_file_loc`, and `ffx_binary_path` for
+ sensible defaults using `$FUCHSIA_DIR`
+
+### Removed
+
+- `ssh_config` from `FuchsiaDevice` config. SSH configs are generated to provide
+a reproducible connection method and ease initial setup.
[unreleased]: https://fuchsia.googlesource.com/antlion/+/refs/tags/v0.1.0..refs/heads/main
diff --git a/README.md b/README.md
index cb10088..658ef42 100644
--- a/README.md
+++ b/README.md
@@ -40,10 +40,7 @@
"testbed": [{
"name": "simple_testbed",
"FuchsiaDevice": [{
- "ip": "fuchsia-00e0-4c01-04df",
- "ssh_priv_key": "/home/fuchsia/.ssh/fuchsia_ed25519",
- "ssh_config": "/home/fuchsia/out/default/ssh-keys/ssh_config",
- "ffx_binary_path": "/home/fuchsia/out/default/host_x64/ffx"
+ "ip": "fuchsia-00e0-4c01-04df"
}]
}],
"logpath": "logs",
diff --git a/src/antlion/controllers/fuchsia_device.py b/src/antlion/controllers/fuchsia_device.py
index ceb1878..fb2c55e 100644
--- a/src/antlion/controllers/fuchsia_device.py
+++ b/src/antlion/controllers/fuchsia_device.py
@@ -20,6 +20,7 @@
import os
import re
import subprocess
+import textwrap
import time
from antlion import context
@@ -36,14 +37,12 @@
from antlion.controllers.fuchsia_lib.lib_controllers.wlan_controller import WlanController
from antlion.controllers.fuchsia_lib.lib_controllers.wlan_policy_controller import WlanPolicyController
from antlion.controllers.fuchsia_lib.package_server import PackageServer
-from antlion.controllers.fuchsia_lib.ssh import DEFAULT_SSH_PORT, DEFAULT_SSH_USER, SSHConfig, SSHProvider, FuchsiaSSHError
+from antlion.controllers.fuchsia_lib.ssh import DEFAULT_SSH_PORT, DEFAULT_SSH_PRIVATE_KEY, DEFAULT_SSH_USER, SSHConfig, SSHProvider, FuchsiaSSHError
from antlion.controllers.fuchsia_lib.utils_lib import flash
MOBLY_CONTROLLER_CONFIG_NAME = "FuchsiaDevice"
ACTS_CONTROLLER_REFERENCE_NAME = "fuchsia_devices"
-CONTROL_PATH_REPLACE_VALUE = " ControlPath /tmp/fuchsia--%r@%h:%p"
-
FUCHSIA_DEVICE_EMPTY_CONFIG_MSG = "Configuration is empty, abort!"
FUCHSIA_DEVICE_NOT_LIST_CONFIG_MSG = "Configuration should be a list, abort!"
FUCHSIA_DEVICE_INVALID_CONFIG = ("Fuchsia device config must be either a str "
@@ -153,33 +152,60 @@
ssh_config: The ssh_config for connecting to the Fuchsia device.
"""
- def __init__(self, fd_conf_data):
- """
- Args:
- fd_conf_data: A dict of a fuchsia device configuration data
- Required keys:
- ip: IP address of fuchsia device
- optional key:
- sl4_port: Port for the sl4f web server on the fuchsia device
- (Default: 80)
- ssh_config: Location of the ssh_config file to connect to
- the fuchsia device
- (Default: None)
- ssh_port: Port for the ssh server on the fuchsia device
- (Default: 22)
- """
+ def __init__(self, fd_conf_data) -> None:
self.conf_data = fd_conf_data
if "ip" not in fd_conf_data:
raise FuchsiaDeviceError(FUCHSIA_DEVICE_NO_IP_MSG)
self.ip: str = fd_conf_data["ip"]
self.orig_ip: str = fd_conf_data["ip"]
self.sl4f_port: int = fd_conf_data.get("sl4f_port", 80)
+ self.ssh_username: str = fd_conf_data.get("ssh_username",
+ DEFAULT_SSH_USER)
self.ssh_port: int = fd_conf_data.get("ssh_port", DEFAULT_SSH_PORT)
- self.ssh_config: Optional[str] = fd_conf_data.get("ssh_config", None)
- self.ssh_priv_key: Optional[str] = fd_conf_data.get(
- "ssh_priv_key", None)
- self.authorized_file: Optional[str] = fd_conf_data.get(
- "authorized_file_loc", None)
+
+ def expand(path: str) -> str:
+ return os.path.expandvars(os.path.expanduser(path))
+
+ def path_from_config(name: str,
+ default: Optional[str] = None) -> Optional[str]:
+ path = fd_conf_data.get(name, default)
+ if not path:
+ return path
+ return expand(path)
+
+ def assert_exists(name: str, path: str) -> None:
+ if not path:
+ raise FuchsiaDeviceError(
+ f'Please specify "${name}" in your configuration file')
+ if not os.path.exists(path):
+ raise FuchsiaDeviceError(
+ f'Please specify a correct "${name}" in your configuration '
+ f'file: "{path}" does not exist')
+
+ self.specific_image: Optional[str] = path_from_config("specific_image")
+ if self.specific_image:
+ assert_exists("specific_image", self.specific_image)
+
+ # Path to a tar.gz archive with pm and amber-files, as necessary for
+ # starting a package server.
+ self.packages_archive_path: Optional[str] = path_from_config(
+ "packages_archive_path", None)
+ if self.packages_archive_path:
+ assert_exists("packages_archive_path", self.packages_archive_path)
+
+ def required_path_from_config(name: str,
+ default: Optional[str] = None) -> str:
+ path = path_from_config(name, default)
+ assert_exists(name, path)
+ return path
+
+ self.ssh_priv_key: str = required_path_from_config(
+ "ssh_priv_key", DEFAULT_SSH_PRIVATE_KEY)
+ self.authorized_file: str = required_path_from_config(
+ "authorized_file_loc", f'{self.ssh_priv_key}.pub')
+ self.ffx_binary_path: str = required_path_from_config(
+ "ffx_binary_path", "${FUCHSIA_DIR}/.jiri_root/bin/ffx")
+
self.serial_number: Optional[str] = fd_conf_data.get(
"serial_number", None)
self.device_type: Optional[str] = fd_conf_data.get("device_type", None)
@@ -189,34 +215,22 @@
self.build_number: Optional[str] = fd_conf_data.get(
"build_number", None)
self.build_type: Optional[str] = fd_conf_data.get("build_type", None)
- self.server_path: Optional[str] = fd_conf_data.get("server_path", None)
- self.specific_image: Optional[str] = fd_conf_data.get(
- "specific_image", None)
- self.ffx_binary_path: Optional[str] = fd_conf_data.get(
- "ffx_binary_path", None)
- # Path to a tar.gz archive with pm and amber-files, as necessary for
- # starting a package server.
- self.packages_archive_path: Optional[str] = fd_conf_data.get(
- "packages_archive_path", None)
+
self.mdns_name: Optional[str] = fd_conf_data.get("mdns_name", None)
- # Instead of the input ssh_config, a new config is generated with proper
- # ControlPath to the test output directory.
- output_path = context.get_current_context().get_base_output_path()
- generated_ssh_config = os.path.join(output_path,
- "ssh_config_{}".format(self.ip))
- self._set_control_path_config(self.ssh_config, generated_ssh_config)
- self.ssh_config = generated_ssh_config
-
- self.ssh_username = fd_conf_data.get("ssh_username", DEFAULT_SSH_USER)
- self.hard_reboot_on_fail = fd_conf_data.get("hard_reboot_on_fail",
- False)
- self.take_bug_report_on_fail = fd_conf_data.get(
+ self.hard_reboot_on_fail: bool = fd_conf_data.get(
+ "hard_reboot_on_fail", False)
+ self.take_bug_report_on_fail: bool = fd_conf_data.get(
"take_bug_report_on_fail", False)
self.device_pdu_config = fd_conf_data.get("PduDevice", None)
- self.config_country_code = fd_conf_data.get(
+ self.config_country_code: str = fd_conf_data.get(
'country_code', FUCHSIA_DEFAULT_COUNTRY_CODE_US).upper()
+ output_path = context.get_current_context().get_base_output_path()
+ self.ssh_config = os.path.join(output_path,
+ "ssh_config_{}".format(self.ip))
+ self._generate_ssh_config(self.ssh_config)
+
# WLAN interface info is populated inside configure_wlan
self.wlan_client_interfaces = {}
self.wlan_ap_interfaces = {}
@@ -323,10 +337,6 @@
calls.
"""
if not hasattr(self, '_ffx'):
- if not self.ffx_binary_path:
- raise FuchsiaConfigError(
- 'Must provide "ffx_binary_path: <path to FFX binary>" in the device config'
- )
if not self.mdns_name:
raise FuchsiaConfigError(
'Must provide "mdns_name: <device mDNS name>" in the device config'
@@ -343,26 +353,33 @@
self._ffx.clean_up()
del self._ffx
- def _set_control_path_config(self, old_config, new_config):
- """Given an input ssh_config, write to a new config with proper
- ControlPath values in place, if it doesn't exist already.
+ def _generate_ssh_config(self, file_path: str):
+ """Generate and write an SSH config for Fuchsia to disk.
Args:
- old_config: string, path to the input config
- new_config: string, path to store the new config
+ file_path: Path to write the generated SSH config
"""
- if os.path.isfile(new_config):
- return
+ content = textwrap.dedent(f"""\
+ Host *
+ CheckHostIP no
+ StrictHostKeyChecking no
+ ForwardAgent no
+ ForwardX11 no
+ GSSAPIDelegateCredentials no
+ UserKnownHostsFile /dev/null
+ User fuchsia
+ IdentitiesOnly yes
+ IdentityFile {self.ssh_priv_key}
+ ControlPersist yes
+ ControlMaster auto
+ ControlPath /tmp/fuchsia--%r@%h:%p
+ ServerAliveInterval 1
+ ServerAliveCountMax 1
+ LogLevel ERROR
+ """)
- ssh_config_copy = ""
-
- with open(old_config, 'r') as file:
- ssh_config_copy = re.sub('(\sControlPath\s.*)',
- CONTROL_PATH_REPLACE_VALUE,
- file.read(),
- flags=re.M)
- with open(new_config, 'w') as file:
- file.write(ssh_config_copy)
+ with open(file_path, 'w') as file:
+ file.write(content)
def init_controllers(self):
# Contains Netstack functions
@@ -874,40 +891,39 @@
Usage:
In FuchsiaDevice config, add "country_code": "<CC>"
"""
- if self.ssh_config:
- # Country code can be None, from antlion config.
- if desired_country_code:
- desired_country_code = desired_country_code.upper()
- response = self.sl4f.regulatory_region_lib.setRegion(
- desired_country_code)
- if response.get('error'):
- raise FuchsiaDeviceError(
- 'Failed to set regulatory domain. Err: %s' %
- response['error'])
+ # Country code can be None, from antlion config.
+ if desired_country_code:
+ desired_country_code = desired_country_code.upper()
+ response = self.sl4f.regulatory_region_lib.setRegion(
+ desired_country_code)
+ if response.get('error'):
+ raise FuchsiaDeviceError(
+ 'Failed to set regulatory domain. Err: %s' %
+ response['error'])
- phy_list_response = self.sl4f.wlan_lib.wlanPhyIdList()
- if phy_list_response.get('error'):
- raise FuchsiaDeviceError(
- f'Failed to get phy list. Err: {response["error"]}')
- phy_list = phy_list_response.get('result')
- if not phy_list:
- raise FuchsiaDeviceError('No phy available in phy list')
- phy_id = phy_list[0]
+ phy_list_response = self.sl4f.wlan_lib.wlanPhyIdList()
+ if phy_list_response.get('error'):
+ raise FuchsiaDeviceError(
+ f'Failed to get phy list. Err: {response["error"]}')
+ phy_list = phy_list_response.get('result')
+ if not phy_list:
+ raise FuchsiaDeviceError('No phy available in phy list')
+ phy_id = phy_list[0]
- end_time = time.time() + FUCHSIA_COUNTRY_CODE_TIMEOUT
- while time.time() < end_time:
- ascii_cc = self.sl4f.wlan_lib.wlanGetCountry(phy_id).get(
- 'result')
- # Convert ascii_cc to string, then compare
- if ascii_cc and (''.join(chr(c) for c in ascii_cc).upper()
- == desired_country_code):
- self.log.debug('Country code successfully set to %s.' %
- desired_country_code)
- return
- self.log.debug('Country code not yet updated. Retrying.')
- time.sleep(1)
- raise FuchsiaDeviceError('Country code never updated to %s' %
- desired_country_code)
+ end_time = time.time() + FUCHSIA_COUNTRY_CODE_TIMEOUT
+ while time.time() < end_time:
+ ascii_cc = self.sl4f.wlan_lib.wlanGetCountry(phy_id).get(
+ 'result')
+ # Convert ascii_cc to string, then compare
+ if ascii_cc and (''.join(chr(c) for c in ascii_cc).upper()
+ == desired_country_code):
+ self.log.debug('Country code successfully set to %s.' %
+ desired_country_code)
+ return
+ self.log.debug('Country code not yet updated. Retrying.')
+ time.sleep(1)
+ raise FuchsiaDeviceError('Country code never updated to %s' %
+ desired_country_code)
def stop_services(self):
"""Stops the ffx daemon and deletes SL4F property."""
@@ -930,11 +946,6 @@
times in one test. Epoch time when the test started. If not
specified, the current time will be used.
"""
- if not self.ssh_config:
- self.log.warn(
- 'Skipping take_bug_report because ssh_config is not specified')
- return
-
if test_name:
self.log.info(
f"Taking snapshot of {self.mdns_name} for {test_name}")
diff --git a/src/antlion/controllers/fuchsia_lib/ssh.py b/src/antlion/controllers/fuchsia_lib/ssh.py
index fd32d99..ec8f762 100644
--- a/src/antlion/controllers/fuchsia_lib/ssh.py
+++ b/src/antlion/controllers/fuchsia_lib/ssh.py
@@ -25,6 +25,7 @@
DEFAULT_SSH_USER: str = "fuchsia"
DEFAULT_SSH_PORT: int = 22
+DEFAULT_SSH_PRIVATE_KEY: str = "~/.ssh/fuchsia_ed25519"
DEFAULT_SSH_TIMEOUT_SEC: int = 60
DEFAULT_SSH_CONNECT_TIMEOUT_SEC: int = 30
DEFAULT_SSH_SERVER_ALIVE_INTERVAL: int = 30
diff --git a/unit_tests/acts_utils_test.py b/unit_tests/acts_utils_test.py
index bf55ec6..1c8b21f 100755
--- a/unit_tests/acts_utils_test.py
+++ b/unit_tests/acts_utils_test.py
@@ -280,8 +280,8 @@
values returned from each individual callable in the order passed in.
"""
ret_values = utils.run_concurrent_actions_no_raise(
- lambda: self.function_returns_passed_in_arg(
- 'ARG1'), lambda: self.function_returns_passed_in_arg('ARG2'),
+ lambda: self.function_returns_passed_in_arg('ARG1'),
+ lambda: self.function_returns_passed_in_arg('ARG2'),
lambda: self.function_returns_passed_in_arg('ARG3'))
self.assertEqual(len(ret_values), 3)
@@ -312,8 +312,8 @@
"""
ret_values = utils.run_concurrent_actions(
- lambda: self.function_returns_passed_in_arg(
- 'ARG1'), lambda: self.function_returns_passed_in_arg('ARG2'),
+ lambda: self.function_returns_passed_in_arg('ARG1'),
+ lambda: self.function_returns_passed_in_arg('ARG2'),
lambda: self.function_returns_passed_in_arg('ARG3'))
self.assertEqual(len(ret_values), 3)
@@ -370,6 +370,7 @@
class IpAddressUtilTest(unittest.TestCase):
+
def test_positive_ipv4_normal_address(self):
ip_address = "192.168.1.123"
self.assertTrue(utils.is_valid_ipv4_address(ip_address))
@@ -432,7 +433,8 @@
self.assertEqual(utils.get_interface_ip_addresses(job, 'wlan1'),
CORRECT_EMPTY_IP_LIST)
- @mock.patch('antlion.controllers.utils_lib.ssh.connection.SshConnection.run')
+ @mock.patch(
+ 'antlion.controllers.utils_lib.ssh.connection.SshConnection.run')
def test_ssh_get_interface_ip_addresses_full(self, ssh_mock):
ssh_mock.side_effect = [
job.Result(stdout=bytes(MOCK_ENO1_IP_ADDRESSES, 'utf-8'),
@@ -442,7 +444,8 @@
utils.get_interface_ip_addresses(SshConnection('mock_settings'),
'eno1'), CORRECT_FULL_IP_LIST)
- @mock.patch('antlion.controllers.utils_lib.ssh.connection.SshConnection.run')
+ @mock.patch(
+ 'antlion.controllers.utils_lib.ssh.connection.SshConnection.run')
def test_ssh_get_interface_ip_addresses_empty(self, ssh_mock):
ssh_mock.side_effect = [
job.Result(stdout=bytes(MOCK_WLAN1_IP_ADDRESSES, 'utf-8'),
@@ -483,14 +486,14 @@
@mock.patch(
'antlion.controllers.fuchsia_lib.sl4f.SL4F._verify_sl4f_connection')
@mock.patch('antlion.controllers.fuchsia_device.'
- 'FuchsiaDevice._set_control_path_config')
+ 'FuchsiaDevice._generate_ssh_config')
@mock.patch('antlion.controllers.'
'fuchsia_lib.netstack.netstack_lib.'
'FuchsiaNetstackLib.netstackListInterfaces')
def test_fuchsia_get_interface_ip_addresses_full(
- self, list_interfaces_mock, control_path_mock,
- verify_sl4f_conn_mock, ssh_run_mock, wait_for_port_mock,
- ffx_mock, sl4f_mock):
+ self, list_interfaces_mock, generate_ssh_config_mock,
+ verify_sl4f_conn_mock, ssh_run_mock, wait_for_port_mock, ffx_mock,
+ sl4f_mock):
# Configure the log path which is required by ACTS logger.
logging.log_path = '/tmp/unit_test_garbage'
@@ -519,14 +522,14 @@
@mock.patch(
'antlion.controllers.fuchsia_lib.sl4f.SL4F._verify_sl4f_connection')
@mock.patch('antlion.controllers.fuchsia_device.'
- 'FuchsiaDevice._set_control_path_config')
+ 'FuchsiaDevice._generate_ssh_config')
@mock.patch('antlion.controllers.'
'fuchsia_lib.netstack.netstack_lib.'
'FuchsiaNetstackLib.netstackListInterfaces')
def test_fuchsia_get_interface_ip_addresses_empty(
- self, list_interfaces_mock, control_path_mock,
- verify_sl4f_conn_mock, ssh_run_mock, wait_for_port_mock,
- ffx_mock, sl4f_mock):
+ self, list_interfaces_mock, generate_ssh_config_mock,
+ verify_sl4f_conn_mock, ssh_run_mock, wait_for_port_mock, ffx_mock,
+ sl4f_mock):
# Configure the log path which is required by ACTS logger.
logging.log_path = '/tmp/unit_test_garbage'
@@ -548,7 +551,9 @@
class GetDeviceTest(unittest.TestCase):
+
class TestDevice:
+
def __init__(self, id, device_type=None) -> None:
self.id = id
if device_type:
