LIO: Handle initiator IQNs as case insensitive

If there is a mismatch in case between the stored
initiator IQN in the LIO ACL vs. the IQN provided
in a later call, rtstool would fail to find the
existing ACL.

Compare IQNs in a case insensitive manner to ensure
they will always match as intended.  (Refer to RFC
3720 section 3.2.6.1 for details on this.)

Closes-Bug: #1522053

Change-Id: I6f535f3f4fbfcbbb49da30cffb08d17b3cac778a

diff --git a/cinder/cmd/rtstool.py b/cinder/cmd/rtstool.py
index 65291180d316dfaf13f5110fb587a465206deb1e..e095b4d7a1208956a82fb73c827f90c9090ad7cd 100644 (file)
--- a/cinder/cmd/rtstool.py
+++ b/cinder/cmd/rtstool.py
@@ -128,7 +128,7 @@ def add_initiator(target_iqn, initiator_iqn, userid, password):
     tpg = next(target.tpgs)  # get the first one
     for acl in tpg.node_acls:
         # See if this ACL configuration already exists
-        if acl.node_wwn == initiator_iqn:
+        if acl.node_wwn.lower() == initiator_iqn.lower():
             # No further action required
             return
 
@@ -143,7 +143,7 @@ def delete_initiator(target_iqn, initiator_iqn):
     target = _lookup_target(target_iqn, initiator_iqn)
     tpg = next(target.tpgs)  # get the first one
     for acl in tpg.node_acls:
-        if acl.node_wwn == initiator_iqn:
+        if acl.node_wwn.lower() == initiator_iqn.lower():
             acl.delete()
             return
 

diff --git a/cinder/tests/unit/test_cmd.py b/cinder/tests/unit/test_cmd.py
index 3c58a4092b44389e5ce3be8acd13c9ca84393d60..de8c574900141a0e244f35d50325406299d77ed3 100644 (file)
--- a/cinder/tests/unit/test_cmd.py
+++ b/cinder/tests/unit/test_cmd.py
@@ -803,6 +803,9 @@ class TestCinderRtstoolCmd(test.TestCase):
         sys.argv = ['cinder-rtstool']
         CONF(sys.argv[1:], project='cinder', version=version.version_string())
 
+        self.INITIATOR_IQN = 'iqn.2015.12.com.example.openstack.i:UNIT1'
+        self.TARGET_IQN = 'iqn.2015.12.com.example.openstack.i:TARGET1'
+
     def tearDown(self):
         super(TestCinderRtstoolCmd, self).tearDown()
 
@@ -973,7 +976,7 @@ class TestCinderRtstoolCmd(test.TestCase):
             self.assertRaises(rtslib_fb.utils.RTSLibError,
                               cinder_rtstool.add_initiator,
                               mock.sentinel.target_iqn,
-                              mock.sentinel.initiator_iqn,
+                              self.INITIATOR_IQN,
                               mock.sentinel.userid,
                               mock.sentinel.password)
 
@@ -984,7 +987,7 @@ class TestCinderRtstoolCmd(test.TestCase):
         self.assertRaises(cinder_rtstool.RtstoolError,
                           cinder_rtstool.add_initiator,
                           mock.sentinel.target_iqn,
-                          mock.sentinel.initiator_iqn,
+                          self.INITIATOR_IQN,
                           mock.sentinel.userid,
                           mock.sentinel.password)
 
@@ -994,15 +997,64 @@ class TestCinderRtstoolCmd(test.TestCase):
     def test_add_initiator_acl_exists(self, rtsroot, node_acl, mapped_lun):
         target_iqn = mock.MagicMock()
         target_iqn.tpgs.return_value = \
