From d88a35e25e1e99ffe28169061d84ffba0359caa5 Mon Sep 17 00:00:00 2001 From: Gil Bregman Date: Wed, 8 Jan 2025 16:40:42 +0200 Subject: [PATCH] Allow listing namespaces for all subsystems. Fixes #943 Signed-off-by: Gil Bregman --- .github/workflows/build-container.yml | 1 + control/cli.py | 66 +++++++++++++++---- control/grpc.py | 27 ++++---- control/proto/gateway.proto | 1 + tests/test_cli.py | 92 ++++++++++++++++++++++++++- 5 files changed, 160 insertions(+), 27 deletions(-) diff --git a/.github/workflows/build-container.yml b/.github/workflows/build-container.yml index 9e6c74e3..2672d32a 100644 --- a/.github/workflows/build-container.yml +++ b/.github/workflows/build-container.yml @@ -322,6 +322,7 @@ jobs: cephnvmf listener list --subsystem $sub cephnvmf host list --subsystem $sub done + cephnvmf namespace list - name: Run bdevperf run: | diff --git a/control/cli.py b/control/cli.py index 0b9d0275..79462136 100644 --- a/control/cli.py +++ b/control/cli.py @@ -1893,11 +1893,15 @@ def get_size_in_bytes(self, sz): def ns_list(self, args): """Lists namespaces on a subsystem.""" + ALL_SUBSYSTEMS = "*" out_func, err_func = self.get_output_functions(args) if args.nsid is not None and args.nsid <= 0: self.cli.parser.error("nsid value must be positive") + if not args.subsystem: + args.subsystem = ALL_SUBSYSTEMS + try: namespaces_info = self.stub.list_namespaces(pb2.list_namespaces_req( subsystem=args.subsystem, @@ -1909,12 +1913,32 @@ def ns_list(self, args): if args.format == "text" or args.format == "plain": if namespaces_info.status == 0: - if args.nsid and len(namespaces_info.namespaces) > 1: - err_func(f"Got more than one namespace for NSID {args.nsid}") - if args.uuid and len(namespaces_info.namespaces) > 1: - err_func(f"Got more than one namespace for UUID {args.uuid}") + if args.subsystem != ALL_SUBSYSTEMS: + if args.nsid and len(namespaces_info.namespaces) > 1: + err_func(f"Got more than one namespace for namespace ID {args.nsid}") + if args.uuid and len(namespaces_info.namespaces) > 1: + err_func(f"Got more than one namespace for UUID {args.uuid}") + if namespaces_info.subsystem_nqn != args.subsystem: + err_func(f"Got namespaces in subsystem " + f"{namespaces_info.subsystem_nqn} which is different than the " + f"requested subsystem {args.subsystem}") + return errno.ENODEV namespaces_list = [] for ns in namespaces_info.namespaces: + if args.subsystem == ALL_SUBSYSTEMS: + if not ns.subsystem_nqn: + err_func(f"Got namespace with ID {ns.nsid} on an unknown subsystem") + subsys_nqn = "" + else: + subsys_nqn = ns.subsystem_nqn + else: + if ns.subsystem_nqn and ns.subsystem_nqn != args.subsystem: + err_func(f"Got a namespace with ID {ns.nsid} in subsystem " + f"{ns.subsystem_nqn} which is different than the " + f"requested one {args.subsystem}") + return errno.ENODEV + subsys_nqn = namespaces_info.subsystem_nqn + if args.nsid and args.nsid != ns.nsid: err_func(f"Failure listing namespace {args.nsid}: " f"Got namespace {ns.nsid} instead") @@ -1937,7 +1961,8 @@ def ns_list(self, args): else: visibility = "Restrictive" - namespaces_list.append([ns.nsid, + namespaces_list.append([subsys_nqn, + ns.nsid, break_string(ns.bdev_name, "-", 2), f"{ns.rbd_pool_name}/{ns.rbd_image_name}", self.format_size(ns.rbd_image_size), @@ -1956,7 +1981,8 @@ def ns_list(self, args): else: table_format = "plain" namespaces_out = tabulate(namespaces_list, - headers=["NSID", + headers=["NQN", + "NSID", "Bdev\nName", "RBD\nImage", "Image\nSize", @@ -1975,15 +2001,27 @@ def ns_list(self, args): prefix = f"Namespace with UUID {args.uuid} in" else: prefix = "Namespaces in" - out_func(f"{prefix} subsystem {args.subsystem}:\n{namespaces_out}") + if args.subsystem == ALL_SUBSYSTEMS: + out_func(f"{prefix} all subsystems:\n{namespaces_out}") + else: + out_func(f"{prefix} subsystem {args.subsystem}:\n{namespaces_out}") else: if args.nsid: - out_func(f"No namespace {args.nsid} in subsystem {args.subsystem}") + if args.subsystem == ALL_SUBSYSTEMS: + out_func(f"No namespace {args.nsid} in any subsystem") + else: + out_func(f"No namespace {args.nsid} in subsystem {args.subsystem}") elif args.uuid: - out_func(f"No namespace with UUID {args.uuid} in subsystem " - f"{args.subsystem}") + if args.subsystem == ALL_SUBSYSTEMS: + out_func(f"No namespace with UUID {args.uuid} in any subsystem") + else: + out_func(f"No namespace with UUID {args.uuid} in subsystem " + f"{args.subsystem}") else: - out_func(f"No namespaces in subsystem {args.subsystem}") + if args.subsystem == ALL_SUBSYSTEMS: + out_func("No namespaces in any subsystem") + else: + out_func(f"No namespaces in subsystem {args.subsystem}") else: err_func(f"{namespaces_info.error_message}") elif args.format == "json" or args.format == "yaml": @@ -2433,7 +2471,11 @@ def ns_change_visibility(self, args): help="Size in bytes or specified unit (K, KB, M, MB, G, GB, T, TB, P, PB)", required=True), ] - ns_list_args_list = ns_common_args + [ + ns_list_args_list = [ + argument("--subsystem", + "-n", + help="Subsystem NQN", + required=False), argument("--nsid", help="Namespace ID", type=int), diff --git a/control/grpc.py b/control/grpc.py index 62341c82..47ea4874 100644 --- a/control/grpc.py +++ b/control/grpc.py @@ -2290,14 +2290,14 @@ def list_namespaces(self, request, context=None): f"context: {context}{peer_msg}") if not request.subsystem: - errmsg = "Failure listing namespaces, missing subsystem NQN" - self.logger.error(errmsg) - return pb2.namespaces_info(status=errno.EINVAL, error_message=errmsg, - subsystem_nqn=request.subsystem, namespaces=[]) + request.subsystem = "*" with self.rpc_lock: try: - ret = rpc_nvmf.nvmf_get_subsystems(self.spdk_rpc_client, nqn=request.subsystem) + if request.subsystem == "*": + ret = rpc_nvmf.nvmf_get_subsystems(self.spdk_rpc_client) + else: + ret = rpc_nvmf.nvmf_get_subsystems(self.spdk_rpc_client, nqn=request.subsystem) self.logger.debug(f"list_namespaces: {ret}") except Exception as ex: errmsg = "Failure listing namespaces" @@ -2314,8 +2314,9 @@ def list_namespaces(self, request, context=None): namespaces = [] for s in ret: try: - if s["nqn"] != request.subsystem: - self.logger.warning(f'Got subsystem {s["nqn"]} instead of ' + subsys_nqn = s["nqn"] + if request.subsystem != "*" and subsys_nqn != request.subsystem: + self.logger.warning(f'Got subsystem {subsys_nqn} instead of ' f'{request.subsystem}, ignore') continue try: @@ -2324,7 +2325,7 @@ def list_namespaces(self, request, context=None): ns_list = [] pass if not ns_list: - self.subsystem_nsid_bdev_and_uuid.remove_namespace(request.subsystem) + self.subsystem_nsid_bdev_and_uuid.remove_namespace(subsys_nqn) for n in ns_list: nsid = n["nsid"] bdev_name = n["bdev_name"] @@ -2341,18 +2342,19 @@ def list_namespaces(self, request, context=None): lb_group = n["anagrpid"] except KeyError: pass - find_ret = self.subsystem_nsid_bdev_and_uuid.find_namespace(request.subsystem, + find_ret = self.subsystem_nsid_bdev_and_uuid.find_namespace(subsys_nqn, nsid) if find_ret.empty(): self.logger.warning(f"Can't find info of namesapce {nsid} in " - f"{request.subsystem}. Visibility status " + f"{subsys_nqn}. Visibility status " f"will be inaccurate") one_ns = pb2.namespace_cli(nsid=nsid, bdev_name=bdev_name, uuid=n["uuid"], load_balancing_group=lb_group, auto_visible=find_ret.auto_visible, - hosts=find_ret.host_list) + hosts=find_ret.host_list, + subsystem_nqn=subsys_nqn) with self.rpc_lock: ns_bdev = self.get_bdev_info(bdev_name) if ns_bdev is None: @@ -2379,7 +2381,8 @@ def list_namespaces(self, request, context=None): self.logger.exception(f"{ns_bdev=} parse error") pass namespaces.append(one_ns) - break + if request.subsystem != "*": + break except Exception: self.logger.exception(f"{s=} parse error") pass diff --git a/control/proto/gateway.proto b/control/proto/gateway.proto index 521897fc..0c56db59 100644 --- a/control/proto/gateway.proto +++ b/control/proto/gateway.proto @@ -523,6 +523,7 @@ message namespace_cli { uint64 w_mbytes_per_second = 12; bool auto_visible = 13; repeated string hosts = 14; + optional string subsystem_nqn = 15; } message namespaces_info { diff --git a/tests/test_cli.py b/tests/test_cli.py index f625f91f..b9975adf 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -24,6 +24,10 @@ image12 = "mytestdevimage12" image13 = "mytestdevimage13" image14 = "mytestdevimage14" +image15 = "mytestdevimage15" +image16 = "mytestdevimage16" +image17 = "mytestdevimage17" +image18 = "mytestdevimage18" pool = "rbd" subsystem = "nqn.2016-06.io.spdk:cnode1" subsystem2 = "nqn.2016-06.io.spdk:cnode2" @@ -33,6 +37,7 @@ subsystem6 = "nqn.2016-06.io.spdk:cnode6" subsystem7 = "nqn.2016-06.io.spdk:cnode7" subsystem8 = "nqn.2016-06.io.spdk:cnode8" +subsystem9 = "nqn.2016-06.io.spdk:cnode9" subsystemX = "nqn.2016-06.io.spdk:cnodeX" discovery_nqn = "nqn.2014-08.org.nvmexpress.discovery" serial = "Ceph00000000000001" @@ -80,7 +85,7 @@ def gateway(config): config.config["gateway"]["group"] = group_name config.config["gateway"]["max_namespaces_with_netmask"] = "3" config.config["gateway"]["max_hosts_per_namespace"] = "3" - config.config["gateway"]["max_subsystems"] = "3" + config.config["gateway"]["max_subsystems"] = "4" config.config["gateway"]["max_namespaces"] = "12" config.config["gateway"]["max_namespaces_per_subsystem"] = "11" config.config["gateway"]["max_hosts_per_subsystem"] = "4" @@ -170,7 +175,7 @@ def test_get_gateway_info(self, caplog, gateway): assert gw_info.spdk_version == spdk_ver assert gw_info.name == gw.gateway_name assert gw_info.hostname == gw.host_name - assert gw_info.max_subsystems == 3 + assert gw_info.max_subsystems == 4 assert gw_info.max_namespaces == 12 assert gw_info.max_namespaces_per_subsystem == 11 assert gw_info.max_hosts_per_subsystem == 4 @@ -1238,6 +1243,78 @@ def test_create_listener_on_discovery(self, caplog, listener, gateway): cli(["listener", "add", "--host-name", host_name] + listener) assert "Can't create a listener for a discovery subsystem" in caplog.text + def test_list_namespaces_all_subsystems(self, caplog): + caplog.clear() + cli(["subsystem", "add", "--subsystem", subsystem9, "--no-group-append"]) + assert f"Adding subsystem {subsystem9}: Successful" in caplog.text + caplog.clear() + cli(["namespace", "del", "--subsystem", subsystem, "--nsid", "10"]) + assert f"Deleting namespace 10 from {subsystem}: Successful" in caplog.text + caplog.clear() + cli(["namespace", "del", "--subsystem", subsystem, "--nsid", "11"]) + assert f"Deleting namespace 11 from {subsystem}: Successful" in caplog.text + caplog.clear() + cli(["namespace", "del", "--subsystem", subsystem, "--nsid", "12"]) + assert f"Deleting namespace 12 from {subsystem}: Successful" in caplog.text + caplog.clear() + cli(["namespace", "add", "--subsystem", subsystem9, "--rbd-pool", pool, + "--rbd-image", image15, "--size", "16MB", "--rbd-create-image"]) + assert f"Adding namespace 1 to {subsystem9}: Successful" in caplog.text + caplog.clear() + cli(["namespace", "add", "--subsystem", subsystem9, "--rbd-pool", pool, + "--rbd-image", image16, "--size", "16MB", "--rbd-create-image"]) + assert f"Adding namespace 2 to {subsystem9}: Successful" in caplog.text + caplog.clear() + cli(["--format", "json", "namespace", "list"]) + assert '"subsystem_nqn": "*"' in caplog.text + assert f'"subsystem_nqn": "{subsystem}"' in caplog.text + assert f'"subsystem_nqn": "{subsystem9}"' in caplog.text + assert '"nsid": 1' in caplog.text + assert '"nsid": 2' in caplog.text + assert '"nsid": 6' in caplog.text + caplog.clear() + cli(["--format", "json", "namespace", "list", "--nsid", "1"]) + assert '"subsystem_nqn": "*"' in caplog.text + assert f'"subsystem_nqn": "{subsystem}"' in caplog.text + assert f'"subsystem_nqn": "{subsystem9}"' in caplog.text + assert '"nsid": 1' in caplog.text + assert '"nsid": 2' not in caplog.text + caplog.clear() + cli(["--format", "json", "namespace", "list", "--nsid", "6"]) + assert '"subsystem_nqn": "*"' in caplog.text + assert f'"subsystem_nqn": "{subsystem}"' in caplog.text + assert f'"subsystem_nqn": "{subsystem9}"' not in caplog.text + assert '"nsid": 6' in caplog.text + assert '"nsid": 1' not in caplog.text + assert '"nsid": 2' not in caplog.text + caplog.clear() + cli(["--format", "json", "namespace", "list", "--uuid", uuid]) + assert '"subsystem_nqn": "*"' in caplog.text + assert f'"subsystem_nqn": "{subsystem}"' in caplog.text + assert f'"subsystem_nqn": "{subsystem9}"' not in caplog.text + assert f'"uuid": "{uuid}"' in caplog.text + + def test_namespace_count_updated(self, caplog): + caplog.clear() + cli(["subsystem", "add", "--subsystem", subsystem5, "--no-group-append"]) + assert f"Adding subsystem {subsystem5}: Successful" in caplog.text + caplog.clear() + cli(["namespace", "add", "--subsystem", subsystem5, "--rbd-pool", pool, + "--rbd-image", image17, "--size", "16MB", "--rbd-create-image"]) + assert f"Adding namespace 1 to {subsystem5}: Successful" in caplog.text + caplog.clear() + cli(["namespace", "add", "--subsystem", subsystem5, "--rbd-pool", pool, + "--rbd-image", image18, "--size", "16MB", "--rbd-create-image"]) + assert f"Failure adding namespace to {subsystem5}: Maximal number of namespaces (12) " \ + f"has already been reached" in caplog.text + caplog.clear() + cli(["subsystem", "del", "--subsystem", subsystem9, "--force"]) + assert f"Deleting subsystem {subsystem9}: Successful" in caplog.text + caplog.clear() + cli(["namespace", "add", "--subsystem", subsystem5, "--rbd-pool", pool, + "--rbd-image", image18, "--size", "16MB", "--rbd-create-image"]) + assert f"Adding namespace 2 to {subsystem5}: Successful" in caplog.text + class TestDelete: @pytest.mark.parametrize("host", host_list) @@ -1392,6 +1469,9 @@ def test_delete_subsystem(self, caplog, gateway): cli(["subsystem", "del", "--subsystem", subsystem2]) assert f"Deleting subsystem {subsystem2}: Successful" in caplog.text caplog.clear() + cli(["subsystem", "del", "--subsystem", subsystem5, "--force"]) + assert f"Deleting subsystem {subsystem5}: Successful" in caplog.text + caplog.clear() cli(["subsystem", "list"]) assert "No subsystems" in caplog.text @@ -1493,8 +1573,14 @@ def test_add_too_many_subsystem(self, caplog, gateway): assert f"Adding subsystem {subsystem6}: Successful" in caplog.text caplog.clear() cli(["subsystem", "add", "--subsystem", subsystem7, "--no-group-append"]) - assert f"Failure creating subsystem {subsystem7}: Maximal number of subsystems (3) has " \ + assert f"Adding subsystem {subsystem7}: Successful" in caplog.text + caplog.clear() + cli(["subsystem", "add", "--subsystem", subsystem8, "--no-group-append"]) + assert f"Failure creating subsystem {subsystem8}: Maximal number of subsystems (4) has " \ f"already been reached" in caplog.text + caplog.clear() + cli(["subsystem", "del", "--subsystem", subsystem7]) + assert f"Deleting subsystem {subsystem7}: Successful" in caplog.text def test_too_many_hosts(self, caplog, gateway): caplog.clear()