diff --git a/openstackclient/compute/v2/server_event.py b/openstackclient/compute/v2/server_event.py index ebf0d526ac..395c2cac09 100644 --- a/openstackclient/compute/v2/server_event.py +++ b/openstackclient/compute/v2/server_event.py @@ -17,8 +17,10 @@ import logging +from cliff import columns import iso8601 -from novaclient import api_versions +from openstack import exceptions as sdk_exceptions +from openstack import utils as sdk_utils from osc_lib.command import command from osc_lib import exceptions from osc_lib import utils @@ -30,6 +32,51 @@ from openstackclient.i18n import _ LOG = logging.getLogger(__name__) +class ServerActionEventColumn(columns.FormattableColumn): + """Custom formatter for server action events. + + Format the :class:`~openstack.compute.v2.server_action.ServerActionEvent` + objects as we'd like. + """ + + def _format_event(self, event): + column_map = {} + hidden_columns = ['id', 'name', 'location'] + _, columns = utils.get_osc_show_columns_for_sdk_resource( + event, + column_map, + hidden_columns, + ) + data = utils.get_item_properties( + event, + columns, + ) + return dict(zip(columns, data)) + + def human_readable(self): + events = [self._format_event(event) for event in self._value] + return utils.format_list_of_dicts(events) + + def machine_readable(self): + events = [self._format_event(event) for event in self._value] + return events + + +def _get_server_event_columns(item, client): + column_map = {} + hidden_columns = ['name', 'server_id', 'links', 'location'] + + if not sdk_utils.supports_microversion(client, '2.58'): + # updated_at was introduced in 2.58 + hidden_columns.append('updated_at') + + return utils.get_osc_show_columns_for_sdk_resource( + item, + column_map, + hidden_columns, + ) + + class ListServerEvent(command.Lister): """List recent events of a server. @@ -38,7 +85,7 @@ class ListServerEvent(command.Lister): """ def get_parser(self, prog_name): - parser = super(ListServerEvent, self).get_parser(prog_name) + parser = super().get_parser(prog_name) parser.add_argument( 'server', metavar='', @@ -90,30 +137,33 @@ class ListServerEvent(command.Lister): return parser def take_action(self, parsed_args): - compute_client = self.app.client_manager.compute + compute_client = self.app.client_manager.sdk_connection.compute kwargs = {} if parsed_args.marker: - if compute_client.api_version < api_versions.APIVersion('2.58'): + if not sdk_utils.supports_microversion(compute_client, '2.58'): msg = _( '--os-compute-api-version 2.58 or greater is required to ' 'support the --marker option' ) raise exceptions.CommandError(msg) + kwargs['marker'] = parsed_args.marker if parsed_args.limit: - if compute_client.api_version < api_versions.APIVersion('2.58'): + if not sdk_utils.supports_microversion(compute_client, '2.58'): msg = _( '--os-compute-api-version 2.58 or greater is required to ' 'support the --limit option' ) raise exceptions.CommandError(msg) + kwargs['limit'] = parsed_args.limit + kwargs['paginated'] = False if parsed_args.changes_since: - if compute_client.api_version < api_versions.APIVersion('2.58'): + if not sdk_utils.supports_microversion(compute_client, '2.58'): msg = _( '--os-compute-api-version 2.58 or greater is required to ' 'support the --changes-since option' @@ -129,7 +179,7 @@ class ListServerEvent(command.Lister): kwargs['changes_since'] = parsed_args.changes_since if parsed_args.changes_before: - if compute_client.api_version < api_versions.APIVersion('2.66'): + if not sdk_utils.supports_microversion(compute_client, '2.66'): msg = _( '--os-compute-api-version 2.66 or greater is required to ' 'support the --changes-before option' @@ -145,10 +195,11 @@ class ListServerEvent(command.Lister): kwargs['changes_before'] = parsed_args.changes_before try: - server_id = utils.find_resource( - compute_client.servers, parsed_args.server, + server_id = compute_client.find_server( + parsed_args.server, + ignore_missing=False ).id - except exceptions.CommandError: + except sdk_exceptions.ResourceNotFound: # If we fail to find the resource, it is possible the server is # deleted. Try once more using the arg directly if it is a # UUID. @@ -157,11 +208,11 @@ class ListServerEvent(command.Lister): else: raise - data = compute_client.instance_action.list(server_id, **kwargs) + data = compute_client.server_actions(server_id, **kwargs) columns = ( 'request_id', - 'instance_uuid', + 'server_id', 'action', 'start_time', ) @@ -200,7 +251,7 @@ class ShowServerEvent(command.ShowOne): """ def get_parser(self, prog_name): - parser = super(ShowServerEvent, self).get_parser(prog_name) + parser = super().get_parser(prog_name) parser.add_argument( 'server', metavar='', @@ -214,13 +265,14 @@ class ShowServerEvent(command.ShowOne): return parser def take_action(self, parsed_args): - compute_client = self.app.client_manager.compute + compute_client = self.app.client_manager.sdk_connection.compute try: - server_id = utils.find_resource( - compute_client.servers, parsed_args.server, + server_id = compute_client.find_server( + parsed_args.server, + ignore_missing=False, ).id - except exceptions.CommandError: + except sdk_exceptions.ResourceNotFound: # If we fail to find the resource, it is possible the server is # deleted. Try once more using the arg directly if it is a # UUID. @@ -229,8 +281,19 @@ class ShowServerEvent(command.ShowOne): else: raise - action_detail = compute_client.instance_action.get( - server_id, parsed_args.request_id + server_action = compute_client.get_server_action( + parsed_args.request_id, server_id, ) - return zip(*sorted(action_detail.to_dict().items())) + column_headers, columns = _get_server_event_columns( + server_action, compute_client, + ) + + return ( + column_headers, + utils.get_item_properties( + server_action, + columns, + formatters={'events': ServerActionEventColumn}, + ), + ) diff --git a/openstackclient/tests/functional/compute/v2/test_server_event.py b/openstackclient/tests/functional/compute/v2/test_server_event.py index 48147507d3..47b5020f5f 100644 --- a/openstackclient/tests/functional/compute/v2/test_server_event.py +++ b/openstackclient/tests/functional/compute/v2/test_server_event.py @@ -50,7 +50,6 @@ class ServerEventTests(common.ComputeTestCase): 'server event show ' + self.server_name + ' ' + request_id, parse_output=True, ) - self.assertEqual(self.server_id, cmd_output.get('instance_uuid')) self.assertEqual(request_id, cmd_output.get('request_id')) self.assertEqual('create', cmd_output.get('action')) self.assertIsNotNone(cmd_output.get('events')) @@ -78,8 +77,6 @@ class ServerEventTests(common.ComputeTestCase): 'server event show ' + self.server_name + ' ' + request_id, parse_output=True, ) - - self.assertEqual(self.server_id, cmd_output.get('instance_uuid')) self.assertEqual(request_id, cmd_output.get('request_id')) self.assertEqual('reboot', cmd_output.get('action')) self.assertIsNotNone(cmd_output.get('events')) @@ -116,7 +113,6 @@ class ServerEventTests(common.ComputeTestCase): 'server event show ' + server_id + ' ' + request_id, parse_output=True, ) - self.assertEqual(server_id, cmd_output.get('instance_uuid')) self.assertEqual(request_id, cmd_output.get('request_id')) self.assertEqual('delete', cmd_output.get('action')) self.assertIsNotNone(cmd_output.get('events')) diff --git a/openstackclient/tests/unit/compute/v2/fakes.py b/openstackclient/tests/unit/compute/v2/fakes.py index f7f0750956..08d4a5749b 100644 --- a/openstackclient/tests/unit/compute/v2/fakes.py +++ b/openstackclient/tests/unit/compute/v2/fakes.py @@ -23,6 +23,7 @@ from openstack.compute.v2 import flavor as _flavor from openstack.compute.v2 import hypervisor as _hypervisor from openstack.compute.v2 import migration as _migration from openstack.compute.v2 import server as _server +from openstack.compute.v2 import server_action as _server_action from openstack.compute.v2 import server_group as _server_group from openstack.compute.v2 import server_interface as _server_interface from openstack.compute.v2 import server_migration as _server_migration @@ -593,45 +594,45 @@ class FakeServer(object): return mock.Mock(side_effect=servers) -class FakeServerEvent(object): - """Fake one or more server event.""" +def create_one_server_action(attrs=None): + """Create a fake server action. - @staticmethod - def create_one_server_event(attrs=None): - """Create a fake server event. + :param attrs: A dictionary with all attributes + :return: A fake openstack.compute.v2.server_action.ServerAction object + """ + attrs = attrs or {} - :param attrs: - A dictionary with all attributes - :return: - A FakeResource object, with id and other attributes - """ - attrs = attrs or {} + # Set default attributes + server_action_info = { + "server_id": "server-event-" + uuid.uuid4().hex, + "user_id": "user-id-" + uuid.uuid4().hex, + "start_time": "2017-02-27T07:47:13.000000", + "request_id": "req-" + uuid.uuid4().hex, + "action": "create", + "message": None, + "project_id": "project-id-" + uuid.uuid4().hex, + "events": [{ + "finish_time": "2017-02-27T07:47:25.000000", + "start_time": "2017-02-27T07:47:15.000000", + "traceback": None, + "event": "compute__do_build_and_run_instance", + "result": "Success" + }] + } + # Overwrite default attributes + server_action_info.update(attrs) - # Set default attributes - server_event_info = { - "instance_uuid": "server-event-" + uuid.uuid4().hex, - "user_id": "user-id-" + uuid.uuid4().hex, - "start_time": "2017-02-27T07:47:13.000000", - "request_id": "req-" + uuid.uuid4().hex, - "action": "create", - "message": None, - "project_id": "project-id-" + uuid.uuid4().hex, - "events": [{ - "finish_time": "2017-02-27T07:47:25.000000", - "start_time": "2017-02-27T07:47:15.000000", - "traceback": None, - "event": "compute__do_build_and_run_instance", - "result": "Success" - }] - } - # Overwrite default attributes - server_event_info.update(attrs) + # We handle events separately since they're nested resources + events = [ + _server_action.ServerActionEvent(**event) + for event in server_action_info.pop('events') + ] - server_event = fakes.FakeResource( - info=copy.deepcopy(server_event_info), - loaded=True, - ) - return server_event + server_action = _server_action.ServerAction( + **server_action_info, + events=events, + ) + return server_action class FakeService(object): diff --git a/openstackclient/tests/unit/compute/v2/test_server_event.py b/openstackclient/tests/unit/compute/v2/test_server_event.py index 058a44d8b6..7b4facd2d2 100644 --- a/openstackclient/tests/unit/compute/v2/test_server_event.py +++ b/openstackclient/tests/unit/compute/v2/test_server_event.py @@ -16,6 +16,7 @@ from unittest import mock import iso8601 from novaclient import api_versions +from openstack import utils as sdk_utils from osc_lib import exceptions from openstackclient.compute.v2 import server_event @@ -29,17 +30,34 @@ class TestServerEvent(compute_fakes.TestComputev2): def setUp(self): super(TestServerEvent, self).setUp() - self.servers_mock = self.app.client_manager.compute.servers - self.servers_mock.reset_mock() - self.events_mock = self.app.client_manager.compute.instance_action - self.events_mock.reset_mock() + self.app.client_manager.sdk_connection = mock.Mock() + self.app.client_manager.sdk_connection.compute = mock.Mock() + self.sdk_client = self.app.client_manager.sdk_connection.compute + self.sdk_client.find_server = mock.Mock() + self.sdk_client.server_actions = mock.Mock() + self.sdk_client.get_server_action = mock.Mock() + self.sdk_client.reset_mock() - self.servers_mock.get.return_value = self.fake_server + patcher = mock.patch.object( + sdk_utils, 'supports_microversion', return_value=True) + self.addCleanup(patcher.stop) + self.supports_microversion_mock = patcher.start() + self._set_mock_microversion( + self.app.client_manager.compute.api_version.get_string()) + + def _set_mock_microversion(self, mock_v): + """Set a specific microversion for the mock supports_microversion().""" + self.supports_microversion_mock.reset_mock(return_value=True) + + self.supports_microversion_mock.side_effect = ( + lambda _, v: + api_versions.APIVersion(v) <= api_versions.APIVersion(mock_v) + ) class TestListServerEvent(TestServerEvent): - fake_event = compute_fakes.FakeServerEvent.create_one_server_event() + fake_event = compute_fakes.create_one_server_action() columns = ( 'Request ID', @@ -49,7 +67,7 @@ class TestListServerEvent(TestServerEvent): ) data = (( fake_event.request_id, - fake_event.instance_uuid, + fake_event.server_id, fake_event.action, fake_event.start_time, ), ) @@ -65,7 +83,7 @@ class TestListServerEvent(TestServerEvent): ) long_data = (( fake_event.request_id, - fake_event.instance_uuid, + fake_event.server_id, fake_event.action, fake_event.start_time, fake_event.message, @@ -74,9 +92,11 @@ class TestListServerEvent(TestServerEvent): ), ) def setUp(self): - super(TestListServerEvent, self).setUp() + super().setUp() + + self.sdk_client.find_server.return_value = self.fake_server + self.sdk_client.server_actions.return_value = [self.fake_event, ] - self.events_mock.list.return_value = [self.fake_event, ] self.cmd = server_event.ListServerEvent(self.app, None) def test_server_event_list(self): @@ -89,11 +109,13 @@ class TestListServerEvent(TestServerEvent): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - columns, data = self.cmd.take_action(parsed_args) - self.servers_mock.get.assert_called_once_with(self.fake_server.name) - self.events_mock.list.assert_called_once_with(self.fake_server.id) + self.sdk_client.find_server.assert_called_with( + self.fake_server.name, + ignore_missing=False, + ) + self.sdk_client.server_actions.assert_called_with(self.fake_server.id) self.assertEqual(self.columns, columns) self.assertEqual(self.data, tuple(data)) @@ -109,18 +131,19 @@ class TestListServerEvent(TestServerEvent): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - columns, data = self.cmd.take_action(parsed_args) - self.servers_mock.get.assert_called_once_with(self.fake_server.name) - self.events_mock.list.assert_called_once_with(self.fake_server.id) + self.sdk_client.find_server.assert_called_with( + self.fake_server.name, + ignore_missing=False, + ) + self.sdk_client.server_actions.assert_called_with(self.fake_server.id) self.assertEqual(self.long_columns, columns) self.assertEqual(self.long_data, tuple(data)) def test_server_event_list_with_changes_since(self): - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.58') + self._set_mock_microversion('2.58') arglist = [ '--changes-since', '2016-03-04T06:27:59Z', @@ -134,9 +157,14 @@ class TestListServerEvent(TestServerEvent): parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.servers_mock.get.assert_called_once_with(self.fake_server.name) - self.events_mock.list.assert_called_once_with( - self.fake_server.id, changes_since='2016-03-04T06:27:59Z') + self.sdk_client.find_server.assert_called_with( + self.fake_server.name, + ignore_missing=False, + ) + self.sdk_client.server_actions.assert_called_with( + self.fake_server.id, + changes_since='2016-03-04T06:27:59Z', + ) self.assertEqual(self.columns, columns) self.assertEqual(tuple(self.data), tuple(data)) @@ -145,8 +173,7 @@ class TestListServerEvent(TestServerEvent): def test_server_event_list_with_changes_since_invalid( self, mock_parse_isotime, ): - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.58') + self._set_mock_microversion('2.58') arglist = [ '--changes-since', 'Invalid time value', @@ -161,17 +188,16 @@ class TestListServerEvent(TestServerEvent): ex = self.assertRaises( exceptions.CommandError, self.cmd.take_action, - parsed_args) + parsed_args, + ) - self.assertIn( - 'Invalid changes-since value:', str(ex)) + self.assertIn('Invalid changes-since value:', str(ex)) mock_parse_isotime.assert_called_once_with( 'Invalid time value' ) def test_server_event_list_with_changes_since_pre_v258(self): - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.57') + self._set_mock_microversion('2.57') arglist = [ '--changes-since', '2016-03-04T06:27:59Z', @@ -186,14 +212,15 @@ class TestListServerEvent(TestServerEvent): ex = self.assertRaises( exceptions.CommandError, self.cmd.take_action, - parsed_args) + parsed_args, + ) self.assertIn( - '--os-compute-api-version 2.58 or greater is required', str(ex)) + '--os-compute-api-version 2.58 or greater is required', str(ex), + ) def test_server_event_list_with_changes_before(self): - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.66') + self._set_mock_microversion('2.66') arglist = [ '--changes-before', '2016-03-04T06:27:59Z', @@ -207,9 +234,14 @@ class TestListServerEvent(TestServerEvent): parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.servers_mock.get.assert_called_once_with(self.fake_server.name) - self.events_mock.list.assert_called_once_with( - self.fake_server.id, changes_before='2016-03-04T06:27:59Z') + self.sdk_client.find_server.assert_called_with( + self.fake_server.name, + ignore_missing=False, + ) + self.sdk_client.server_actions.assert_called_with( + self.fake_server.id, + changes_before='2016-03-04T06:27:59Z', + ) self.assertEqual(self.columns, columns) self.assertEqual(tuple(self.data), tuple(data)) @@ -218,8 +250,7 @@ class TestListServerEvent(TestServerEvent): def test_server_event_list_with_changes_before_invalid( self, mock_parse_isotime, ): - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.66') + self._set_mock_microversion('2.66') arglist = [ '--changes-before', 'Invalid time value', @@ -236,15 +267,13 @@ class TestListServerEvent(TestServerEvent): self.cmd.take_action, parsed_args) - self.assertIn( - 'Invalid changes-before value:', str(ex)) + self.assertIn('Invalid changes-before value:', str(ex)) mock_parse_isotime.assert_called_once_with( 'Invalid time value' ) def test_server_event_list_with_changes_before_pre_v266(self): - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.65') + self._set_mock_microversion('2.65') arglist = [ '--changes-before', '2016-03-04T06:27:59Z', @@ -262,11 +291,12 @@ class TestListServerEvent(TestServerEvent): parsed_args) self.assertIn( - '--os-compute-api-version 2.66 or greater is required', str(ex)) + '--os-compute-api-version 2.66 or greater is required', + str(ex), + ) def test_server_event_list_with_limit(self): - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.58') + self._set_mock_microversion('2.58') arglist = [ '--limit', '1', @@ -280,12 +310,14 @@ class TestListServerEvent(TestServerEvent): parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.cmd.take_action(parsed_args) - self.events_mock.list.assert_called_once_with( - self.fake_server.id, limit=1) + self.sdk_client.server_actions.assert_called_with( + self.fake_server.id, + limit=1, + paginated=False, + ) def test_server_event_list_with_limit_pre_v258(self): - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.57') + self._set_mock_microversion('2.57') arglist = [ '--limit', '1', @@ -300,14 +332,16 @@ class TestListServerEvent(TestServerEvent): ex = self.assertRaises( exceptions.CommandError, self.cmd.take_action, - parsed_args) + parsed_args, + ) self.assertIn( - '--os-compute-api-version 2.58 or greater is required', str(ex)) + '--os-compute-api-version 2.58 or greater is required', + str(ex), + ) def test_server_event_list_with_marker(self): - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.58') + self._set_mock_microversion('2.58') arglist = [ '--marker', 'test_event', @@ -321,12 +355,13 @@ class TestListServerEvent(TestServerEvent): parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.cmd.take_action(parsed_args) - self.events_mock.list.assert_called_once_with( - self.fake_server.id, marker='test_event') + self.sdk_client.server_actions.assert_called_with( + self.fake_server.id, + marker='test_event', + ) def test_server_event_list_with_marker_pre_v258(self): - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.57') + self._set_mock_microversion('2.57') arglist = [ '--marker', 'test_event', @@ -349,12 +384,11 @@ class TestListServerEvent(TestServerEvent): class TestShowServerEvent(TestServerEvent): - fake_event = compute_fakes.FakeServerEvent.create_one_server_event() - + fake_event = compute_fakes.create_one_server_action() columns = ( 'action', 'events', - 'instance_uuid', + 'id', 'message', 'project_id', 'request_id', @@ -363,8 +397,8 @@ class TestShowServerEvent(TestServerEvent): ) data = ( fake_event.action, - fake_event.events, - fake_event.instance_uuid, + server_event.ServerActionEventColumn(fake_event.events), + fake_event.id, fake_event.message, fake_event.project_id, fake_event.request_id, @@ -373,9 +407,11 @@ class TestShowServerEvent(TestServerEvent): ) def setUp(self): - super(TestShowServerEvent, self).setUp() + super().setUp() + + self.sdk_client.find_server.return_value = self.fake_server + self.sdk_client.get_server_action.return_value = self.fake_event - self.events_mock.get.return_value = self.fake_event self.cmd = server_event.ShowServerEvent(self.app, None) def test_server_event_show(self): @@ -389,12 +425,16 @@ class TestShowServerEvent(TestServerEvent): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - columns, data = self.cmd.take_action(parsed_args) - self.servers_mock.get.assert_called_once_with(self.fake_server.name) - self.events_mock.get.assert_called_once_with( - self.fake_server.id, self.fake_event.request_id) + self.sdk_client.find_server.assert_called_with( + self.fake_server.name, + ignore_missing=False, + ) + self.sdk_client.get_server_action.assert_called_with( + self.fake_event.request_id, + self.fake_server.id, + ) self.assertEqual(self.columns, columns) self.assertEqual(self.data, data) diff --git a/releasenotes/notes/migrate-server-events-to-sdk-6a1f5dce582df245.yaml b/releasenotes/notes/migrate-server-events-to-sdk-6a1f5dce582df245.yaml new file mode 100644 index 0000000000..b62e5dab32 --- /dev/null +++ b/releasenotes/notes/migrate-server-events-to-sdk-6a1f5dce582df245.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + The ``server event list`` and ``server event show`` commands have been + migrated to SDK.