Show actual NQN in CLI when adding a subsystem

Fixes #854 Signed-off-by: Gil Bregman <gbregman@il.ibm.com>
ceph · Sep 8, 2024 · f7cff15 · f7cff15
1 parent b62fa34
commit f7cff15
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 13 deletions.
diff --git a/control/cli.py b/control/cli.py
@@ -678,11 +678,20 @@ def subsystem_add(self, args):
         try:
             ret = self.stub.create_subsystem(req)
         except Exception as ex:
-            ret = pb2.req_status(status = errno.EINVAL, error_message = f"Failure adding subsystem {args.subsystem}:\n{ex}")
+            ret = pb2.subsys_status(status = errno.EINVAL, error_message = f"Failure adding subsystem {args.subsystem}:\n{ex}",
+                                    nqn = args.subsystem)
+
+        new_nqn = ""
+        try:
+            new_nqn = ret.nqn
+        except Exception as ex:  # In case of an old gateway the returned value wouldn't have the nqn field
+           pass
+        if not new_nqn:
+            new_nqn = args.subsystem
 
         if args.format == "text" or args.format == "plain":
             if ret.status == 0:
-                out_func(f"Adding subsystem {args.subsystem}: Successful")
+                out_func(f"Adding subsystem {new_nqn}: Successful")
             else:
                 err_func(f"{ret.error_message}")
         elif args.format == "json" or args.format == "yaml":

diff --git a/control/grpc.py b/control/grpc.py
@@ -641,30 +641,30 @@ def create_subsystem_safe(self, request, context):
         if not request.enable_ha:
             errmsg = f"{create_subsystem_error_prefix}: HA must be enabled for subsystems"
             self.logger.error(f"{errmsg}")
-            return pb2.req_status(status = errno.EINVAL, error_message = errmsg)
+            return pb2.subsys_status(status = errno.EINVAL, error_message = errmsg, nqn = request.subsystem_nqn)
 
         if not request.subsystem_nqn:
             errmsg = f"Failure creating subsystem, missing subsystem NQN"
             self.logger.error(f"{errmsg}")
-            return pb2.req_status(status = errno.EINVAL, error_message = errmsg)
+            return pb2.subsys_status(status = errno.EINVAL, error_message = errmsg, nqn = request.subsystem_nqn)
 
         errmsg = ""
         if not GatewayState.is_key_element_valid(request.subsystem_nqn):
             errmsg = f"{create_subsystem_error_prefix}: Invalid NQN \"{request.subsystem_nqn}\", contains invalid characters"
             self.logger.error(f"{errmsg}")
-            return pb2.req_status(status = errno.EINVAL, error_message = errmsg)
+            return pb2.subsys_status(status = errno.EINVAL, error_message = errmsg, nqn = request.subsystem_nqn)
 
         if self.verify_nqns:
             rc = GatewayUtils.is_valid_nqn(request.subsystem_nqn)
             if rc[0] != 0:
                 errmsg = f"{create_subsystem_error_prefix}: {rc[1]}"
                 self.logger.error(f"{errmsg}")
-                return pb2.req_status(status = rc[0], error_message = errmsg)
+                return pb2.subsys_status(status = rc[0], error_message = errmsg, nqn = request.subsystem_nqn)
 
         if GatewayUtils.is_discovery_nqn(request.subsystem_nqn):
             errmsg = f"{create_subsystem_error_prefix}: Can't create a discovery subsystem"
             self.logger.error(f"{errmsg}")
-            return pb2.req_status(status = errno.EINVAL, error_message = errmsg)
+            return pb2.subsys_status(status = errno.EINVAL, error_message = errmsg, nqn = request.subsystem_nqn)
 
         if context:
             if request.no_group_append or not self.gateway_group:
@@ -701,7 +701,7 @@ def create_subsystem_safe(self, request, context):
                 if subsys_already_exists or subsys_using_serial:
                     errmsg = f"{create_subsystem_error_prefix}: {errmsg}"
                     self.logger.error(f"{errmsg}")
