Skip to content

Commit

Permalink
Merge pull request #1064 from gbregman/devel
Browse files Browse the repository at this point in the history 
Don't delete other subsys namespace when deleting a subsys
  • Loading branch information
@gbregman
gbregman authored Jan 29, 2025
2 parents fae5b73 + 30f1ca9 commit 31fdf9f
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 29 deletions.
2 changes: 1 addition & 1 deletion control/grpc.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1477,7 +1477,7 @@ def delete_subsystem(self, request, context=None):
peer_msg = self.get_peer_message(context)
delete_subsystem_error_prefix = f"Failure deleting subsystem {request.subsystem_nqn}"
self.logger.info(f"Received request to delete subsystem {request.subsystem_nqn}, "
f"context: {context}{peer_msg}")
f"force: {request.force}, context: {context}{peer_msg}")

if not request.subsystem_nqn:
errmsg = "Failure deleting subsystem, missing subsystem NQN"
Expand Down
56 changes: 28 additions & 28 deletions control/state.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,77 +50,80 @@ def is_key_element_valid(s: str) -> bool:
return True

def build_namespace_key(subsystem_nqn: str, nsid) -> str:
key = GatewayState.NAMESPACE_PREFIX + subsystem_nqn
key = GatewayState.NAMESPACE_PREFIX + subsystem_nqn + GatewayState.OMAP_KEY_DELIMITER
if nsid is not None:
key += GatewayState.OMAP_KEY_DELIMITER + str(nsid)
key += str(nsid)
return key

def build_namespace_lbgroup_key(subsystem_nqn: str, nsid) -> str:
key = GatewayState.NAMESPACE_LB_GROUP_PREFIX + subsystem_nqn
key = GatewayState.NAMESPACE_LB_GROUP_PREFIX + subsystem_nqn + \
GatewayState.OMAP_KEY_DELIMITER
if nsid is not None:
key += GatewayState.OMAP_KEY_DELIMITER + str(nsid)
key += str(nsid)
return key

def build_namespace_visibility_key(subsystem_nqn: str, nsid) -> str:
key = GatewayState.NAMESPACE_VISIBILITY_PREFIX + subsystem_nqn
key = GatewayState.NAMESPACE_VISIBILITY_PREFIX + subsystem_nqn + \
GatewayState.OMAP_KEY_DELIMITER
if nsid is not None:
key += GatewayState.OMAP_KEY_DELIMITER + str(nsid)
key += str(nsid)
return key

def build_namespace_trash_image_key(subsystem_nqn: str, nsid) -> str:
key = GatewayState.NAMESPACE_TRASH_IMAGE_PREFIX + subsystem_nqn
key = GatewayState.NAMESPACE_TRASH_IMAGE_PREFIX + subsystem_nqn + \
GatewayState.OMAP_KEY_DELIMITER
if nsid is not None:
key += GatewayState.OMAP_KEY_DELIMITER + str(nsid)
key += str(nsid)
return key

def build_namespace_qos_key(subsystem_nqn: str, nsid) -> str:
key = GatewayState.NAMESPACE_QOS_PREFIX + subsystem_nqn
key = GatewayState.NAMESPACE_QOS_PREFIX + subsystem_nqn + GatewayState.OMAP_KEY_DELIMITER
if nsid is not None:
key += GatewayState.OMAP_KEY_DELIMITER + str(nsid)
key += str(nsid)
return key

def build_namespace_host_key(subsystem_nqn: str, nsid, host: str) -> str:
key = GatewayState.NAMESPACE_HOST_PREFIX + subsystem_nqn
key = GatewayState.NAMESPACE_HOST_PREFIX + subsystem_nqn + GatewayState.OMAP_KEY_DELIMITER
if nsid is not None:
key += GatewayState.OMAP_KEY_DELIMITER + str(nsid)
key += str(nsid) + GatewayState.OMAP_KEY_DELIMITER
if host:
key += GatewayState.OMAP_KEY_DELIMITER + host
key += host
return key

def build_subsystem_key(subsystem_nqn: str) -> str:
return GatewayState.SUBSYSTEM_PREFIX + subsystem_nqn

def build_host_key(subsystem_nqn: str, host_nqn: str) -> str:
key = GatewayState.HOST_PREFIX + subsystem_nqn
key = GatewayState.HOST_PREFIX + subsystem_nqn + GatewayState.OMAP_KEY_DELIMITER
if host_nqn is not None:
key += GatewayState.OMAP_KEY_DELIMITER + host_nqn
key += host_nqn
return key