-            [{'node_acls': mock.sentinel.initiator_iqn}]
-        acl = mock.MagicMock(node_wwn=mock.sentinel.initiator_iqn)
+            [{'node_acls': self.INITIATOR_IQN}]
+        acl = mock.MagicMock(node_wwn=self.INITIATOR_IQN)
+        tpg = mock.MagicMock(node_acls=[acl])
+        tpgs = iter([tpg])
+        target = mock.MagicMock(tpgs=tpgs, wwn=self.TARGET_IQN)
+        rtsroot.return_value = mock.MagicMock(targets=[target])
+
+        cinder_rtstool.add_initiator(self.TARGET_IQN,
+                                     self.INITIATOR_IQN,
+                                     mock.sentinel.userid,
+                                     mock.sentinel.password)
+        self.assertFalse(node_acl.called)
+        self.assertFalse(mapped_lun.called)
+
+    @mock.patch.object(rtslib_fb, 'MappedLUN')
+    @mock.patch.object(rtslib_fb, 'NodeACL')
+    @mock.patch.object(rtslib_fb.root, 'RTSRoot')
+    def test_add_initiator_acl_exists_case_1(self,
+                                             rtsroot,
+                                             node_acl,
+                                             mapped_lun):
+        """Ensure initiator iqns are handled in a case-insensitive manner."""
+        target_iqn = mock.MagicMock()
+        target_iqn.tpgs.return_value = \
+            [{'node_acls': self.INITIATOR_IQN.lower()}]
+        acl = mock.MagicMock(node_wwn=self.INITIATOR_IQN)
         tpg = mock.MagicMock(node_acls=[acl])
         tpgs = iter([tpg])
         target = mock.MagicMock(tpgs=tpgs, wwn=target_iqn)
         rtsroot.return_value = mock.MagicMock(targets=[target])
 
         cinder_rtstool.add_initiator(target_iqn,
-                                     mock.sentinel.initiator_iqn,
+                                     self.INITIATOR_IQN,
+                                     mock.sentinel.userid,
+                                     mock.sentinel.password)
+        self.assertFalse(node_acl.called)
+        self.assertFalse(mapped_lun.called)
+
+    @mock.patch.object(rtslib_fb, 'MappedLUN')
+    @mock.patch.object(rtslib_fb, 'NodeACL')
+    @mock.patch.object(rtslib_fb.root, 'RTSRoot')
+    def test_add_initiator_acl_exists_case_2(self,
+                                             rtsroot,
+                                             node_acl,
+                                             mapped_lun):
+        """Ensure initiator iqns are handled in a case-insensitive manner."""
+        iqn_lower = self.INITIATOR_IQN.lower()
+        target_iqn = mock.MagicMock()
+        target_iqn.tpgs.return_value = \
+            [{'node_acls': self.INITIATOR_IQN}]
+        acl = mock.MagicMock(node_wwn=iqn_lower)
+        tpg = mock.MagicMock(node_acls=[acl])
+        tpgs = iter([tpg])
+        target = mock.MagicMock(tpgs=tpgs, wwn=target_iqn)
+        rtsroot.return_value = mock.MagicMock(targets=[target])
+
+        cinder_rtstool.add_initiator(target_iqn,
+                                     self.INITIATOR_IQN,
                                      mock.sentinel.userid,
                                      mock.sentinel.password)
         self.assertFalse(node_acl.called)
@@ -1014,7 +1066,7 @@ class TestCinderRtstoolCmd(test.TestCase):
     def test_add_initiator(self, rtsroot, node_acl, mapped_lun):
         target_iqn = mock.MagicMock()
         target_iqn.tpgs.return_value = \
-            [{'node_acls': mock.sentinel.initiator_iqn}]
+            [{'node_acls': self.INITIATOR_IQN}]
         tpg = mock.MagicMock()
         tpgs = iter([tpg])
         target = mock.MagicMock(tpgs=tpgs, wwn=target_iqn)
@@ -1025,11 +1077,11 @@ class TestCinderRtstoolCmd(test.TestCase):
         node_acl.return_value = acl_new
 
         cinder_rtstool.add_initiator(target_iqn,
-                                     mock.sentinel.initiator_iqn,
+                                     self.INITIATOR_IQN,
                                      mock.sentinel.userid,
                                      mock.sentinel.password)
         node_acl.assert_called_once_with(tpg,
-                                         mock.sentinel.initiator_iqn,
+                                         self.INITIATOR_IQN,
                                          mode='create')
         mapped_lun.assert_called_once_with(acl_new, 0, tpg_lun=0)
 
@@ -1058,6 +1110,40 @@ class TestCinderRtstoolCmd(test.TestCase):
         target.delete.assert_called_once_with()
         storage_object.delete.assert_called_once_with()
 
+    @mock.patch.object(rtslib_fb, 'MappedLUN')
+    @mock.patch.object(rtslib_fb, 'NodeACL')
+    @mock.patch.object(rtslib_fb.root, 'RTSRoot')
+    def test_delete_initiator(self, rtsroot, node_acl, mapped_lun):
+        target_iqn = mock.MagicMock()
+        target_iqn.tpgs.return_value = \
+            [{'node_acls': self.INITIATOR_IQN}]
+        acl = mock.MagicMock(node_wwn=self.INITIATOR_IQN)
+        tpg = mock.MagicMock(node_acls=[acl])
+        tpgs = iter([tpg])
+        target = mock.MagicMock(tpgs=tpgs, wwn=target_iqn)
+        rtsroot.return_value = mock.MagicMock(targets=[target])
+
+        cinder_rtstool.delete_initiator(target_iqn,
+                                        self.INITIATOR_IQN)
+
+    @mock.patch.object(rtslib_fb, 'MappedLUN')
+    @mock.patch.object(rtslib_fb, 'NodeACL')
+    @mock.patch.object(rtslib_fb.root, 'RTSRoot')
+    def test_delete_initiator_case(self, rtsroot, node_acl, mapped_lun):
+        """Ensure iqns are handled in a case-insensitive manner."""
+        initiator_iqn_lower = self.INITIATOR_IQN.lower()
+        target_iqn = mock.MagicMock()
+        target_iqn.tpgs.return_value = \
+            [{'node_acls': initiator_iqn_lower}]
+        acl = mock.MagicMock(node_wwn=self.INITIATOR_IQN)
+        tpg = mock.MagicMock(node_acls=[acl])
+        tpgs = iter([tpg])
+        target = mock.MagicMock(tpgs=tpgs, wwn=target_iqn)
+        rtsroot.return_value = mock.MagicMock(targets=[target])
+
+        cinder_rtstool.delete_initiator(target_iqn,
+                                        self.INITIATOR_IQN)
+
     @mock.patch.object(cinder_rtstool, 'os', autospec=True)
     @mock.patch.object(cinder_rtstool, 'rtslib_fb', autospec=True)
     def test_save_with_filename(self, mock_rtslib, mock_os):