From 31278ff5f77b152b5ef7a4197e15c441c72ff163 Mon Sep 17 00:00:00 2001 From: Steve Baker Date: Thu, 9 Jun 2016 08:47:17 +1200 Subject: Implement client side of event list --nested-depth This change does the following: - cleans up the usage of get_events so that marker and limit are only specified in their dedicated arguments, not also in event_args (also, specifying only in event_args still works) - first attempts server-side nested_depth support - falls back to the old recursive approach if the response data lacks a link with the ref root_stack Since there is a fallback for old heat APIs, the client change can land before or after the heat change I27e1ffb770e00a7f929c081b2a505e2007f5d584 Change-Id: I43d12ec61ec359222184f07c170de3c97481f1ba Closes-Bug: #1588561 --- heatclient/common/event_utils.py | 72 +++++++++++----- heatclient/osc/v1/event.py | 7 -- heatclient/osc/v1/stack.py | 12 +-- heatclient/tests/unit/osc/v1/test_event.py | 10 +-- heatclient/tests/unit/test_shell.py | 133 +++++++++++++++++++++++++++-- heatclient/v1/events.py | 3 + 6 files changed, 188 insertions(+), 49 deletions(-) diff --git a/heatclient/common/event_utils.py b/heatclient/common/event_utils.py index 65f89c5..d490ecf 100644 --- a/heatclient/common/event_utils.py +++ b/heatclient/common/event_utils.py @@ -67,29 +67,57 @@ def get_hook_events(hc, stack_id, event_args, nested_depth=0, 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) - if nested_depth > 0: - 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] + + 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] return events diff --git a/heatclient/osc/v1/event.py b/heatclient/osc/v1/event.py index 233b3ed..c9fe689 100644 --- a/heatclient/osc/v1/event.py +++ b/heatclient/osc/v1/event.py @@ -157,8 +157,6 @@ class ListEvent(lister.Lister): kwargs = { 'resource_name': parsed_args.resource, - 'limit': parsed_args.limit, - 'marker': parsed_args.marker, 'filters': heat_utils.format_parameters(parsed_args.filter), 'sort_dir': 'asc' } @@ -168,10 +166,6 @@ class ListEvent(lister.Lister): raise exc.CommandError(msg) if parsed_args.nested_depth: - # Until the API supports recursive event listing we'll have to do - # the marker/limit filtering client-side - del kwargs['marker'] - del kwargs['limit'] columns.append('stack_name') nested_depth = parsed_args.nested_depth else: @@ -185,7 +179,6 @@ class ListEvent(lister.Lister): marker = parsed_args.marker try: while True: - kwargs['marker'] = marker events = event_utils.get_events( client, stack_id=parsed_args.stack, diff --git a/heatclient/osc/v1/stack.py b/heatclient/osc/v1/stack.py index c845ee0..5c9dea4 100644 --- a/heatclient/osc/v1/stack.py +++ b/heatclient/osc/v1/stack.py @@ -342,8 +342,8 @@ class UpdateStack(show.ShowOne): # find the last event to use as the marker events = event_utils.get_events(client, stack_id=parsed_args.stack, - event_args={'sort_dir': 'desc', - 'limit': 1}) + event_args={'sort_dir': 'desc'}, + limit=1) marker = events[0].id if events else None client.stacks.update(**fields) @@ -660,8 +660,8 @@ class DeleteStack(command.Command): events = event_utils.get_events(heat_client, stack_id=sid, event_args={ - 'sort_dir': 'desc', - 'limit': 1}) + 'sort_dir': 'desc'}, + limit=1) if events: marker = events[0].id except heat_exc.CommandError as ex: @@ -1025,8 +1025,8 @@ def _stack_action(stack, parsed_args, heat_client, action, action_name=None): # find the last event to use as the marker events = event_utils.get_events(heat_client, stack_id=stack, - event_args={'sort_dir': 'desc', - 'limit': 1}) + event_args={'sort_dir': 'desc'}, + limit=1) marker = events[0].id if events else None try: diff --git a/heatclient/tests/unit/osc/v1/test_event.py b/heatclient/tests/unit/osc/v1/test_event.py index 31896bb..746c5c7 100644 --- a/heatclient/tests/unit/osc/v1/test_event.py +++ b/heatclient/tests/unit/osc/v1/test_event.py @@ -99,8 +99,6 @@ class TestEventList(TestEvent): defaults = { 'stack_id': 'my_stack', 'resource_name': None, - 'limit': None, - 'marker': None, 'filters': {}, 'sort_dir': 'asc' } @@ -169,8 +167,7 @@ class TestEventList(TestEvent): def test_event_list_nested_depth(self): arglist = ['my_stack', '--nested-depth', '3', '--format', 'table'] kwargs = copy.deepcopy(self.defaults) - del kwargs['marker'] - del kwargs['limit'] + kwargs['nested_depth'] = 3 cols = copy.deepcopy(self.fields) cols[-1] = 'stack_name' cols.append('logical_resource_id') @@ -178,7 +175,10 @@ class TestEventList(TestEvent): columns, data = self.cmd.take_action(parsed_args) - self.event_client.list.assert_called_with(**kwargs) + self.event_client.list.assert_has_calls([ + mock.call(**kwargs), + mock.call(**self.defaults) + ]) self.assertEqual(cols, columns) def test_event_list_sort(self): diff --git a/heatclient/tests/unit/test_shell.py b/heatclient/tests/unit/test_shell.py index f471a29..7587a12 100644 --- a/heatclient/tests/unit/test_shell.py +++ b/heatclient/tests/unit/test_shell.py @@ -2767,14 +2767,18 @@ class ShellTestEventsNested(ShellBase): for r in required: self.assertRegex(list_text, r) - def _stub_event_list_response(self, stack_id, nested_id, timestamps): + def _stub_event_list_response_old_api(self, stack_id, nested_id, + timestamps, first_request): # Stub events for parent stack ev_resp_dict = {"events": [{"id": "p_eventid1", "event_time": timestamps[0]}, {"id": "p_eventid2", "event_time": timestamps[3]}]} - self.mock_request_get('/stacks/%s/events?sort_dir=asc' % stack_id, - ev_resp_dict) + self.mock_request_get(first_request, ev_resp_dict) + + # response lacks root_stack link, fetch nested events recursively + self.mock_request_get('/stacks/%s/events?sort_dir=asc' + % stack_id, ev_resp_dict) # Stub resources for parent, including one nested res_resp_dict = {"resources": [ @@ -2794,7 +2798,7 @@ class ShellTestEventsNested(ShellBase): self.mock_request_get('/stacks/%s/events?sort_dir=asc' % nested_id, nev_resp_dict) - def test_shell_nested_depth(self): + def test_shell_nested_depth_old_api(self): self.register_keystone_auth_fixture() stack_id = 'teststack/1' nested_id = 'nested/2' @@ -2802,7 +2806,10 @@ class ShellTestEventsNested(ShellBase): "2014-01-06T16:15:00Z", # nested n_eventid1 "2014-01-06T16:16:00Z", # nested n_eventid2 "2014-01-06T16:17:00Z") # parent p_eventid2 - self._stub_event_list_response(stack_id, nested_id, timestamps) + first_request = ('/stacks/%s/events?nested_depth=1&sort_dir=asc' + % stack_id) + self._stub_event_list_response_old_api( + stack_id, nested_id, timestamps, first_request) self.m.ReplayAll() list_text = self.shell('event-list %s --nested-depth 1' % stack_id) required = ['id', 'p_eventid1', 'p_eventid2', 'n_eventid1', @@ -2814,7 +2821,7 @@ class ShellTestEventsNested(ShellBase): self.assertRegex(list_text, "%s.*\n.*%s.*\n.*%s.*\n.*%s" % timestamps) - def test_shell_nested_depth_marker(self): + def test_shell_nested_depth_marker_old_api(self): self.register_keystone_auth_fixture() stack_id = 'teststack/1' nested_id = 'nested/2' @@ -2822,7 +2829,10 @@ class ShellTestEventsNested(ShellBase): "2014-01-06T16:15:00Z", # nested n_eventid1 "2014-01-06T16:16:00Z", # nested n_eventid2 "2014-01-06T16:17:00Z") # parent p_eventid2 - self._stub_event_list_response(stack_id, nested_id, timestamps) + first_request = ('/stacks/%s/events?marker=n_eventid1&nested_depth=1' + '&sort_dir=asc' % stack_id) + self._stub_event_list_response_old_api( + stack_id, nested_id, timestamps, first_request) self.m.ReplayAll() list_text = self.shell( 'event-list %s --nested-depth 1 --marker n_eventid1' % stack_id) @@ -2836,7 +2846,7 @@ class ShellTestEventsNested(ShellBase): self.assertRegex(list_text, "%s.*\n.*%s.*\n.*%s.*" % timestamps[1:]) - def test_shell_nested_depth_limit(self): + def test_shell_nested_depth_limit_old_api(self): self.register_keystone_auth_fixture() stack_id = 'teststack/1' nested_id = 'nested/2' @@ -2844,7 +2854,10 @@ class ShellTestEventsNested(ShellBase): "2014-01-06T16:15:00Z", # nested n_eventid1 "2014-01-06T16:16:00Z", # nested n_eventid2 "2014-01-06T16:17:00Z") # parent p_eventid2 - self._stub_event_list_response(stack_id, nested_id, timestamps) + first_request = ('/stacks/%s/events?limit=2&nested_depth=1' + '&sort_dir=asc' % stack_id) + self._stub_event_list_response_old_api( + stack_id, nested_id, timestamps, first_request) self.m.ReplayAll() list_text = self.shell( 'event-list %s --nested-depth 1 --limit 2' % stack_id) @@ -2858,6 +2871,104 @@ class ShellTestEventsNested(ShellBase): self.assertRegex(list_text, "%s.*\n.*%s.*\n" % timestamps[:2]) + def _nested_events(self): + links = [ + {"rel": "self"}, + {"rel": "resource"}, + {"rel": "stack"}, + {"rel": "root_stack"} + ] + return [ + { + "id": "p_eventid1", + "event_time": '2014-01-06T16:14:00Z', + "stack_id": '1', + "resource_name": 'the_stack', + "resource_status": 'CREATE_IN_PROGRESS', + "resource_status_reason": 'Stack CREATE started', + "links": links, + }, { + "id": 'n_eventid1', + "event_time": '2014-01-06T16:15:00Z', + "stack_id": '2', + "resource_name": 'nested_stack', + "resource_status": 'CREATE_IN_PROGRESS', + "resource_status_reason": 'Stack CREATE started', + "links": links, + }, { + "id": 'n_eventid2', + "event_time": '2014-01-06T16:16:00Z', + "stack_id": '2', + "resource_name": 'nested_stack', + "resource_status": 'CREATE_COMPLETE', + "resource_status_reason": 'Stack CREATE completed', + "links": links, + }, { + "id": "p_eventid2", + "event_time": '2014-01-06T16:17:00Z', + "stack_id": '1', + "resource_name": 'the_stack', + "resource_status": 'CREATE_COMPLETE', + "resource_status_reason": 'Stack CREATE completed', + "links": links, + }, + ] + + def test_shell_nested_depth(self): + self.register_keystone_auth_fixture() + stack_id = 'teststack/1' + nested_events = self._nested_events() + ev_resp_dict = {'events': nested_events} + + url = '/stacks/%s/events?nested_depth=1&sort_dir=asc' % stack_id + self.mock_request_get(url, ev_resp_dict) + self.m.ReplayAll() + list_text = self.shell('event-list %s --nested-depth 1 --format log' + % stack_id) + self.assertEqual('''\ +2014-01-06 16:14:00Z [the_stack]: CREATE_IN_PROGRESS Stack CREATE started +2014-01-06 16:15:00Z [nested_stack]: CREATE_IN_PROGRESS Stack CREATE started +2014-01-06 16:16:00Z [nested_stack]: CREATE_COMPLETE Stack CREATE completed +2014-01-06 16:17:00Z [the_stack]: CREATE_COMPLETE Stack CREATE completed +''', list_text) + + def test_shell_nested_depth_marker(self): + self.register_keystone_auth_fixture() + stack_id = 'teststack/1' + nested_events = self._nested_events() + ev_resp_dict = {'events': nested_events[1:]} + + url = ('/stacks/%s/events?marker=n_eventid1&nested_depth=1' + '&sort_dir=asc' % stack_id) + self.mock_request_get(url, ev_resp_dict) + self.m.ReplayAll() + list_text = self.shell('event-list %s --nested-depth 1 --format log ' + '--marker n_eventid1' + % stack_id) + self.assertEqual('''\ +2014-01-06 16:15:00Z [nested_stack]: CREATE_IN_PROGRESS Stack CREATE started +2014-01-06 16:16:00Z [nested_stack]: CREATE_COMPLETE Stack CREATE completed +2014-01-06 16:17:00Z [the_stack]: CREATE_COMPLETE Stack CREATE completed +''', list_text) + + def test_shell_nested_depth_limit(self): + self.register_keystone_auth_fixture() + stack_id = 'teststack/1' + nested_events = self._nested_events() + ev_resp_dict = {'events': nested_events[:2]} + + url = ('/stacks/%s/events?limit=2&nested_depth=1&sort_dir=asc' + % stack_id) + self.mock_request_get(url, ev_resp_dict) + self.m.ReplayAll() + list_text = self.shell('event-list %s --nested-depth 1 --format log ' + '--limit 2' + % stack_id) + self.assertEqual('''\ +2014-01-06 16:14:00Z [the_stack]: CREATE_IN_PROGRESS Stack CREATE started +2014-01-06 16:15:00Z [nested_stack]: CREATE_IN_PROGRESS Stack CREATE started +''', list_text) + class ShellTestHookFunctions(ShellBase): def setUp(self): @@ -2892,6 +3003,10 @@ class ShellTestHookFunctions(ShellBase): "event_time": "2014-01-06T16:17:00Z", "resource_name": "p_res", "resource_status_reason": hook_reason}]} + + url = '/stacks/%s/events?nested_depth=1&sort_dir=asc' % stack_id + self.mock_request_get(url, ev_resp_dict) + # this api doesn't support nested_depth, fetch events recursively self.mock_request_get('/stacks/%s/events?sort_dir=asc' % stack_id, ev_resp_dict) diff --git a/heatclient/v1/events.py b/heatclient/v1/events.py index d348927..5e98aa0 100644 --- a/heatclient/v1/events.py +++ b/heatclient/v1/events.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. +import collections from oslo_utils import encodeutils import six from six.moves.urllib import parse @@ -65,6 +66,8 @@ class EventManager(stacks.StackChildManager): parse.quote(stack_id, ''), parse.quote(encodeutils.safe_encode(resource_name), '')) if params: + # convert to a sorted dict for python3 predictible order + params = collections.OrderedDict(sorted(params.items())) url += '?%s' % parse.urlencode(params, True) return self._list(url, 'events') -- cgit v1.2.1