def build_host_key_key(subsystem_nqn: str, host_nqn: str) -> str:
key = GatewayState.HOST_KEY_PREFIX + subsystem_nqn
key = GatewayState.HOST_KEY_PREFIX + subsystem_nqn + GatewayState.OMAP_KEY_DELIMITER
if host_nqn is not None:
key += GatewayState.OMAP_KEY_DELIMITER + host_nqn
key += host_nqn
return key

def build_subsystem_key_key(subsystem_nqn: str) -> str:
return GatewayState.SUBSYSTEM_KEY_PREFIX + subsystem_nqn

def build_partial_listener_key(subsystem_nqn: str, host: str) -> str:
key = GatewayState.LISTENER_PREFIX + subsystem_nqn
key = GatewayState.LISTENER_PREFIX + subsystem_nqn + GatewayState.OMAP_KEY_DELIMITER
if host:
key += GatewayState.OMAP_KEY_DELIMITER + host
key += host + GatewayState.OMAP_KEY_DELIMITER
return key

def build_listener_key_suffix(host: str, trtype: str, traddr: str, trsvcid: int) -> str:
if host:
return GatewayState.OMAP_KEY_DELIMITER + host + \
return host + \
GatewayState.OMAP_KEY_DELIMITER + trtype + GatewayState.OMAP_KEY_DELIMITER + \
traddr + GatewayState.OMAP_KEY_DELIMITER + str(trsvcid)
if trtype:
return GatewayState.OMAP_KEY_DELIMITER + trtype + \
return trtype + \
GatewayState.OMAP_KEY_DELIMITER + traddr + \
GatewayState.OMAP_KEY_DELIMITER + str(trsvcid)
return GatewayState.OMAP_KEY_DELIMITER + traddr + \
return traddr + \
GatewayState.OMAP_KEY_DELIMITER + str(trsvcid)

