From 143ce0ddd1fa92adf4e7ea1082147ae253b15008 Mon Sep 17 00:00:00 2001 From: cferreiragonz Date: Thu, 16 Jan 2025 15:16:42 +0100 Subject: [PATCH] Refs #22627: Add EASY_MODE to parser Signed-off-by: cferreiragonz --- test/system/tools/fastdds/CMakeLists.txt | 2 + .../tools/fastdds/test_discovery_parser.py | 66 +++++++++++++++++++ .../process_handler/process_handler.py | 28 +++++++- .../xmlrpc_local/local_client.py | 4 +- tools/fastdds/discovery/parser.py | 48 ++++++++++---- 5 files changed, 130 insertions(+), 18 deletions(-) diff --git a/test/system/tools/fastdds/CMakeLists.txt b/test/system/tools/fastdds/CMakeLists.txt index 75b6900c3fe..c2c8f1cc767 100644 --- a/test/system/tools/fastdds/CMakeLists.txt +++ b/test/system/tools/fastdds/CMakeLists.txt @@ -67,12 +67,14 @@ if(Python3_Interpreter_FOUND) TestDiscoveryParser.test_parser_auto TestDiscoveryParser.test_parser_auto_domain_arg TestDiscoveryParser.test_parser_auto_domain_env + TestDiscoveryParser.test_parser_auto_easy_mode_domain_env TestDiscoveryParser.test_parser_auto_ros_static_peers TestDiscoveryParser.test_parser_start TestDiscoveryParser.test_parser_start_ros_static_peers TestDiscoveryParser.test_parser_start_with_arg TestDiscoveryParser.test_parser_stop_when_off TestDiscoveryParser.test_parser_stop_when_on + TestDiscoveryParser.test_parser_stop_whith_unknown_args TestDiscoveryParser.test_parser_list_when_off TestDiscoveryParser.test_parser_list_when_on TestDiscoveryParser.test_parser_add_when_off diff --git a/test/system/tools/fastdds/test_discovery_parser.py b/test/system/tools/fastdds/test_discovery_parser.py index 4fd4da8b213..7fd7cb073c3 100644 --- a/test/system/tools/fastdds/test_discovery_parser.py +++ b/test/system/tools/fastdds/test_discovery_parser.py @@ -33,12 +33,16 @@ def __init__(self, methodName = "test_fastdds_daemon"): self.check_command = '' # Attribute to check the domain sent to the RPC server to act as index self.domain = 0 + # Attribute to check the value of the third attr (easy_mode or check_server) sent to the RPC server + self.third_attr = '' def side_effect_rpc(self, *args, **kwargs): domain = args[0] used_cmd = args[1] + third_attr = args[2] assert(domain == self.domain) assert(len(used_cmd) == len(self.check_command)) + assert(third_attr == self.third_attr) for i in range(len(self.check_command)): assert(used_cmd[i] == self.check_command[i]) return args @@ -175,6 +179,36 @@ def test_parser_auto_domain_env(self, mock_rpc_stopall, mock_rpc_nbrequest, mock mock_rpc_brequest.assert_not_called() mock_exit.assert_not_called() + @patch('parser.sys.exit') + @patch('parser.is_daemon_running') + @patch('parser.spawn_daemon') + @patch('parser.client_cli.run_request_b') + @patch('parser.client_cli.run_request_nb') + @patch('parser.client_cli.stop_all_request') + def test_parser_auto_easy_mode_domain_env(self, mock_rpc_stopall, mock_rpc_nbrequest, mock_rpc_brequest, mock_spawn, mock_is_running, mock_exit): + mock_rpc_stopall.return_value = 'Mocked request' + mock_rpc_nbrequest.return_value = 'Mocked request' + mock_rpc_brequest.return_value = 'Mocked request' + mock_spawn.return_value = True + + self.set_env_values('EASY_MODE', '127.0.0.1') + self.domain = 42 + self.third_attr = '127.0.0.1' + self.check_command = [str(command_to_int[Command.AUTO]), '-d', '42', '127.0.0.1:42'] + mock_rpc_nbrequest.side_effect = self.side_effect_rpc + + # The parser is not responsible of adding the EASY_MODE argument to the command. This is done in Fast DDS. + # The parser only checks if the EASY_MODE variable is set to pass it to the RPC server as third argument + argv = ['auto', '-d', '42', '127.0.0.1:42'] + parser = Parser(argv) + + mock_is_running.assert_not_called() + mock_spawn.assert_called_once() + mock_rpc_stopall.assert_not_called() + mock_rpc_nbrequest.assert_called_once() + mock_rpc_brequest.assert_not_called() + mock_exit.assert_not_called() + @patch('parser.sys.exit') @patch('parser.is_daemon_running') @patch('parser.spawn_daemon') @@ -333,6 +367,32 @@ def test_parser_stop_when_on(self, mock_rpc_stop_once, mock_rpc_nbrequest, mock_ mock_rpc_brequest.assert_not_called() mock_exit.assert_not_called() + @patch('parser.sys.exit') + @patch('parser.is_daemon_running') + @patch('parser.shutdown_daemon') + @patch('parser.client_cli.run_request_b') + @patch('parser.client_cli.run_request_nb') + @patch('parser.client_cli.stop_request') + def test_parser_stop_whith_unknown_args(self, mock_rpc_stop_once, mock_rpc_nbrequest, mock_rpc_brequest, mock_shutdown, mock_is_running, mock_exit): + mock_rpc_stop_once.return_value = 'Mocked request' + mock_rpc_nbrequest.return_value = 'Mocked request' + mock_rpc_brequest.return_value = 'Mocked request' + mock_is_running.return_value = True + mock_shutdown.return_value = True + + self.check_command = [str(command_to_int[Command.STOP]), '-d', '0'] + mock_rpc_brequest.side_effect = self.side_effect_rpc + + argv = ['stop', '-d', '0', 'extra_arg'] + parser = Parser(argv) + + mock_is_running.assert_called_once() + mock_shutdown.assert_not_called() + mock_rpc_stop_once.assert_not_called() + mock_rpc_nbrequest.assert_not_called() + mock_rpc_brequest.assert_not_called() + mock_exit.assert_called_once() + @patch('parser.sys.exit') @patch('parser.is_daemon_running') @patch('parser.spawn_daemon') @@ -346,6 +406,7 @@ def test_parser_list_when_off(self, mock_rpc_stopall, mock_rpc_nbrequest, mock_r mock_is_running.return_value = False mock_spawn.return_value = True + self.third_attr = False self.check_command = [str(command_to_int[Command.LIST]), '-d', '0'] mock_rpc_brequest.side_effect = self.side_effect_rpc @@ -376,6 +437,7 @@ def test_parser_list_when_on(self, mock_rpc_stopall, mock_rpc_nbrequest, mock_rp mock_is_running.return_value = True mock_spawn.return_value = True + self.third_attr = False self.check_command = [str(command_to_int[Command.LIST]), '-d', '0'] mock_rpc_brequest.side_effect = self.side_effect_rpc @@ -402,6 +464,7 @@ def test_parser_add_when_off(self, mock_rpc_stopall, mock_rpc_nbrequest, mock_rp mock_is_running.return_value = False mock_spawn.return_value = True + self.third_attr = True self.check_command = [str(command_to_int[Command.ADD]), '-d', '0'] mock_rpc_brequest.side_effect = self.side_effect_rpc @@ -432,6 +495,7 @@ def test_parser_add_when_on(self, mock_rpc_stopall, mock_rpc_nbrequest, mock_rpc mock_is_running.return_value = True mock_spawn.return_value = True + self.third_attr = True self.check_command = [str(command_to_int[Command.ADD]), '-d', '0'] mock_rpc_brequest.side_effect = self.side_effect_rpc @@ -458,6 +522,7 @@ def test_parser_set_when_off(self, mock_rpc_stopall, mock_rpc_nbrequest, mock_rp mock_is_running.return_value = False mock_spawn.return_value = True + self.third_attr = True self.check_command = [str(command_to_int[Command.SET]), '-d', '0'] mock_rpc_brequest.side_effect = self.side_effect_rpc @@ -488,6 +553,7 @@ def test_parser_set_when_on(self, mock_rpc_stopall, mock_rpc_nbrequest, mock_rpc mock_is_running.return_value = True mock_spawn.return_value = True + self.third_attr = True self.check_command = [str(command_to_int[Command.SET]), '-d', '0'] mock_rpc_brequest.side_effect = self.side_effect_rpc diff --git a/tools/fastdds/discovery/fastdds_daemon/process_handler/process_handler.py b/tools/fastdds/discovery/fastdds_daemon/process_handler/process_handler.py index 3d7adfbee6c..831bd51f7d0 100644 --- a/tools/fastdds/discovery/fastdds_daemon/process_handler/process_handler.py +++ b/tools/fastdds/discovery/fastdds_daemon/process_handler/process_handler.py @@ -20,7 +20,10 @@ class ProcessHandler: def __init__(self, toolpath: str): - self.processes = {} # Dictionary to store process information + # Dictionary to store process information: domain -> process + self.processes = {} + # Dictionary to store remote connections of each domain: domain -> connection + self.remote_connections = {} # Path to the tool to be executed. It is stored in the object to avoid # passing it as an argument in every call and to enable daemon calls between # host and docker containers with --net=host option. @@ -41,10 +44,19 @@ def _get_signal(self, sig: int): elif sig == 2: return signal.SIGKILL - def run_process_nb(self, domain: int, command: list): - """ Used for starting new servers. Commands 'start' and 'auto'.""" + def run_process_nb(self, domain: int, command: list, easy_mode: str): + """ + Used for starting new servers. Commands 'start' and 'auto'. + If @easy_mode is empty, it means the 'EASY_MODE' variable was not set, that is, + it is a direct call from the CLI. + Note that 'easy_mode' has already been added to the command string in the parser, + it is only used here to avoid using regex to find its value and differentiate + 'auto' and 'start' commands. + """ with self._lock: if domain in self.processes: + if easy_mode != '' and easy_mode != self.remote_connections[domain]: + return f"Error: DS for Domain '{domain}' already points to '{self.remote_connections[domain]}'." return f"Discovery server for Domain '{domain}' is already running." # Start a new subprocess in a non-blocking way command.insert(0, self.toolpath) @@ -60,6 +72,16 @@ def run_process_nb(self, domain: int, command: list): if 'started' in output: self.processes[domain] = process + # Remote connections are always the last element of the command + # Cpp tool should fail if more than one argument is received + remote_connection = '' + if len(command) > 4: + ip_re = r"(\d{1,3}(?:\.\d{1,3}){3})" + match = re.search(ip_re, command[-1]) + if match: + ip = match.group(1) + remote_connection = ip + self.remote_connections[domain] = remote_connection else: # Strip ANSI colors from the error message stderr = os.read(process.stderr.fileno(), 1024).decode('utf-8') diff --git a/tools/fastdds/discovery/fastdds_daemon/xmlrpc_local/local_client.py b/tools/fastdds/discovery/fastdds_daemon/xmlrpc_local/local_client.py index 5bf61af1ce2..76a6aa8a835 100644 --- a/tools/fastdds/discovery/fastdds_daemon/xmlrpc_local/local_client.py +++ b/tools/fastdds/discovery/fastdds_daemon/xmlrpc_local/local_client.py @@ -15,10 +15,10 @@ import discovery.fastdds_daemon.daemon as daemon import xmlrpc.client -def run_request_nb(domain, command) -> str: +def run_request_nb(domain, command, remote) -> str: server_url = daemon.get_xmlrpc_server_url() with xmlrpc.client.ServerProxy(server_url) as proxy: - return(proxy.run_process_nb(domain, command)) + return(proxy.run_process_nb(domain, command, remote)) def run_request_b(domain, command, check_server) -> str: server_url = daemon.get_xmlrpc_server_url() diff --git a/tools/fastdds/discovery/parser.py b/tools/fastdds/discovery/parser.py index 78b1e374ba9..0d502bd546f 100644 --- a/tools/fastdds/discovery/parser.py +++ b/tools/fastdds/discovery/parser.py @@ -40,6 +40,7 @@ DOMAIN_ENV_VAR = "ROS_DOMAIN_ID" REMOTE_SERVERS_ENV_VAR = "ROS_STATIC_PEERS" +EASY_MODE_ENV_VAR = "EASY_MODE" class Command(Enum): SERVER = "server" # Does not need to be specified @@ -183,10 +184,22 @@ def __init__(self, argv): self.__stop_daemon() for p in Path(self.__shm_dir()).glob("*_servers.txt"): p.unlink() - elif command_int == command_to_int[Command.AUTO] or command_int == command_to_int[Command.START]: + elif command_int == command_to_int[Command.AUTO]: + self.__start_daemon(tool_path) + easy_mode = self.__get_easy_mode_from_env() + if easy_mode is None: + self.__add_remote_servers_to_args(args_for_cpp) + easy_mode = '' + output = client_cli.run_request_nb(domain, args_for_cpp, easy_mode) + print(output) + if 'Error starting Server' in output: + raise SystemExit(1) # Exit with error code + elif 'Error: DS for Domain' in output: + raise SystemExit(2) + elif command_int == command_to_int[Command.START]: self.__start_daemon(tool_path) self.__add_remote_servers_to_args(args_for_cpp) - output = client_cli.run_request_nb(domain, args_for_cpp) + output = client_cli.run_request_nb(domain, args_for_cpp, '') print(output) if 'Error starting Server' in output: raise SystemExit(1) # Exit with error code @@ -194,6 +207,9 @@ def __init__(self, argv): if not self.__is_daemon_running(): print('The Fast DDS daemon is not running.') raise SystemExit(0) + if unknown_args: + print(f"Unknown arguments: {unknown_args}") + raise SystemExit(0) print(client_cli.stop_request(domain, get_sig_idx(signal.SIGTERM))) elif command_int == command_to_int[Command.ADD]: if not self.__is_daemon_running(): @@ -211,10 +227,8 @@ def __init__(self, argv): print('The Fast DDS daemon is not running. No servers to list.') raise SystemExit(0) print(client_cli.run_request_b(domain, args_for_cpp, False)) - pass elif command_int == command_to_int[Command.INFO]: print('Info mode not implemented yet.') - pass else: print('Fast DDS CLI Error: Unknown command') @@ -305,15 +319,15 @@ def __get_domain_id_from_env(self) -> int: return id def __get_remote_servers_from_env(self) -> str: - """ - Obtain the remote servers from the environment. - Default to empty string if not found. - """ - servers = '' - env_value = os.getenv(REMOTE_SERVERS_ENV_VAR) - if env_value is not None: - servers = env_value - return servers + """ + Obtain the remote servers from the environment. + Default to empty string if not found. + """ + servers = '' + env_value = os.getenv(REMOTE_SERVERS_ENV_VAR) + if env_value is not None: + servers = env_value + return servers def __add_remote_servers_to_args(self, args) -> None: """ @@ -323,6 +337,14 @@ def __add_remote_servers_to_args(self, args) -> None: if remote_servers != '': args.append(remote_servers) + def __get_easy_mode_from_env(self) -> str: + """ + Obtain the value present in 'EASY_MODE' from the environment. + """ + value = None + value = os.getenv(EASY_MODE_ENV_VAR) + return value + def __shm_dir(self): """ Calculate the shm directory.