-                    return pb2.req_status(status=errno.EEXIST, error_message=errmsg)
+                    return pb2.subsys_status(status=errno.EEXIST, error_message=errmsg, nqn = request.subsystem_nqn)
                 ret = rpc_nvmf.nvmf_create_subsystem(
                     self.spdk_rpc_client,
                     nqn=request.subsystem_nqn,
@@ -722,12 +722,12 @@ def create_subsystem_safe(self, request, context):
                 if resp:
                     status = resp["code"]
                     errmsg = f"{create_subsystem_error_prefix}: {resp['message']}"
-                return pb2.req_status(status=status, error_message=errmsg)
+                return pb2.subsys_status(status=status, error_message=errmsg, nqn = request.subsystem_nqn)
 
             # Just in case SPDK failed with no exception
             if not ret:
                 self.logger.error(create_subsystem_error_prefix)
-                return pb2.req_status(status=errno.EINVAL, error_message=create_subsystem_error_prefix)
+                return pb2.subsys_status(status=errno.EINVAL, error_message=create_subsystem_error_prefix, nqn = request.subsystem_nqn)
 
             if context:
                 # Update gateway state
@@ -739,9 +739,9 @@ def create_subsystem_safe(self, request, context):
                     errmsg = f"Error persisting subsystem {request.subsystem_nqn}"
                     self.logger.exception(errmsg)
                     errmsg = f"{errmsg}:\n{ex}"
-                    return pb2.req_status(status=errno.EINVAL, error_message=errmsg)
+                    return pb2.subsys_status(status=errno.EINVAL, error_message=errmsg, nqn = request.subsystem_nqn)
 
-        return pb2.req_status(status=0, error_message=os.strerror(0))
+        return pb2.subsys_status(status=0, error_message=os.strerror(0), nqn = request.subsystem_nqn)
 
     def create_subsystem(self, request, context=None):
         return self.execute_grpc_function(self.create_subsystem_safe, request, context)

diff --git a/control/proto/gateway.proto b/control/proto/gateway.proto
@@ -37,7 +37,7 @@ service Gateway {
 	rpc namespace_add(namespace_add_req) returns (nsid_status) {}
 
 	// Creates a subsystem
-	rpc create_subsystem(create_subsystem_req) returns(req_status) {}
+	rpc create_subsystem(create_subsystem_req) returns(subsys_status) {}
 
 	// Deletes a subsystem
 	rpc delete_subsystem(delete_subsystem_req) returns(req_status) {}
@@ -288,6 +288,12 @@ message req_status {
 	string error_message = 2;
 }
 
+message subsys_status {
+	int32 status = 1;
+	string error_message = 2;
+	string nqn = 3;
+}
+
 message nsid_status {
 	int32 status = 1;
 	string error_message = 2;

diff --git a/tests/test_cli.py b/tests/test_cli.py
@@ -972,6 +972,7 @@ class TestSubsysWithGroupName:
     def test_create_subsys_group_name(self, caplog, gateway):
         caplog.clear()
         cli(["subsystem", "add", "--subsystem", subsystem3])
+        assert f"Adding subsystem {subsystem3}.{group_name}: Successful" in caplog.text
         assert f"Subsystem NQN was changed to {subsystem3}.{group_name}, adding the group name" in caplog.text
         assert f"create_subsystem {subsystem3}.{group_name}: True" in caplog.text
         assert f"create_subsystem {subsystem3}: True" not in caplog.text
@@ -980,6 +981,7 @@ def test_create_subsys_group_name(self, caplog, gateway):
         assert f'"nqn": "{subsystem3}"' not in caplog.text
         caplog.clear()
         cli(["subsystem", "add", "--subsystem", subsystem4, "--no-group-append"])
+        assert f"Adding subsystem {subsystem4}: Successful" in caplog.text
         assert f"Subsystem NQN will not be changed" in caplog.text
         assert f"create_subsystem {subsystem4}.{group_name}: True" not in caplog.text
         assert f"create_subsystem {subsystem4}: True" in caplog.text