From 0421d123971f28a1444c1015d32decdc13903f0f Mon Sep 17 00:00:00 2001 From: Adam Harwell Date: Thu, 19 Sep 2019 22:02:02 -0700 Subject: [PATCH] Modernize testing and clients Update to use a new-style zuul role for devstack testing. Remove references to Shade/os_client_config and use OpenstackSDK. Change-Id: Ieecc4043b613e333948c32e7e93ad1d383d1f3ec --- .zuul.yaml | 12 +- README.rst | 14 +- ospurge/main.py | 82 +++++------- ospurge/resources/base.py | 6 +- ospurge/tests/resources/test_cinder.py | 8 +- ospurge/tests/resources/test_designate.py | 4 +- ospurge/tests/resources/test_glance.py | 6 +- ospurge/tests/resources/test_heat.py | 4 +- ospurge/tests/resources/test_neutron.py | 14 +- ospurge/tests/resources/test_nova.py | 4 +- ospurge/tests/resources/test_swift.py | 9 +- ospurge/tests/test_main.py | 153 +++++++++++++--------- ospurge/tests/test_utils.py | 55 +------- ospurge/utils.py | 42 ------ playbooks/ospurge-functional/post.yaml | 15 --- playbooks/ospurge-functional/run.yaml | 53 -------- requirements.txt | 3 +- tools/func-tests.sh | 5 +- tools/populate.sh | 4 +- tools/post_test_hook.sh | 37 ------ 20 files changed, 177 insertions(+), 353 deletions(-) delete mode 100644 playbooks/ospurge-functional/post.yaml delete mode 100644 playbooks/ospurge-functional/run.yaml delete mode 100755 tools/post_test_hook.sh diff --git a/.zuul.yaml b/.zuul.yaml index aedd338..97ceaec 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -1,14 +1,20 @@ - job: name: ospurge-functional - parent: legacy-dsvm-base - run: playbooks/ospurge-functional/run.yaml - post-run: playbooks/ospurge-functional/post.yaml + parent: devstack-tox-functional-consumer timeout: 3600 required-projects: - openstack/devstack-gate - x/ospurge - openstack/designate - openstack/python-designateclient + roles: + - zuul: openstack-infra/devstack + vars: + devstack_plugins: + designate: https://opendev.org/openstack/designate + devstack_localrc: + DESIGNATE_SERVICE_PORT_DNS: 5322 + tox_envlist: functional irrelevant-files: &dsvm-irrelevant-files - ^(test-|)requirements.txt$ - ^.*\.rst$ diff --git a/README.rst b/README.rst index adfeed1..580fb51 100644 --- a/README.rst +++ b/README.rst @@ -293,7 +293,7 @@ How to contribute ----------------- OSPurge is hosted on the OpenStack infrastructure and is using -`Gerrit `_ to +`Gerrit `_ to manage contributions. You can contribute to the project by following the `OpenStack Development workflow `_. @@ -301,21 +301,19 @@ Start hacking right away with: .. code-block:: console - $ git clone https://git.openstack.org/openstack/ospurge + $ git clone https://opendev.org/x/ospurge Design decisions ---------------- -* OSPurge depends on `os-client-config`_ to manage authentication. This way, +* OSPurge depends on `openstacksdk`_ to manage authentication. This way, environment variables (OS_*) and CLI options are properly handled. -* OSPurge is built on top of `shade`_. shade is a simple client library for - interacting with OpenStack clouds. With shade, OSPurge can focus on the +* OSPurge is built on top of `openstacksdk`_. OpenstackSDK is a client library + for interacting with OpenStack clouds. With the SDK, OSPurge can focus on the cleaning resources logic and not on properly building the various Python OpenStack clients and dealing with their not-so-intuitive API. -.. _shade: https://github.com/openstack-infra/shade/ -.. _os-client-config: https://github.com/openstack/os-client-config - +.. _openstacksdk: https://github.com/openstack/openstacksdk diff --git a/ospurge/main.py b/ospurge/main.py index 0809887..b747754 100644 --- a/ospurge/main.py +++ b/ospurge/main.py @@ -18,12 +18,10 @@ import sys import threading import typing +from openstack.config import loader from openstack import connection from openstack import exceptions as os_exceptions -import os_client_config -import shade - from ospurge import exceptions from ospurge import utils @@ -70,6 +68,10 @@ def create_argument_parser(): help="Purge only the specified resource type. Repeat to delete " "several types at once." ) + parser.add_argument( + "--os-identity-api-version", default=3, + help="Identity API version, default=3" + ) group = parser.add_mutually_exclusive_group(required=True) group.add_argument( @@ -93,55 +95,40 @@ class CredentialsManager(object): self.revoke_role_after_purge = False self.disable_project_after_purge = False - self.cloud = None # type: Optional[shade.OpenStackCloud] - self.connection = None # type: Optional[connection.Connection] - - if options.purge_own_project: - self.cloud = shade.openstack_cloud(argparse=options, config=config) - self.user_id = self.cloud.keystone_session.get_user_id() - self.project_id = self.cloud.keystone_session.get_project_id() - else: - self.connection = connection.Connection( - config=config.get_one(argparse=options) - ) - self.user_id = self.connection.identity.get_user_id() + self.cloud = connection.Connection( + config=config.get_one(argparse=options) + ) + self.admin_cloud = None # type: Optional[connection.Connection] + if not options.purge_own_project: try: # Only admins can do that. - project = self.connection.identity.get_tenant( - options.purge_project) - except ( - os_exceptions.ResourceNotFound, os_exceptions.NotFoundException - ): + project = self.cloud.get_project(options.purge_project) + if not project: + raise os_exceptions.SDKException() + # If project is not enabled, we must disable it after purge. + self.disable_project_after_purge = not project.is_enabled + except os_exceptions.SDKException: raise exceptions.OSProjectNotFound( "Unable to find project '{}'".format(options.purge_project) ) - self.project_id = project.id + self.admin_cloud = self.cloud + self.cloud = self.admin_cloud.connect_as_project( + options.purge_project) - # If project is not enabled, we must disable it after purge. - self.disable_project_after_purge = not project.is_enabled + self.user_id = self.cloud.current_user_id + self.project_id = self.cloud.current_project_id - # Reuse the information passed to get the `OperatorCloud` but - # change the project. This way we bind/re-scope to the project - # we want to purge, not the project we authenticated to. - self.cloud = shade.openstack_cloud( - **utils.replace_project_info( - self.connection.config.config, - self.project_id - ) - ) - - auth_args = self.cloud.cloud_config.get_auth_args() logging.warning( "Going to list and/or delete resources from project '%s'", - options.purge_project or auth_args.get('project_name') - or auth_args.get('project_id') + options.purge_project or self.cloud.current_project.name + or self.cloud.current_project_id ) def ensure_role_on_project(self): - if self.connection and self.connection.grant_role( - self.options.admin_role_name, - project=self.options.purge_project, user=self.user_id + if self.admin_cloud and self.admin_cloud.grant_role( + self.options.admin_role_name, + project=self.options.purge_project, user=self.user_id ): logging.warning( "Role 'Member' granted to user '%s' on project '%s'", @@ -150,7 +137,7 @@ class CredentialsManager(object): self.revoke_role_after_purge = True def revoke_role_on_project(self): - self.connection.revoke_role( + self.admin_cloud.revoke_role( self.options.admin_role_name, user=self.user_id, project=self.options.purge_project) logging.warning( @@ -159,13 +146,13 @@ class CredentialsManager(object): ) def ensure_enabled_project(self): - if self.connection and self.disable_project_after_purge: - self.connection.update_project(self.project_id, enabled=True) + if self.admin_cloud and self.disable_project_after_purge: + self.admin_cloud.update_project(self.project_id, enabled=True) logging.warning("Project '%s' was disabled before purge and it is " "now enabled", self.options.purge_project) def disable_project(self): - self.connection.update_project(self.project_id, enabled=False) + self.admin_cloud.update_project(self.project_id, enabled=False) logging.warning("Project '%s' was disabled before purge and it is " "now also disabled", self.options.purge_project) @@ -191,11 +178,7 @@ def runner(resource_mngr, options, exit): # If we want to delete only specific resources, many things # can go wrong, so we basically ignore all exceptions. - if options.resource: - exc = shade.OpenStackCloudException - else: - exc = shade.OpenStackCloudResourceNotFound - + exc = os_exceptions.OpenStackCloudException utils.call_and_ignore_exc(exc, resource_mngr.delete, resource) except Exception as exc: @@ -222,11 +205,10 @@ def runner(resource_mngr, options, exit): exit.set() -@utils.monkeypatch_oscc_logging_warning def main(): parser = create_argument_parser() - cloud_config = os_client_config.OpenStackConfig() + cloud_config = loader.OpenStackConfig() cloud_config.register_argparse_arguments(parser, sys.argv) options = parser.parse_args() diff --git a/ospurge/resources/base.py b/ospurge/resources/base.py index f548e1c..a2da40f 100644 --- a/ospurge/resources/base.py +++ b/ospurge/resources/base.py @@ -25,7 +25,7 @@ from ospurge import exceptions if TYPE_CHECKING: # pragma: no cover import argparse # noqa: F401 - import shade # noqa: F401 + import openstack.connection as os_connection # noqa: F401 from typing import Optional # noqa: F401 @@ -90,7 +90,7 @@ else: # pragma: no cover here class BaseServiceResource(object): def __init__(self): self.cleanup_project_id = None # type: Optional[str] - self.cloud = None # type: Optional[shade.OpenStackCloud] + self.cloud = None # type: Optional[os_connection.Connection] self.options = None # type: Optional[argparse.Namespace] @@ -126,7 +126,7 @@ class ServiceResource(six.with_metaclass(CodingStyleMixin, if project_id: return project_id == self.cleanup_project_id else: - # Uncomment the following line once Shade and all OpenStack + # Uncomment the following line once the SDK and all OpenStack # services returns the resource owner. In the mean time no need # to be worrying. # logging.warning("Can't determine owner of resource %s", resource) diff --git a/ospurge/tests/resources/test_cinder.py b/ospurge/tests/resources/test_cinder.py index 1a36495..07dafbc 100644 --- a/ospurge/tests/resources/test_cinder.py +++ b/ospurge/tests/resources/test_cinder.py @@ -11,7 +11,7 @@ # under the License. import unittest -import shade +import openstack.connection from ospurge.resources import cinder from ospurge.tests import mock @@ -19,7 +19,7 @@ from ospurge.tests import mock class TestBackups(unittest.TestCase): def setUp(self): - self.cloud = mock.Mock(spec_set=shade.openstackcloud.OpenStackCloud) + self.cloud = mock.Mock(spec_set=openstack.connection.Connection) self.creds_manager = mock.Mock(cloud=self.cloud) def test_list(self): @@ -40,7 +40,7 @@ class TestBackups(unittest.TestCase): class TestSnapshots(unittest.TestCase): def setUp(self): - self.cloud = mock.Mock(spec_set=shade.openstackcloud.OpenStackCloud) + self.cloud = mock.Mock(spec_set=openstack.connection.Connection) self.creds_manager = mock.Mock(cloud=self.cloud) def test_list(self): @@ -63,7 +63,7 @@ class TestSnapshots(unittest.TestCase): class TestVolumes(unittest.TestCase): def setUp(self): - self.cloud = mock.Mock(spec_set=shade.openstackcloud.OpenStackCloud) + self.cloud = mock.Mock(spec_set=openstack.connection.Connection) self.creds_manager = mock.Mock(cloud=self.cloud, project_id=42) def test_check_prerequisite(self): diff --git a/ospurge/tests/resources/test_designate.py b/ospurge/tests/resources/test_designate.py index 72b15b1..b7e7dea 100644 --- a/ospurge/tests/resources/test_designate.py +++ b/ospurge/tests/resources/test_designate.py @@ -11,7 +11,7 @@ # under the License. import unittest -import shade +import openstack.connection from ospurge.resources import designate from ospurge.tests import mock @@ -19,7 +19,7 @@ from ospurge.tests import mock class TestZones(unittest.TestCase): def setUp(self): - self.cloud = mock.Mock(spec_set=shade.openstackcloud.OpenStackCloud) + self.cloud = mock.Mock(spec_set=openstack.connection.Connection) self.creds_manager = mock.Mock(cloud=self.cloud) def test_list_without_service(self): diff --git a/ospurge/tests/resources/test_glance.py b/ospurge/tests/resources/test_glance.py index 504fc11..0eb66b8 100644 --- a/ospurge/tests/resources/test_glance.py +++ b/ospurge/tests/resources/test_glance.py @@ -11,7 +11,7 @@ # under the License. import unittest -import shade +import openstack.connection from ospurge.resources import glance from ospurge.tests import mock @@ -19,7 +19,7 @@ from ospurge.tests import mock class TestListImagesMixin(unittest.TestCase): def setUp(self): - self.cloud = mock.Mock(spec_set=shade.openstackcloud.OpenStackCloud) + self.cloud = mock.Mock(spec_set=openstack.connection.Connection) self.img_lister = glance.ListImagesMixin() self.img_lister.cloud = self.cloud self.img_lister.cleanup_project_id = 42 @@ -53,7 +53,7 @@ class TestListImagesMixin(unittest.TestCase): class TestImages(unittest.TestCase): def setUp(self): - self.cloud = mock.Mock(spec_set=shade.openstackcloud.OpenStackCloud) + self.cloud = mock.Mock(spec_set=openstack.connection.Connection) self.creds_manager = mock.Mock(cloud=self.cloud, project_id=42) @mock.patch.object(glance.ListImagesMixin, 'list_images_by_owner') diff --git a/ospurge/tests/resources/test_heat.py b/ospurge/tests/resources/test_heat.py index d21af8f..5e20444 100644 --- a/ospurge/tests/resources/test_heat.py +++ b/ospurge/tests/resources/test_heat.py @@ -11,7 +11,7 @@ # under the License. import unittest -import shade +import openstack.connection from ospurge.resources import heat from ospurge.tests import mock @@ -19,7 +19,7 @@ from ospurge.tests import mock class TestStacks(unittest.TestCase): def setUp(self): - self.cloud = mock.Mock(spec_set=shade.openstackcloud.OpenStackCloud) + self.cloud = mock.Mock(spec_set=openstack.connection.Connection) self.creds_manager = mock.Mock(cloud=self.cloud) def test_list_without_service(self): diff --git a/ospurge/tests/resources/test_neutron.py b/ospurge/tests/resources/test_neutron.py index d33b8a4..203111f 100644 --- a/ospurge/tests/resources/test_neutron.py +++ b/ospurge/tests/resources/test_neutron.py @@ -11,7 +11,7 @@ # under the License. import unittest -import shade +import openstack.connection from ospurge.resources import neutron from ospurge.tests import mock @@ -19,7 +19,7 @@ from ospurge.tests import mock class TestFloatingIPs(unittest.TestCase): def setUp(self): - self.cloud = mock.Mock(spec_set=shade.openstackcloud.OpenStackCloud) + self.cloud = mock.Mock(spec_set=openstack.connection.Connection) self.creds_manager = mock.Mock(cloud=self.cloud) def test_check_prerequisite(self): @@ -55,7 +55,7 @@ class TestFloatingIPs(unittest.TestCase): class TestRouterInterfaces(unittest.TestCase): def setUp(self): - self.cloud = mock.Mock(spec_set=shade.openstackcloud.OpenStackCloud) + self.cloud = mock.Mock(spec_set=openstack.connection.Connection) self.creds_manager = mock.Mock(cloud=self.cloud) def test_check_prerequisite(self): @@ -103,7 +103,7 @@ class TestRouterInterfaces(unittest.TestCase): class TestRouters(unittest.TestCase): def setUp(self): - self.cloud = mock.Mock(spec_set=shade.openstackcloud.OpenStackCloud) + self.cloud = mock.Mock(spec_set=openstack.connection.Connection) self.creds_manager = mock.Mock(cloud=self.cloud) def test_check_prerequisite(self): @@ -139,7 +139,7 @@ class TestRouters(unittest.TestCase): class TestPorts(unittest.TestCase): def setUp(self): - self.cloud = mock.Mock(spec_set=shade.openstackcloud.OpenStackCloud) + self.cloud = mock.Mock(spec_set=openstack.connection.Connection) self.creds_manager = mock.Mock(cloud=self.cloud) def test_list(self): @@ -167,7 +167,7 @@ class TestPorts(unittest.TestCase): class TestNetworks(unittest.TestCase): def setUp(self): - self.cloud = mock.Mock(spec_set=shade.openstackcloud.OpenStackCloud) + self.cloud = mock.Mock(spec_set=openstack.connection.Connection) self.creds_manager = mock.Mock(cloud=self.cloud) def test_check_prerequisite(self): @@ -211,7 +211,7 @@ class TestNetworks(unittest.TestCase): class TestSecurityGroups(unittest.TestCase): def setUp(self): - self.cloud = mock.Mock(spec_set=shade.openstackcloud.OpenStackCloud) + self.cloud = mock.Mock(spec_set=openstack.connection.Connection) self.creds_manager = mock.Mock(cloud=self.cloud) def test_list(self): diff --git a/ospurge/tests/resources/test_nova.py b/ospurge/tests/resources/test_nova.py index 06019a7..03488ff 100644 --- a/ospurge/tests/resources/test_nova.py +++ b/ospurge/tests/resources/test_nova.py @@ -11,7 +11,7 @@ # under the License. import unittest -import shade +import openstack.connection from ospurge.resources import nova from ospurge.tests import mock @@ -19,7 +19,7 @@ from ospurge.tests import mock class TestServers(unittest.TestCase): def setUp(self): - self.cloud = mock.Mock(spec_set=shade.openstackcloud.OpenStackCloud) + self.cloud = mock.Mock(spec_set=openstack.connection.Connection) self.creds_manager = mock.Mock(cloud=self.cloud) def test_list(self): diff --git a/ospurge/tests/resources/test_swift.py b/ospurge/tests/resources/test_swift.py index e0a735c..0fb3968 100644 --- a/ospurge/tests/resources/test_swift.py +++ b/ospurge/tests/resources/test_swift.py @@ -11,17 +11,16 @@ # under the License. import unittest +import openstack.connection from six.moves import urllib_parse -import shade - from ospurge.resources import swift from ospurge.tests import mock class TestListObjectsMixin(unittest.TestCase): def setUp(self): - self.cloud = mock.Mock(spec_set=shade.openstackcloud.OpenStackCloud) + self.cloud = mock.Mock(spec_set=openstack.connection.Connection) self.obj_lister = swift.ListObjectsMixin() self.obj_lister.cloud = self.cloud @@ -48,7 +47,7 @@ class TestListObjectsMixin(unittest.TestCase): class TestObjects(unittest.TestCase): def setUp(self): - self.cloud = mock.Mock(spec_set=shade.openstackcloud.OpenStackCloud) + self.cloud = mock.Mock(spec_set=openstack.connection.Connection) self.creds_manager = mock.Mock(cloud=self.cloud) def test_check_prerequisite(self): @@ -98,7 +97,7 @@ class TestObjects(unittest.TestCase): class TestContainers(unittest.TestCase): def setUp(self): - self.cloud = mock.Mock(spec_set=shade.openstackcloud.OpenStackCloud) + self.cloud = mock.Mock(spec_set=openstack.connection.Connection) self.creds_manager = mock.Mock(cloud=self.cloud) @mock.patch('ospurge.resources.swift.ListObjectsMixin.list_objects') diff --git a/ospurge/tests/test_main.py b/ospurge/tests/test_main.py index 07690e5..e146e3f 100644 --- a/ospurge/tests/test_main.py +++ b/ospurge/tests/test_main.py @@ -14,15 +14,12 @@ import logging import types import unittest -import shade.exc - from openstack import exceptions as os_exceptions from ospurge import exceptions from ospurge import main from ospurge.resources.base import ServiceResource from ospurge.tests import mock -from ospurge import utils try: @@ -129,7 +126,7 @@ class TestFunctions(unittest.TestCase): def test_runner_with_recoverable_exception(self): class MyEndpointNotFound(Exception): pass - exc = shade.exc.OpenStackCloudException("") + exc = os_exceptions.OpenStackCloudException("") exc.inner_exception = (MyEndpointNotFound, ) resource_manager = mock.Mock(list=mock.Mock(side_effect=exc)) exit = mock.Mock() @@ -145,19 +142,18 @@ class TestFunctions(unittest.TestCase): self.assertFalse(exit.set.called) @mock.patch.object(main, 'connection') - @mock.patch.object(main, 'os_client_config', autospec=True) - @mock.patch.object(main, 'shade') + @mock.patch.object(main, 'loader', autospec=True) @mock.patch('argparse.ArgumentParser.parse_args') @mock.patch('threading.Event', autospec=True) @mock.patch('concurrent.futures.ThreadPoolExecutor', autospec=True) @mock.patch('sys.exit', autospec=True) - def test_main(self, m_sys_exit, m_tpe, m_event, m_parse_args, m_shade, + def test_main(self, m_sys_exit, m_tpe, m_event, m_parse_args, m_oscc, m_conn): m_tpe.return_value.__enter__.return_value.map.side_effect = \ KeyboardInterrupt m_parse_args.return_value.purge_own_project = False m_parse_args.return_value.resource = None - m_conn.Connection().identity.get_tenant().is_enabled = False + m_conn.Connection().get_project().is_enabled = False main.main() @@ -181,14 +177,13 @@ class TestFunctions(unittest.TestCase): self.assertIsInstance(m_sys_exit.call_args[0][0], int) @mock.patch.object(main, 'connection') - @mock.patch.object(main, 'os_client_config', autospec=True) - @mock.patch.object(main, 'shade') + @mock.patch.object(main, 'loader', autospec=True) @mock.patch('argparse.ArgumentParser.parse_args') @mock.patch('threading.Event', autospec=True) @mock.patch('concurrent.futures.ThreadPoolExecutor', autospec=True) @mock.patch('sys.exit', autospec=True) def test_main_resource(self, m_sys_exit, m_tpe, m_event, m_parse_args, - m_shade, m_oscc, m_conn): + m_oscc, m_conn): m_tpe.return_value.__enter__.return_value.map.side_effect = \ KeyboardInterrupt m_parse_args.return_value.purge_own_project = False @@ -202,86 +197,88 @@ class TestFunctions(unittest.TestCase): self.assertIsInstance(obj, ServiceResource) -@mock.patch.object(main, 'os_client_config') +@mock.patch.object(main, 'loader') @mock.patch.object(main, 'connection') -@mock.patch.object(main, 'shade') class TestCredentialsManager(unittest.TestCase): - def test_init_with_purge_own_project(self, m_shade, m_conn, m_osc): + def test_init_with_purge_own_project(self, m_conn, m_osc): _options = SimpleNamespace( purge_own_project=True, purge_project=None) _config = m_osc.OpenStackConfig() - creds_mgr = main.CredentialsManager(_options, _config) + creds_manager = main.CredentialsManager(_options, _config) - self.assertEqual(_options, creds_mgr.options) - self.assertEqual(False, creds_mgr.revoke_role_after_purge) - self.assertEqual(False, creds_mgr.disable_project_after_purge) - self.assertIsNone(creds_mgr.connection) + self.assertEqual(_options, creds_manager.options) + self.assertEqual(False, creds_manager.revoke_role_after_purge) + self.assertEqual(False, creds_manager.disable_project_after_purge) + self.assertEqual(m_conn.Connection.return_value, + creds_manager.cloud) - m_shade.openstack_cloud.assert_called_once_with( - argparse=_options, config=_config) - self.assertEqual(m_shade.openstack_cloud.return_value, - creds_mgr.cloud) + m_conn.Connection.assert_called_once_with( + config=_config.get_one()) + self.assertEqual(m_conn.Connection.return_value, + creds_manager.cloud) self.assertEqual( - creds_mgr.cloud.keystone_session.get_user_id(), - creds_mgr.user_id + creds_manager.cloud.current_user_id, + creds_manager.user_id ) self.assertEqual( - creds_mgr.cloud.keystone_session.get_project_id(), - creds_mgr.project_id + creds_manager.cloud.current_project_id, + creds_manager.project_id ) - creds_mgr.cloud.cloud_config.get_auth_args.assert_called_once_with() - - @mock.patch.object(utils, 'replace_project_info') - def test_init_with_purge_project(self, m_replace, m_shade, m_conn, m_osc): + def test_init_with_purge_project(self, m_conn, m_osc): _config = m_osc.OpenStackConfig() _options = SimpleNamespace( purge_own_project=False, purge_project=mock.sentinel.purge_project) - creds_mgr = main.CredentialsManager(_options, _config) + creds_manager = main.CredentialsManager(_options, _config) m_conn.Connection.assert_called_once_with(config=_config.get_one()) - self.assertEqual(m_conn.Connection.return_value, - creds_mgr.connection) - - creds_mgr.connection.identity.get_tenant.assert_called_once_with( + m_conn.Connection().get_project.assert_called_once_with( _options.purge_project) self.assertEqual( - creds_mgr.connection.identity.get_user_id.return_value, - creds_mgr.user_id + m_conn.Connection.return_value, + creds_manager.admin_cloud ) self.assertEqual( - creds_mgr.connection.identity.get_tenant().id, - creds_mgr.project_id + m_conn.Connection().connect_as_project.return_value, + creds_manager.cloud ) - self.assertFalse(creds_mgr.disable_project_after_purge) - self.assertEqual( - m_shade.openstack_cloud.return_value, - creds_mgr.cloud - ) - m_replace.assert_called_once_with( - creds_mgr.connection.config.config, - creds_mgr.project_id - ) - creds_mgr.cloud.cloud_config.get_auth_args.assert_called_once_with() - def test_init_with_project_not_found(self, m_shade, m_conn, m_osc): - m_conn.Connection.return_value.identity.get_tenant\ - .side_effect = os_exceptions.NotFoundException + self.assertEqual( + creds_manager.cloud.current_user_id, + creds_manager.user_id + ) + self.assertEqual( + creds_manager.cloud.current_project_id, + creds_manager.project_id + ) + self.assertFalse(creds_manager.disable_project_after_purge) + + def test_init_with_project_not_found(self, m_conn, m_osc): + m_conn.Connection().get_project.return_value = None self.assertRaises( exceptions.OSProjectNotFound, main.CredentialsManager, mock.Mock(purge_own_project=False), m_osc.OpenStackConfig() ) - def test_ensure_role_on_project(self, m_shade, m_conn, m_osc): + def test_ensure_role_on_project(self, m_conn, m_osc): options = mock.Mock(purge_own_project=False) config = m_osc.OpenStackConfig() creds_manager = main.CredentialsManager(options, config) creds_manager.ensure_role_on_project() - m_conn.Connection.return_value.grant_role.assert_called_once_with( + self.assertEqual( + m_conn.Connection.return_value, + creds_manager.admin_cloud + ) + self.assertEqual( + m_conn.Connection().connect_as_project.return_value, + creds_manager.cloud + ) + + creds_manager.admin_cloud.grant_role.assert_called_once_with( options.admin_role_name, project=options.purge_project, user=mock.ANY) self.assertEqual(True, creds_manager.revoke_role_after_purge) @@ -293,24 +290,45 @@ class TestCredentialsManager(unittest.TestCase): creds_manager.ensure_role_on_project() self.assertEqual(False, creds_manager.revoke_role_after_purge) - def test_revoke_role_on_project(self, m_shade, m_conn, m_osc): + def test_revoke_role_on_project(self, m_conn, m_osc): options = mock.Mock(purge_own_project=False) config = m_osc.OpenStackConfig() creds_manager = main.CredentialsManager(options, config) creds_manager.revoke_role_on_project() - m_conn.Connection().revoke_role.assert_called_once_with( + self.assertEqual( + m_conn.Connection.return_value, + creds_manager.admin_cloud + ) + self.assertEqual( + m_conn.Connection().connect_as_project.return_value, + creds_manager.cloud + ) + + creds_manager.admin_cloud.revoke_role.assert_called_once_with( options.admin_role_name, project=options.purge_project, user=mock.ANY) - def test_ensure_enabled_project(self, m_shade, m_conn, m_osc): - m_conn.Connection().identity.get_tenant().is_enabled = False + def test_ensure_enabled_project(self, m_conn, m_osc): + m_conn.Connection().get_project().is_enabled = False + options = mock.Mock(purge_own_project=False) creds_manager = main.CredentialsManager( - mock.Mock(purge_own_project=False), m_osc.OpenStackConfig()) + options, m_osc.OpenStackConfig()) creds_manager.ensure_enabled_project() + self.assertEqual( + m_conn.Connection.return_value, + creds_manager.admin_cloud + ) + self.assertEqual( + m_conn.Connection().connect_as_project.return_value, + creds_manager.cloud + ) + self.assertEqual(True, creds_manager.disable_project_after_purge) - m_conn.Connection().update_project.assert_called_once_with( + m_conn.Connection().connect_as_project.assert_called_once_with( + options.purge_project) + creds_manager.admin_cloud.update_project.assert_called_once_with( mock.ANY, enabled=True) # If project is enabled before purge, no need to disable it after @@ -321,12 +339,23 @@ class TestCredentialsManager(unittest.TestCase): self.assertEqual(False, creds_manager.disable_project_after_purge) self.assertEqual(1, m_conn.Connection().update_project.call_count) - def test_disable_project(self, m_shade, m_conn, m_osc): + def test_disable_project(self, m_conn, m_osc): options = mock.Mock(purge_own_project=False) config = m_osc.OpenStackConfig() creds_manager = main.CredentialsManager(options, config) creds_manager.disable_project() + self.assertEqual( + m_conn.Connection.return_value, + creds_manager.admin_cloud + ) + self.assertEqual( + m_conn.Connection().connect_as_project.return_value, + creds_manager.cloud + ) + + m_conn.Connection().connect_as_project.assert_called_once_with( + options.purge_project) m_conn.Connection().update_project.assert_called_once_with( mock.ANY, enabled=False ) diff --git a/ospurge/tests/test_utils.py b/ospurge/tests/test_utils.py index 5ba066e..0818082 100644 --- a/ospurge/tests/test_utils.py +++ b/ospurge/tests/test_utils.py @@ -9,18 +9,15 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. -import logging import os +import pkg_resources import types import typing import unittest -import pkg_resources - +from openstack import exceptions as os_exceptions import six -import shade - from ospurge.resources.base import ServiceResource from ospurge.tests import mock from ospurge import utils @@ -42,27 +39,6 @@ def register_test_entry_point(): class TestUtils(unittest.TestCase): - def test_replace_project_info_in_config(self): - config = { - 'cloud': 'foo', - 'auth': { - 'project_name': 'bar' - } - } - new_conf = utils.replace_project_info( - config, mock.sentinel.project) - - self.assertEqual(new_conf, { - 'auth': { - 'project_id': mock.sentinel.project - } - }) - self.assertEqual(config, { - 'cloud': 'foo', - 'auth': { - 'project_name': 'bar' - } - }) def test_load_ospurge_resource_modules(self): modules = utils.load_ospurge_resource_modules() @@ -100,36 +76,15 @@ class TestUtils(unittest.TestCase): def test_call_and_ignore_notfound(self): def raiser(): - raise shade.exc.OpenStackCloudResourceNotFound("") + raise os_exceptions.OpenStackCloudException("") self.assertIsNone( utils.call_and_ignore_exc( - shade.exc.OpenStackCloudResourceNotFound, raiser + os_exceptions.OpenStackCloudException, raiser ) ) m = mock.Mock() utils.call_and_ignore_exc( - shade.exc.OpenStackCloudResourceNotFound, m, 42) + os_exceptions.OpenStackCloudException, m, 42) self.assertEqual([mock.call(42)], m.call_args_list) - - @mock.patch('logging.getLogger', autospec=True) - def test_monkeypatch_oscc_logging_warning(self, mock_getLogger): - oscc_target = 'os_client_config.cloud_config' - m_oscc_logger, m_other_logger = mock.Mock(), mock.Mock() - - mock_getLogger.side_effect = \ - lambda m: m_oscc_logger if m == oscc_target else m_other_logger - - @utils.monkeypatch_oscc_logging_warning - def f(): - logging.getLogger(oscc_target).warning("foo") - logging.getLogger(oscc_target).warning("!catalog entry not found!") - logging.getLogger("other").warning("!catalog entry not found!") - - f() - - self.assertEqual([mock.call.warning('foo'), ], - m_oscc_logger.mock_calls) - self.assertEqual([mock.call.warning('!catalog entry not found!')], - m_other_logger.mock_calls) diff --git a/ospurge/utils.py b/ospurge/utils.py index b9f4dda..b4f1a7a 100644 --- a/ospurge/utils.py +++ b/ospurge/utils.py @@ -9,8 +9,6 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. -import copy -import functools import importlib import logging import os @@ -70,48 +68,8 @@ def get_resource_classes(resources=None): return [c for c in all_classes if regex.match(c.__name__)] -def monkeypatch_oscc_logging_warning(f): - """ - Monkey-patch logging.warning() method to silence 'os_client_config' when - it complains that a Keystone catalog entry is not found. This warning - benignly happens when, for instance, we try to cleanup a Neutron resource - but Neutron is not available on the target cloud environment. - """ - oscc_target = 'os_client_config.cloud_config' - orig_logging = logging.getLogger(oscc_target).warning - - def logging_warning(msg, *args, **kwargs): - if 'catalog entry not found' not in msg: - orig_logging(msg, *args, **kwargs) - - @functools.wraps(f) - def wrapper(*args, **kwargs): - try: - setattr(logging.getLogger(oscc_target), 'warning', logging_warning) - return f(*args, **kwargs) - finally: - setattr(logging.getLogger(oscc_target), 'warning', orig_logging) - - return wrapper - - def call_and_ignore_exc(exc, f, *args): try: f(*args) except exc as e: logging.debug("The following exception was ignored: %r", e) - - -def replace_project_info(config, new_project_id): - """ - Replace all tenant/project info in a `os_client_config` config dict with - a new project. This is used to bind/scope to another project. - """ - new_conf = copy.deepcopy(config) - new_conf.pop('cloud', None) - new_conf['auth'].pop('project_name', None) - new_conf['auth'].pop('project_id', None) - - new_conf['auth']['project_id'] = new_project_id - - return new_conf diff --git a/playbooks/ospurge-functional/post.yaml b/playbooks/ospurge-functional/post.yaml deleted file mode 100644 index e07f551..0000000 --- a/playbooks/ospurge-functional/post.yaml +++ /dev/null @@ -1,15 +0,0 @@ -- hosts: primary - tasks: - - - name: Copy files from {{ ansible_user_dir }}/workspace/ on node - synchronize: - src: '{{ ansible_user_dir }}/workspace/' - dest: '{{ zuul.executor.log_root }}' - mode: pull - copy_links: true - verify_host: true - rsync_opts: - - --include=/logs/** - - --include=*/ - - --exclude=* - - --prune-empty-dirs diff --git a/playbooks/ospurge-functional/run.yaml b/playbooks/ospurge-functional/run.yaml deleted file mode 100644 index 2d892fc..0000000 --- a/playbooks/ospurge-functional/run.yaml +++ /dev/null @@ -1,53 +0,0 @@ -- hosts: all - tasks: - - - name: Ensure legacy workspace directory - file: - path: '{{ ansible_user_dir }}/workspace' - state: directory - - - shell: - cmd: | - set -e - set -x - cat > clonemap.yaml << EOF - clonemap: - - name: openstack/devstack-gate - dest: devstack-gate - EOF - /usr/zuul-env/bin/zuul-cloner -m clonemap.yaml --cache-dir /opt/git \ - https://opendev.org \ - openstack/devstack-gate - executable: /bin/bash - chdir: '{{ ansible_user_dir }}/workspace' - environment: '{{ zuul | zuul_legacy_vars }}' - - - shell: - cmd: | - set -e - set -x - export PYTHONUNBUFFERED=true - export DEVSTACK_GATE_TEMPEST=0 - export DEVSTACK_GATE_NEUTRON=1 - export BRANCH_OVERRIDE=default - - export DEVSTACK_LOCAL_CONFIG="enable_plugin designate https://opendev.org/openstack/designate" - export DEVSTACK_LOCAL_CONFIG+=$'\n'"DESIGNATE_SERVICE_PORT_DNS=5322" - export PROJECTS="openstack/designate $PROJECTS" - - export PROJECTS="x/ospurge $PROJECTS" - if [ "$BRANCH_OVERRIDE" != "default" ] ; then - export OVERRIDE_ZUUL_BRANCH=$BRANCH_OVERRIDE - fi - - - function post_test_hook { - bash -xe $BASE/new/ospurge/tools/post_test_hook.sh - } - export -f post_test_hook - - cp devstack-gate/devstack-vm-gate-wrap.sh ./safe-devstack-vm-gate-wrap.sh - ./safe-devstack-vm-gate-wrap.sh - executable: /bin/bash - chdir: '{{ ansible_user_dir }}/workspace' - environment: '{{ zuul | zuul_legacy_vars }}' diff --git a/requirements.txt b/requirements.txt index 43c20d2..ebc64a0 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,8 +1,7 @@ -os-client-config>=1.22.0 # Apache-2.0 pbr>=1.8 # Apache-2.0 six -shade>=1.13.1 typing>=3.5.2.2 # PSF +openstacksdk # Apache-2.0 # Python 2.7 dependencies funcsigs; python_version < '3.0' diff --git a/tools/func-tests.sh b/tools/func-tests.sh index 0c04600..de69aab 100755 --- a/tools/func-tests.sh +++ b/tools/func-tests.sh @@ -110,7 +110,7 @@ for i in ${!pid[@]}; do unset "pid[$i]" done - +echo "Done populating. Moving on to cleanup." ######################## ### Cleanup @@ -127,6 +127,9 @@ assert_compute && assert_network && assert_volume tox -e run -- \ --os-auth-url http://localhost/identity \ + --os-cacert /opt/stack/data/ca-bundle.pem \ + --os-identity-api-version 3 \ + --os-region-name $OS_REGION_NAME \ --os-username demo --os-project-name invisible_to_admin \ --os-password $invisible_to_admin_demo_pass \ --os-domain-id=$OS_PROJECT_DOMAIN_ID \ diff --git a/tools/populate.sh b/tools/populate.sh index 25bbc3e..f72f5e3 100755 --- a/tools/populate.sh +++ b/tools/populate.sh @@ -15,8 +15,8 @@ # OS_PROJECT_NAME with various resources. The purpose is to test # ospurge. -# Be strict -set -ueo pipefail +# Be strict but don't exit automatically on error (exit_on_failure handles that) +set -uo pipefail function exit_on_failure { RET_CODE=$? diff --git a/tools/post_test_hook.sh b/tools/post_test_hook.sh deleted file mode 100755 index 408ed91..0000000 --- a/tools/post_test_hook.sh +++ /dev/null @@ -1,37 +0,0 @@ -#!/bin/sh - -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -export OSPURGE_DIR="$BASE/new/ospurge" - -cd $OSPURGE_DIR -sudo chown -R stack:stack $OSPURGE_DIR - -CLOUDS_YAML=/etc/openstack/clouds.yaml - -if [ ! -e ${CLOUDS_YAML} ]; then - # stable/liberty had clouds.yaml in the home/base directory - sudo mkdir -p /etc/openstack - sudo cp $BASE/new/.config/openstack/clouds.yaml ${CLOUDS_YAML} - sudo chown -R stack:stack /etc/openstack -fi - - -echo "Running OSpurge functional test suite" -set +e -sudo -E -H -u stack tox -e functional -EXIT_CODE=$? -set -e - -exit $EXIT_CODE -