Skip to content

Commit

Permalink
Do not display an exception in case we delete a non-existing namespac…
Browse files Browse the repository at this point in the history 
…e host.

Fixes #964

Signed-off-by: Gil Bregman <[email protected]>
  • Loading branch information
@gbregman
gbregman committed Dec 22, 2024
1 parent 88f10c3 commit 23a49b2
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 30 deletions.
61 changes: 36 additions & 25 deletions control/grpc.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2160,58 +2160,62 @@ def namespace_add_host_safe(self, request, context):
self.logger.info(f"Received request to add host {request.host_nqn} to namespace {request.nsid} on {request.subsystem_nqn}, context: {context}{peer_msg}")

if not request.nsid:
errmsg = f"Failure adding host to namespace, missing NSID"
errmsg = f"Failure adding host to namespace: Missing NSID"
self.logger.error(f"{errmsg}")
return pb2.req_status(status = errno.EINVAL, error_message = errmsg)

if not request.subsystem_nqn:
errmsg = f"Failure adding host to namespace {request.nsid}, missing subsystem NQN"
errmsg = f"Failure adding host to namespace {request.nsid}: Missing subsystem NQN"
self.logger.error(f"{errmsg}")
return pb2.req_status(status = errno.EINVAL, error_message = errmsg)

if not request.host_nqn:
errmsg = f"Failure adding host to namespace {request.nsid} on {request.subsystem_nqn}, missing host NQN"
errmsg = f"Failure adding host to namespace {request.nsid} on {request.subsystem_nqn}: Missing host NQN"
self.logger.error(f"{errmsg}")
return pb2.req_status(status = errno.EINVAL, error_message = errmsg)

if request.host_nqn == "*":
errmsg = f"Failure adding host to namespace {request.nsid} on {request.subsystem_nqn}, host can't be \"*\""
errmsg = f"Failure adding host to namespace {request.nsid} on {request.subsystem_nqn}: Host NQN can't be \"*\""
self.logger.error(f"{errmsg}")
return pb2.req_status(status=errno.EINVAL, error_message=errmsg)

if self.verify_nqns:
rc = GatewayUtils.is_valid_nqn(request.subsystem_nqn)
if rc[0] != 0:
errmsg = f"Failure adding host {request.host_nqn} to namespace {request.nsid} on {request.subsystem_nqn}, invalid subsystem NQN: {rc[1]}"
errmsg = f"Failure adding host {request.host_nqn} to namespace {request.nsid} on {request.subsystem_nqn}: Invalid subsystem NQN: {rc[1]}"
self.logger.error(f"{errmsg}")
return pb2.req_status(status = rc[0], error_message = errmsg)
rc = GatewayUtils.is_valid_nqn(request.host_nqn)
if rc[0] != 0:
errmsg = f"Failure adding host {request.host_nqn} to namespace {request.nsid} on {request.subsystem_nqn}, invalid host NQN: {rc[1]}"
errmsg = f"Failure adding host {request.host_nqn} to namespace {request.nsid} on {request.subsystem_nqn}: Invalid host NQN: {rc[1]}"
self.logger.error(f"{errmsg}")
return pb2.req_status(status = rc[0], error_message = errmsg)

if GatewayUtils.is_discovery_nqn(request.subsystem_nqn):
errmsg = f"Failure adding host to namespace {request.nsid} on {request.subsystem_nqn}, subsystem NQN can't be a discovery NQN"
errmsg = f"Failure adding host to namespace {request.nsid} on {request.subsystem_nqn}: Subsystem NQN can't be a discovery NQN"
self.logger.error(f"{errmsg}")
return pb2.req_status(status=errno.EINVAL, error_message=errmsg)

if GatewayUtils.is_discovery_nqn(request.host_nqn):
errmsg = f"Failure adding host to namespace {request.nsid} on {request.subsystem_nqn}, host NQN can't be a discovery NQN"
errmsg = f"Failure adding host to namespace {request.nsid} on {request.subsystem_nqn}: Host NQN can't be a discovery NQN"
self.logger.error(f"{errmsg}")
return pb2.req_status(status=errno.EINVAL, error_message=errmsg)

find_ret = self.subsystem_nsid_bdev_and_uuid.find_namespace(request.subsystem_nqn, request.nsid)
if not find_ret.empty():
if not find_ret.no_auto_visible:
errmsg = f"Failure adding host {request.host_nqn} to namespace {request.nsid} on {request.subsystem_nqn}, namespace is visible to all hosts"
self.logger.error(f"{errmsg}")
return pb2.req_status(status=errno.EINVAL, error_message=errmsg)
if find_ret.empty():
errmsg = f"Failure adding host {request.host_nqn} to namespace {request.nsid} on {request.subsystem_nqn}: Can't find namespace"
self.logger.error(errmsg)
return pb2.namespace_io_stats_info(status=errno.ENODEV, error_message=errmsg)

