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

@@ -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(
+        aggregate = compute_client.find_aggregate(
-            compute_client.aggregates,
+            parsed_args.aggregate, ignore_missing=False)
-            parsed_args.aggregate,
-        )
-        data = compute_client.aggregates.add_host(aggregate, parsed_args.host)
 
-        info = {}
+        aggregate = compute_client.add_host_to_aggregate(
-        info.update(data._info)
+            aggregate.id, parsed_args.host)
 
-        # Special mapping for columns to make the output easier to read:
+        display_columns, columns = _get_aggregate_columns(aggregate)
-        # 'metadata' --> 'properties'
+        data = utils.get_item_properties(
-        info.update(
+            aggregate, columns, formatters=_aggregate_formatters)
-            {
+        return (display_columns, data)
-                'hosts': format_columns.ListColumn(info.pop('hosts')),
-                'properties': format_columns.DictColumn(info.pop('metadata')),
-            },
-        )
-        return zip(*sorted(info.items()))
 
 
 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 = {}
+        attrs = {'name': parsed_args.name}
-        data = compute_client.aggregates.create(
+
-            parsed_args.name,
+        if parsed_args.zone:
-            parsed_args.zone,
+            attrs['availability_zone'] = parsed_args.zone
-        )
+
-        info.update(data._info)
+        aggregate = compute_client.create_aggregate(**attrs)
 
         if parsed_args.property:
-            info.update(compute_client.aggregates.set_metadata(
+            aggregate = compute_client.set_aggregate_metadata(
-                data,
+                aggregate.id,
                 parsed_args.property,
-            )._info)
+            )
 
