Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not display an exception in case we delete a non-existing namespace host #997

Merged
merged 1 commit into from
Dec 22, 2024
Merged

Do not display an exception in case we delete a non-existing namespace host #997

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 36 additions & 25 deletions control/grpc.py
View file
Open in desktop
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
Open in desktop
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
Loading