if find_ret.host_count() >= self.max_hosts_per_namespace:
errmsg = f"Failure adding host {request.host_nqn} to namespace {request.nsid} on {request.subsystem_nqn}, maximal host count for namespace ({self.max_hosts_per_namespace}) was already reached"
self.logger.error(f"{errmsg}")
return pb2.req_status(status=errno.E2BIG, error_message=errmsg)
if not find_ret.no_auto_visible:
errmsg = f"Failure adding host {request.host_nqn} to namespace {request.nsid} on {request.subsystem_nqn}: Namespace is visible to all hosts"
self.logger.error(f"{errmsg}")
return pb2.req_status(status=errno.EINVAL, error_message=errmsg)

if find_ret.host_count() >= self.max_hosts_per_namespace:
errmsg = f"Failure adding host {request.host_nqn} to namespace {request.nsid} on {request.subsystem_nqn}, maximal host count for namespace ({self.max_hosts_per_namespace}) was already reached"
self.logger.error(f"{errmsg}")
return pb2.req_status(status=errno.E2BIG, error_message=errmsg)

omap_lock = self.omap_lock.get_omap_lock_to_use(context)
with omap_lock:
Expand Down Expand Up @@ -2262,45 +2266,50 @@ def namespace_delete_host_safe(self, request, context):
return pb2.req_status(status = errno.EINVAL, error_message = errmsg)

if not request.subsystem_nqn:
errmsg = f"Failure deleting host from namespace {request.nsid}, missing subsystem NQN"
errmsg = f"Failure deleting host from namespace {request.nsid}: Missing subsystem NQN"
self.logger.error(f"{errmsg}")
return pb2.req_status(status = errno.EINVAL, error_message = errmsg)

if not request.host_nqn:
errmsg = f"Failure deleting host from namespace {request.nsid} on {request.subsystem_nqn}, missing host NQN"
errmsg = f"Failure deleting host from namespace {request.nsid} on {request.subsystem_nqn}: Missing host NQN"
self.logger.error(f"{errmsg}")
return pb2.req_status(status = errno.EINVAL, error_message = errmsg)

if request.host_nqn == "*":
errmsg = f"Failure deleting host from namespace {request.nsid} on {request.subsystem_nqn}, host can't be \"*\""
errmsg = f"Failure deleting host from namespace {request.nsid} on {request.subsystem_nqn}: Host NQN can't be \"*\""
self.logger.error(f"{errmsg}")
return pb2.req_status(status=errno.EINVAL, error_message=errmsg)

if self.verify_nqns:
rc = GatewayUtils.is_valid_nqn(request.subsystem_nqn)
if rc[0] != 0:
errmsg = f"Failure deleting host {request.host_nqn} from namespace {request.nsid} on {request.subsystem_nqn}, invalid subsystem NQN: {rc[1]}"
errmsg = f"Failure deleting host {request.host_nqn} from namespace {request.nsid} on {request.subsystem_nqn}: Invalid subsystem NQN: {rc[1]}"
self.logger.error(f"{errmsg}")
return pb2.req_status(status = rc[0], error_message = errmsg)
rc = GatewayUtils.is_valid_nqn(request.host_nqn)
if rc[0] != 0:
errmsg = f"Failure deleting host {request.host_nqn} from namespace {request.nsid} on {request.subsystem_nqn}, invalid host NQN: {rc[1]}"
errmsg = f"Failure deleting host {request.host_nqn} from namespace {request.nsid} on {request.subsystem_nqn}: Invalid host NQN: {rc[1]}"
self.logger.error(f"{errmsg}")
return pb2.req_status(status = rc[0], error_message = errmsg)

if GatewayUtils.is_discovery_nqn(request.subsystem_nqn):
errmsg = f"Failure deleting host from namespace {request.nsid} on {request.subsystem_nqn}, subsystem NQN can't be a discovery NQN"
errmsg = f"Failure deleting host from namespace {request.nsid} on {request.subsystem_nqn}: Subsystem NQN can't be a discovery NQN"
self.logger.error(f"{errmsg}")
return pb2.req_status(status=errno.EINVAL, error_message=errmsg)

