From 702dfddf23036e6ec79e4b9d5eac7d09637971b8 Mon Sep 17 00:00:00 2001 From: Tomer Haskalovitch Date: Sun, 6 Jul 2025 23:15:50 +0300 Subject: [PATCH] mgr/dashboard: nvmeof cli rename ns to namespace, fixes for text responses, subsys add params Signed-off-by: Tomer Haskalovitch --- .../mgr/dashboard/controllers/nvmeof.py | 89 ++++++++++++++----- src/pybind/mgr/dashboard/model/nvmeof.py | 16 ++-- src/pybind/mgr/dashboard/openapi.yaml | 11 ++- .../mgr/dashboard/services/nvmeof_cli.py | 34 ++++--- .../mgr/dashboard/tests/test_nvmeof_cli.py | 80 +++++++++++++++++ src/pybind/mgr/mgr_module.py | 5 +- 6 files changed, 183 insertions(+), 52 deletions(-) diff --git a/src/pybind/mgr/dashboard/controllers/nvmeof.py b/src/pybind/mgr/dashboard/controllers/nvmeof.py index 7fe19fcd3a999..22847d672a35c 100644 --- a/src/pybind/mgr/dashboard/controllers/nvmeof.py +++ b/src/pybind/mgr/dashboard/controllers/nvmeof.py @@ -32,8 +32,9 @@ else: @APIRouter("/nvmeof/gateway", Scope.NVME_OF) @APIDoc("NVMe-oF Gateway Management API", "NVMe-oF Gateway") class NVMeoFGateway(RESTController): - - @NvmeofCLICommand("nvmeof gw info", model.GatewayInfo) + @NvmeofCLICommand( + "nvmeof gateway info", model.GatewayInfo, alias="nvmeof gw info" + ) @EndpointDoc("Get information about the NVMeoF gateway") @convert_to_model(model.GatewayInfo) @handle_nvmeof_error @@ -57,7 +58,9 @@ else: @ReadPermission @Endpoint('GET', '/version') - @NvmeofCLICommand("nvmeof gw version", model.GatewayVersion) + @NvmeofCLICommand( + "nvmeof gateway version", model.GatewayVersion, alias="nvmeof gw version" + ) @EndpointDoc("Get the version of the NVMeoF gateway") @convert_to_model(model.GatewayVersion) @handle_nvmeof_error @@ -71,7 +74,10 @@ else: @ReadPermission @Endpoint('GET', '/log_level') - @NvmeofCLICommand("nvmeof gw get_log_level", model.GatewayLogLevelInfo) + @NvmeofCLICommand( + "nvmeof gateway get_log_level", model.GatewayLogLevelInfo, + alias="nvmeof gw get_log_level" + ) @EndpointDoc("Get NVMeoF gateway log level information") @convert_to_model(model.GatewayLogLevelInfo) @handle_nvmeof_error @@ -84,7 +90,8 @@ else: @ReadPermission @Endpoint('PUT', '/log_level') - @NvmeofCLICommand("nvmeof gw set_log_level", model.RequestStatus) + @NvmeofCLICommand( + "nvmeof gateway set_log_level", model.RequestStatus, alias="nvmeof gw set_log_level") @EndpointDoc("Set NVMeoF gateway log levels") @convert_to_model(model.RequestStatus) @handle_nvmeof_error @@ -195,11 +202,15 @@ else: ) @convert_to_model(model.SubsystemStatus) @handle_nvmeof_error - def create(self, nqn: str, enable_ha: bool = True, max_namespaces: int = 4096, + def create(self, nqn: str, enable_ha: Optional[bool] = True, + max_namespaces: Optional[int] = 4096, no_group_append: Optional[bool] = True, + serial_number: Optional[str] = None, dhchap_key: Optional[str] = None, gw_group: Optional[str] = None, traddr: Optional[str] = None): return NVMeoFClient(gw_group=gw_group, traddr=traddr).stub.create_subsystem( NVMeoFClient.pb2.create_subsystem_req( - subsystem_nqn=nqn, max_namespaces=max_namespaces, enable_ha=enable_ha + subsystem_nqn=nqn, serial_number=serial_number, + max_namespaces=max_namespaces, enable_ha=enable_ha, + no_group_append=no_group_append, dhchap_key=dhchap_key ) ) @@ -356,7 +367,9 @@ else: @APIDoc("NVMe-oF Subsystem Namespace Management API", "NVMe-oF Subsystem Namespace") class NVMeoFNamespace(RESTController): @pick("namespaces") - @NvmeofCLICommand("nvmeof ns list", model.NamespaceList) + @NvmeofCLICommand( + "nvmeof namespace list", model.NamespaceList, alias="nvmeof ns list" + ) @EndpointDoc( "List all NVMeoF namespaces in a subsystem", parameters={ @@ -372,7 +385,8 @@ else: ) @pick("namespaces", first=True) - @NvmeofCLICommand("nvmeof ns get", model.NamespaceList) + @NvmeofCLICommand( + "nvmeof namespace get", model.NamespaceList, alias="nvmeof ns get") @EndpointDoc( "Get info from specified NVMeoF namespace", parameters={ @@ -391,7 +405,10 @@ else: @ReadPermission @Endpoint('GET', '{nsid}/io_stats') - @NvmeofCLICommand("nvmeof ns get_io_stats", model.NamespaceIOStats) + @NvmeofCLICommand( + "nvmeof namespace get_io_stats", model.NamespaceIOStats, + alias="nvmeof ns get_io_stats" + ) @EndpointDoc( "Get IO stats from specified NVMeoF namespace", parameters={ @@ -409,7 +426,9 @@ else: subsystem_nqn=nqn, nsid=int(nsid)) ) - @NvmeofCLICommand("nvmeof ns add", model.NamespaceCreation) + @NvmeofCLICommand( + "nvmeof namespace add", model.NamespaceCreation, alias="nvmeof ns add" + ) @EndpointDoc( "Create a new NVMeoF namespace", parameters={ @@ -472,7 +491,8 @@ else: @ReadPermission @Endpoint('PUT', '{nsid}/set_qos') - @NvmeofCLICommand("nvmeof ns set_qos", model=model.RequestStatus) + @NvmeofCLICommand( + "nvmeof namespace set_qos", model=model.RequestStatus, alias="nvmeof ns set_qos") @EndpointDoc( "set QOS for specified NVMeoF namespace", parameters={ @@ -521,7 +541,10 @@ else: @ReadPermission @Endpoint('PUT', '{nsid}/change_load_balancing_group') - @NvmeofCLICommand("nvmeof ns change_load_balancing_group", model=model.RequestStatus) + @NvmeofCLICommand( + "nvmeof namespace change_load_balancing_group", model=model.RequestStatus, + alias="nvmeof ns change_load_balancing_group" + ) @EndpointDoc( "set the load balancing group for specified NVMeoF namespace", parameters={ @@ -553,7 +576,9 @@ else: @ReadPermission @Endpoint('PUT', '{nsid}/resize') - @NvmeofCLICommand("nvmeof ns resize", model=model.RequestStatus) + @NvmeofCLICommand( + "nvmeof namespace resize", model=model.RequestStatus, alias="nvmeof ns resize" + ) @EndpointDoc( "resize the specified NVMeoF namespace", parameters={ @@ -585,7 +610,9 @@ else: @ReadPermission @Endpoint('PUT', '{nsid}/add_host') - @NvmeofCLICommand("nvmeof ns add_host", model=model.RequestStatus) + @NvmeofCLICommand( + "nvmeof namespace add_host", model=model.RequestStatus, alias="nvmeof ns add_host" + ) @EndpointDoc( "Adds a host to the specified NVMeoF namespace", parameters={ @@ -621,7 +648,9 @@ else: @ReadPermission @Endpoint('PUT', '{nsid}/del_host') - @NvmeofCLICommand("nvmeof ns del_host", model=model.RequestStatus) + @NvmeofCLICommand( + "nvmeof namespace del_host", model=model.RequestStatus, alias="nvmeof ns del_host" + ) @EndpointDoc( "Removes a host from the specified NVMeoF namespace", parameters={ @@ -652,7 +681,10 @@ else: @ReadPermission @Endpoint('PUT', '{nsid}/change_visibility') - @NvmeofCLICommand("nvmeof ns change_visibility", model=model.RequestStatus) + @NvmeofCLICommand( + "nvmeof namespace change_visibility", model=model.RequestStatus, + alias="nvmeof ns change_visibility" + ) @EndpointDoc( "changes the visibility of the specified NVMeoF namespace to all or selected hosts", parameters={ @@ -685,7 +717,10 @@ else: @ReadPermission @Endpoint('PUT', '{nsid}/set_auto_resize') - @NvmeofCLICommand("nvmeof ns set_auto_resize", model=model.RequestStatus) + @NvmeofCLICommand( + "nvmeof namespace set_auto_resize", model=model.RequestStatus, + alias="nvmeof ns set_auto_resize" + ) @EndpointDoc( "Enable or disable namespace auto resize when RBD image is resized", parameters={ @@ -720,7 +755,10 @@ else: @ReadPermission @Endpoint('PUT', '{nsid}/set_rbd_trash_image') - @NvmeofCLICommand("nvmeof ns set_rbd_trash_image", model=model.RequestStatus) + @NvmeofCLICommand( + "nvmeof namespace set_rbd_trash_image", model=model.RequestStatus, + alias="nvmeof ns set_rbd_trash_image" + ) @EndpointDoc( "changes the trash image on delete of the specified NVMeoF \ namespace to all or selected hosts", @@ -755,7 +793,10 @@ else: @ReadPermission @Endpoint('PUT', '{nsid}/refresh_size') - @NvmeofCLICommand("nvmeof ns refresh_size", model=model.RequestStatus) + @NvmeofCLICommand( + "nvmeof namespace refresh_size", model=model.RequestStatus, + alias="nvmeof ns refresh_size" + ) @EndpointDoc( "refresh the specified NVMeoF namespace to current RBD image size", parameters={ @@ -783,7 +824,9 @@ else: ) @pick("namespaces", first=True) - @NvmeofCLICommand("nvmeof ns update", model.NamespaceList) + @NvmeofCLICommand( + "nvmeof namespace update", model.NamespaceList, alias="nvmeof ns update" + ) @EndpointDoc( "Update an existing NVMeoF namespace", parameters={ @@ -873,7 +916,7 @@ else: return response @empty_response - @NvmeofCLICommand("nvmeof ns del", model.RequestStatus) + @NvmeofCLICommand("nvmeof namespace del", model.RequestStatus, alias="nvmeof ns del") @EndpointDoc( "Delete an existing NVMeoF namespace", parameters={ @@ -924,7 +967,7 @@ else: @convert_to_model(model.HostsInfo, finalize=_update_hosts) @handle_nvmeof_error def list( - self, nqn: str, clear_alerts: Optional[bool], + self, nqn: str, clear_alerts: Optional[bool] = None, gw_group: Optional[str] = None, traddr: Optional[str] = None ): return NVMeoFClient(gw_group=gw_group, traddr=traddr).stub.list_hosts( diff --git a/src/pybind/mgr/dashboard/model/nvmeof.py b/src/pybind/mgr/dashboard/model/nvmeof.py index af9b3bb415cfa..43dd490c8a946 100644 --- a/src/pybind/mgr/dashboard/model/nvmeof.py +++ b/src/pybind/mgr/dashboard/model/nvmeof.py @@ -15,7 +15,7 @@ class CliHeader: class GatewayInfo(NamedTuple): - bool_status: bool + bool_status: Annotated[bool, CliFlags.DROP] status: int error_message: str hostname: str @@ -26,11 +26,11 @@ class GatewayInfo(NamedTuple): addr: str port: int load_balancing_group: Annotated[int, CliHeader('LB Group')] - max_hosts: int - max_hosts_per_subsystem: int - max_namespaces: int - max_namespaces_per_subsystem: int - max_subsystems: int + max_hosts: Annotated[int, CliFlags.DROP] + max_hosts_per_subsystem: Annotated[int, CliFlags.DROP] + max_namespaces: Annotated[int, CliFlags.DROP] + max_namespaces_per_subsystem: Annotated[int, CliFlags.DROP] + max_subsystems: Annotated[int, CliFlags.DROP] spdk_version: Optional[str] = "" @@ -189,7 +189,7 @@ class Host(NamedTuple): nqn: str use_psk: Optional[bool] use_dhchap: Optional[bool] - disconnected_due_to_keepalive_timeout: Optional[bool] + disconnected_due_to_keepalive_timeout: Annotated[Optional[bool], CliFlags.DROP] class HostsInfo(NamedTuple): @@ -197,7 +197,7 @@ class HostsInfo(NamedTuple): error_message: str allow_any_host: bool subsystem_nqn: str - hosts: List[Host] + hosts: Annotated[List[Host], CliFlags.EXCLUSIVE_LIST] class RequestStatus(NamedTuple): diff --git a/src/pybind/mgr/dashboard/openapi.yaml b/src/pybind/mgr/dashboard/openapi.yaml index a73202bc8408d..e74eff6bc5c35 100755 --- a/src/pybind/mgr/dashboard/openapi.yaml +++ b/src/pybind/mgr/dashboard/openapi.yaml @@ -8557,6 +8557,8 @@ paths: application/json: schema: properties: + dhchap_key: + type: string enable_ha: default: true description: Enable high availability @@ -8568,9 +8570,14 @@ paths: default: 4096 description: Maximum number of namespaces type: integer + no_group_append: + default: true + type: boolean nqn: description: NVMeoF subsystem NQN type: string + serial_number: + type: integer traddr: type: string required: @@ -8740,10 +8747,10 @@ paths: required: true schema: type: string - - description: Clear any host alert signal after getting its value + - allowEmptyValue: true + description: Clear any host alert signal after getting its value in: query name: clear_alerts - required: true schema: type: boolean - allowEmptyValue: true diff --git a/src/pybind/mgr/dashboard/services/nvmeof_cli.py b/src/pybind/mgr/dashboard/services/nvmeof_cli.py index 3e10e7f13a50e..b6be1dd726939 100644 --- a/src/pybind/mgr/dashboard/services/nvmeof_cli.py +++ b/src/pybind/mgr/dashboard/services/nvmeof_cli.py @@ -97,23 +97,26 @@ class AnnotatedDataTextOutputFormatter(OutputFormatter): return self._get_list_text_output(data) return self._get_object_text_output(data) + def _get_row(self, columns, data_obj): + row = [] + for col in columns: + col_val = data_obj.get(col) + if col_val is None: + col_val = '' + row.append(str(col_val)) + return row + def _get_list_text_output(self, data): columns = list(dict.fromkeys([key for obj in data for key in obj.keys()])) table = self._create_table(columns) for d in data: - row = [] - for col in columns: - row.append(str(d.get(col))) - table.add_row(row) + table.add_row(self._get_row(columns, d)) return table.get_string() def _get_object_text_output(self, data): columns = [k for k in data.keys() if k not in ["status", "error_message"]] table = self._create_table(columns) - row = [] - for col in columns: - row.append(str(data.get(col))) - table.add_row(row) + table.add_row(self._get_row(columns, data)) return table.get_string() def _is_list_of_complex_type(self, value): @@ -200,27 +203,22 @@ class AnnotatedDataTextOutputFormatter(OutputFormatter): class NvmeofCLICommand(CLICommand): desc: str - def __init__(self, prefix, model: Type[NamedTuple], perm='rw', poll=False): + def __init__(self, prefix, model: Type[NamedTuple], alias=None, perm='rw', poll=False): super().__init__(prefix, perm, poll) self._output_formatter = AnnotatedDataTextOutputFormatter() self._model = model + self._alias = alias def _use_api_endpoint_desc_if_available(self, func): if not self.desc and hasattr(func, 'doc_info'): self.desc = func.doc_info.get('summary', '') def __call__(self, func) -> HandlerFuncType: # type: ignore - # pylint: disable=useless-super-delegation - """ - This method is being overriden solely to be able to disable the linters checks for typing. - The NvmeofCLICommand decorator assumes a different type returned from the - function it wraps compared to CLICmmand, breaking a Liskov substitution principal, - hence triggering linters alerts. - """ - resp = super().__call__(func) + if self._alias: + NvmeofCLICommand(self._alias, model=self._model)._register_handler(func) + resp = super().__call__(func) self._use_api_endpoint_desc_if_available(func) - return resp def call(self, diff --git a/src/pybind/mgr/dashboard/tests/test_nvmeof_cli.py b/src/pybind/mgr/dashboard/tests/test_nvmeof_cli.py index fd3583e6c6e27..f5fa3fa8a3720 100644 --- a/src/pybind/mgr/dashboard/tests/test_nvmeof_cli.py +++ b/src/pybind/mgr/dashboard/tests/test_nvmeof_cli.py @@ -164,6 +164,50 @@ class TestNvmeofCLICommand: del NvmeofCLICommand.COMMANDS[test_cmd] assert test_cmd not in NvmeofCLICommand.COMMANDS + def test_command_alias_calls_command(self, base_call_mock): + test_cmd = "test command1" + test_alias = "test alias1" + + class Model(NamedTuple): + a: str + b: int + + @NvmeofCLICommand(test_cmd, Model, alias=test_alias) + def func(_): # noqa # pylint: disable=unused-variable + return {'a': '1', 'b': 2} + + assert test_cmd in NvmeofCLICommand.COMMANDS + assert test_alias in NvmeofCLICommand.COMMANDS + + result = NvmeofCLICommand.COMMANDS[test_cmd].call(MagicMock(), {}) + assert result.retval == 0 + assert result.stdout == ( + "+-+\n" + "|A|\n" + "+-+\n" + "|b|\n" + "+-+" + ) + assert result.stderr == '' + base_call_mock.assert_called_once() + + result = NvmeofCLICommand.COMMANDS[test_alias].call(MagicMock(), {}) + assert result.retval == 0 + assert result.stdout == ( + "+-+\n" + "|A|\n" + "+-+\n" + "|b|\n" + "+-+" + ) + assert result.stderr == '' + assert base_call_mock.call_count == 2 + + del NvmeofCLICommand.COMMANDS[test_cmd] + del NvmeofCLICommand.COMMANDS[test_alias] + assert test_cmd not in NvmeofCLICommand.COMMANDS + assert test_alias not in NvmeofCLICommand.COMMANDS + class TestNVMeoFConfCLI(unittest.TestCase, CLICommandTestMixin): def setUp(self): @@ -339,6 +383,42 @@ class TestNVMeoFConfCLI(unittest.TestCase, CLICommandTestMixin): class TestAnnotatedDataTextOutputFormatter(): + def test_no_annotation(self): + class Sample(NamedTuple): + name: str + age: int + byte: int + + data = {'name': 'Alice', 'age': 30, "byte": 20971520} + + formatter = AnnotatedDataTextOutputFormatter() + output = formatter.format_output(data, Sample) + assert output == ( + '+-----+---+--------+\n' + '|Name |Age|Byte |\n' + '+-----+---+--------+\n' + '|Alice|30 |20971520|\n' + '+-----+---+--------+' + ) + + def test_none_to_empty_str_annotation(self): + class Sample(NamedTuple): + name: str + age: int + byte: int + + data = {'name': 'Alice', 'age': 30, "byte": None} + + formatter = AnnotatedDataTextOutputFormatter() + output = formatter.format_output(data, Sample) + assert output == ( + '+-----+---+----+\n' + '|Name |Age|Byte|\n' + '+-----+---+----+\n' + '|Alice|30 | |\n' + '+-----+---+----+' + ) + def test_size_bytes_annotation(self): class Sample(NamedTuple): name: str diff --git a/src/pybind/mgr/mgr_module.py b/src/pybind/mgr/mgr_module.py index 9487d9be0170b..2830a0b162276 100644 --- a/src/pybind/mgr/mgr_module.py +++ b/src/pybind/mgr/mgr_module.py @@ -473,12 +473,15 @@ class CLICommand(object): self.desc, self.arg_spec, self.first_default, self.args = \ self._load_func_metadata(f) - def __call__(self, func: HandlerFuncType) -> HandlerFuncType: + def _register_handler(self, func: HandlerFuncType) -> HandlerFuncType: self.store_func_metadata(func) self.func = func self.COMMANDS[self.prefix] = self return self.func + def __call__(self, func: HandlerFuncType) -> HandlerFuncType: + return self._register_handler(func) + def _get_arg_value(self, kwargs_switch: bool, key: str, val: Any) -> Tuple[bool, str, Any]: def start_kwargs() -> bool: if isinstance(val, str) and '=' in val: -- 2.39.5