From c8c098b3f5232616dccb8564b91805354c13452a Mon Sep 17 00:00:00 2001 From: Monty Taylor Date: Mon, 27 Mar 2017 09:35:34 -0500 Subject: [PATCH] Remove python-heatclient and replace with REST The changes in the unittests are because testtools does explicit exception matching, not subclass matching. Also the first GET call is actually supposed to be a 302 redirect. I do not understand why the first form worked with heatclient. Change-Id: I4b23304c09a1b985cc595a75587dbbc0472d450a --- requirements.txt | 1 - shade/_heat/event_utils.py | 126 ++++++--------------------------- shade/_tasks.py | 25 ------- shade/_utils.py | 13 ---- shade/openstackcloud.py | 63 ++++++++--------- shade/tests/unit/base.py | 1 + shade/tests/unit/test_shade.py | 14 ---- shade/tests/unit/test_stack.py | 72 ++++++++++++++----- 8 files changed, 107 insertions(+), 208 deletions(-) diff --git a/requirements.txt b/requirements.txt index d671b3a60..5b5550669 100644 --- a/requirements.txt +++ b/requirements.txt @@ -16,7 +16,6 @@ python-keystoneclient>=0.11.0 python-cinderclient>=1.3.1 python-neutronclient>=2.3.10 python-ironicclient>=0.10.0 -python-heatclient>=1.0.0 python-designateclient>=2.1.0 dogpile.cache>=0.5.3 diff --git a/shade/_heat/event_utils.py b/shade/_heat/event_utils.py index ab3c27cd0..8f63daef1 100644 --- a/shade/_heat/event_utils.py +++ b/shade/_heat/event_utils.py @@ -12,115 +12,33 @@ # License for the specific language governing permissions and limitations # under the License. +import collections import time -from shade._heat import utils -import heatclient.exc as exc +def get_events(cloud, stack_id, event_args, marker=None, limit=None): + # TODO(mordred) FIX THIS ONCE assert_calls CAN HANDLE QUERY STRINGS + params = collections.OrderedDict() + for k in sorted(event_args.keys()): + params[k] = event_args[k] -def get_events(hc, stack_id, event_args, nested_depth=0, - marker=None, limit=None): - event_args = dict(event_args) if marker: event_args['marker'] = marker if limit: event_args['limit'] = limit - if not nested_depth: - # simple call with no nested_depth - return _get_stack_events(hc, stack_id, event_args) - # assume an API which supports nested_depth - event_args['nested_depth'] = nested_depth - events = _get_stack_events(hc, stack_id, event_args) + events = cloud._orchestration_client.get( + '/stacks/{id}/events'.format(id=stack_id), + params=params) - if not events: - return events - - first_links = getattr(events[0], 'links', []) - root_stack_link = [l for l in first_links - if l.get('rel') == 'root_stack'] - if root_stack_link: - # response has a root_stack link, indicating this is an API which - # supports nested_depth - return events - - # API doesn't support nested_depth, do client-side paging and recursive - # event fetch - marker = event_args.pop('marker', None) - limit = event_args.pop('limit', None) - event_args.pop('nested_depth', None) - events = _get_stack_events(hc, stack_id, event_args) - events.extend(_get_nested_events(hc, nested_depth, - stack_id, event_args)) - # Because there have been multiple stacks events mangled into - # one list, we need to sort before passing to print_list - # Note we can't use the prettytable sortby_index here, because - # the "start" option doesn't allow post-sort slicing, which - # will be needed to make "--marker" work for nested_depth lists - events.sort(key=lambda x: x.event_time) - - # Slice the list if marker is specified - if marker: - try: - marker_index = [e.id for e in events].index(marker) - events = events[marker_index:] - except ValueError: - pass - - # Slice the list if limit is specified - if limit: - limit_index = min(int(limit), len(events)) - events = events[:limit_index] + # Show which stack the event comes from (for nested events) + for e in events: + e['stack_name'] = stack_id.split("/")[0] return events -def _get_nested_ids(hc, stack_id): - nested_ids = [] - try: - resources = hc.resources.list(stack_id=stack_id) - except exc.HTTPNotFound: - raise exc.CommandError('Stack not found: %s' % stack_id) - for r in resources: - nested_id = utils.resource_nested_identifier(r) - if nested_id: - nested_ids.append(nested_id) - return nested_ids - - -def _get_nested_events(hc, nested_depth, stack_id, event_args): - # FIXME(shardy): this is very inefficient, we should add nested_depth to - # the event_list API in a future heat version, but this will be required - # until kilo heat is EOL. - nested_ids = _get_nested_ids(hc, stack_id) - nested_events = [] - for n_id in nested_ids: - stack_events = _get_stack_events(hc, n_id, event_args) - if stack_events: - nested_events.extend(stack_events) - if nested_depth > 1: - next_depth = nested_depth - 1 - nested_events.extend(_get_nested_events( - hc, next_depth, n_id, event_args)) - return nested_events - - -def _get_stack_events(hc, stack_id, event_args): - event_args['stack_id'] = stack_id - try: - events = hc.events.list(**event_args) - except exc.HTTPNotFound as ex: - # it could be the stack or resource that is not found - # just use the message that the server sent us. - raise exc.CommandError(str(ex)) - else: - # Show which stack the event comes from (for nested events) - for e in events: - e.stack_name = stack_id.split("/")[0] - return events - - -def poll_for_events(hc, stack_name, action=None, poll_period=5, marker=None, - nested_depth=0): +def poll_for_events( + cloud, stack_name, action=None, poll_period=5, marker=None): """Continuously poll events and logs for performed action on stack.""" if action: @@ -133,19 +51,19 @@ def poll_for_events(hc, stack_name, action=None, poll_period=5, marker=None, msg_template = "\n Stack %(name)s %(status)s \n" def is_stack_event(event): - if getattr(event, 'resource_name', '') != stack_name: + if event.get('resource_name', '') != stack_name: return False - phys_id = getattr(event, 'physical_resource_id', '') + phys_id = event.get('physical_resource_id', '') links = dict((l.get('rel'), - l.get('href')) for l in getattr(event, 'links', [])) + l.get('href')) for l in event.get('links', [])) stack_id = links.get('stack', phys_id).rsplit('/', 1)[-1] return stack_id == phys_id while True: - events = get_events(hc, stack_id=stack_name, nested_depth=nested_depth, - event_args={'sort_dir': 'asc', - 'marker': marker}) + events = get_events( + cloud, stack_id=stack_name, + event_args={'sort_dir': 'asc', 'marker': marker}) if len(events) == 0: no_event_polls += 1 @@ -165,8 +83,8 @@ def poll_for_events(hc, stack_name, action=None, poll_period=5, marker=None, if no_event_polls >= 2: # after 2 polls with no events, fall back to a stack get - stack = hc.stacks.get(stack_name, resolve_outputs=False) - stack_status = stack.stack_status + stack = cloud.get_stack(stack_name) + stack_status = stack['stack_status'] msg = msg_template % dict( name=stack_name, status=stack_status) if stop_check(stack_status): diff --git a/shade/_tasks.py b/shade/_tasks.py index 9904eba8f..d0975b3b9 100644 --- a/shade/_tasks.py +++ b/shade/_tasks.py @@ -699,31 +699,6 @@ class RolesForUser(task_manager.Task): return client.keystone_client.roles.roles_for_user(**self.args) -class StackList(task_manager.Task): - def main(self, client): - return client.heat_client.stacks.list() - - -class StackCreate(task_manager.Task): - def main(self, client): - return client.heat_client.stacks.create(**self.args) - - -class StackUpdate(task_manager.Task): - def main(self, client): - return client.heat_client.stacks.update(**self.args) - - -class StackDelete(task_manager.Task): - def main(self, client): - return client.heat_client.stacks.delete(self.args['id']) - - -class StackGet(task_manager.Task): - def main(self, client): - return client.heat_client.stacks.get(**self.args) - - class ZoneList(task_manager.Task): def main(self, client): return client.designate_client.zones.list() diff --git a/shade/_utils.py b/shade/_utils.py index 238986685..82233f795 100644 --- a/shade/_utils.py +++ b/shade/_utils.py @@ -25,7 +25,6 @@ import sys import time from decorator import decorator -from heatclient import exc as heat_exc from neutronclient.common import exceptions as neutron_exc from novaclient import exceptions as nova_exc @@ -418,18 +417,6 @@ def cache_on_arguments(*cache_on_args, **cache_on_kwargs): return _inner_cache_on_arguments -@contextlib.contextmanager -def heat_exceptions(error_message): - try: - yield - except heat_exc.NotFound as e: - raise exc.OpenStackCloudResourceNotFound( - "{msg}: {exc}".format(msg=error_message, exc=str(e))) - except Exception as e: - raise exc.OpenStackCloudException( - "{msg}: {exc}".format(msg=error_message, exc=str(e))) - - @contextlib.contextmanager def neutron_exceptions(error_message): try: diff --git a/shade/openstackcloud.py b/shade/openstackcloud.py index 32a84f8f4..988a81172 100644 --- a/shade/openstackcloud.py +++ b/shade/openstackcloud.py @@ -31,7 +31,6 @@ import requestsexceptions from six.moves import urllib import cinderclient.exceptions as cinder_exceptions -from heatclient import exc as heat_exceptions import keystoneauth1.exceptions import novaclient.exceptions as nova_exceptions @@ -1095,6 +1094,17 @@ class OpenStackCloud(_normalize.Normalizer): @property def heat_client(self): + warnings.warn( + 'Using shade to get a heat_client object is deprecated. If you' + ' need a raw heatclient.client.Client object, please use' + ' make_legacy_client in os-client-config instead') + try: + import heatclient # flake8: noqa + except ImportError: + self.log.error( + 'heatclient is no longer a dependency of shade. You need to' + ' install python-heatclient directly.') + raise if self._heat_client is None: self._heat_client = self._get_client('orchestration') return self._heat_client @@ -1261,11 +1271,9 @@ class OpenStackCloud(_normalize.Normalizer): environment=env, timeout_mins=timeout // 60, ) - with _utils.heat_exceptions("Error creating stack {name}".format( - name=name)): - self.manager.submit_task(_tasks.StackCreate(**params)) + self._orchestration_client.post('/stacks', json=params) if wait: - event_utils.poll_for_events(self.heat_client, stack_name=name, + event_utils.poll_for_events(self, stack_name=name, action='CREATE') return self.get_stack(name) @@ -1308,7 +1316,6 @@ class OpenStackCloud(_normalize.Normalizer): template_object=template_object, files=files) params = dict( - stack_id=name_or_id, disable_rollback=not rollback, parameters=parameters, template=template, @@ -1318,17 +1325,14 @@ class OpenStackCloud(_normalize.Normalizer): ) if wait: # find the last event to use as the marker - events = event_utils.get_events(self.heat_client, - name_or_id, - event_args={'sort_dir': 'desc', - 'limit': 1}) + events = event_utils.get_events( + self, name_or_id, event_args={'sort_dir': 'desc', 'limit': 1}) marker = events[0].id if events else None - with _utils.heat_exceptions("Error updating stack {name}".format( - name=name_or_id)): - self.manager.submit_task(_tasks.StackUpdate(**params)) + self._orchestration_client.put( + '/stacks/{name_or_id}'.format(name_or_id=name_or_id), json=params) if wait: - event_utils.poll_for_events(self.heat_client, + event_utils.poll_for_events(self, name_or_id, action='UPDATE', marker=marker) @@ -1352,24 +1356,20 @@ class OpenStackCloud(_normalize.Normalizer): if wait: # find the last event to use as the marker - events = event_utils.get_events(self.heat_client, - name_or_id, - event_args={'sort_dir': 'desc', - 'limit': 1}) + events = event_utils.get_events( + self, name_or_id, event_args={'sort_dir': 'desc', 'limit': 1}) marker = events[0].id if events else None - with _utils.heat_exceptions("Failed to delete stack {id}".format( - id=name_or_id)): - self.manager.submit_task(_tasks.StackDelete(id=stack['id'])) + self._orchestration_client.delete( + '/stacks/{id}'.format(id=stack['id'])) + if wait: try: - event_utils.poll_for_events(self.heat_client, + event_utils.poll_for_events(self, stack_name=name_or_id, action='DELETE', marker=marker) - except (heat_exceptions.NotFound, heat_exceptions.CommandError): - # heatclient might raise NotFound or CommandError on - # not found during poll_for_events + except OpenStackCloudHTTPError: pass stack = self.get_stack(name_or_id) if stack and stack['stack_status'] == 'DELETE_FAILED': @@ -1770,7 +1770,7 @@ class OpenStackCloud(_normalize.Normalizer): OpenStack API call. """ with _utils.shade_exceptions("Error fetching stack list"): - stacks = self.manager.submit_task(_tasks.StackList()) + stacks = self._orchestration_client.get('/stacks') return self._normalize_stacks(stacks) def list_server_security_groups(self, server): @@ -2773,16 +2773,15 @@ class OpenStackCloud(_normalize.Normalizer): # so a StackGet can always be used for name or ID. with _utils.shade_exceptions("Error fetching stack"): try: - stack = self.manager.submit_task( - _tasks.StackGet(stack_id=name_or_id)) + stack = self._orchestration_client.get( + '/stacks/{name_or_id}'.format(name_or_id=name_or_id)) # Treat DELETE_COMPLETE stacks as a NotFound if stack['stack_status'] == 'DELETE_COMPLETE': return [] - stacks = [stack] - except heat_exceptions.NotFound: + except OpenStackCloudURINotFound: return [] - nstacks = self._normalize_stacks(stacks) - return _utils._filter_list(nstacks, name_or_id, filters) + stack = self._normalize_stack(stack) + return _utils._filter_list([stack], name_or_id, filters) return _utils._get_entity( _search_one_stack, name_or_id, filters) diff --git a/shade/tests/unit/base.py b/shade/tests/unit/base.py index 2f91c87c5..bb9f225d9 100644 --- a/shade/tests/unit/base.py +++ b/shade/tests/unit/base.py @@ -555,6 +555,7 @@ class RequestsMockTestCase(BaseTestCase): zip(self.calls, self.adapter.request_history)): if stop_after and x > stop_after: break + self.assertEqual( (call['method'], call['url']), (history.method, history.url), 'REST mismatch on call {index}'.format(index=x)) diff --git a/shade/tests/unit/test_shade.py b/shade/tests/unit/test_shade.py index aef3e8948..3dbc2e232 100644 --- a/shade/tests/unit/test_shade.py +++ b/shade/tests/unit/test_shade.py @@ -16,7 +16,6 @@ import munch from neutronclient.common import exceptions as n_exc import testtools -from os_client_config import cloud_config import shade from shade import _utils from shade import exc @@ -73,19 +72,6 @@ class TestShade(base.TestCase): self.assertRaises(exc.OpenStackCloudException, self.cloud.list_servers) - @mock.patch.object(cloud_config.CloudConfig, 'get_session') - @mock.patch.object(cloud_config.CloudConfig, 'get_legacy_client') - def test_heat_args(self, get_legacy_client_mock, get_session_mock): - session_mock = mock.Mock() - get_session_mock.return_value = session_mock - self.cloud.heat_client - get_legacy_client_mock.assert_called_once_with( - service_key='orchestration', - client_class=None, - interface_key=None, - pass_version_arg=True, - ) - @mock.patch.object(shade.OpenStackCloud, 'neutron_client') def test_list_networks(self, mock_neutron): net1 = {'id': '1', 'name': 'net1'} diff --git a/shade/tests/unit/test_stack.py b/shade/tests/unit/test_stack.py index 45f3de1e8..c32022992 100644 --- a/shade/tests/unit/test_stack.py +++ b/shade/tests/unit/test_stack.py @@ -55,10 +55,7 @@ class TestStack(base.RequestsMockTestCase): endpoint=fakes.ORCHESTRATION_ENDPOINT), status_code=404) ]) - with testtools.ExpectedException( - shade.OpenStackCloudException, - "Error fetching stack list" - ): + with testtools.ExpectedException(shade.OpenStackCloudURINotFound): self.cloud.list_stacks() self.assert_calls() @@ -110,10 +107,7 @@ class TestStack(base.RequestsMockTestCase): endpoint=fakes.ORCHESTRATION_ENDPOINT), status_code=404) ]) - with testtools.ExpectedException( - shade.OpenStackCloudException, - "Error fetching stack list" - ): + with testtools.ExpectedException(shade.OpenStackCloudURINotFound): self.cloud.search_stacks() def test_delete_stack(self): @@ -122,7 +116,11 @@ class TestStack(base.RequestsMockTestCase): uri='{endpoint}/stacks/{name}'.format( endpoint=fakes.ORCHESTRATION_ENDPOINT, name=self.stack_name), - json={"stack": self.stack}), + status_code=302, + headers=dict( + location='{endpoint}/stacks/{name}/{id}'.format( + endpoint=fakes.ORCHESTRATION_ENDPOINT, + id=self.stack_id, name=self.stack_name))), dict(method='GET', uri='{endpoint}/stacks/{name}/{id}'.format( endpoint=fakes.ORCHESTRATION_ENDPOINT, @@ -152,7 +150,11 @@ class TestStack(base.RequestsMockTestCase): uri='{endpoint}/stacks/{id}'.format( endpoint=fakes.ORCHESTRATION_ENDPOINT, id=self.stack_id), - json={"stack": self.stack}), + status_code=302, + headers=dict( + location='{endpoint}/stacks/{name}/{id}'.format( + endpoint=fakes.ORCHESTRATION_ENDPOINT, + id=self.stack_id, name=self.stack_name))), dict(method='GET', uri='{endpoint}/stacks/{name}/{id}'.format( endpoint=fakes.ORCHESTRATION_ENDPOINT, @@ -165,7 +167,7 @@ class TestStack(base.RequestsMockTestCase): status_code=400, reason="ouch"), ]) - with testtools.ExpectedException(shade.OpenStackCloudException): + with testtools.ExpectedException(shade.OpenStackCloudBadRequest): self.cloud.delete_stack(self.stack_id) self.assert_calls() @@ -179,7 +181,11 @@ class TestStack(base.RequestsMockTestCase): uri='{endpoint}/stacks/{id}'.format( endpoint=fakes.ORCHESTRATION_ENDPOINT, id=self.stack_id), - json={"stack": self.stack}), + status_code=302, + headers=dict( + location='{endpoint}/stacks/{name}/{id}'.format( + endpoint=fakes.ORCHESTRATION_ENDPOINT, + id=self.stack_id, name=self.stack_name))), dict(method='GET', uri='{endpoint}/stacks/{name}/{id}'.format( endpoint=fakes.ORCHESTRATION_ENDPOINT, @@ -229,7 +235,11 @@ class TestStack(base.RequestsMockTestCase): uri='{endpoint}/stacks/{id}'.format( endpoint=fakes.ORCHESTRATION_ENDPOINT, id=self.stack_id), - json={"stack": self.stack}), + status_code=302, + headers=dict( + location='{endpoint}/stacks/{name}/{id}'.format( + endpoint=fakes.ORCHESTRATION_ENDPOINT, + id=self.stack_id, name=self.stack_name))), dict(method='GET', uri='{endpoint}/stacks/{name}/{id}'.format( endpoint=fakes.ORCHESTRATION_ENDPOINT, @@ -261,7 +271,11 @@ class TestStack(base.RequestsMockTestCase): uri='{endpoint}/stacks/{id}'.format( endpoint=fakes.ORCHESTRATION_ENDPOINT, id=self.stack_id, name=self.stack_name), - json={'stack': failed_stack}), + status_code=302, + headers=dict( + location='{endpoint}/stacks/{name}/{id}'.format( + endpoint=fakes.ORCHESTRATION_ENDPOINT, + id=self.stack_id, name=self.stack_name))), dict(method='GET', uri='{endpoint}/stacks/{name}/{id}'.format( endpoint=fakes.ORCHESTRATION_ENDPOINT, @@ -298,7 +312,11 @@ class TestStack(base.RequestsMockTestCase): uri='{endpoint}/stacks/{name}'.format( endpoint=fakes.ORCHESTRATION_ENDPOINT, name=self.stack_name), - json={"stack": self.stack}), + status_code=302, + headers=dict( + location='{endpoint}/stacks/{name}/{id}'.format( + endpoint=fakes.ORCHESTRATION_ENDPOINT, + id=self.stack_id, name=self.stack_name))), dict( method='GET', uri='{endpoint}/stacks/{name}/{id}'.format( @@ -351,7 +369,11 @@ class TestStack(base.RequestsMockTestCase): uri='{endpoint}/stacks/{name}'.format( endpoint=fakes.ORCHESTRATION_ENDPOINT, name=self.stack_name), - json={"stack": self.stack}), + status_code=302, + headers=dict( + location='{endpoint}/stacks/{name}/{id}'.format( + endpoint=fakes.ORCHESTRATION_ENDPOINT, + id=self.stack_id, name=self.stack_name))), dict( method='GET', uri='{endpoint}/stacks/{name}/{id}'.format( @@ -390,7 +412,11 @@ class TestStack(base.RequestsMockTestCase): uri='{endpoint}/stacks/{name}'.format( endpoint=fakes.ORCHESTRATION_ENDPOINT, name=self.stack_name), - json={"stack": self.stack}), + status_code=302, + headers=dict( + location='{endpoint}/stacks/{name}/{id}'.format( + endpoint=fakes.ORCHESTRATION_ENDPOINT, + id=self.stack_id, name=self.stack_name))), dict( method='GET', uri='{endpoint}/stacks/{name}/{id}'.format( @@ -452,7 +478,11 @@ class TestStack(base.RequestsMockTestCase): uri='{endpoint}/stacks/{name}'.format( endpoint=fakes.ORCHESTRATION_ENDPOINT, name=self.stack_name), - json={"stack": self.stack}), + status_code=302, + headers=dict( + location='{endpoint}/stacks/{name}/{id}'.format( + endpoint=fakes.ORCHESTRATION_ENDPOINT, + id=self.stack_id, name=self.stack_name))), dict( method='GET', uri='{endpoint}/stacks/{name}/{id}'.format( @@ -473,7 +503,11 @@ class TestStack(base.RequestsMockTestCase): uri='{endpoint}/stacks/{name}'.format( endpoint=fakes.ORCHESTRATION_ENDPOINT, name=self.stack_name), - json={"stack": self.stack}), + status_code=302, + headers=dict( + location='{endpoint}/stacks/{name}/{id}'.format( + endpoint=fakes.ORCHESTRATION_ENDPOINT, + id=self.stack_id, name=self.stack_name))), dict(method='GET', uri='{endpoint}/stacks/{name}/{id}'.format( endpoint=fakes.ORCHESTRATION_ENDPOINT,