def build_listener_key(subsystem_nqn: str, host: str, trtype: str,
Expand Down Expand Up @@ -1138,13 +1141,10 @@ def update(self) -> bool:
only_trash_image_changed = []
only_host_key_changed = []
only_subsystem_key_changed = []
ns_prefix = GatewayState.build_namespace_key("nqn", None)
host_prefix = GatewayState.build_host_key("nqn", None)
subsystem_prefix = GatewayState.build_subsystem_key("nqn")
for key in changed.keys():
self.logger.info(f"Changed key: {key} local-state: {local_state_dict[key]}"
f" omap-state: {omap_state_dict[key]}")
if key.startswith(ns_prefix):
if key.startswith(GatewayState.NAMESPACE_PREFIX):
(should_process, new_lb_grp_id) = self.namespace_only_lb_group_id_changed(
local_state_dict[key],
omap_state_dict[key])
Expand Down Expand Up @@ -1172,7 +1172,7 @@ def update(self) -> bool:
f"flag has changed. "
f"The new flag is {new_trash_image}")
only_trash_image_changed.append((key, new_trash_image))
elif key.startswith(host_prefix):
elif key.startswith(GatewayState.HOST_PREFIX):
(should_process,
new_dhchap_key,
new_key_encrypted) = self.host_only_key_changed(
Expand All @@ -1182,7 +1182,7 @@ def update(self) -> bool:
self.logger.debug(f"Found {key} where only the key has changed. The "
f"new DHCHAP key is {new_dhchap_key}")
only_host_key_changed.append((key, new_dhchap_key, new_key_encrypted))
elif key.startswith(subsystem_prefix):
elif key.startswith(GatewayState.SUBSYSTEM_PREFIX):
(should_process,
new_dhchap_key,
new_key_encrypted) = self.subsystem_only_key_changed(
Expand Down
92 changes: 92 additions & 0 deletions tests/test_cli.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@
image17 = "mytestdevimage17"
image18 = "mytestdevimage18"
image19 = "mytestdevimage19"
image20 = "mytestdevimage20"
image21 = "mytestdevimage21"
image22 = "mytestdevimage22"
image23 = "mytestdevimage23"
pool = "rbd"
subsystem = "nqn.2016-06.io.spdk:cnode1"
subsystem2 = "nqn.2016-06.io.spdk:cnode2"
Expand Down Expand Up @@ -1769,3 +1773,91 @@ def test_delete_rbd_image(self, caplog, gateway):
caplog.clear()
cli(["namespace", "del", "--subsystem", subsystem10, "--nsid", "1"])
assert f"Deleting namespace 1 from {subsystem10}: Successful" in caplog.text


class TestSubsystemWithIdenticalPrefix:
def test_subsystem_with_identical_prefix(self, caplog, gateway):
gw, stub = gateway
# Make sure one NQN is a prefix of the other
assert subsystem10.startswith(subsystem)
# Clean old subsystems as we are limited to only 4
subs = cli_test(["subsystem", "list"])
for s in subs.subsystems:
caplog.clear()
cli(["subsystem", "del", "--subsystem", s.nqn, "--force"])
assert f"Deleting subsystem {s.nqn}: Successful" in caplog.text
caplog.clear()
cli(["subsystem", "list"])
assert "No subsystems" in caplog.text
# OK, all clear, now we can add the subsystems where one NQN is a prefix of the other
caplog.clear()
cli(["subsystem", "add", "--subsystem", subsystem, "--no-group-append"])
assert f"Adding subsystem {subsystem}: Successful" in caplog.text
caplog.clear()
cli(["subsystem", "add", "--subsystem", subsystem10, "--no-group-append"])
assert f"Adding subsystem {subsystem10}: Successful" in caplog.text
caplog.clear()
cli(["namespace", "add", "--subsystem", subsystem, "--rbd-pool", pool,
"--rbd-image", image20, "--rbd-create-image", "--size", "16MB",
"--rbd-trash-image-on-delete"])
assert f"Adding namespace 1 to {subsystem}: Successful" in caplog.text
caplog.clear()
cli(["namespace", "add", "--subsystem", subsystem, "--rbd-pool", pool,
"--rbd-image", image21, "--rbd-create-image", "--size", "16MB",
"--rbd-trash-image-on-delete"])
assert f"Adding namespace 2 to {subsystem}: Successful" in caplog.text
caplog.clear()
cli(["namespace", "add", "--subsystem", subsystem10, "--rbd-pool", pool,
"--rbd-image", image22, "--rbd-create-image", "--size", "16MB",
"--rbd-trash-image-on-delete"])
assert f"Adding namespace 1 to {subsystem10}: Successful" in caplog.text
caplog.clear()
cli(["namespace", "add", "--subsystem", subsystem10, "--rbd-pool", pool,
"--rbd-image", image23, "--rbd-create-image", "--size", "16MB",
"--rbd-trash-image-on-delete"])
assert f"Adding namespace 2 to {subsystem10}: Successful" in caplog.text
caplog.clear()
cli(["listener", "add", "--subsystem", subsystem, "--host-name", host_name,
"--verify-host-name", "-a", addr, "-s", "4440", "-f", "ipv4"])
assert f"Adding {subsystem} listener at {addr}:{4440}: Successful" in caplog.text
caplog.clear()
cli(["listener", "add", "--subsystem", subsystem10, "--host-name", host_name,
"--verify-host-name", "-a", addr, "-s", "4450", "-f", "ipv4"])
assert f"Adding {subsystem10} listener at {addr}:{4450}: Successful" in caplog.text
caplog.clear()
cli(["--format", "json", "listener", "list", "--subsystem", subsystem])
assert '"trsvcid": 4440,' in caplog.text
assert '"trsvcid": 4450,' not in caplog.text
caplog.clear()
cli(["--format", "json", "listener", "list", "--subsystem", subsystem10])
assert '"trsvcid": 4440,' not in caplog.text
assert '"trsvcid": 4450,' in caplog.text
found = 0
found10 = 0
state = gw.gateway_state.omap.get_state()
for key, val in state.items():
if not key.startswith(gw.gateway_state.local.NAMESPACE_PREFIX):
continue
valstr = val.decode()
if f'"subsystem_nqn": "{subsystem}",' in valstr:
found += 1
elif f'"subsystem_nqn": "{subsystem10}",' in valstr:
found10 += 1
assert found == 2
assert found10 == 2
caplog.clear()
cli(["subsystem", "del", "--subsystem", subsystem, "--force"])
assert f"Deleting subsystem {subsystem}: Successful" in caplog.text
found = 0
found10 = 0
state = gw.gateway_state.omap.get_state()
for key, val in state.items():
if not key.startswith(gw.gateway_state.local.NAMESPACE_PREFIX):
continue
valstr = val.decode()
if f'"subsystem_nqn": "{subsystem}",' in valstr:
found += 1
elif f'"subsystem_nqn": "{subsystem10}",' in valstr:
found10 += 1
assert found == 0
assert found10 == 2

0 comments on commit 31fdf9f

Please sign in to comment.