-        # Special mapping for columns to make the output easier to read:
+        display_columns, columns = _get_aggregate_columns(aggregate)
-        # 'metadata' --> 'properties'
+        data = utils.get_item_properties(
-        hosts = None
+            aggregate, columns, formatters=_aggregate_formatters)
-        properties = None
+        return (display_columns, data)
-        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()))
 
 
 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(
+                aggregate = compute_client.find_aggregate(
-                    compute_client.aggregates, a)
+                    a, ignore_missing=False)
-                compute_client.aggregates.delete(data.id)
+                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:
+            for aggregate in aggregates:
-                if 'availability_zone' in d.metadata:
+                if 'availability_zone' in aggregate.metadata:
-                    d.metadata.pop('availability_zone')
+                    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,
+        data = (
-                (utils.get_item_properties(
+            utils.get_item_properties(
-                    s, columns,
+                s, columns, formatters=_aggregate_formatters
-                    formatters={
+            ) for s in aggregates)
-                        'Hosts': format_columns.ListColumn,
+        return (column_headers, data)
-                        'Metadata': format_columns.DictColumn,
-                    },
-                ) for s in 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(
+        aggregate = compute_client.find_aggregate(
-            compute_client.aggregates,
+            parsed_args.aggregate, ignore_missing=False)
-            parsed_args.aggregate,
-        )
-        data = compute_client.aggregates.remove_host(
-            aggregate,
-            parsed_args.host,
-        )
 
-        info = {}
+        aggregate = compute_client.remove_host_from_aggregate(
-        info.update(data._info)
+            aggregate.id, parsed_args.host)
 
-        # Special mapping for columns to make the output easier to read:
+        display_columns, columns = _get_aggregate_columns(aggregate)
-        # 'metadata' --> 'properties'
+        data = utils.get_item_properties(
-        info.update(
+            aggregate, columns, formatters=_aggregate_formatters)
-            {
+        return (display_columns, data)
-                'hosts': format_columns.ListColumn(info.pop('hosts')),
-                'properties': format_columns.DictColumn(info.pop('metadata')),
-            },
-        )
-        return zip(*sorted(info.items()))
 
 
 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
+        compute_client = self.app.client_manager.sdk_connection.compute
-        aggregate = utils.find_resource(
+        aggregate = compute_client.find_aggregate(
-            compute_client.aggregates,
+            parsed_args.aggregate, ignore_missing=False)
-            parsed_args.aggregate,
-        )
 
         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(
+            compute_client.update_aggregate(aggregate.id, **kwargs)
-                aggregate,
-                kwargs
-            )
 
         set_property = {}
         if parsed_args.no_property:
-            # NOTE(RuiChen): "availability_zone" is removed from response of
+            # NOTE(RuiChen): "availability_zone" can not be unset from
-            #                aggregate show and create commands, don't see it
+            # properties. It is already excluded from show and create output.
-            #                anywhere, so pop it, avoid the unexpected server
-            #                exception(can't unset the availability zone from
-            #                aggregate metadata in nova).
             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(
+            compute_client.set_aggregate_metadata(
-                aggregate,
+                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
+        compute_client = self.app.client_manager.sdk_connection.compute
-        data = utils.find_resource(
+        aggregate = compute_client.find_aggregate(
-            compute_client.aggregates,
+            parsed_args.aggregate, ignore_missing=False)
-            parsed_args.aggregate,
+
-        )
         # Remove availability_zone from metadata because Nova doesn't
-        if 'availability_zone' in data.metadata:
+        if 'availability_zone' in aggregate.metadata:
-            data.metadata.pop('availability_zone')
+            aggregate.metadata.pop('availability_zone')
 
-        # Special mapping for columns to make the output easier to read:
+        display_columns, columns = _get_aggregate_columns(aggregate)
-        # 'metadata' --> 'properties'
+        data = utils.get_item_properties(
-        data._info.update(
+            aggregate, columns, formatters=_aggregate_formatters)
-            {
+        return (display_columns, data)
-                '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()))
 
 
 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
+        compute_client = self.app.client_manager.sdk_connection.compute
-        aggregate = utils.find_resource(
+        aggregate = compute_client.find_aggregate(
-            compute_client.aggregates,
+            parsed_args.aggregate, ignore_missing=False)
-            parsed_args.aggregate)
 
         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,
+            compute_client.set_aggregate_metadata(
-                                                   unset_property)
+                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='<aggregate>',
+            help=_("Aggregate (name or ID)")
+        )
+        parser.add_argument(
+            'image',
+            metavar='<image>',
+            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)

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.app.client_manager.sdk_connection = mock.Mock()
-        self.aggregate_mock.reset_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.sdk_client.find_aggregate.return_value = self.fake_ag
-        self.aggregate_mock.add_host.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.sdk_client.find_aggregate.assert_called_once_with(
-        self.aggregate_mock.add_host.assert_called_once_with(self.fake_ag,
+            parsed_args.aggregate, ignore_missing=False)
-                                                             parsed_args.host)
+        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.sdk_client.create_aggregate.return_value = self.fake_ag
-        self.aggregate_mock.set_metadata.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,
+        self.sdk_client.create_aggregate.assert_called_once_with(
-                                                           None)
+            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,
+        self.sdk_client.create_aggregate.assert_called_once_with(
-                                                           parsed_args.zone)
+            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,
+        self.sdk_client.create_aggregate.assert_called_once_with(
-                                                           None)
+            name=parsed_args.name)
-        self.aggregate_mock.set_metadata.assert_called_once_with(
+        self.sdk_client.set_aggregate_metadata.assert_called_once_with(
-            self.fake_ag, parsed_args.property)
+            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.cmd.take_action(parsed_args)
-        self.aggregate_mock.get.assert_called_once_with(self.fake_ags[0].id)
+        self.sdk_client.find_aggregate.assert_called_once_with(
-        self.aggregate_mock.delete.assert_called_once_with(self.fake_ags[0].id)
+            self.fake_ags[0].id, ignore_missing=False)
-        self.assertIsNone(result)
+        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))
+            calls.append(call(a.id, ignore_missing=False))
-        self.aggregate_mock.delete.assert_has_calls(calls)
+        self.sdk_client.find_aggregate.assert_has_calls(calls)
-        self.assertIsNone(result)
+        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]
+        self.sdk_client.find_aggregate.side_effect = [
-        with mock.patch.object(utils, 'find_resource',
+            self.fake_ags[0], sdk_exceptions.NotFoundException]
-                               side_effect=find_mock_result) as find_mock:
+        try:
-            try:
+            self.cmd.take_action(parsed_args)
-                self.cmd.take_action(parsed_args)
+            self.fail('CommandError should be raised.')
-                self.fail('CommandError should be raised.')
+        except exceptions.CommandError as e:
-            except exceptions.CommandError as e:
+            self.assertEqual('1 of 2 aggregates failed to delete.',
-                self.assertEqual('1 of 2 aggregates failed to delete.',
+                             str(e))
-                                 str(e))
 
-            find_mock.assert_any_call(self.aggregate_mock, self.fake_ags[0].id)
+        calls = []
-            find_mock.assert_any_call(self.aggregate_mock, 'unexist_aggregate')
+        for a in arglist:
-
+            calls.append(call(a, ignore_missing=False))
-            self.assertEqual(2, find_mock.call_count)
+        self.sdk_client.find_aggregate.assert_has_calls(calls)
-            self.aggregate_mock.delete.assert_called_once_with(
+        self.sdk_client.delete_aggregate.assert_called_with(
-                self.fake_ags[0].id
+            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.sdk_client.find_aggregate.return_value = self.fake_ag
-        self.aggregate_mock.remove_host.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.sdk_client.find_aggregate.assert_called_once_with(
-        self.aggregate_mock.remove_host.assert_called_once_with(
+            parsed_args.aggregate, ignore_missing=False)
-            self.fake_ag, parsed_args.host)
+        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.sdk_client.find_aggregate.assert_called_once_with(
-        self.assertNotCalled(self.aggregate_mock.update)
+            parsed_args.aggregate, ignore_missing=False)
-        self.assertNotCalled(self.aggregate_mock.set_metadata)
+        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.sdk_client.find_aggregate.assert_called_once_with(
-        self.aggregate_mock.update.assert_called_once_with(
+            parsed_args.aggregate, ignore_missing=False)
-            self.fake_ag, {'name': parsed_args.name})
+        self.sdk_client.update_aggregate.assert_called_once_with(
-        self.assertNotCalled(self.aggregate_mock.set_metadata)
+            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.sdk_client.find_aggregate.assert_called_once_with(
-        self.aggregate_mock.update.assert_called_once_with(
+            parsed_args.aggregate, ignore_missing=False)
-            self.fake_ag, {'availability_zone': parsed_args.zone})
+        self.sdk_client.update_aggregate.assert_called_once_with(
-        self.assertNotCalled(self.aggregate_mock.set_metadata)
+            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.sdk_client.find_aggregate.assert_called_once_with(
-        self.assertNotCalled(self.aggregate_mock.update)
+            parsed_args.aggregate, ignore_missing=False)
-        self.aggregate_mock.set_metadata.assert_called_once_with(
+        self.assertNotCalled(self.sdk_client.update_aggregate)
-            self.fake_ag, parsed_args.property)
+        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.sdk_client.find_aggregate.assert_called_once_with(
-        self.assertNotCalled(self.aggregate_mock.update)
+            parsed_args.aggregate, ignore_missing=False)
-        self.aggregate_mock.set_metadata.assert_called_once_with(
+        self.assertNotCalled(self.sdk_client.update_aggregate)
-            self.fake_ag, {'key1': None, 'key2': 'value2'})
+        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.sdk_client.find_aggregate.assert_called_once_with(
-        self.assertNotCalled(self.aggregate_mock.update)
+            parsed_args.aggregate, ignore_missing=False)
-        self.aggregate_mock.set_metadata.assert_called_once_with(
+        self.assertNotCalled(self.sdk_client.update_aggregate)
-            self.fake_ag, {'key1': None})
+        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.sdk_client.find_aggregate.assert_called_once_with(
-        self.aggregate_mock.update.assert_called_once_with(
+            parsed_args.aggregate, ignore_missing=False)
-            self.fake_ag, {'availability_zone': parsed_args.zone})
+        self.sdk_client.update_aggregate.assert_called_once_with(
-        self.aggregate_mock.set_metadata.assert_called_once_with(
+            self.fake_ag.id, availability_zone=parsed_args.zone)
-            self.fake_ag, {'key1': None})
+        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])

releasenotes/notes/switch-aggregate-to-sdk-ced451a0f28bf6ea.yaml

@@ -0,0 +1,4 @@
+---
+features:
+  - Switch aggregate operations to use SDK
+  - Adds 'aggregate cache image' operation

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