From f36a34b675e69a811d5cd48f6bcfd6ce7bda6a5a Mon Sep 17 00:00:00 2001 From: Artem Goncharov Date: Tue, 10 Nov 2020 13:56:11 +0100 Subject: [PATCH] Switch compute aggregate functions to SDK Continue journey towards having OSC consuming SDK for nova part. Depends-On: https://review.opendev.org/#/c/762131/ Change-Id: Id16e6c47aa93f02f15f49e1f59f73fecaa3e3b80 --- openstackclient/compute/v2/aggregate.py | 257 +++++++++--------- .../tests/unit/compute/v2/test_aggregate.py | 257 ++++++++++++------ ...tch-aggregate-to-sdk-ced451a0f28bf6ea.yaml | 4 + setup.cfg | 1 + 4 files changed, 310 insertions(+), 209 deletions(-) create mode 100644 releasenotes/notes/switch-aggregate-to-sdk-ced451a0f28bf6ea.yaml diff --git a/openstackclient/compute/v2/aggregate.py b/openstackclient/compute/v2/aggregate.py index 599659a3e..8b70f4264 100644 --- a/openstackclient/compute/v2/aggregate.py +++ b/openstackclient/compute/v2/aggregate.py @@ -18,6 +18,7 @@ import logging +from openstack import utils as sdk_utils from osc_lib.cli import format_columns from osc_lib.cli import parseractions from osc_lib.command import command @@ -30,6 +31,25 @@ from openstackclient.i18n import _ LOG = logging.getLogger(__name__) +_aggregate_formatters = { + 'Hosts': format_columns.ListColumn, + 'Metadata': format_columns.DictColumn, + 'hosts': format_columns.ListColumn, + 'metadata': format_columns.DictColumn, +} + + +def _get_aggregate_columns(item): + # To maintain backwards compatibility we need to rename sdk props to + # whatever OSC was using before + column_map = { + 'metadata': 'properties', + } + hidden_columns = ['links', 'location'] + return utils.get_osc_show_columns_for_sdk_resource( + item, column_map, hidden_columns) + + class AddAggregateHost(command.ShowOne): _description = _("Add host to aggregate") @@ -48,26 +68,18 @@ class AddAggregateHost(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 - aggregate = utils.find_resource( - compute_client.aggregates, - parsed_args.aggregate, - ) - data = compute_client.aggregates.add_host(aggregate, parsed_args.host) + aggregate = compute_client.find_aggregate( + parsed_args.aggregate, ignore_missing=False) - info = {} - info.update(data._info) + aggregate = compute_client.add_host_to_aggregate( + aggregate.id, parsed_args.host) - # Special mapping for columns to make the output easier to read: - # 'metadata' --> 'properties' - info.update( - { - 'hosts': format_columns.ListColumn(info.pop('hosts')), - 'properties': format_columns.DictColumn(info.pop('metadata')), - }, - ) - return zip(*sorted(info.items())) + display_columns, columns = _get_aggregate_columns(aggregate) + data = utils.get_item_properties( + aggregate, columns, formatters=_aggregate_formatters) + return (display_columns, data) class CreateAggregate(command.ShowOne): @@ -95,36 +107,25 @@ class CreateAggregate(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 - info = {} - data = compute_client.aggregates.create( - parsed_args.name, - parsed_args.zone, - ) - info.update(data._info) + attrs = {'name': parsed_args.name} + + if parsed_args.zone: + attrs['availability_zone'] = parsed_args.zone + + aggregate = compute_client.create_aggregate(**attrs) if parsed_args.property: - info.update(compute_client.aggregates.set_metadata( - data, + aggregate = compute_client.set_aggregate_metadata( + aggregate.id, parsed_args.property, - )._info) + ) - # Special mapping for columns to make the output easier to read: - # 'metadata' --> 'properties' - hosts = None - properties = None - if 'hosts' in info.keys(): - hosts = format_columns.ListColumn(info.pop('hosts')) - if 'metadata' in info.keys(): - properties = format_columns.DictColumn(info.pop('metadata')) - info.update( - { - 'hosts': hosts, - 'properties': properties, - }, - ) - return zip(*sorted(info.items())) + display_columns, columns = _get_aggregate_columns(aggregate) + data = utils.get_item_properties( + aggregate, columns, formatters=_aggregate_formatters) + return (display_columns, data) class DeleteAggregate(command.Command): @@ -141,13 +142,14 @@ class DeleteAggregate(command.Command): return parser def take_action(self, parsed_args): - compute_client = self.app.client_manager.compute + compute_client = self.app.client_manager.sdk_connection.compute result = 0 for a in parsed_args.aggregate: try: - data = utils.find_resource( - compute_client.aggregates, a) - compute_client.aggregates.delete(data.id) + aggregate = compute_client.find_aggregate( + a, ignore_missing=False) + compute_client.delete_aggregate( + aggregate.id, ignore_missing=False) except Exception as e: result += 1 LOG.error(_("Failed to delete aggregate with name or " @@ -175,15 +177,15 @@ class ListAggregate(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 - data = compute_client.aggregates.list() + aggregates = list(compute_client.aggregates()) if parsed_args.long: # Remove availability_zone from metadata because Nova doesn't - for d in data: - if 'availability_zone' in d.metadata: - d.metadata.pop('availability_zone') + for aggregate in aggregates: + if 'availability_zone' in aggregate.metadata: + aggregate.metadata.pop('availability_zone') # This is the easiest way to change column headers column_headers = ( "ID", @@ -204,14 +206,11 @@ class ListAggregate(command.Lister): "Availability Zone", ) - return (column_headers, - (utils.get_item_properties( - s, columns, - formatters={ - 'Hosts': format_columns.ListColumn, - 'Metadata': format_columns.DictColumn, - }, - ) for s in data)) + data = ( + utils.get_item_properties( + s, columns, formatters=_aggregate_formatters + ) for s in aggregates) + return (column_headers, data) class RemoveAggregateHost(command.ShowOne): @@ -232,29 +231,18 @@ class RemoveAggregateHost(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 - aggregate = utils.find_resource( - compute_client.aggregates, - parsed_args.aggregate, - ) - data = compute_client.aggregates.remove_host( - aggregate, - parsed_args.host, - ) + aggregate = compute_client.find_aggregate( + parsed_args.aggregate, ignore_missing=False) - info = {} - info.update(data._info) + aggregate = compute_client.remove_host_from_aggregate( + aggregate.id, parsed_args.host) - # Special mapping for columns to make the output easier to read: - # 'metadata' --> 'properties' - info.update( - { - 'hosts': format_columns.ListColumn(info.pop('hosts')), - 'properties': format_columns.DictColumn(info.pop('metadata')), - }, - ) - return zip(*sorted(info.items())) + display_columns, columns = _get_aggregate_columns(aggregate) + data = utils.get_item_properties( + aggregate, columns, formatters=_aggregate_formatters) + return (display_columns, data) class SetAggregate(command.Command): @@ -296,11 +284,9 @@ class SetAggregate(command.Command): def take_action(self, parsed_args): - compute_client = self.app.client_manager.compute - aggregate = utils.find_resource( - compute_client.aggregates, - parsed_args.aggregate, - ) + compute_client = self.app.client_manager.sdk_connection.compute + aggregate = compute_client.find_aggregate( + parsed_args.aggregate, ignore_missing=False) kwargs = {} if parsed_args.name: @@ -308,18 +294,12 @@ class SetAggregate(command.Command): if parsed_args.zone: kwargs['availability_zone'] = parsed_args.zone if kwargs: - compute_client.aggregates.update( - aggregate, - kwargs - ) + compute_client.update_aggregate(aggregate.id, **kwargs) set_property = {} if parsed_args.no_property: - # NOTE(RuiChen): "availability_zone" is removed from response of - # aggregate show and create commands, don't see it - # anywhere, so pop it, avoid the unexpected server - # exception(can't unset the availability zone from - # aggregate metadata in nova). + # NOTE(RuiChen): "availability_zone" can not be unset from + # properties. It is already excluded from show and create output. set_property.update({key: None for key in aggregate.metadata.keys() if key != 'availability_zone'}) @@ -327,8 +307,8 @@ class SetAggregate(command.Command): set_property.update(parsed_args.property) if set_property: - compute_client.aggregates.set_metadata( - aggregate, + compute_client.set_aggregate_metadata( + aggregate.id, set_property ) @@ -347,31 +327,18 @@ class ShowAggregate(command.ShowOne): def take_action(self, parsed_args): - compute_client = self.app.client_manager.compute - data = utils.find_resource( - compute_client.aggregates, - parsed_args.aggregate, - ) + compute_client = self.app.client_manager.sdk_connection.compute + aggregate = compute_client.find_aggregate( + parsed_args.aggregate, ignore_missing=False) + # Remove availability_zone from metadata because Nova doesn't - if 'availability_zone' in data.metadata: - data.metadata.pop('availability_zone') + if 'availability_zone' in aggregate.metadata: + aggregate.metadata.pop('availability_zone') - # Special mapping for columns to make the output easier to read: - # 'metadata' --> 'properties' - data._info.update( - { - 'hosts': format_columns.ListColumn( - data._info.pop('hosts') - ), - 'properties': format_columns.DictColumn( - data._info.pop('metadata') - ), - }, - ) - - info = {} - info.update(data._info) - return zip(*sorted(info.items())) + display_columns, columns = _get_aggregate_columns(aggregate) + data = utils.get_item_properties( + aggregate, columns, formatters=_aggregate_formatters) + return (display_columns, data) class UnsetAggregate(command.Command): @@ -394,14 +361,56 @@ class UnsetAggregate(command.Command): return parser def take_action(self, parsed_args): - compute_client = self.app.client_manager.compute - aggregate = utils.find_resource( - compute_client.aggregates, - parsed_args.aggregate) + compute_client = self.app.client_manager.sdk_connection.compute + aggregate = compute_client.find_aggregate( + parsed_args.aggregate, ignore_missing=False) unset_property = {} if parsed_args.property: unset_property.update({key: None for key in parsed_args.property}) if unset_property: - compute_client.aggregates.set_metadata(aggregate, - unset_property) + compute_client.set_aggregate_metadata( + aggregate, unset_property) + + +class CacheImageForAggregate(command.Command): + _description = _("Request image caching for aggregate") + # NOTE(gtema): According to stephenfin and dansmith there is no and will + # not be anything to return. + + def get_parser(self, prog_name): + parser = super(CacheImageForAggregate, self).get_parser(prog_name) + parser.add_argument( + 'aggregate', + metavar='', + help=_("Aggregate (name or ID)") + ) + parser.add_argument( + 'image', + metavar='', + nargs='+', + help=_("Image ID to request caching for aggregate (name or ID). " + "May be specified multiple times.") + ) + return parser + + def take_action(self, parsed_args): + compute_client = self.app.client_manager.sdk_connection.compute + + if not sdk_utils.supports_microversion(compute_client, '2.81'): + msg = _( + 'This operation requires server support for ' + 'API microversion 2.81' + ) + raise exceptions.CommandError(msg) + + aggregate = compute_client.find_aggregate( + parsed_args.aggregate, ignore_missing=False) + + images = [] + for img in parsed_args.image: + image = self.app.client_manager.sdk_connection.image.find_image( + img, ignore_missing=False) + images.append(image.id) + + compute_client.aggregate_precache_images(aggregate.id, images) diff --git a/openstackclient/tests/unit/compute/v2/test_aggregate.py b/openstackclient/tests/unit/compute/v2/test_aggregate.py index cd0c1525e..e4c13ea55 100644 --- a/openstackclient/tests/unit/compute/v2/test_aggregate.py +++ b/openstackclient/tests/unit/compute/v2/test_aggregate.py @@ -16,12 +16,14 @@ from unittest import mock from unittest.mock import call +from openstack import exceptions as sdk_exceptions +from openstack import utils as sdk_utils from osc_lib.cli import format_columns from osc_lib import exceptions -from osc_lib import utils from openstackclient.compute.v2 import aggregate from openstackclient.tests.unit.compute.v2 import fakes as compute_fakes +from openstackclient.tests.unit.image.v2 import fakes as image_fakes class TestAggregate(compute_fakes.TestComputev2): @@ -48,8 +50,17 @@ class TestAggregate(compute_fakes.TestComputev2): super(TestAggregate, self).setUp() # Get a shortcut to the AggregateManager Mock - self.aggregate_mock = self.app.client_manager.compute.aggregates - self.aggregate_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.aggregates = mock.Mock() + self.sdk_client.find_aggregate = mock.Mock() + self.sdk_client.create_aggregate = mock.Mock() + self.sdk_client.update_aggregate = mock.Mock() + self.sdk_client.update_aggregate = mock.Mock() + self.sdk_client.set_aggregate_metadata = mock.Mock() + self.sdk_client.add_host_to_aggregate = mock.Mock() + self.sdk_client.remove_host_from_aggregate = mock.Mock() class TestAggregateAddHost(TestAggregate): @@ -57,8 +68,8 @@ class TestAggregateAddHost(TestAggregate): def setUp(self): super(TestAggregateAddHost, self).setUp() - self.aggregate_mock.get.return_value = self.fake_ag - self.aggregate_mock.add_host.return_value = self.fake_ag + self.sdk_client.find_aggregate.return_value = self.fake_ag + self.sdk_client.add_host_to_aggregate.return_value = self.fake_ag self.cmd = aggregate.AddAggregateHost(self.app, None) def test_aggregate_add_host(self): @@ -72,9 +83,10 @@ class TestAggregateAddHost(TestAggregate): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.aggregate_mock.get.assert_called_once_with(parsed_args.aggregate) - self.aggregate_mock.add_host.assert_called_once_with(self.fake_ag, - parsed_args.host) + self.sdk_client.find_aggregate.assert_called_once_with( + parsed_args.aggregate, ignore_missing=False) + self.sdk_client.add_host_to_aggregate.assert_called_once_with( + self.fake_ag.id, parsed_args.host) self.assertEqual(self.columns, columns) self.assertItemEqual(self.data, data) @@ -84,8 +96,8 @@ class TestAggregateCreate(TestAggregate): def setUp(self): super(TestAggregateCreate, self).setUp() - self.aggregate_mock.create.return_value = self.fake_ag - self.aggregate_mock.set_metadata.return_value = self.fake_ag + self.sdk_client.create_aggregate.return_value = self.fake_ag + self.sdk_client.set_aggregate_metadata.return_value = self.fake_ag self.cmd = aggregate.CreateAggregate(self.app, None) def test_aggregate_create(self): @@ -97,8 +109,8 @@ class TestAggregateCreate(TestAggregate): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.aggregate_mock.create.assert_called_once_with(parsed_args.name, - None) + self.sdk_client.create_aggregate.assert_called_once_with( + name=parsed_args.name) self.assertEqual(self.columns, columns) self.assertItemEqual(self.data, data) @@ -114,8 +126,8 @@ class TestAggregateCreate(TestAggregate): parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.aggregate_mock.create.assert_called_once_with(parsed_args.name, - parsed_args.zone) + self.sdk_client.create_aggregate.assert_called_once_with( + name=parsed_args.name, availability_zone=parsed_args.zone) self.assertEqual(self.columns, columns) self.assertItemEqual(self.data, data) @@ -131,10 +143,10 @@ class TestAggregateCreate(TestAggregate): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.aggregate_mock.create.assert_called_once_with(parsed_args.name, - None) - self.aggregate_mock.set_metadata.assert_called_once_with( - self.fake_ag, parsed_args.property) + self.sdk_client.create_aggregate.assert_called_once_with( + name=parsed_args.name) + self.sdk_client.set_aggregate_metadata.assert_called_once_with( + self.fake_ag.id, parsed_args.property) self.assertEqual(self.columns, columns) self.assertItemEqual(self.data, data) @@ -146,7 +158,7 @@ class TestAggregateDelete(TestAggregate): def setUp(self): super(TestAggregateDelete, self).setUp() - self.aggregate_mock.get = ( + self.sdk_client.find_aggregate = ( compute_fakes.FakeAggregate.get_aggregates(self.fake_ags)) self.cmd = aggregate.DeleteAggregate(self.app, None) @@ -158,10 +170,11 @@ class TestAggregateDelete(TestAggregate): ('aggregate', [self.fake_ags[0].id]), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - result = self.cmd.take_action(parsed_args) - self.aggregate_mock.get.assert_called_once_with(self.fake_ags[0].id) - self.aggregate_mock.delete.assert_called_once_with(self.fake_ags[0].id) - self.assertIsNone(result) + self.cmd.take_action(parsed_args) + self.sdk_client.find_aggregate.assert_called_once_with( + self.fake_ags[0].id, ignore_missing=False) + self.sdk_client.delete_aggregate.assert_called_once_with( + self.fake_ags[0].id, ignore_missing=False) def test_delete_multiple_aggregates(self): arglist = [] @@ -172,13 +185,13 @@ class TestAggregateDelete(TestAggregate): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - result = self.cmd.take_action(parsed_args) + self.cmd.take_action(parsed_args) calls = [] for a in self.fake_ags: - calls.append(call(a.id)) - self.aggregate_mock.delete.assert_has_calls(calls) - self.assertIsNone(result) + calls.append(call(a.id, ignore_missing=False)) + self.sdk_client.find_aggregate.assert_has_calls(calls) + self.sdk_client.delete_aggregate.assert_has_calls(calls) def test_delete_multiple_agggregates_with_exception(self): arglist = [ @@ -191,23 +204,21 @@ class TestAggregateDelete(TestAggregate): parsed_args = self.check_parser(self.cmd, arglist, verifylist) - find_mock_result = [self.fake_ags[0], exceptions.CommandError] - with mock.patch.object(utils, 'find_resource', - side_effect=find_mock_result) as find_mock: - try: - self.cmd.take_action(parsed_args) - self.fail('CommandError should be raised.') - except exceptions.CommandError as e: - self.assertEqual('1 of 2 aggregates failed to delete.', - str(e)) + self.sdk_client.find_aggregate.side_effect = [ + self.fake_ags[0], sdk_exceptions.NotFoundException] + try: + self.cmd.take_action(parsed_args) + self.fail('CommandError should be raised.') + except exceptions.CommandError as e: + self.assertEqual('1 of 2 aggregates failed to delete.', + str(e)) - find_mock.assert_any_call(self.aggregate_mock, self.fake_ags[0].id) - find_mock.assert_any_call(self.aggregate_mock, 'unexist_aggregate') - - self.assertEqual(2, find_mock.call_count) - self.aggregate_mock.delete.assert_called_once_with( - self.fake_ags[0].id - ) + calls = [] + for a in arglist: + calls.append(call(a, ignore_missing=False)) + self.sdk_client.find_aggregate.assert_has_calls(calls) + self.sdk_client.delete_aggregate.assert_called_with( + self.fake_ags[0].id, ignore_missing=False) class TestAggregateList(TestAggregate): @@ -245,7 +256,7 @@ class TestAggregateList(TestAggregate): def setUp(self): super(TestAggregateList, self).setUp() - self.aggregate_mock.list.return_value = [self.fake_ag] + self.sdk_client.aggregates.return_value = [self.fake_ag] self.cmd = aggregate.ListAggregate(self.app, None) def test_aggregate_list(self): @@ -275,11 +286,11 @@ class TestAggregateRemoveHost(TestAggregate): def setUp(self): super(TestAggregateRemoveHost, self).setUp() - self.aggregate_mock.get.return_value = self.fake_ag - self.aggregate_mock.remove_host.return_value = self.fake_ag + self.sdk_client.find_aggregate.return_value = self.fake_ag + self.sdk_client.remove_host_from_aggregate.return_value = self.fake_ag self.cmd = aggregate.RemoveAggregateHost(self.app, None) - def test_aggregate_add_host(self): + def test_aggregate_remove_host(self): arglist = [ 'ag1', 'host1', @@ -290,9 +301,10 @@ class TestAggregateRemoveHost(TestAggregate): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.aggregate_mock.get.assert_called_once_with(parsed_args.aggregate) - self.aggregate_mock.remove_host.assert_called_once_with( - self.fake_ag, parsed_args.host) + self.sdk_client.find_aggregate.assert_called_once_with( + parsed_args.aggregate, ignore_missing=False) + self.sdk_client.remove_host_from_aggregate.assert_called_once_with( + self.fake_ag.id, parsed_args.host) self.assertEqual(self.columns, columns) self.assertItemEqual(self.data, data) @@ -302,7 +314,7 @@ class TestAggregateSet(TestAggregate): def setUp(self): super(TestAggregateSet, self).setUp() - self.aggregate_mock.get.return_value = self.fake_ag + self.sdk_client.find_aggregate.return_value = self.fake_ag self.cmd = aggregate.SetAggregate(self.app, None) def test_aggregate_set_no_option(self): @@ -315,9 +327,10 @@ class TestAggregateSet(TestAggregate): parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.aggregate_mock.get.assert_called_once_with(parsed_args.aggregate) - self.assertNotCalled(self.aggregate_mock.update) - self.assertNotCalled(self.aggregate_mock.set_metadata) + self.sdk_client.find_aggregate.assert_called_once_with( + parsed_args.aggregate, ignore_missing=False) + self.assertNotCalled(self.sdk_client.update_aggregate) + self.assertNotCalled(self.sdk_client.set_aggregate_metadata) self.assertIsNone(result) def test_aggregate_set_with_name(self): @@ -332,10 +345,11 @@ class TestAggregateSet(TestAggregate): parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.aggregate_mock.get.assert_called_once_with(parsed_args.aggregate) - self.aggregate_mock.update.assert_called_once_with( - self.fake_ag, {'name': parsed_args.name}) - self.assertNotCalled(self.aggregate_mock.set_metadata) + self.sdk_client.find_aggregate.assert_called_once_with( + parsed_args.aggregate, ignore_missing=False) + self.sdk_client.update_aggregate.assert_called_once_with( + self.fake_ag.id, name=parsed_args.name) + self.assertNotCalled(self.sdk_client.set_aggregate_metadata) self.assertIsNone(result) def test_aggregate_set_with_zone(self): @@ -350,10 +364,11 @@ class TestAggregateSet(TestAggregate): parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.aggregate_mock.get.assert_called_once_with(parsed_args.aggregate) - self.aggregate_mock.update.assert_called_once_with( - self.fake_ag, {'availability_zone': parsed_args.zone}) - self.assertNotCalled(self.aggregate_mock.set_metadata) + self.sdk_client.find_aggregate.assert_called_once_with( + parsed_args.aggregate, ignore_missing=False) + self.sdk_client.update_aggregate.assert_called_once_with( + self.fake_ag.id, availability_zone=parsed_args.zone) + self.assertNotCalled(self.sdk_client.set_aggregate_metadata) self.assertIsNone(result) def test_aggregate_set_with_property(self): @@ -369,10 +384,11 @@ class TestAggregateSet(TestAggregate): parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.aggregate_mock.get.assert_called_once_with(parsed_args.aggregate) - self.assertNotCalled(self.aggregate_mock.update) - self.aggregate_mock.set_metadata.assert_called_once_with( - self.fake_ag, parsed_args.property) + self.sdk_client.find_aggregate.assert_called_once_with( + parsed_args.aggregate, ignore_missing=False) + self.assertNotCalled(self.sdk_client.update_aggregate) + self.sdk_client.set_aggregate_metadata.assert_called_once_with( + self.fake_ag.id, parsed_args.property) self.assertIsNone(result) def test_aggregate_set_with_no_property_and_property(self): @@ -388,10 +404,11 @@ class TestAggregateSet(TestAggregate): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.aggregate_mock.get.assert_called_once_with(parsed_args.aggregate) - self.assertNotCalled(self.aggregate_mock.update) - self.aggregate_mock.set_metadata.assert_called_once_with( - self.fake_ag, {'key1': None, 'key2': 'value2'}) + self.sdk_client.find_aggregate.assert_called_once_with( + parsed_args.aggregate, ignore_missing=False) + self.assertNotCalled(self.sdk_client.update_aggregate) + self.sdk_client.set_aggregate_metadata.assert_called_once_with( + self.fake_ag.id, {'key1': None, 'key2': 'value2'}) self.assertIsNone(result) def test_aggregate_set_with_no_property(self): @@ -405,10 +422,11 @@ class TestAggregateSet(TestAggregate): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.aggregate_mock.get.assert_called_once_with(parsed_args.aggregate) - self.assertNotCalled(self.aggregate_mock.update) - self.aggregate_mock.set_metadata.assert_called_once_with( - self.fake_ag, {'key1': None}) + self.sdk_client.find_aggregate.assert_called_once_with( + parsed_args.aggregate, ignore_missing=False) + self.assertNotCalled(self.sdk_client.update_aggregate) + self.sdk_client.set_aggregate_metadata.assert_called_once_with( + self.fake_ag.id, {'key1': None}) self.assertIsNone(result) def test_aggregate_set_with_zone_and_no_property(self): @@ -424,11 +442,12 @@ class TestAggregateSet(TestAggregate): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.aggregate_mock.get.assert_called_once_with(parsed_args.aggregate) - self.aggregate_mock.update.assert_called_once_with( - self.fake_ag, {'availability_zone': parsed_args.zone}) - self.aggregate_mock.set_metadata.assert_called_once_with( - self.fake_ag, {'key1': None}) + self.sdk_client.find_aggregate.assert_called_once_with( + parsed_args.aggregate, ignore_missing=False) + self.sdk_client.update_aggregate.assert_called_once_with( + self.fake_ag.id, availability_zone=parsed_args.zone) + self.sdk_client.set_aggregate_metadata.assert_called_once_with( + self.fake_ag.id, {'key1': None}) self.assertIsNone(result) @@ -457,7 +476,7 @@ class TestAggregateShow(TestAggregate): def setUp(self): super(TestAggregateShow, self).setUp() - self.aggregate_mock.get.return_value = self.fake_ag + self.sdk_client.find_aggregate.return_value = self.fake_ag self.cmd = aggregate.ShowAggregate(self.app, None) def test_aggregate_show(self): @@ -469,7 +488,8 @@ class TestAggregateShow(TestAggregate): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.aggregate_mock.get.assert_called_once_with(parsed_args.aggregate) + self.sdk_client.find_aggregate.assert_called_once_with( + parsed_args.aggregate, ignore_missing=False) self.assertEqual(self.columns, columns) self.assertItemEqual(self.data, tuple(data)) @@ -480,7 +500,7 @@ class TestAggregateUnset(TestAggregate): def setUp(self): super(TestAggregateUnset, self).setUp() - self.aggregate_mock.get.return_value = self.fake_ag + self.sdk_client.find_aggregate.return_value = self.fake_ag self.cmd = aggregate.UnsetAggregate(self.app, None) def test_aggregate_unset(self): @@ -495,7 +515,7 @@ class TestAggregateUnset(TestAggregate): parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.aggregate_mock.set_metadata.assert_called_once_with( + self.sdk_client.set_aggregate_metadata.assert_called_once_with( self.fake_ag, {'unset_key': None}) self.assertIsNone(result) @@ -512,7 +532,7 @@ class TestAggregateUnset(TestAggregate): parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.aggregate_mock.set_metadata.assert_called_once_with( + self.sdk_client.set_aggregate_metadata.assert_called_once_with( self.fake_ag, {'unset_key1': None, 'unset_key2': None}) self.assertIsNone(result) @@ -526,5 +546,72 @@ class TestAggregateUnset(TestAggregate): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.assertNotCalled(self.aggregate_mock.set_metadata) + self.assertNotCalled(self.sdk_client.set_aggregate_metadata) self.assertIsNone(result) + + +class TestAggregateCacheImage(TestAggregate): + + images = image_fakes.FakeImage.create_images(count=2) + + def setUp(self): + super(TestAggregateCacheImage, self).setUp() + + self.sdk_client.find_aggregate.return_value = self.fake_ag + self.find_image_mock = mock.Mock(side_effect=self.images) + self.app.client_manager.sdk_connection.image.find_image = \ + self.find_image_mock + + self.cmd = aggregate.CacheImageForAggregate(self.app, None) + + @mock.patch.object(sdk_utils, 'supports_microversion', return_value=False) + def test_aggregate_not_supported(self, sm_mock): + arglist = [ + 'ag1', + 'im1' + ] + verifylist = [ + ('aggregate', 'ag1'), + ('image', ['im1']), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.assertRaises( + exceptions.CommandError, + self.cmd.take_action, + parsed_args + ) + + @mock.patch.object(sdk_utils, 'supports_microversion', return_value=True) + def test_aggregate_add_single_image(self, sm_mock): + arglist = [ + 'ag1', + 'im1' + ] + verifylist = [ + ('aggregate', 'ag1'), + ('image', ['im1']), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.cmd.take_action(parsed_args) + self.sdk_client.find_aggregate.assert_called_once_with( + parsed_args.aggregate, ignore_missing=False) + self.sdk_client.aggregate_precache_images.assert_called_once_with( + self.fake_ag.id, [self.images[0].id]) + + @mock.patch.object(sdk_utils, 'supports_microversion', return_value=True) + def test_aggregate_add_multiple_images(self, sm_mock): + arglist = [ + 'ag1', + 'im1', + 'im2', + ] + verifylist = [ + ('aggregate', 'ag1'), + ('image', ['im1', 'im2']), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.cmd.take_action(parsed_args) + self.sdk_client.find_aggregate.assert_called_once_with( + parsed_args.aggregate, ignore_missing=False) + self.sdk_client.aggregate_precache_images.assert_called_once_with( + self.fake_ag.id, [self.images[0].id, self.images[1].id]) diff --git a/releasenotes/notes/switch-aggregate-to-sdk-ced451a0f28bf6ea.yaml b/releasenotes/notes/switch-aggregate-to-sdk-ced451a0f28bf6ea.yaml new file mode 100644 index 000000000..0df2d6355 --- /dev/null +++ b/releasenotes/notes/switch-aggregate-to-sdk-ced451a0f28bf6ea.yaml @@ -0,0 +1,4 @@ +--- +features: + - Switch aggregate operations to use SDK + - Adds 'aggregate cache image' operation diff --git a/setup.cfg b/setup.cfg index a29852e35..9299fb918 100644 --- a/setup.cfg +++ b/setup.cfg @@ -65,6 +65,7 @@ openstack.compute.v2 = aggregate_set = openstackclient.compute.v2.aggregate:SetAggregate aggregate_show = openstackclient.compute.v2.aggregate:ShowAggregate aggregate_unset = openstackclient.compute.v2.aggregate:UnsetAggregate + aggregate_cache_image = openstackclient.compute.v2.aggregate:CacheImageForAggregate compute_service_delete = openstackclient.compute.v2.service:DeleteService compute_service_list = openstackclient.compute.v2.service:ListService