if GatewayUtils.is_discovery_nqn(request.host_nqn):
errmsg = f"Failure deleting host from namespace {request.nsid} on {request.subsystem_nqn}, host NQN can't be a discovery NQN"
errmsg = f"Failure deleting host from namespace {request.nsid} on {request.subsystem_nqn}: Host NQN can't be a discovery NQN"
self.logger.error(f"{errmsg}")
return pb2.req_status(status=errno.EINVAL, error_message=errmsg)

find_ret = self.subsystem_nsid_bdev_and_uuid.find_namespace(request.subsystem_nqn, request.nsid)
if find_ret.empty():
errmsg = f"Failure deleting host {request.host_nqn} from namespace {request.nsid} on {request.subsystem_nqn}: Can't find namespace"
self.logger.error(errmsg)
return pb2.namespace_io_stats_info(status=errno.ENODEV, error_message=errmsg)

if not find_ret.empty() and not find_ret.no_auto_visible:
errmsg = f"Failure deleting host from namespace {request.nsid} on {request.subsystem_nqn}, namespace is visible to all hosts"
errmsg = f"Failure deleting host from namespace {request.nsid} on {request.subsystem_nqn}: Namespace is visible to all hosts"
self.logger.error(f"{errmsg}")
return pb2.req_status(status=errno.EINVAL, error_message=errmsg)

Expand All @@ -2327,6 +2336,8 @@ def namespace_delete_host_safe(self, request, context):
# Update gateway state
try:
self.gateway_state.remove_namespace_host(request.subsystem_nqn, request.nsid, request.host_nqn)
except KeyError:
pass
except Exception as ex:
errmsg = f"Error persisting deletion of host {request.host_nqn} for namespace {request.nsid} on {request.subsystem_nqn}"
self.logger.exception(errmsg)
Expand Down
10 changes: 5 additions & 5 deletions tests/test_cli.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,7 @@ def test_add_too_many_hosts_to_namespace(self, caplog, gateway):
def test_add_all_hosts_to_namespace(self, caplog, gateway):
caplog.clear()
cli(["namespace", "add_host", "--subsystem", subsystem, "--nsid", "8", "--host-nqn", "*"])
assert f"Failure adding host to namespace 8 on {subsystem}, host can't be \"*\"" in caplog.text
assert f"Failure adding host to namespace 8 on {subsystem}: Host NQN can't be \"*\"" in caplog.text

def test_add_namespace_no_such_subsys(self, caplog, gateway):
caplog.clear()
Expand Down Expand Up @@ -529,22 +529,22 @@ def test_add_too_many_namespaces_to_a_subsystem(self, caplog, gateway):
def test_add_discovery_to_namespace(self, caplog, gateway):
caplog.clear()
cli(["namespace", "add_host", "--subsystem", subsystem, "--nsid", "8", "--host-nqn", discovery_nqn])
assert f"Failure adding host to namespace 8 on {subsystem}, host NQN can't be a discovery NQN" in caplog.text
assert f"Failure adding host to namespace 8 on {subsystem}: Host NQN can't be a discovery NQN" in caplog.text

def test_add_junk_host_to_namespace(self, caplog, gateway):
caplog.clear()
cli(["namespace", "add_host", "--subsystem", subsystem, "--nsid", "8", "--host-nqn", "junk"])
assert f"Failure adding host junk to namespace 8 on {subsystem}, invalid host NQN" in caplog.text
assert f"Failure adding host junk to namespace 8 on {subsystem}: Invalid host NQN" in caplog.text

def test_add_host_to_namespace_junk_subsystem(self, caplog, gateway):
caplog.clear()
cli(["namespace", "add_host", "--subsystem", "junk", "--nsid", "8", "--host-nqn", "nqn.2016-06.io.spdk:hostXX"])
assert f"Failure adding host nqn.2016-06.io.spdk:hostXX to namespace 8 on junk, invalid subsystem NQN" in caplog.text
assert f"Failure adding host nqn.2016-06.io.spdk:hostXX to namespace 8 on junk: Invalid subsystem NQN" in caplog.text

def test_add_host_to_wrong_namespace(self, caplog, gateway):
caplog.clear()
cli(["namespace", "add_host", "--subsystem", subsystem, "--nsid", "1", "--host-nqn", "nqn.2016-06.io.spdk:host10"])
assert f"Failure adding host nqn.2016-06.io.spdk:host10 to namespace 1 on {subsystem}, namespace is visible to all hosts" in caplog.text
assert f"Failure adding host nqn.2016-06.io.spdk:host10 to namespace 1 on {subsystem}: Namespace is visible to all hosts" in caplog.text

def test_add_too_many_namespaces_with_hosts(self, caplog, gateway):
caplog.clear()
Expand Down

0 comments on commit 23a49b2

Please sign in to comment.