Fix batch deleting issue in volume_type.unset_keys()

cinderclient volume_type.unset_keys() only delete the first key even if
multiple keys are applied. The response of manager._delete() is tuple object,
it is not "None" if the deleting success, so the batch operation would
be broken when the first key is deleted successfully. The patch fix this
issue.

Change-Id: I60378a32cdc52aacdf869d69b246dec7eb6cdb77
Closes-Bug: #1596511

cinderclient/tests/unit/utils.py

@@ -40,9 +40,9 @@ class TestCase(testtools.TestCase):
             stderr = self.useFixture(fixtures.StringStream('stderr')).stream
             self.useFixture(fixtures.MonkeyPatch('sys.stderr', stderr))
 
-    def _assert_request_id(self, obj):
+    def _assert_request_id(self, obj, count=1):
         self.assertTrue(hasattr(obj, 'request_ids'))
-        self.assertEqual(REQUEST_ID, obj.request_ids)
+        self.assertEqual(REQUEST_ID * count, obj.request_ids)
 
     def assert_called_anytime(self, method, url, body=None,
                               partial_body=None):

cinderclient/tests/unit/v1/fakes.py

@@ -472,6 +472,9 @@ class FakeHTTPClient(base_client.HTTPClient):
     def delete_types_1_extra_specs_k(self, **kw):
         return(204, {}, None)
 
+    def delete_types_1_extra_specs_m(self, **kw):
+        return(204, {}, None)
+
     def delete_types_1(self, **kw):
         return (202, {}, None)
 

cinderclient/tests/unit/v1/test_types.py

@@ -37,11 +37,17 @@ class TypesTest(utils.TestCase):
                          '/types/1/extra_specs',
                          {'extra_specs': {'k': 'v'}})
 
-    def test_unsset_keys(self):
+    def test_unset_keys(self):
         t = cs.volume_types.get(1)
         t.unset_keys(['k'])
         cs.assert_called('DELETE', '/types/1/extra_specs/k')
 
+    def test_unset_multiple_keys(self):
+        t = cs.volume_types.get(1)
+        t.unset_keys(['k', 'm'])
+        cs.assert_called_anytime('DELETE', '/types/1/extra_specs/k')
+        cs.assert_called_anytime('DELETE', '/types/1/extra_specs/m')
+
     def test_delete(self):
         cs.volume_types.delete(1)
         cs.assert_called('DELETE', '/types/1')

cinderclient/tests/unit/v2/fakes.py

@@ -689,6 +689,9 @@ class FakeHTTPClient(base_client.HTTPClient):
     def delete_types_1_extra_specs_k(self, **kw):
         return(204, {}, None)
 
+    def delete_types_1_extra_specs_m(self, **kw):
+        return(204, {}, None)
+
     def delete_types_1(self, **kw):
         return (202, {}, None)
 

cinderclient/tests/unit/v2/test_types.py

@@ -81,17 +81,24 @@ class TypesTest(utils.TestCase):
 
     def test_set_key(self):
         t = cs.volume_types.get(1)
-        t.set_keys({'k': 'v'})
+        res = t.set_keys({'k': 'v'})
         cs.assert_called('POST',
                          '/types/1/extra_specs',
                          {'extra_specs': {'k': 'v'}})
-        self._assert_request_id(t)
+        self._assert_request_id(res)
 
-    def test_unsset_keys(self):
+    def test_unset_keys(self):
         t = cs.volume_types.get(1)
-        t.unset_keys(['k'])
+        res = t.unset_keys(['k'])
         cs.assert_called('DELETE', '/types/1/extra_specs/k')
-        self._assert_request_id(t)
+        self._assert_request_id(res)
+
+    def test_unset_multiple_keys(self):
+        t = cs.volume_types.get(1)
+        res = t.unset_keys(['k', 'm'])
+        cs.assert_called_anytime('DELETE', '/types/1/extra_specs/k')
+        cs.assert_called_anytime('DELETE', '/types/1/extra_specs/m')
+        self._assert_request_id(res, count=2)
 
     def test_delete(self):
         t = cs.volume_types.delete(1)

cinderclient/v1/volume_types.py

@@ -62,16 +62,12 @@ class VolumeType(base.Resource):
         """
 
         # NOTE(jdg): This wasn't actually doing all of the keys before
-        # the return in the loop resulted in ony ONE key being unset.
+        # the return in the loop resulted in only ONE key being unset,
-        # since on success the return was NONE, we'll only interrupt the loop
+        # since on success the return was None, we'll only interrupt
-        # and return if there's an error
+        # the loop and if an exception is raised.
-        resp = None
         for k in keys:
-            resp = self.manager._delete(
+            self.manager._delete("/types/%s/extra_specs/%s" %
-                "/types/%s/extra_specs/%s" % (
+                                 (base.getid(self), k))
-                base.getid(self), k))
-            if resp is not None:
-                return resp
 
 
 class VolumeTypeManager(base.ManagerWithFind):

cinderclient/v3/volume_types.py

@@ -17,6 +17,7 @@
 """Volume Type interface."""
 
 from cinderclient import base
+from cinderclient.openstack.common.apiclient import base as common_base
 
 
 class VolumeType(base.Resource):
@@ -63,15 +64,17 @@ class VolumeType(base.Resource):
         """
 
         # NOTE(jdg): This wasn't actually doing all of the keys before
-        # the return in the loop resulted in ony ONE key being unset.
+        # the return in the loop resulted in only ONE key being unset,
-        # since on success the return was NONE, we'll only interrupt the loop
+        # since on success the return was ListWithMeta class, we'll only
-        # and return if there's an error
+        # interrupt the loop and if an exception is raised.
+        response_list = []
         for k in keys:
-            resp = self.manager._delete(
+            resp, body = self.manager._delete(
                 "/types/%s/extra_specs/%s" % (
                 base.getid(self), k))
-            if resp is not None:
+            response_list.append(resp)
-                return resp
+
+        return common_base.ListWithMeta([], response_list)
 
 
 class VolumeTypeManager(base.ManagerWithFind):