From 33e844ff5e378399ba3f1675b11b867a9650dec2 Mon Sep 17 00:00:00 2001 From: Gil Bregman Date: Wed, 21 Aug 2024 09:59:50 +0300 Subject: [PATCH] Change --host argument to --host-nqn in host add and del commands. Fixes #735 Signed-off-by: Gil Bregman --- .github/workflows/build-container.yml | 2 +- README.md | 4 ++-- control/cli.py | 30 ++++++++++++--------------- mk/demo.mk | 2 +- mk/demosecure.mk | 4 ++-- tests/ha/setup.sh | 2 +- tests/ha/setup_mtls.sh | 2 +- tests/test_cli.py | 16 +++++++------- tests/test_psk.py | 20 +++++++++--------- 9 files changed, 39 insertions(+), 43 deletions(-) diff --git a/.github/workflows/build-container.yml b/.github/workflows/build-container.yml index 009f6182..bc71921f 100644 --- a/.github/workflows/build-container.yml +++ b/.github/workflows/build-container.yml @@ -639,7 +639,7 @@ jobs: echo ℹ️ Create listener address $ip gateway $name cli_gw $ip listener add --subsystem $NQN --host-name $name --traddr $ip --trsvcid $NVMEOF_IO_PORT done - cli_gw $gw1 host add --subsystem $NQN --host "*" + cli_gw $gw1 host add --subsystem $NQN --host-nqn "*" for gw in $GW1 $GW2; do ip=$(container_ip $gw) echo ℹ️ Subsystems for name $gw ip $ip diff --git a/README.md b/README.md index a02d559b..dd58eec5 100644 --- a/README.md +++ b/README.md @@ -102,7 +102,7 @@ docker-compose run --rm nvmeof-cli --server-address 192.168.13.3 --server-port Adding listener 192.168.13.3:4420 to nqn.2016-06.io.spdk:cnode1: Successful docker-compose run --rm nvmeof-cli --server-address 2001:db8::3 --server-port 5500 listener add --subsystem "nqn.2016-06.io.spdk:cnode1" --host-name fbca1a3d3ed8 --traddr 2001:db8::3 --trsvcid 4420 --adrfam IPV6 Adding listener [2001:db8::3]:4420 to nqn.2016-06.io.spdk:cnode1: Successful -docker-compose run --rm nvmeof-cli --server-address 192.168.13.3 --server-port 5500 host add --subsystem "nqn.2016-06.io.spdk:cnode1" --host "*" +docker-compose run --rm nvmeof-cli --server-address 192.168.13.3 --server-port 5500 host add --subsystem "nqn.2016-06.io.spdk:cnode1" --host-nqn "*" Allowing any host for nqn.2016-06.io.spdk:cnode1: Successful ``` @@ -143,7 +143,7 @@ The same configuration can also be manually run: 1. Define which hosts can connect: ```bash - cephnvmf host add --subsystem nqn.2016-06.io.spdk:cnode1 --host "*" + cephnvmf host add --subsystem nqn.2016-06.io.spdk:cnode1 --host-nqn "*" ``` These can also be run by setting environment variables `CEPH_NVMEOF_SERVER_ADDRESS` and `CEPH_NVMEOF_SERVER_PORT` before running nvmeof-cli commands, example: diff --git a/control/cli.py b/control/cli.py index 5264554a..e0257073 100644 --- a/control/cli.py +++ b/control/cli.py @@ -1034,27 +1034,25 @@ def host_add(self, args): """Add a host to a subsystem.""" out_func, err_func = self.get_output_functions(args) - if not args.host: - self.cli.parser.error("--host argument is mandatory for add command") - if args.host == "*" and args.psk: + if args.host_nqn == "*" and args.psk: self.cli.parser.error("PSK is only allowed for specific hosts") - req = pb2.add_host_req(subsystem_nqn=args.subsystem, host_nqn=args.host, psk=args.psk) + req = pb2.add_host_req(subsystem_nqn=args.subsystem, host_nqn=args.host_nqn, psk=args.psk) try: ret = self.stub.add_host(req) except Exception as ex: - if args.host == "*": + if args.host_nqn == "*": errmsg = f"Failure allowing open host access to {args.subsystem}" else: - errmsg = f"Failure adding host {args.host} to {args.subsystem}" + errmsg = f"Failure adding host {args.host_nqn} to {args.subsystem}" ret = pb2.req_status(status = errno.EINVAL, error_message = f"{errmsg}:\n{ex}") if args.format == "text" or args.format == "plain": if ret.status == 0: - if args.host == "*": + if args.host_nqn == "*": out_func(f"Allowing open host access to {args.subsystem}: Successful") else: - out_func(f"Adding host {args.host} to {args.subsystem}: Successful") + out_func(f"Adding host {args.host_nqn} to {args.subsystem}: Successful") else: err_func(f"{ret.error_message}") elif args.format == "json" or args.format == "yaml": @@ -1079,25 +1077,23 @@ def host_del(self, args): """Delete a host from a subsystem.""" out_func, err_func = self.get_output_functions(args) - if not args.host: - self.cli.parser.error("--host argument is mandatory for del command") - req = pb2.remove_host_req(subsystem_nqn=args.subsystem, host_nqn=args.host) + req = pb2.remove_host_req(subsystem_nqn=args.subsystem, host_nqn=args.host_nqn) try: ret = self.stub.remove_host(req) except Exception as ex: - if args.host == "*": + if args.host_nqn == "*": errmsg = f"Failure disabling open host access to {args.subsystem}" else: - errmsg = f"Failure removing host {args.host} access to {args.subsystem}" + errmsg = f"Failure removing host {args.host_nqn} access to {args.subsystem}" ret = pb2.req_status(status = errno.EINVAL, error_message = f"{errmsg}:\n{ex}") if args.format == "text" or args.format == "plain": if ret.status == 0: - if args.host == "*": + if args.host_nqn == "*": out_func(f"Disabling open host access to {args.subsystem}: Successful") else: - out_func(f"Removing host {args.host} access from {args.subsystem}: Successful") + out_func(f"Removing host {args.host_nqn} access from {args.subsystem}: Successful") else: err_func(f"{ret.error_message}") elif args.format == "json" or args.format == "yaml": @@ -1172,11 +1168,11 @@ def host_list(self, args): argument("--subsystem", "-n", help="Subsystem NQN", required=True), ] host_add_args = host_common_args + [ - argument("--host", "-t", help="Host NQN", required=True), + argument("--host-nqn", "-t", help="Host NQN", required=True), argument("--psk", help="Host's PSK key", required=False), ] host_del_args = host_common_args + [ - argument("--host", "-t", help="Host NQN", required=True), + argument("--host-nqn", "-t", help="Host NQN", required=True), ] host_list_args = host_common_args + [ ] diff --git a/mk/demo.mk b/mk/demo.mk index 77b4acb8..7cf3aba9 100644 --- a/mk/demo.mk +++ b/mk/demo.mk @@ -6,6 +6,6 @@ demo: $(NVMEOF_CLI) namespace add --subsystem $(NQN) --rbd-pool $(RBD_POOL) --rbd-image $(RBD_IMAGE_NAME) --size $(RBD_IMAGE_SIZE) --rbd-create-image $(NVMEOF_CLI) listener add --subsystem $(NQN) --host-name `docker ps -q -f name=$(NVMEOF_CONTAINER_NAME)` --traddr $(NVMEOF_IP_ADDRESS) --trsvcid $(NVMEOF_IO_PORT) $(NVMEOF_CLI_IPV6) listener add --subsystem $(NQN) --host-name `docker ps -q -f name=$(NVMEOF_CONTAINER_NAME)` --traddr $(NVMEOF_IPV6_ADDRESS) --trsvcid $(NVMEOF_IO_PORT) --adrfam IPV6 - $(NVMEOF_CLI) host add --subsystem $(NQN) --host "*" + $(NVMEOF_CLI) host add --subsystem $(NQN) --host-nqn "*" .PHONY: demo diff --git a/mk/demosecure.mk b/mk/demosecure.mk index ab8809c7..96588bd0 100644 --- a/mk/demosecure.mk +++ b/mk/demosecure.mk @@ -9,7 +9,7 @@ demosecure: $(NVMEOF_CLI) namespace add --subsystem $(NQN) --rbd-pool $(RBD_POOL) --rbd-image $(RBD_IMAGE_NAME) --size $(RBD_IMAGE_SIZE) --rbd-create-image $(NVMEOF_CLI) listener add --subsystem $(NQN) --host-name `docker ps -q -f name=$(NVMEOF_CONTAINER_NAME)` --traddr $(NVMEOF_IP_ADDRESS) --trsvcid $(NVMEOF_IO_PORT) --secure $(NVMEOF_CLI) listener add --subsystem $(NQN) --host-name `docker ps -q -f name=$(NVMEOF_CONTAINER_NAME)` --traddr $(NVMEOF_IP_ADDRESS) --trsvcid $(NVMEOF_IO_PORT2) - $(NVMEOF_CLI) host add --subsystem $(NQN) --host "$(HOSTNQN)" --psk "NVMeTLSkey-1:01:YzrPElk4OYy1uUERriPwiiyEJE/+J5ckYpLB+5NHMsR2iBuT:" - $(NVMEOF_CLI) host add --subsystem $(NQN) --host "$(HOSTNQN2)" + $(NVMEOF_CLI) host add --subsystem $(NQN) --host-nqn "$(HOSTNQN)" --psk "NVMeTLSkey-1:01:YzrPElk4OYy1uUERriPwiiyEJE/+J5ckYpLB+5NHMsR2iBuT:" + $(NVMEOF_CLI) host add --subsystem $(NQN) --host-nqn "$(HOSTNQN2)" .PHONY: demosecure diff --git a/tests/ha/setup.sh b/tests/ha/setup.sh index 6c409ab5..930b9217 100755 --- a/tests/ha/setup.sh +++ b/tests/ha/setup.sh @@ -11,7 +11,7 @@ docker-compose run --rm nvmeof-cli --server-address $GW1_IP --server-port 5500 docker-compose run --rm nvmeof-cli --server-address $GW1_IP --server-port 5500 namespace add --subsystem $NQN --rbd-pool rbd --rbd-image demo_image2 --size 10M --rbd-create-image -l 2 docker-compose run --rm nvmeof-cli --server-address $GW1_IP --server-port 5500 listener add --subsystem $NQN --host-name $GW1_NAME --traddr $GW1_IP --trsvcid 4420 docker-compose run --rm nvmeof-cli --server-address $GW2_IP --server-port 5500 listener add --subsystem $NQN --host-name $GW2_NAME --traddr $GW2_IP --trsvcid 4420 -docker-compose run --rm nvmeof-cli --server-address $GW1_IP --server-port 5500 host add --subsystem $NQN --host "*" +docker-compose run --rm nvmeof-cli --server-address $GW1_IP --server-port 5500 host add --subsystem $NQN --host-nqn "*" docker-compose run --rm nvmeof-cli --server-address $GW1_IP --server-port 5500 get_subsystems docker-compose run --rm nvmeof-cli --server-address $GW2_IP --server-port 5500 get_subsystems diff --git a/tests/ha/setup_mtls.sh b/tests/ha/setup_mtls.sh index acbb8356..15c10999 100755 --- a/tests/ha/setup_mtls.sh +++ b/tests/ha/setup_mtls.sh @@ -8,6 +8,6 @@ docker-compose run --rm nvmeof-cli --server-address $GW1_IP --server-port 5500 docker-compose run --rm nvmeof-cli --server-address $GW1_IP --server-port 5500 --server-cert /etc/ceph/server.crt --client-key /etc/ceph/client.key --client-cert /etc/ceph/client.crt namespace add --subsystem $NQN --rbd-pool rbd --rbd-image demo_image1 --size 10M --rbd-create-image -l 1 #docker-compose run --rm nvmeof-cli --server-address $GW1_IP --server-port 5500 --server-cert /etc/ceph/server.crt --client-key /etc/ceph/client.key --client-cert /etc/ceph/client.crt namespace add --subsystem $NQN --rbd-pool rbd --rbd-image demo_image2 --size 10M --rbd-create-image -l 2 docker-compose run --rm nvmeof-cli --server-address $GW1_IP --server-port 5500 --server-cert /etc/ceph/server.crt --client-key /etc/ceph/client.key --client-cert /etc/ceph/client.crt listener add --subsystem $NQN --host-name $GW1_NAME --traddr $GW1_IP --trsvcid 4420 -docker-compose run --rm nvmeof-cli --server-address $GW1_IP --server-port 5500 --server-cert /etc/ceph/server.crt --client-key /etc/ceph/client.key --client-cert /etc/ceph/client.crt host add --subsystem $NQN --host "*" +docker-compose run --rm nvmeof-cli --server-address $GW1_IP --server-port 5500 --server-cert /etc/ceph/server.crt --client-key /etc/ceph/client.key --client-cert /etc/ceph/client.crt host add --subsystem $NQN --host-nqn "*" docker-compose run --rm nvmeof-cli --server-address $GW1_IP --server-port 5500 --server-cert /etc/ceph/server.crt --client-key /etc/ceph/client.key --client-cert /etc/ceph/client.crt get_subsystems diff --git a/tests/test_cli.py b/tests/test_cli.py index 75cc1851..ba8ef591 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -636,10 +636,10 @@ def test_add_host(self, caplog, host): except SystemExit as sysex: rc = int(str(sysex)) pass - assert "error: the following arguments are required: --host/-t" in caplog.text + assert "error: the following arguments are required: --host-nqn/-t" in caplog.text assert rc == 2 caplog.clear() - cli(["host", "add", "--subsystem", subsystem, "--host", host]) + cli(["host", "add", "--subsystem", subsystem, "--host-nqn", host]) if host == "*": assert f"Allowing open host access to {subsystem}: Successful" in caplog.text else: @@ -647,17 +647,17 @@ def test_add_host(self, caplog, host): def test_add_host_invalid_nqn(self, caplog): caplog.clear() - cli(["host", "add", "--subsystem", subsystem, "--host", "nqn.2016"]) + cli(["host", "add", "--subsystem", subsystem, "--host-nqn", "nqn.2016"]) assert f'NQN "nqn.2016" is too short, minimal length is 11' in caplog.text caplog.clear() - cli(["host", "add", "--subsystem", subsystem, "--host", "nqn.2X16-06.io.spdk:host1"]) + cli(["host", "add", "--subsystem", subsystem, "--host-nqn", "nqn.2X16-06.io.spdk:host1"]) assert f"invalid date code" in caplog.text caplog.clear() - cli(["host", "add", "--subsystem", subsystem, "--host", "nqn.2016-06.io.spdk:host1_X"]) + cli(["host", "add", "--subsystem", subsystem, "--host-nqn", "nqn.2016-06.io.spdk:host1_X"]) assert f"Invalid host NQN" in caplog.text assert f"contains invalid characters" in caplog.text caplog.clear() - cli(["host", "add", "--subsystem", f"{subsystem}_X", "--host", "nqn.2016-06.io.spdk:host2"]) + cli(["host", "add", "--subsystem", f"{subsystem}_X", "--host-nqn", "nqn.2016-06.io.spdk:host2"]) assert f"Invalid subsystem NQN" in caplog.text assert f"contains invalid characters" in caplog.text @@ -754,10 +754,10 @@ def test_remove_host(self, caplog, host, gateway): except SystemExit as sysex: rc = int(str(sysex)) pass - assert "error: the following arguments are required: --host/-t" in caplog.text + assert "error: the following arguments are required: --host-nqn/-t" in caplog.text assert rc == 2 caplog.clear() - cli(["host", "del", "--subsystem", subsystem, "--host", host]) + cli(["host", "del", "--subsystem", subsystem, "--host-nqn", host]) if host == "*": assert f"Disabling open host access to {subsystem}: Successful" in caplog.text else: diff --git a/tests/test_psk.py b/tests/test_psk.py index 2af89844..92d93f8d 100644 --- a/tests/test_psk.py +++ b/tests/test_psk.py @@ -72,7 +72,7 @@ def test_setup(caplog, gateway): def test_allow_any_host(caplog, gateway): caplog.clear() - cli(["host", "add", "--subsystem", subsystem, "--host", "*"]) + cli(["host", "add", "--subsystem", subsystem, "--host-nqn", "*"]) assert f"Allowing open host access to {subsystem}: Successful" in caplog.text def test_create_secure_with_any_host(caplog, gateway): @@ -82,7 +82,7 @@ def test_create_secure_with_any_host(caplog, gateway): def test_remove_any_host_access(caplog, gateway): caplog.clear() - cli(["host", "del", "--subsystem", subsystem, "--host", "*"]) + cli(["host", "del", "--subsystem", subsystem, "--host-nqn", "*"]) assert f"Disabling open host access to {subsystem}: Successful" in caplog.text def test_create_secure(caplog, gateway): @@ -90,13 +90,13 @@ def test_create_secure(caplog, gateway): cli(["listener", "add", "--subsystem", subsystem, "--host-name", host_name, "-a", addr, "-s", "5001", "--secure"]) assert f"Adding {subsystem} listener at {addr}:5001: Successful" in caplog.text caplog.clear() - cli(["host", "add", "--subsystem", subsystem, "--host", hostnqn, "--psk", hostpsk]) + cli(["host", "add", "--subsystem", subsystem, "--host-nqn", hostnqn, "--psk", hostpsk]) assert f"Adding host {hostnqn} to {subsystem}: Successful" in caplog.text caplog.clear() - cli(["host", "add", "--subsystem", subsystem, "--host", hostnqn2, "--psk", hostpsk2]) + cli(["host", "add", "--subsystem", subsystem, "--host-nqn", hostnqn2, "--psk", hostpsk2]) assert f"Adding host {hostnqn2} to {subsystem}: Successful" in caplog.text caplog.clear() - cli(["host", "add", "--subsystem", subsystem, "--host", hostnqn4, "--psk", hostpsk4]) + cli(["host", "add", "--subsystem", subsystem, "--host-nqn", hostnqn4, "--psk", hostpsk4]) assert f"Adding host {hostnqn4} to {subsystem}: Successful" in caplog.text def test_create_not_secure(caplog, gateway): @@ -104,22 +104,22 @@ def test_create_not_secure(caplog, gateway): cli(["listener", "add", "--subsystem", subsystem, "--host-name", host_name, "-a", addr, "-s", "5002"]) assert f"Adding {subsystem} listener at {addr}:5002: Successful" in caplog.text caplog.clear() - cli(["host", "add", "--subsystem", subsystem, "--host", hostnqn6]) + cli(["host", "add", "--subsystem", subsystem, "--host-nqn", hostnqn6]) assert f"Adding host {hostnqn6} to {subsystem}: Successful" in caplog.text caplog.clear() - cli(["host", "add", "--subsystem", subsystem, "--host", hostnqn7]) + cli(["host", "add", "--subsystem", subsystem, "--host-nqn", hostnqn7]) assert f"Adding host {hostnqn7} to {subsystem}: Successful" in caplog.text def test_create_secure_junk_key(caplog, gateway): caplog.clear() - cli(["host", "add", "--subsystem", subsystem, "--host", hostnqn3, "--psk", hostpsk3]) + cli(["host", "add", "--subsystem", subsystem, "--host-nqn", hostnqn3, "--psk", hostpsk3]) assert f"Failure adding host {hostnqn3} to {subsystem}" in caplog.text def test_create_secure_no_key(caplog, gateway): caplog.clear() rc = 0 try: - cli(["host", "add", "--subsystem", subsystem, "--host", hostnqn5, "--psk"]) + cli(["host", "add", "--subsystem", subsystem, "--host-nqn", hostnqn5, "--psk"]) except SystemExit as sysex: rc = int(str(sysex)) pass @@ -157,7 +157,7 @@ def test_allow_any_host_with_psk(caplog, gateway): caplog.clear() rc = 0 try: - cli(["host", "add", "--subsystem", subsystem, "--host", "*", "--psk", hostpsk]) + cli(["host", "add", "--subsystem", subsystem, "--host-nqn", "*", "--psk", hostpsk]) except SystemExit as sysex: rc = int(str(sysex)) pass