Update the "use storpool's api for attach/detach" patch
Update tests and fix linting issues.
Change-Id: If18645c8d2810ae8520aaf4dbb9caeb2b61c2e86
diff --git a/patches/openstack/os-brick/attach-globalid.patch b/patches/openstack/os-brick/attach-globalid.patch
index a1f9ef7..71834be 100644
--- a/patches/openstack/os-brick/attach-globalid.patch
+++ b/patches/openstack/os-brick/attach-globalid.patch
@@ -1,4 +1,4 @@
-From eeea4310317d10a6f730777e1e0b1cb73cb004d4 Mon Sep 17 00:00:00 2001
+From 315448f05b310d4e4b2bad084ddcdc888e0c2cd0 Mon Sep 17 00:00:00 2001
From: Biser Milanov <biser.milanov@storpool.com>
Date: Tue, 23 May 2023 17:55:09 +0300
Subject: [PATCH] storpool.py: Use StorPool's API for Attach/Detach
@@ -13,12 +13,12 @@
Change-Id: I0fabcde1dc249b9b09f99ee0b7d759432972314a
---
- os_brick/initiator/connectors/storpool.py | 81 ++++++++--
- .../initiator/connectors/test_storpool.py | 151 +++++++++++-------
- 2 files changed, 165 insertions(+), 67 deletions(-)
+ os_brick/initiator/connectors/storpool.py | 84 +++++--
+ .../initiator/connectors/test_storpool.py | 219 +++++++++++++-----
+ 2 files changed, 236 insertions(+), 67 deletions(-)
diff --git a/os_brick/initiator/connectors/storpool.py b/os_brick/initiator/connectors/storpool.py
-index e1bd55c..72d2acf 100644
+index e1bd55c..9752ad9 100644
--- a/os_brick/initiator/connectors/storpool.py
+++ b/os_brick/initiator/connectors/storpool.py
@@ -26,6 +26,7 @@ from os_brick import utils
@@ -40,14 +40,17 @@
if spopenstack is not None:
try:
self._attach = spopenstack.AttachDB(log=LOG)
-@@ -46,6 +51,35 @@ class StorPoolConnector(base.BaseLinuxConnector):
+@@ -46,6 +51,38 @@ class StorPoolConnector(base.BaseLinuxConnector):
else:
self._attach = None
+ def _detach_retry(self, sp_ourid, volume):
-+ """Retries attempt to handle LUKS tests-related failures:"
++ """Retry detaching.
++
++ Retries attempt to handle LUKS tests-related failures:
+ busy: volume ... open at ...
+ """
++
+ count = 10
+ while True:
+ try:
@@ -76,7 +79,7 @@
@staticmethod
def get_connector_properties(root_helper, *args, **kwargs):
"""The StorPool connector properties."""
-@@ -76,16 +110,29 @@ class StorPoolConnector(base.BaseLinuxConnector):
+@@ -76,16 +113,29 @@ class StorPoolConnector(base.BaseLinuxConnector):
if mode is None or mode not in ('rw', 'ro'):
raise exception.BrickException(
'Invalid access_mode specified in the connection data.')
@@ -116,7 +119,7 @@
@utils.connect_volume_undo_prepare_result(unlink_after=True)
def disconnect_volume(self, connection_properties, device_info,
-@@ -124,9 +171,17 @@ class StorPoolConnector(base.BaseLinuxConnector):
+@@ -124,9 +174,17 @@ class StorPoolConnector(base.BaseLinuxConnector):
raise exception.BrickException(
'Invalid StorPool connection data, no volume ID specified.')
volume = self._attach.volumeName(volume_id)
@@ -138,11 +141,33 @@
def get_search_path(self):
return '/dev/storpool'
diff --git a/os_brick/tests/initiator/connectors/test_storpool.py b/os_brick/tests/initiator/connectors/test_storpool.py
-index 614deba..9cc1076 100644
+index 614deba..b539e2b 100644
--- a/os_brick/tests/initiator/connectors/test_storpool.py
+++ b/os_brick/tests/initiator/connectors/test_storpool.py
-@@ -27,44 +27,13 @@ def volumeNameExt(vid):
+@@ -25,46 +25,35 @@ def volumeNameExt(vid):
+ return 'os--volume--{id}'.format(id=vid)
+
++def faulty_api(req):
++ faulty_api.real_fn(req)
++ if faulty_api.fail_count > 0:
++ faulty_api.fail_count -= 1
++ raise MockApiError("busy",
++ "'os--volume--sp-vol-1' is open at client 19")
++
++
++class MockApiError(Exception):
++ def __init__(self, name, desc):
++ super(MockApiError, self).__init__()
++ self.name = name
++ self.desc = desc
++
++
++class MockVolumeInfo(object):
++ def __init__(self, global_id):
++ self.globalId = global_id
++
++
class MockStorPoolADB(object):
def __init__(self, log):
- self.requests = {}
@@ -189,15 +214,27 @@
def volumeName(self, vid):
return volumeNameExt(vid)
-@@ -101,6 +70,7 @@ class StorPoolConnectorTestCase(test_connector.ConnectorTestCase):
+@@ -74,6 +63,10 @@ spopenstack = mock.Mock()
+ spopenstack.AttachDB = MockStorPoolADB
+ connector.spopenstack = spopenstack
+
++spapi = mock.Mock()
++spapi.ApiError = MockApiError
++connector.spapi = spapi
++
+
+ class StorPoolConnectorTestCase(test_connector.ConnectorTestCase):
+ def volumeName(self, vid):
+@@ -101,6 +94,8 @@ class StorPoolConnectorTestCase(test_connector.ConnectorTestCase):
'client_id': 1,
'access_mode': 'rw',
}
+ self.fakeGlobalId = 'OneNiceGlobalId'
++ self.api_calls_retry_max = 10
self.fakeConnection = None
self.fakeSize = 1024 * 1024 * 1024
-@@ -109,30 +79,56 @@ class StorPoolConnectorTestCase(test_connector.ConnectorTestCase):
+@@ -109,30 +104,56 @@ class StorPoolConnectorTestCase(test_connector.ConnectorTestCase):
self.adb = self.connector._attach
def test_connect_volume(self):
@@ -219,8 +256,8 @@
+ volume_name = volumeNameExt(self.fakeProp['volume'])
+ api = mock.MagicMock(spec=['volumesReassignWait', 'volumeInfo'])
+ api.volumesReassignWait = mock.MagicMock(spec=['__call__'])
-+ api.volumeInfo = mock.Mock(
-+ return_value={'data': {'globalId': self.fakeGlobalId}})
++ volume_info = MockVolumeInfo(self.fakeGlobalId)
++ api.volumeInfo = mock.Mock(return_value=volume_info)
+
+ with mock.patch.object(
+ self.adb, attribute='api', spec=['__call__']
@@ -266,7 +303,7 @@
+ fake_api.return_value = api
+ reassign_wait_data = {'reassign': [
+ {'volume': volumeNameExt(self.fakeProp['volume']),
-+ 'detach': [1]}]}
++ 'detach': [1], 'force': False}]}
+
+ self.connector.disconnect_volume(self.fakeProp, None)
+ self.assertEqual(api.volumesReassignWait.mock_calls[0],
@@ -274,7 +311,7 @@
def test_connect_exceptions(self):
"""Raise exceptions on missing connection information"""
-@@ -146,6 +142,53 @@ class StorPoolConnectorTestCase(test_connector.ConnectorTestCase):
+@@ -146,6 +167,96 @@ class StorPoolConnectorTestCase(test_connector.ConnectorTestCase):
self.assertRaises(exception.BrickException,
self.connector.disconnect_volume, c, None)
@@ -295,6 +332,7 @@
+ """Handle SP API exceptions"""
+ api = mock.MagicMock(spec=['volumesReassignWait', 'volumeInfo'])
+ api.volumesReassignWait = mock.MagicMock(spec=['__call__'])
++ # The generic exception should bypass the SP API exception handling
+ api.volumesReassignWait.side_effect = Exception()
+ api.volumeInfo = mock.MagicMock(spec=['__call__'])
+
@@ -325,6 +363,48 @@
+ self.connector.disconnect_volume, self.fakeProp,
+ None)
+
++ # Test the retry logic
++ faulty_api.fail_count = self.api_calls_retry_max - 1
++ faulty_api.real_fn = mock.MagicMock(spec=['__call__'])
++ api.volumesReassignWait = faulty_api
++ api.volumeInfo = mock.MagicMock(spec=['__call__'])
++
++ with mock.patch.object(
++ self.adb, attribute='api', spec=['__call__']
++ ) as fake_api:
++ fake_api.return_value = api
++ reassign_wait_data = {'reassign': [
++ {'volume': volumeNameExt(self.fakeProp['volume']),
++ 'detach': [1], 'force': False}]}
++
++ self.connector.disconnect_volume(self.fakeProp, None)
++ self.assertEqual(self.api_calls_retry_max,
++ len(faulty_api.real_fn.mock_calls))
++ for mock_call in faulty_api.real_fn.mock_calls:
++ self.assertEqual(mock_call, mock.call(reassign_wait_data))
++
++ faulty_api.fail_count = self.api_calls_retry_max
++ faulty_api.real_fn = mock.MagicMock(spec=['__call__'])
++ api.volumesReassignWait = faulty_api
++ api.volumeInfo = mock.MagicMock(spec=['__call__'])
++
++ with mock.patch.object(
++ self.adb, attribute='api', spec=['__call__']
++ ) as fake_api:
++ fake_api.return_value = api
++ reassign_wait_data = {'reassign': [
++ {'volume': volumeNameExt(self.fakeProp['volume']),
++ 'detach': [1], 'force': False}]}
++
++ self.connector.disconnect_volume(self.fakeProp, None)
++ self.assertEqual(self.api_calls_retry_max + 1,
++ len(faulty_api.real_fn.mock_calls))
++ for mock_call in faulty_api.real_fn.mock_calls[:-1]:
++ self.assertEqual(mock_call, mock.call(reassign_wait_data))
++ reassign_wait_data['reassign'][0]['force'] = True
++ self.assertEqual(faulty_api.real_fn.mock_calls[-1],
++ mock.call(reassign_wait_data))
++
def test_extend_volume(self):
if self.fakeConnection is None:
self.test_connect_volume()