Merge "Ensure cleanup context uses the supplied api_versions in the task"
This commit is contained in:
@@ -29,7 +29,7 @@ LOG = logging.getLogger(__name__)
|
||||
|
||||
class SeekAndDestroy(object):
|
||||
|
||||
def __init__(self, manager_cls, admin, users):
|
||||
def __init__(self, manager_cls, admin, users, api_versions=None):
|
||||
"""Resource deletion class.
|
||||
|
||||
This class contains method exterminate() that finds and deletes
|
||||
@@ -42,20 +42,24 @@ class SeekAndDestroy(object):
|
||||
self.manager_cls = manager_cls
|
||||
self.admin = admin
|
||||
self.users = users or []
|
||||
self.api_versions = api_versions
|
||||
|
||||
@staticmethod
|
||||
def _get_cached_client(user, cache=None):
|
||||
def _get_cached_client(user, cache=None, api_versions=None):
|
||||
"""Simplifies initialization and caching OpenStack clients."""
|
||||
|
||||
if not user:
|
||||
return None
|
||||
|
||||
if not isinstance(cache, dict):
|
||||
return osclients.Clients(user["credential"])
|
||||
return osclients.Clients(user["credential"], api_info=api_versions)
|
||||
|
||||
key = user["credential"]
|
||||
if api_versions:
|
||||
key = str((user["credential"], sorted(api_versions.items())))
|
||||
else:
|
||||
key = user["credential"]
|
||||
if key not in cache:
|
||||
cache[key] = osclients.Clients(key)
|
||||
cache[key] = osclients.Clients(key, api_info=api_versions)
|
||||
|
||||
return cache[key]
|
||||
|
||||
@@ -146,12 +150,16 @@ class SeekAndDestroy(object):
|
||||
if self.admin and (not self.users
|
||||
or self.manager_cls._perform_for_admin_only):
|
||||
manager = self.manager_cls(
|
||||
admin=self._get_cached_client(self.admin))
|
||||
admin=self._get_cached_client(
|
||||
self.admin,
|
||||
api_versions=self.api_versions))
|
||||
_publish(self.admin, None, manager)
|
||||
|
||||
else:
|
||||
visited_tenants = set()
|
||||
admin_client = self._get_cached_client(self.admin)
|
||||
admin_client = self._get_cached_client(
|
||||
self.admin,
|
||||
api_versions=self.api_versions)
|
||||
for user in self.users:
|
||||
if (self.manager_cls._tenant_resource
|
||||
and user["tenant_id"] in visited_tenants):
|
||||
@@ -160,7 +168,9 @@ class SeekAndDestroy(object):
|
||||
visited_tenants.add(user["tenant_id"])
|
||||
manager = self.manager_cls(
|
||||
admin=admin_client,
|
||||
user=self._get_cached_client(user),
|
||||
user=self._get_cached_client(
|
||||
user,
|
||||
api_versions=self.api_versions),
|
||||
tenant_uuid=user["tenant_id"])
|
||||
|
||||
_publish(self.admin, user, manager)
|
||||
@@ -176,8 +186,12 @@ class SeekAndDestroy(object):
|
||||
|
||||
manager = self.manager_cls(
|
||||
resource=raw_resource,
|
||||
admin=self._get_cached_client(admin, cache=cache),
|
||||
user=self._get_cached_client(user, cache=cache),
|
||||
admin=self._get_cached_client(admin,
|
||||
cache=cache,
|
||||
api_versions=self.api_versions),
|
||||
user=self._get_cached_client(user,
|
||||
cache=cache,
|
||||
api_versions=self.api_versions),
|
||||
tenant_uuid=user and user["tenant_id"])
|
||||
|
||||
self._delete_single_resource(manager)
|
||||
@@ -248,7 +262,8 @@ def find_resource_managers(names=None, admin_required=None):
|
||||
return resource_managers
|
||||
|
||||
|
||||
def cleanup(names=None, admin_required=None, admin=None, users=None):
|
||||
def cleanup(names=None, admin_required=None, admin=None, users=None,
|
||||
api_versions=None):
|
||||
"""Generic cleaner.
|
||||
|
||||
This method goes through all plugins. Filter those and left only plugins
|
||||
@@ -279,4 +294,4 @@ def cleanup(names=None, admin_required=None, admin=None, users=None):
|
||||
LOG.debug("Cleaning up %(service)s %(resource)s objects" %
|
||||
{"service": manager._service,
|
||||
"resource": manager._resource})
|
||||
SeekAndDestroy(manager, admin, users).exterminate()
|
||||
SeekAndDestroy(manager, admin, users, api_versions).exterminate()
|
||||
|
||||
@@ -64,10 +64,12 @@ class AdminCleanup(CleanupMixin, context.Context):
|
||||
|
||||
@logging.log_task_wrapper(LOG.info, _("admin resources cleanup"))
|
||||
def cleanup(self):
|
||||
manager.cleanup(names=self.config,
|
||||
admin_required=True,
|
||||
admin=self.context["admin"],
|
||||
users=self.context.get("users", []))
|
||||
manager.cleanup(
|
||||
names=self.config,
|
||||
admin_required=True,
|
||||
admin=self.context["admin"],
|
||||
users=self.context.get("users", []),
|
||||
api_versions=self.context["config"].get("api_versions"))
|
||||
|
||||
|
||||
# NOTE(amaretskiy): Set maximum order to run this last
|
||||
@@ -89,6 +91,9 @@ class UserCleanup(CleanupMixin, context.Context):
|
||||
|
||||
@logging.log_task_wrapper(LOG.info, _("user resources cleanup"))
|
||||
def cleanup(self):
|
||||
manager.cleanup(names=self.config,
|
||||
admin_required=False,
|
||||
users=self.context.get("users", []))
|
||||
manager.cleanup(
|
||||
names=self.config,
|
||||
admin_required=False,
|
||||
users=self.context.get("users", []),
|
||||
api_versions=self.context["config"].get("api_versions")
|
||||
)
|
||||
|
||||
@@ -32,6 +32,7 @@ class SeekAndDestroyTestCase(test.TestCase):
|
||||
self.assertIsNone(manager.SeekAndDestroy._get_cached_client(None))
|
||||
|
||||
users = [{"credential": "a"}, {"credential": "b"}]
|
||||
|
||||
cache = {}
|
||||
|
||||
self.assertEqual(
|
||||
@@ -46,6 +47,45 @@ class SeekAndDestroyTestCase(test.TestCase):
|
||||
manager.SeekAndDestroy._get_cached_client(users[0], cache=cache),
|
||||
manager.SeekAndDestroy._get_cached_client(users[1], cache=cache))
|
||||
|
||||
@mock.patch("%s.osclients.Clients" % BASE,
|
||||
side_effect=[mock.MagicMock(), mock.MagicMock()])
|
||||
def test__get_cached_client_with_api_versions(self, mock_clients):
|
||||
self.assertIsNone(manager.SeekAndDestroy._get_cached_client(None))
|
||||
|
||||
user = {"credential": "a"}
|
||||
api_versions = [
|
||||
{"cinder": {"version": "1", "service_type": "volume"}},
|
||||
{"cinder": {"version": "2", "service_type": "volumev2"}}
|
||||
]
|
||||
|
||||
cache = {}
|
||||
|
||||
self.assertEqual(
|
||||
manager.SeekAndDestroy._get_cached_client(
|
||||
user, cache=cache, api_versions=api_versions[0]),
|
||||
manager.SeekAndDestroy._get_cached_client(
|
||||
user, cache=cache, api_versions=api_versions[0]))
|
||||
|
||||
self.assertEqual(
|
||||
manager.SeekAndDestroy._get_cached_client(
|
||||
user,
|
||||
cache=cache,
|
||||
api_versions=api_versions[1]),
|
||||
manager.SeekAndDestroy._get_cached_client(
|
||||
user,
|
||||
cache=cache,
|
||||
api_versions=api_versions[1]))
|
||||
|
||||
self.assertNotEqual(
|
||||
manager.SeekAndDestroy._get_cached_client(
|
||||
user,
|
||||
cache=cache,
|
||||
api_versions=api_versions[0]),
|
||||
manager.SeekAndDestroy._get_cached_client(
|
||||
user,
|
||||
cache=cache,
|
||||
api_versions=api_versions[1]))
|
||||
|
||||
@mock.patch("%s.LOG" % BASE)
|
||||
def test__delete_single_resource(self, mock_log):
|
||||
mock_resource = mock.MagicMock(_max_attempts=3, _timeout=10,
|
||||
@@ -116,7 +156,8 @@ class SeekAndDestroyTestCase(test.TestCase):
|
||||
|
||||
queue = []
|
||||
publish(queue)
|
||||
mock__get_cached_client.assert_called_once_with(admin)
|
||||
mock__get_cached_client.assert_called_once_with(admin,
|
||||
api_versions=None)
|
||||
mock_mgr.assert_called_once_with(
|
||||
admin=mock__get_cached_client.return_value)
|
||||
self.assertEqual(queue, [(admin, None, x) for x in range(1, 4)])
|
||||
@@ -131,7 +172,8 @@ class SeekAndDestroyTestCase(test.TestCase):
|
||||
|
||||
queue = []
|
||||
publish(queue)
|
||||
mock__get_cached_client.assert_called_once_with(admin)
|
||||
mock__get_cached_client.assert_called_once_with(admin,
|
||||
api_versions=None)
|
||||
mock_mgr.assert_called_once_with(
|
||||
admin=mock__get_cached_client.return_value)
|
||||
self.assertEqual(queue, [(admin, None, x) for x in range(1, 4)])
|
||||
@@ -164,9 +206,9 @@ class SeekAndDestroyTestCase(test.TestCase):
|
||||
mock.call().list()
|
||||
])
|
||||
mock__get_cached_client.assert_has_calls([
|
||||
mock.call(admin),
|
||||
mock.call(users[0]),
|
||||
mock.call(users[1])
|
||||
mock.call(admin, api_versions=None),
|
||||
mock.call(users[0], api_versions=None),
|
||||
mock.call(users[1], api_versions=None)
|
||||
])
|
||||
expected_queue = [(admin, users[0], x) for x in range(1, 4)]
|
||||
expected_queue += [(admin, users[1], x) for x in range(4, 6)]
|
||||
@@ -202,9 +244,9 @@ class SeekAndDestroyTestCase(test.TestCase):
|
||||
mock.call().list()
|
||||
])
|
||||
mock__get_cached_client.assert_has_calls([
|
||||
mock.call(None),
|
||||
mock.call(users[0]),
|
||||
mock.call(users[2])
|
||||
mock.call(None, api_versions=None),
|
||||
mock.call(users[0], api_versions=None),
|
||||
mock.call(users[2], api_versions=None)
|
||||
])
|
||||
self.assertEqual(queue, [(None, users[0], x) for x in range(1, 4)])
|
||||
|
||||
@@ -227,8 +269,8 @@ class SeekAndDestroyTestCase(test.TestCase):
|
||||
user=mock__get_cached_client.return_value,
|
||||
tenant_uuid=user1["tenant_id"])
|
||||
mock__get_cached_client.assert_has_calls([
|
||||
mock.call(admin, cache=cache),
|
||||
mock.call(user1, cache=cache)
|
||||
mock.call(admin, cache=cache, api_versions=None),
|
||||
mock.call(user1, cache=cache, api_versions=None)
|
||||
])
|
||||
mock__delete_single_resource.assert_called_once_with(
|
||||
mock_mgr.return_value)
|
||||
@@ -245,8 +287,8 @@ class SeekAndDestroyTestCase(test.TestCase):
|
||||
tenant_uuid=None)
|
||||
|
||||
mock__get_cached_client.assert_has_calls([
|
||||
mock.call(admin, cache=cache),
|
||||
mock.call(None, cache=cache)
|
||||
mock.call(admin, cache=cache, api_versions=None),
|
||||
mock.call(None, cache=cache, api_versions=None)
|
||||
])
|
||||
mock__delete_single_resource.assert_called_once_with(
|
||||
mock_mgr.return_value)
|
||||
@@ -344,11 +386,42 @@ class ResourceManagerTestCase(test.TestCase):
|
||||
|
||||
mock_seek_and_destroy.assert_has_calls([
|
||||
mock.call(
|
||||
mock_find_resource_managers.return_value[0], "admin", ["user"]
|
||||
mock_find_resource_managers.return_value[0], "admin",
|
||||
["user"], None
|
||||
),
|
||||
mock.call().exterminate(),
|
||||
mock.call(
|
||||
mock_find_resource_managers.return_value[1], "admin", ["user"]
|
||||
mock_find_resource_managers.return_value[1], "admin",
|
||||
["user"], None
|
||||
),
|
||||
mock.call().exterminate()
|
||||
])
|
||||
|
||||
@mock.patch("%s.SeekAndDestroy" % BASE)
|
||||
@mock.patch("%s.find_resource_managers" % BASE,
|
||||
return_value=[mock.MagicMock(), mock.MagicMock()])
|
||||
def test_cleanup_with_api_versions(self,
|
||||
mock_find_resource_managers,
|
||||
mock_seek_and_destroy):
|
||||
manager.cleanup(names=["a", "b"], admin_required=True,
|
||||
admin="admin", users=["user"],
|
||||
api_versions={"cinder": {
|
||||
"version": "1", "service_type": "volume"
|
||||
}})
|
||||
|
||||
mock_find_resource_managers.assert_called_once_with(["a", "b"], True)
|
||||
|
||||
mock_seek_and_destroy.assert_has_calls([
|
||||
mock.call(
|
||||
mock_find_resource_managers.return_value[0], "admin",
|
||||
["user"],
|
||||
{"cinder": {"service_type": "volume", "version": "1"}}
|
||||
),
|
||||
mock.call().exterminate(),
|
||||
mock.call(
|
||||
mock_find_resource_managers.return_value[1], "admin",
|
||||
["user"],
|
||||
{"cinder": {"service_type": "volume", "version": "1"}}
|
||||
),
|
||||
mock.call().exterminate()
|
||||
])
|
||||
|
||||
Reference in New Issue
Block a user