Skip to content

Commit

Permalink
Change --host argument to --host-nqn in host add and del commands.
Browse files Browse the repository at this point in the history
Fixes #735

Signed-off-by: Gil Bregman <gbregman@il.ibm.com>
  • Loading branch information
gbregman committed Aug 21, 2024
1 parent e3bf616 commit 33e844f
Show file tree
Hide file tree
Showing 9 changed files with 39 additions and 43 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build-container.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
```

Expand Down Expand Up @@ -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:
Expand Down
30 changes: 13 additions & 17 deletions control/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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":
Expand All @@ -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":
Expand Down Expand Up @@ -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 + [
]
Expand Down
2 changes: 1 addition & 1 deletion mk/demo.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions mk/demosecure.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion tests/ha/setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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

2 changes: 1 addition & 1 deletion tests/ha/setup_mtls.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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

16 changes: 8 additions & 8 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -636,28 +636,28 @@ 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:
assert f"Adding host {host} to {subsystem}: Successful" in caplog.text

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

Expand Down Expand Up @@ -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:
Expand Down
20 changes: 10 additions & 10 deletions tests/test_psk.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -82,44 +82,44 @@ 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):
caplog.clear()
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):
caplog.clear()
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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 33e844f

Please sign in to comment.