<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-family:arial,sans-serif"><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Nov 23, 2023 at 10:14 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Format according to the Google format and PEP257, with slight<br>
deviations.<br>
<br>
Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech><br>
---<br>
dts/framework/testbed_model/os_session.py | 272 ++++++++++++++++------<br>
1 file changed, 205 insertions(+), 67 deletions(-)<br>
<br>
diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/testbed_model/os_session.py<br>
index 76e595a518..cfdbd1c4bd 100644<br>
--- a/dts/framework/testbed_model/os_session.py<br>
+++ b/dts/framework/testbed_model/os_session.py<br>
@@ -2,6 +2,26 @@<br>
# Copyright(c) 2023 PANTHEON.tech s.r.o.<br>
# Copyright(c) 2023 University of New Hampshire<br>
<br>
+"""OS-aware remote session.<br>
+<br>
+DPDK supports multiple different operating systems, meaning it can run on these different operating<br>
+systems. This module defines the common API that OS-unaware layers use and translates the API into<br>
+OS-aware calls/utility usage.<br>
+<br>
+Note:<br>
+ Running commands with administrative privileges requires OS awareness. This is the only layer<br>
+ that's aware of OS differences, so this is where non-privileged command get converted<br>
+ to privileged commands.<br>
+<br>
+Example:<br>
+ A user wishes to remove a directory on a remote :class:`~.sut_node.SutNode`.<br>
+ The :class:`~.sut_node.SutNode` object isn't aware what OS the node is running - it delegates<br>
+ the OS translation logic to :attr:`~.node.Node.main_session`. The SUT node calls<br>
+ :meth:`~OSSession.remove_remote_dir` with a generic, OS-unaware path and<br>
+ the :attr:`~.node.Node.main_session` translates that to ``rm -rf`` if the node's OS is Linux<br>
+ and other commands for other OSs. It also translates the path to match the underlying OS.<br>
+"""<br>
+<br>
from abc import ABC, abstractmethod<br>
from collections.abc import Iterable<br>
from ipaddress import IPv4Interface, IPv6Interface<br>
@@ -28,10 +48,16 @@<br>
<br>
<br>
class OSSession(ABC):<br>
- """<br>
- The OS classes create a DTS node remote session and implement OS specific<br>
+ """OS-unaware to OS-aware translation API definition.<br>
+<br>
+ The OSSession classes create a remote session to a DTS node and implement OS specific<br>
behavior. There a few control methods implemented by the base class, the rest need<br>
- to be implemented by derived classes.<br>
+ to be implemented by subclasses.<br>
+<br>
+ Attributes:<br>
+ name: The name of the session.<br>
+ remote_session: The remote session maintaining the connection to the node.<br>
+ interactive_session: The interactive remote session maintaining the connection to the node.<br>
"""<br>
<br>
_config: NodeConfiguration<br>
@@ -46,6 +72,15 @@ def __init__(<br>
name: str,<br>
logger: DTSLOG,<br>
):<br>
+ """Initialize the OS-aware session.<br>
+<br>
+ Connect to the node right away and also create an interactive remote session.<br>
+<br>
+ Args:<br>
+ node_config: The test run configuration of the node to connect to.<br>
+ name: The name of the session.<br>
+ logger: The logger instance this session will use.<br>
+ """<br>
self._config = node_config<br>
<a href="http://self.name" rel="noreferrer" target="_blank">self.name</a> = name<br>
self._logger = logger<br>
@@ -53,15 +88,15 @@ def __init__(<br>
self.interactive_session = create_interactive_session(node_config, logger)<br>
<br>
def close(self, force: bool = False) -> None:<br>
- """<br>
- Close the remote session.<br>
+ """Close the underlying remote session.<br>
+<br>
+ Args:<br>
+ force: Force the closure of the connection.<br>
"""<br>
self.remote_session.close(force)<br>
<br>
def is_alive(self) -> bool:<br>
- """<br>
- Check whether the remote session is still responding.<br>
- """<br>
+ """Check whether the underlying remote session is still responding."""<br>
return self.remote_session.is_alive()<br>
<br>
def send_command(<br>
@@ -72,10 +107,23 @@ def send_command(<br>
verify: bool = False,<br>
env: dict | None = None,<br>
) -> CommandResult:<br>
- """<br>
- An all-purpose API in case the command to be executed is already<br>
- OS-agnostic, such as when the path to the executed command has been<br>
- constructed beforehand.<br>
+ """An all-purpose API for OS-agnostic commands.<br>
+<br>
+ This can be used for an execution of a portable command that's executed the same way<br>
+ on all operating systems, such as Python.<br>
+<br>
+ The :option:`--timeout` command line argument and the :envvar:`DTS_TIMEOUT`<br>
+ environment variable configure the timeout of command execution.<br>
+<br>
+ Args:<br>
+ command: The command to execute.<br>
+ timeout: Wait at most this long in seconds for `command` execution to complete.<br>
+ privileged: Whether to run the command with administrative privileges.<br>
+ verify: If :data:`True`, will check the exit code of the command.<br>
+ env: A dictionary with environment variables to be used with the command execution.<br>
+<br>
+ Raises:<br>
+ RemoteCommandExecutionError: If verify is :data:`True` and the command failed.<br>
"""<br>
if privileged:<br>
command = self._get_privileged_command(command)<br>
@@ -89,8 +137,20 @@ def create_interactive_shell(<br>
privileged: bool,<br>
app_args: str,<br>
) -> InteractiveShellType:<br>
- """<br>
- See "create_interactive_shell" in SutNode<br>
+ """Factory for interactive session handlers.<br>
+<br>
+ Instantiate `shell_cls` according to the remote OS specifics.<br>
+<br>
+ Args:<br>
+ shell_cls: The class of the shell.<br>
+ timeout: Timeout for reading output from the SSH channel. If you are<br>
+ reading from the buffer and don't receive any data within the timeout<br>
+ it will throw an error.<br>
+ privileged: Whether to run the shell with administrative privileges.<br>
+ app_args: The arguments to be passed to the application.<br>
+<br>
+ Returns:<br>
+ An instance of the desired interactive application shell.<br>
"""<br>
return shell_cls(<br>
self.interactive_session.session,<br>
@@ -114,27 +174,42 @@ def _get_privileged_command(command: str) -> str:<br>
<br>
@abstractmethod<br>
def guess_dpdk_remote_dir(self, remote_dir: str | PurePath) -> PurePath:<br>
- """<br>
- Try to find DPDK remote dir in remote_dir.<br>
+ """Try to find DPDK directory in `remote_dir`.<br>
+<br>
+ The directory is the one which is created after the extraction of the tarball. The files<br>
+ are usually extracted into a directory starting with ``dpdk-``.<br>
+<br>
+ Returns:<br>
+ The absolute path of the DPDK remote directory, empty path if not found.<br>
"""<br>
<br>
@abstractmethod<br>
def get_remote_tmp_dir(self) -> PurePath:<br>
- """<br>
- Get the path of the temporary directory of the remote OS.<br>
+ """Get the path of the temporary directory of the remote OS.<br>
+<br>
+ Returns:<br>
+ The absolute path of the temporary directory.<br>
"""<br>
<br>
@abstractmethod<br>
def get_dpdk_build_env_vars(self, arch: Architecture) -> dict:<br>
- """<br>
- Create extra environment variables needed for the target architecture. Get<br>
- information from the node if needed.<br>
+ """Create extra environment variables needed for the target architecture.<br>
+<br>
+ Different architectures may require different configuration, such as setting 32-bit CFLAGS.<br>
+<br>
+ Returns:<br>
+ A dictionary with keys as environment variables.<br>
"""<br>
<br>
@abstractmethod<br>
def join_remote_path(self, *args: str | PurePath) -> PurePath:<br>
- """<br>
- Join path parts using the path separator that fits the remote OS.<br>
+ """Join path parts using the path separator that fits the remote OS.<br>
+<br>
+ Args:<br>
+ args: Any number of paths to join.<br>
+<br>
+ Returns:<br>
+ The resulting joined path.<br>
"""<br>
<br>
@abstractmethod<br>
@@ -143,13 +218,13 @@ def copy_from(<br>
source_file: str | PurePath,<br>
destination_file: str | PurePath,<br>
) -> None:<br>
- """Copy a file from the remote Node to the local filesystem.<br>
+ """Copy a file from the remote node to the local filesystem.<br>
<br>
- Copy source_file from the remote Node associated with this remote<br>
- session to destination_file on the local filesystem.<br>
+ Copy `source_file` from the remote node associated with this remote<br>
+ session to `destination_file` on the local filesystem.<br>
<br>
Args:<br>
- source_file: the file on the remote Node.<br>
+ source_file: the file on the remote node.<br>
destination_file: a file or directory path on the local filesystem.<br>
"""<br>
<br>
@@ -159,14 +234,14 @@ def copy_to(<br>
source_file: str | PurePath,<br>
destination_file: str | PurePath,<br>
) -> None:<br>
- """Copy a file from local filesystem to the remote Node.<br>
+ """Copy a file from local filesystem to the remote node.<br>
<br>
- Copy source_file from local filesystem to destination_file<br>
- on the remote Node associated with this remote session.<br>
+ Copy `source_file` from local filesystem to `destination_file`<br>
+ on the remote node associated with this remote session.<br>
<br>
Args:<br>
source_file: the file on the local filesystem.<br>
- destination_file: a file or directory path on the remote Node.<br>
+ destination_file: a file or directory path on the remote node.<br>
"""<br>
<br>
@abstractmethod<br>
@@ -176,8 +251,12 @@ def remove_remote_dir(<br>
recursive: bool = True,<br>
force: bool = True,<br>
) -> None:<br>
- """<br>
- Remove remote directory, by default remove recursively and forcefully.<br>
+ """Remove remote directory, by default remove recursively and forcefully.<br>
+<br>
+ Args:<br>
+ remote_dir_path: The path of the directory to remove.<br>
+ recursive: If :data:`True`, also remove all contents inside the directory.<br>
+ force: If :data:`True`, ignore all warnings and try to remove at all costs.<br>
"""<br>
<br>
@abstractmethod<br>
@@ -186,9 +265,12 @@ def extract_remote_tarball(<br>
remote_tarball_path: str | PurePath,<br>
expected_dir: str | PurePath | None = None,<br>
) -> None:<br>
- """<br>
- Extract remote tarball in place. If expected_dir is a non-empty string, check<br>
- whether the dir exists after extracting the archive.<br>
+ """Extract remote tarball in its remote directory.<br>
+<br>
+ Args:<br>
+ remote_tarball_path: The path of the tarball on the remote node.<br>
+ expected_dir: If non-empty, check whether `expected_dir` exists after extracting<br>
+ the archive.<br>
"""<br>
<br>
@abstractmethod<br>
@@ -201,69 +283,119 @@ def build_dpdk(<br>
rebuild: bool = False,<br>
timeout: float = SETTINGS.compile_timeout,<br>
) -> None:<br>
- """<br>
- Build DPDK in the input dir with specified environment variables and meson<br>
- arguments.<br>
+ """Build DPDK on the remote node.<br>
+<br>
+ An extracted DPDK tarball must be present on the node. The build consists of two steps::<br>
+<br>
+ meson setup <meson args> remote_dpdk_dir remote_dpdk_build_dir<br>
+ ninja -C remote_dpdk_build_dir<br>
+<br>
+ The :option:`--compile-timeout` command line argument and the :envvar:`DTS_COMPILE_TIMEOUT`<br>
+ environment variable configure the timeout of DPDK build.<br>
+<br>
+ Args:<br>
+ env_vars: Use these environment variables then building DPDK.<br></blockquote><div><br></div><div><div style="font-family:arial,sans-serif" class="gmail_default">I think this is meant to be "when building DPDK" instead.<br></div><div style="font-family:arial,sans-serif" class="gmail_default"> </div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ meson_args: Use these meson arguments when building DPDK.<br>
+ remote_dpdk_dir: The directory on the remote node where DPDK will be built.<br>
+ remote_dpdk_build_dir: The target build directory on the remote node.<br>
+ rebuild: If :data:`True`, do a subsequent build with ``meson configure`` instead<br>
+ of ``meson setup``.<br>
+ timeout: Wait at most this long in seconds for the build execution to complete.<br>
"""<br>
<br>
@abstractmethod<br>
def get_dpdk_version(self, version_path: str | PurePath) -> str:<br>
- """<br>
- Inspect DPDK version on the remote node from version_path.<br>
+ """Inspect the DPDK version on the remote node.<br>
+<br>
+ Args:<br>
+ version_path: The path to the VERSION file containing the DPDK version.<br>
+<br>
+ Returns:<br>
+ The DPDK version.<br>
"""<br>
<br>
@abstractmethod<br>
def get_remote_cpus(self, use_first_core: bool) -> list[LogicalCore]:<br>
- """<br>
- Compose a list of LogicalCores present on the remote node.<br>
- If use_first_core is False, the first physical core won't be used.<br>
+ r"""Get the list of :class:`~.cpu.LogicalCore`\s on the remote node.<br>
+<br>
+ Args:<br>
+ use_first_core: If :data:`False`, the first physical core won't be used.<br>
+<br>
+ Returns:<br>
+ The logical cores present on the node.<br>
"""<br>
<br>
@abstractmethod<br>
def kill_cleanup_dpdk_apps(self, dpdk_prefix_list: Iterable[str]) -> None:<br>
- """<br>
- Kill and cleanup all DPDK apps identified by dpdk_prefix_list. If<br>
- dpdk_prefix_list is empty, attempt to find running DPDK apps to kill and clean.<br>
+ """Kill and cleanup all DPDK apps.<br>
+<br>
+ Args:<br>
+ dpdk_prefix_list: Kill all apps identified by `dpdk_prefix_list`.<br>
+ If `dpdk_prefix_list` is empty, attempt to find running DPDK apps to kill and clean.<br>
"""<br>
<br>
@abstractmethod<br>
def get_dpdk_file_prefix(self, dpdk_prefix: str) -> str:<br>
- """<br>
- Get the DPDK file prefix that will be used when running DPDK apps.<br>
+ """Make OS-specific modification to the DPDK file prefix.<br>
+<br>
+ Args:<br>
+ dpdk_prefix: The OS-unaware file prefix.<br>
+<br>
+ Returns:<br>
+ The OS-specific file prefix.<br>
"""<br>
<br>
@abstractmethod<br>
- def setup_hugepages(self, hugepage_amount: int, force_first_numa: bool) -> None:<br>
- """<br>
- Get the node's Hugepage Size, configure the specified amount of hugepages<br>
+ def setup_hugepages(self, hugepage_count: int, force_first_numa: bool) -> None:<br>
+ """Configure hugepages on the node.<br>
+<br>
+ Get the node's Hugepage Size, configure the specified count of hugepages<br>
if needed and mount the hugepages if needed.<br>
- If force_first_numa is True, configure hugepages just on the first socket.<br>
+<br>
+ Args:<br>
+ hugepage_count: Configure this many hugepages.<br>
+ force_first_numa: If :data:`True`, configure hugepages just on the first numa node.<br>
"""<br>
<br>
@abstractmethod<br>
def get_compiler_version(self, compiler_name: str) -> str:<br>
- """<br>
- Get installed version of compiler used for DPDK<br>
+ """Get installed version of compiler used for DPDK.<br>
+<br>
+ Args:<br>
+ compiler_name: The name of the compiler executable.<br>
+<br>
+ Returns:<br>
+ The compiler's version.<br>
"""<br>
<br>
@abstractmethod<br>
def get_node_info(self) -> NodeInfo:<br>
- """<br>
- Collect information about the node<br>
+ """Collect additional information about the node.<br>
+<br>
+ Returns:<br>
+ Node information.<br>
"""<br>
<br>
@abstractmethod<br>
def update_ports(self, ports: list[Port]) -> None:<br>
- """<br>
- Get additional information about ports:<br>
- Logical name (e.g. enp7s0) if applicable<br>
- Mac address<br>
+ """Get additional information about ports from the operating system and update them.<br>
+<br>
+ The additional information is:<br>
+<br>
+ * Logical name (e.g. ``enp7s0``) if applicable,<br>
+ * Mac address.<br>
+<br>
+ Args:<br>
+ ports: The ports to update.<br>
"""<br>
<br>
@abstractmethod<br>
def configure_port_state(self, port: Port, enable: bool) -> None:<br>
- """<br>
- Enable/disable port.<br>
+ """Enable/disable `port` in the operating system.<br>
+<br>
+ Args:<br>
+ port: The port to configure.<br>
+ enable: If :data:`True`, enable the port, otherwise shut it down.<br>
"""<br>
<br>
@abstractmethod<br>
@@ -273,12 +405,18 @@ def configure_port_ip_address(<br>
port: Port,<br>
delete: bool,<br>
) -> None:<br>
- """<br>
- Configure (add or delete) an IP address of the input port.<br>
+ """Configure an IP address on `port` in the operating system.<br>
+<br>
+ Args:<br>
+ address: The address to configure.<br>
+ port: The port to configure.<br>
+ delete: If :data:`True`, remove the IP address, otherwise configure it.<br>
"""<br>
<br>
@abstractmethod<br>
def configure_ipv4_forwarding(self, enable: bool) -> None:<br>
- """<br>
- Enable IPv4 forwarding in the underlying OS.<br>
+ """Enable IPv4 forwarding in the operating system.<br>
+<br>
+ Args:<br>
+ enable: If :data:`True`, enable the forwarding, otherwise disable it.<br>
"""<br>
-- <br>
2.34.1<br>
<br>
</blockquote></div></div>