diff --git a/lower-constraints.txt b/lower-constraints.txt index b489501b8..e08b05d22 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -45,7 +45,7 @@ msgpack-python==0.4.0 munch==2.1.0 netaddr==0.7.18 netifaces==0.10.4 -openstacksdk==0.51.0 +openstacksdk==0.52.0 os-service-types==1.7.0 os-testr==1.0.0 osc-lib==2.2.0 diff --git a/openstackclient/compute/v2/flavor.py b/openstackclient/compute/v2/flavor.py index 00431b7b7..8477e8efb 100644 --- a/openstackclient/compute/v2/flavor.py +++ b/openstackclient/compute/v2/flavor.py @@ -17,7 +17,8 @@ import logging -from novaclient import api_versions +from openstack import exceptions as sdk_exceptions +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 @@ -33,10 +34,7 @@ LOG = logging.getLogger(__name__) _formatters = { 'extra_specs': format_columns.DictColumn, - # Unless we finish switch to use SDK resources this need to be doubled this - # way - 'properties': format_columns.DictColumn, - 'Properties': format_columns.DictColumn + 'properties': format_columns.DictColumn } @@ -51,29 +49,10 @@ def _get_flavor_columns(item): } hidden_columns = ['links', 'location'] - return utils.get_osc_show_columns_for_sdk_resource( item, column_map, hidden_columns) -def _find_flavor(compute_client, flavor): - try: - return compute_client.flavors.get(flavor) - except Exception as ex: - if type(ex).__name__ == 'NotFound': - pass - else: - raise - try: - return compute_client.flavors.find(name=flavor, is_public=None) - except Exception as ex: - if type(ex).__name__ == 'NotFound': - msg = _("No flavor with a name or ID of '%s' exists.") % flavor - raise exceptions.CommandError(msg) - else: - raise - - class CreateFlavor(command.ShowOne): _description = _("Create new flavor") @@ -87,9 +66,7 @@ class CreateFlavor(command.ShowOne): parser.add_argument( "--id", metavar="", - default='auto', - help=_("Unique flavor ID; 'auto' creates a UUID " - "(default: auto)") + help=_("Unique flavor ID") ) parser.add_argument( "--ram", @@ -170,32 +147,36 @@ class CreateFlavor(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 identity_client = self.app.client_manager.identity if parsed_args.project and parsed_args.public: msg = _("--project is only allowed with --private") raise exceptions.CommandError(msg) + args = { + 'name': parsed_args.name, + 'ram': parsed_args.ram, + 'vcpus': parsed_args.vcpus, + 'disk': parsed_args.disk, + 'id': parsed_args.id, + 'ephemeral': parsed_args.ephemeral, + 'swap': parsed_args.swap, + 'rxtx_factor': parsed_args.rxtx_factor, + 'is_public': parsed_args.public, + } + if parsed_args.description: - if compute_client.api_version < api_versions.APIVersion("2.55"): - msg = _("--os-compute-api-version 2.55 or later is required") + if not sdk_utils.supports_microversion(compute_client, '2.55'): + msg = _( + 'The --description parameter requires server support for ' + 'API microversion 2.55' + ) raise exceptions.CommandError(msg) - args = ( - parsed_args.name, - parsed_args.ram, - parsed_args.vcpus, - parsed_args.disk, - parsed_args.id, - parsed_args.ephemeral, - parsed_args.swap, - parsed_args.rxtx_factor, - parsed_args.public, - parsed_args.description - ) + args['description'] = parsed_args.description - flavor = compute_client.flavors.create(*args) + flavor = compute_client.create_flavor(**args) if parsed_args.project: try: @@ -204,7 +185,7 @@ class CreateFlavor(command.ShowOne): parsed_args.project, parsed_args.project_domain, ).id - compute_client.flavor_access.add_tenant_access( + compute_client.flavor_add_tenant_access( flavor.id, project_id) except Exception as e: msg = _("Failed to add project %(project)s access to " @@ -212,19 +193,14 @@ class CreateFlavor(command.ShowOne): LOG.error(msg, {'project': parsed_args.project, 'e': e}) if parsed_args.property: try: - flavor.set_keys(parsed_args.property) + flavor = compute_client.create_flavor_extra_specs( + flavor, parsed_args.property) except Exception as e: LOG.error(_("Failed to set flavor property: %s"), e) - flavor_info = flavor._info.copy() - flavor_info['properties'] = flavor.get_keys() - - display_columns, columns = _get_flavor_columns(flavor_info) - data = utils.get_dict_properties( - flavor_info, columns, - formatters=_formatters, - mixed_case_fields=['OS-FLV-DISABLED:disabled', - 'OS-FLV-EXT-DATA:ephemeral']) + display_columns, columns = _get_flavor_columns(flavor) + data = utils.get_dict_properties(flavor, columns, + formatters=_formatters) return (display_columns, data) @@ -243,12 +219,12 @@ class DeleteFlavor(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 f in parsed_args.flavor: try: - flavor = _find_flavor(compute_client, f) - compute_client.flavors.delete(flavor.id) + flavor = compute_client.find_flavor(f, ignore_missing=False) + compute_client.delete_flavor(flavor.id) except Exception as e: result += 1 LOG.error(_("Failed to delete flavor with name or " @@ -307,37 +283,63 @@ class ListFlavor(command.Lister): return parser def take_action(self, parsed_args): - compute_client = self.app.client_manager.compute - columns = ( - "ID", - "Name", - "RAM", - "Disk", - "Ephemeral", - "VCPUs", - "Is Public", - ) - + compute_client = self.app.client_manager.sdk_connection.compute # is_public is ternary - None means give all flavors, # True is public only and False is private only # By default Nova assumes True and gives admins public flavors # and flavors from their own projects only. is_public = None if parsed_args.all else parsed_args.public - data = compute_client.flavors.list(is_public=is_public, - marker=parsed_args.marker, - limit=parsed_args.limit) + query_attrs = { + 'is_public': is_public + } + if parsed_args.marker: + query_attrs['marker'] = parsed_args.marker + if parsed_args.limit: + query_attrs['limit'] = parsed_args.limit + if parsed_args.limit or parsed_args.marker: + # User passed explicit pagination request, switch off SDK + # pagination + query_attrs['paginated'] = False + data = list(compute_client.flavors(**query_attrs)) + # Even if server supports 2.61 some policy might stop it sending us + # extra_specs. So try to fetch them if they are absent + for f in data: + if not f.extra_specs: + compute_client.fetch_flavor_extra_specs(f) + + columns = ( + "id", + "name", + "ram", + "disk", + "ephemeral", + "vcpus", + "is_public" + ) if parsed_args.long: - columns = columns + ( + columns += ( + "swap", + "rxtx_factor", + "extra_specs", + ) + + column_headers = ( + "ID", + "Name", + "RAM", + "Disk", + "Ephemeral", + "VCPUs", + "Is Public" + ) + if parsed_args.long: + column_headers += ( "Swap", "RXTX Factor", "Properties", ) - for f in data: - f.properties = f.get_keys() - - column_headers = columns return (column_headers, (utils.get_item_properties( @@ -387,24 +389,42 @@ class SetFlavor(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 identity_client = self.app.client_manager.identity - flavor = _find_flavor(compute_client, parsed_args.flavor) + try: + flavor = compute_client.find_flavor( + parsed_args.flavor, + get_extra_specs=True, + ignore_missing=False) + except sdk_exceptions.ResourceNotFound as e: + raise exceptions.CommandError(e.message) + + if parsed_args.description: + if not sdk_utils.supports_microversion(compute_client, '2.55'): + msg = _( + 'The --description parameter requires server support for ' + 'API microversion 2.55' + ) + raise exceptions.CommandError(msg) + + compute_client.update_flavor( + flavor=flavor.id, description=parsed_args.description) result = 0 - key_list = [] if parsed_args.no_property: try: - for key in flavor.get_keys().keys(): - key_list.append(key) - flavor.unset_keys(key_list) + for key in flavor.extra_specs.keys(): + compute_client.delete_flavor_extra_specs_property( + flavor.id, key) except Exception as e: LOG.error(_("Failed to clear flavor property: %s"), e) result += 1 + if parsed_args.property: try: - flavor.set_keys(parsed_args.property) + compute_client.create_flavor_extra_specs( + flavor.id, parsed_args.property) except Exception as e: LOG.error(_("Failed to set flavor property: %s"), e) result += 1 @@ -420,7 +440,7 @@ class SetFlavor(command.Command): parsed_args.project, parsed_args.project_domain, ).id - compute_client.flavor_access.add_tenant_access( + compute_client.flavor_add_tenant_access( flavor.id, project_id) except Exception as e: LOG.error(_("Failed to set flavor access to project: %s"), e) @@ -430,13 +450,6 @@ class SetFlavor(command.Command): raise exceptions.CommandError(_("Command Failed: One or more of" " the operations failed")) - if parsed_args.description: - if compute_client.api_version < api_versions.APIVersion("2.55"): - msg = _("--os-compute-api-version 2.55 or later is required") - raise exceptions.CommandError(msg) - compute_client.flavors.update(flavor=flavor.id, - description=parsed_args.description) - class ShowFlavor(command.ShowOne): _description = _("Display flavor details") @@ -451,35 +464,32 @@ class ShowFlavor(command.ShowOne): return parser def take_action(self, parsed_args): - compute_client = self.app.client_manager.compute - resource_flavor = _find_flavor(compute_client, parsed_args.flavor) + compute_client = self.app.client_manager.sdk_connection.compute + flavor = compute_client.find_flavor( + parsed_args.flavor, get_extra_specs=True, ignore_missing=False) access_projects = None # get access projects list of this flavor - if not resource_flavor.is_public: + if not flavor.is_public: try: - flavor_access = compute_client.flavor_access.list( - flavor=resource_flavor.id) - access_projects = [utils.get_field(access, 'tenant_id') - for access in flavor_access] + flavor_access = compute_client.get_flavor_access( + flavor=flavor.id) + access_projects = [ + utils.get_field(access, 'tenant_id') + for access in flavor_access] except Exception as e: msg = _("Failed to get access projects list " "for flavor '%(flavor)s': %(e)s") LOG.error(msg, {'flavor': parsed_args.flavor, 'e': e}) - flavor = resource_flavor._info.copy() - flavor.update({ - 'access_project_ids': access_projects - }) - - flavor['properties'] = resource_flavor.get_keys() + # Since we need to inject "access_project_id" into resource - convert + # it to dict and treat it respectively + flavor = flavor.to_dict() + flavor['access_project_ids'] = access_projects display_columns, columns = _get_flavor_columns(flavor) data = utils.get_dict_properties( - flavor, columns, - formatters=_formatters, - mixed_case_fields=['OS-FLV-DISABLED:disabled', - 'OS-FLV-EXT-DATA:ephemeral']) + flavor, columns, formatters=_formatters) return (display_columns, data) @@ -512,32 +522,40 @@ class UnsetFlavor(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 identity_client = self.app.client_manager.identity - flavor = _find_flavor(compute_client, parsed_args.flavor) + try: + flavor = compute_client.find_flavor( + parsed_args.flavor, + get_extra_specs=True, + ignore_missing=False) + except sdk_exceptions.ResourceNotFound as e: + raise exceptions.CommandError(_(e.message)) result = 0 if parsed_args.property: - try: - flavor.unset_keys(parsed_args.property) - except Exception as e: - LOG.error(_("Failed to unset flavor property: %s"), e) - result += 1 + for key in parsed_args.property: + try: + compute_client.delete_flavor_extra_specs_property( + flavor.id, key) + except sdk_exceptions.SDKException as e: + LOG.error(_("Failed to unset flavor property: %s"), e) + result += 1 if parsed_args.project: try: if flavor.is_public: msg = _("Cannot remove access for a public flavor") raise exceptions.CommandError(msg) - else: - project_id = identity_common.find_project( - identity_client, - parsed_args.project, - parsed_args.project_domain, - ).id - compute_client.flavor_access.remove_tenant_access( - flavor.id, project_id) + + project_id = identity_common.find_project( + identity_client, + parsed_args.project, + parsed_args.project_domain, + ).id + compute_client.flavor_remove_tenant_access( + flavor.id, project_id) except Exception as e: LOG.error(_("Failed to remove flavor access from project: %s"), e) diff --git a/openstackclient/tests/unit/compute/v2/fakes.py b/openstackclient/tests/unit/compute/v2/fakes.py index 3a06d2713..d3d037a9a 100644 --- a/openstackclient/tests/unit/compute/v2/fakes.py +++ b/openstackclient/tests/unit/compute/v2/fakes.py @@ -19,6 +19,7 @@ from unittest import mock import uuid from novaclient import api_versions +from openstack.compute.v2 import flavor as _flavor from openstackclient.api import compute_v2 from openstackclient.tests.unit import fakes @@ -164,7 +165,6 @@ class FakeComputev2Client(object): self.extensions.resource_class = fakes.FakeResource(None, {}) self.flavors = mock.Mock() - self.flavors.resource_class = fakes.FakeResource(None, {}) self.flavor_access = mock.Mock() self.flavor_access.resource_class = fakes.FakeResource(None, {}) @@ -777,27 +777,13 @@ class FakeFlavor(object): 'os-flavor-access:is_public': True, 'description': 'description', 'OS-FLV-EXT-DATA:ephemeral': 0, - 'properties': {'property': 'value'}, + 'extra_specs': {'property': 'value'}, } # Overwrite default attributes. flavor_info.update(attrs) - # Set default methods. - flavor_methods = { - 'set_keys': None, - 'unset_keys': None, - 'get_keys': {'property': 'value'}, - } - - flavor = fakes.FakeResource(info=copy.deepcopy(flavor_info), - methods=flavor_methods, - loaded=True) - - # Set attributes with special mappings in nova client. - flavor.disabled = flavor_info['OS-FLV-DISABLED:disabled'] - flavor.is_public = flavor_info['os-flavor-access:is_public'] - flavor.ephemeral = flavor_info['OS-FLV-EXT-DATA:ephemeral'] + flavor = _flavor.Flavor(**flavor_info) return flavor diff --git a/openstackclient/tests/unit/compute/v2/test_flavor.py b/openstackclient/tests/unit/compute/v2/test_flavor.py index 2828d74e3..8625b7120 100644 --- a/openstackclient/tests/unit/compute/v2/test_flavor.py +++ b/openstackclient/tests/unit/compute/v2/test_flavor.py @@ -12,11 +12,11 @@ # License for the specific language governing permissions and limitations # under the License. # - from unittest import mock -from unittest.mock import call -import novaclient +from openstack.compute.v2 import flavor as _flavor +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 @@ -31,13 +31,19 @@ class TestFlavor(compute_fakes.TestComputev2): def setUp(self): super(TestFlavor, self).setUp() - # Get a shortcut to the FlavorManager Mock - self.flavors_mock = self.app.client_manager.compute.flavors - self.flavors_mock.reset_mock() - - # Get a shortcut to the FlavorAccessManager Mock - self.flavor_access_mock = self.app.client_manager.compute.flavor_access - self.flavor_access_mock.reset_mock() + # SDK 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.flavors = mock.Mock() + self.sdk_client.find_flavor = mock.Mock() + self.sdk_client.delete_flavor = mock.Mock() + self.sdk_client.update_flavor = mock.Mock() + self.sdk_client.flavor_add_tenant_access = mock.Mock() + self.sdk_client.flavor_remove_tenant_access = mock.Mock() + self.sdk_client.create_flavor_extra_specs = mock.Mock() + self.sdk_client.update_flavor_extra_specs_property = mock.Mock() + self.sdk_client.delete_flavor_extra_specs_property = mock.Mock() self.projects_mock = self.app.client_manager.identity.projects self.projects_mock.reset_mock() @@ -48,6 +54,7 @@ class TestFlavorCreate(TestFlavor): flavor = compute_fakes.FakeFlavor.create_one_flavor( attrs={'links': 'flavor-links'}) project = identity_fakes.FakeProject.create_one_project() + columns = ( 'OS-FLV-DISABLED:disabled', 'OS-FLV-EXT-DATA:ephemeral', @@ -60,17 +67,32 @@ class TestFlavorCreate(TestFlavor): 'ram', 'rxtx_factor', 'swap', - 'vcpus', + 'vcpus' ) + data = ( - flavor.disabled, + flavor.is_disabled, flavor.ephemeral, flavor.description, flavor.disk, flavor.id, flavor.name, flavor.is_public, - format_columns.DictColumn(flavor.properties), + format_columns.DictColumn(flavor.extra_specs), + flavor.ram, + flavor.rxtx_factor, + flavor.swap, + flavor.vcpus, + ) + data_private = ( + flavor.is_disabled, + flavor.ephemeral, + flavor.description, + flavor.disk, + flavor.id, + flavor.name, + False, + format_columns.DictColumn(flavor.extra_specs), flavor.ram, flavor.rxtx_factor, flavor.swap, @@ -82,7 +104,7 @@ class TestFlavorCreate(TestFlavor): # Return a project self.projects_mock.get.return_value = self.project - self.flavors_mock.create.return_value = self.flavor + self.sdk_client.create_flavor.return_value = self.flavor self.cmd = flavor.CreateFlavor(self.app, None) def test_flavor_create_default_options(self): @@ -95,20 +117,20 @@ class TestFlavorCreate(TestFlavor): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - default_args = ( - self.flavor.name, - 256, - 1, - 0, - 'auto', - 0, - 0, - 1.0, - True, - None, - ) + default_args = { + 'name': self.flavor.name, + 'ram': 256, + 'vcpus': 1, + 'disk': 0, + 'id': None, + 'ephemeral': 0, + 'swap': 0, + 'rxtx_factor': 1.0, + 'is_public': True, + } + columns, data = self.cmd.take_action(parsed_args) - self.flavors_mock.create.assert_called_once_with(*default_args) + self.sdk_client.create_flavor.assert_called_once_with(**default_args) self.assertEqual(self.columns, columns) self.assertItemEqual(self.data, data) @@ -143,29 +165,44 @@ class TestFlavorCreate(TestFlavor): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - args = ( - self.flavor.name, - self.flavor.ram, - self.flavor.vcpus, - self.flavor.disk, - self.flavor.id, - self.flavor.ephemeral, - self.flavor.swap, - self.flavor.rxtx_factor, - self.flavor.is_public, - self.flavor.description, - ) - self.app.client_manager.compute.api_version = 2.55 - with mock.patch.object(novaclient.api_versions, - 'APIVersion', - return_value=2.55): - columns, data = self.cmd.take_action(parsed_args) - self.flavors_mock.create.assert_called_once_with(*args) - self.flavor.set_keys.assert_called_once_with({'property': 'value'}) - self.flavor.get_keys.assert_called_once_with() + args = { + 'name': self.flavor.name, + 'ram': self.flavor.ram, + 'vcpus': self.flavor.vcpus, + 'disk': self.flavor.disk, + 'id': self.flavor.id, + 'ephemeral': self.flavor.ephemeral, + 'swap': self.flavor.swap, + 'rxtx_factor': self.flavor.rxtx_factor, + 'is_public': self.flavor.is_public, + 'description': self.flavor.description + } - self.assertEqual(self.columns, columns) - self.assertItemEqual(self.data, data) + props = {'property': 'value'} + + # SDK updates the flavor object instance. In order to make the + # verification clear and preciese let's create new flavor and change + # expected props this way + create_flavor = _flavor.Flavor(**self.flavor) + expected_flavor = _flavor.Flavor(**self.flavor) + expected_flavor.extra_specs = props + # convert expected data tuple to list to be able to modify it + cmp_data = list(self.data) + cmp_data[7] = format_columns.DictColumn(props) + self.sdk_client.create_flavor.return_value = create_flavor + self.sdk_client.create_flavor_extra_specs.return_value = \ + expected_flavor + + with mock.patch.object(sdk_utils, 'supports_microversion', + return_value=True): + columns, data = self.cmd.take_action(parsed_args) + self.sdk_client.create_flavor.assert_called_once_with(**args) + self.sdk_client.create_flavor_extra_specs.assert_called_once_with( + create_flavor, props) + self.sdk_client.get_flavor_access.assert_not_called() + + self.assertEqual(self.columns, columns) + self.assertItemEqual(tuple(cmp_data), data) def test_flavor_create_other_options(self): @@ -200,33 +237,47 @@ class TestFlavorCreate(TestFlavor): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - args = ( - self.flavor.name, - self.flavor.ram, - self.flavor.vcpus, - self.flavor.disk, - 'auto', - self.flavor.ephemeral, - self.flavor.swap, - self.flavor.rxtx_factor, - self.flavor.is_public, - self.flavor.description, - ) - self.app.client_manager.compute.api_version = 2.55 - with mock.patch.object(novaclient.api_versions, - 'APIVersion', - return_value=2.55): + args = { + 'name': self.flavor.name, + 'ram': self.flavor.ram, + 'vcpus': self.flavor.vcpus, + 'disk': self.flavor.disk, + 'id': 'auto', + 'ephemeral': self.flavor.ephemeral, + 'swap': self.flavor.swap, + 'rxtx_factor': self.flavor.rxtx_factor, + 'is_public': False, + 'description': self.flavor.description + } + + props = {'key1': 'value1', 'key2': 'value2'} + + # SDK updates the flavor object instance. In order to make the + # verification clear and preciese let's create new flavor and change + # expected props this way + create_flavor = _flavor.Flavor(**self.flavor) + expected_flavor = _flavor.Flavor(**self.flavor) + expected_flavor.extra_specs = props + expected_flavor.is_public = False + # convert expected data tuple to list to be able to modify it + cmp_data = list(self.data_private) + cmp_data[7] = format_columns.DictColumn(props) + self.sdk_client.create_flavor.return_value = create_flavor + self.sdk_client.create_flavor_extra_specs.return_value = \ + expected_flavor + + with mock.patch.object(sdk_utils, 'supports_microversion', + return_value=True): columns, data = self.cmd.take_action(parsed_args) - self.flavors_mock.create.assert_called_once_with(*args) - self.flavor_access_mock.add_tenant_access.assert_called_with( + self.sdk_client.create_flavor.assert_called_once_with(**args) + self.sdk_client.flavor_add_tenant_access.assert_called_with( self.flavor.id, self.project.id, ) - self.flavor.set_keys.assert_called_with( - {'key1': 'value1', 'key2': 'value2'}) - self.flavor.get_keys.assert_called_with() + self.sdk_client.create_flavor_extra_specs.assert_called_with( + create_flavor, props) self.assertEqual(self.columns, columns) - self.assertItemEqual(self.data, data) + self.assertItemEqual(cmp_data, data) def test_public_flavor_create_with_project(self): arglist = [ @@ -278,29 +329,28 @@ class TestFlavorCreate(TestFlavor): ('name', self.flavor.name), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - self.app.client_manager.compute.api_version = 2.55 - with mock.patch.object(novaclient.api_versions, - 'APIVersion', - return_value=2.55): + with mock.patch.object(sdk_utils, 'supports_microversion', + return_value=True): + columns, data = self.cmd.take_action(parsed_args) - args = ( - self.flavor.name, - self.flavor.ram, - self.flavor.vcpus, - self.flavor.disk, - self.flavor.id, - self.flavor.ephemeral, - self.flavor.swap, - self.flavor.rxtx_factor, - False, - 'fake description', - ) + args = { + 'name': self.flavor.name, + 'ram': self.flavor.ram, + 'vcpus': self.flavor.vcpus, + 'disk': self.flavor.disk, + 'id': self.flavor.id, + 'ephemeral': self.flavor.ephemeral, + 'swap': self.flavor.swap, + 'rxtx_factor': self.flavor.rxtx_factor, + 'is_public': self.flavor.is_public, + 'description': 'fake description' + } - self.flavors_mock.create.assert_called_once_with(*args) + self.sdk_client.create_flavor.assert_called_once_with(**args) self.assertEqual(self.columns, columns) - self.assertItemEqual(self.data, data) + self.assertItemEqual(self.data_private, data) def test_flavor_create_with_description_api_older(self): arglist = [ @@ -318,10 +368,8 @@ class TestFlavorCreate(TestFlavor): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - self.app.client_manager.compute.api_version = 2.54 - with mock.patch.object(novaclient.api_versions, - 'APIVersion', - return_value=2.55): + with mock.patch.object(sdk_utils, 'supports_microversion', + return_value=False): self.assertRaises(exceptions.CommandError, self.cmd.take_action, parsed_args) @@ -333,9 +381,7 @@ class TestFlavorDelete(TestFlavor): def setUp(self): super(TestFlavorDelete, self).setUp() - self.flavors_mock.get = ( - compute_fakes.FakeFlavor.get_flavors(self.flavors)) - self.flavors_mock.delete.return_value = None + self.sdk_client.delete_flavor.return_value = None self.cmd = flavor.DeleteFlavor(self.app, None) @@ -348,9 +394,13 @@ class TestFlavorDelete(TestFlavor): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.sdk_client.find_flavor.return_value = self.flavors[0] + result = self.cmd.take_action(parsed_args) - self.flavors_mock.delete.assert_called_with(self.flavors[0].id) + self.sdk_client.find_flavor.assert_called_with(self.flavors[0].id, + ignore_missing=False) + self.sdk_client.delete_flavor.assert_called_with(self.flavors[0].id) self.assertIsNone(result) def test_delete_multiple_flavors(self): @@ -362,12 +412,17 @@ class TestFlavorDelete(TestFlavor): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.sdk_client.find_flavor.side_effect = self.flavors + result = self.cmd.take_action(parsed_args) - calls = [] - for f in self.flavors: - calls.append(call(f.id)) - self.flavors_mock.delete.assert_has_calls(calls) + find_calls = [ + mock.call(i.id, ignore_missing=False) for i in self.flavors + ] + delete_calls = [mock.call(i.id) for i in self.flavors] + self.sdk_client.find_flavor.assert_has_calls(find_calls) + self.sdk_client.delete_flavor.assert_has_calls(delete_calls) self.assertIsNone(result) def test_multi_flavors_delete_with_exception(self): @@ -380,11 +435,10 @@ class TestFlavorDelete(TestFlavor): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - find_mock_result = [self.flavors[0], exceptions.CommandError] - self.flavors_mock.get = ( - mock.Mock(side_effect=find_mock_result) - ) - self.flavors_mock.find.side_effect = exceptions.NotFound(None) + self.sdk_client.find_flavor.side_effect = [ + self.flavors[0], + sdk_exceptions.ResourceNotFound + ] try: self.cmd.take_action(parsed_args) @@ -392,15 +446,18 @@ class TestFlavorDelete(TestFlavor): except exceptions.CommandError as e: self.assertEqual('1 of 2 flavors failed to delete.', str(e)) - self.flavors_mock.get.assert_any_call(self.flavors[0].id) - self.flavors_mock.get.assert_any_call('unexist_flavor') - self.flavors_mock.delete.assert_called_once_with(self.flavors[0].id) + find_calls = [ + mock.call(self.flavors[0].id, ignore_missing=False), + mock.call('unexist_flavor', ignore_missing=False), + ] + delete_calls = [mock.call(self.flavors[0].id)] + self.sdk_client.find_flavor.assert_has_calls(find_calls) + self.sdk_client.delete_flavor.assert_has_calls(delete_calls) class TestFlavorList(TestFlavor): - # Return value of self.flavors_mock.list(). - flavors = compute_fakes.FakeFlavor.create_flavors(count=1) + _flavor = compute_fakes.FakeFlavor.create_one_flavor() columns = ( 'ID', @@ -418,24 +475,27 @@ class TestFlavorList(TestFlavor): ) data = (( - flavors[0].id, - flavors[0].name, - flavors[0].ram, - flavors[0].disk, - flavors[0].ephemeral, - flavors[0].vcpus, - flavors[0].is_public, - ), ) + _flavor.id, + _flavor.name, + _flavor.ram, + _flavor.disk, + _flavor.ephemeral, + _flavor.vcpus, + _flavor.is_public, + ),) data_long = (data[0] + ( - flavors[0].swap, - flavors[0].rxtx_factor, - format_columns.DictColumn(flavors[0].properties) + _flavor.swap, + _flavor.rxtx_factor, + format_columns.DictColumn(_flavor.extra_specs) ), ) def setUp(self): super(TestFlavorList, self).setUp() - self.flavors_mock.list.return_value = self.flavors + self.api_mock = mock.Mock() + self.api_mock.side_effect = [[self._flavor], [], ] + + self.sdk_client.flavors = self.api_mock # Get the command object to test self.cmd = flavor.ListFlavor(self.app, None) @@ -458,16 +518,14 @@ class TestFlavorList(TestFlavor): # Set expected values kwargs = { 'is_public': True, - 'limit': None, - 'marker': None } - self.flavors_mock.list.assert_called_with( + self.sdk_client.flavors.assert_called_with( **kwargs ) self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data), tuple(data)) + self.assertEqual(self.data, tuple(data)) def test_flavor_list_all_flavors(self): arglist = [ @@ -487,16 +545,14 @@ class TestFlavorList(TestFlavor): # Set expected values kwargs = { 'is_public': None, - 'limit': None, - 'marker': None } - self.flavors_mock.list.assert_called_with( + self.sdk_client.flavors.assert_called_with( **kwargs ) self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data), tuple(data)) + self.assertEqual(self.data, tuple(data)) def test_flavor_list_private_flavors(self): arglist = [ @@ -516,16 +572,14 @@ class TestFlavorList(TestFlavor): # Set expected values kwargs = { 'is_public': False, - 'limit': None, - 'marker': None } - self.flavors_mock.list.assert_called_with( + self.sdk_client.flavors.assert_called_with( **kwargs ) self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data), tuple(data)) + self.assertEqual(self.data, tuple(data)) def test_flavor_list_public_flavors(self): arglist = [ @@ -545,16 +599,14 @@ class TestFlavorList(TestFlavor): # Set expected values kwargs = { 'is_public': True, - 'limit': None, - 'marker': None } - self.flavors_mock.list.assert_called_with( + self.sdk_client.flavors.assert_called_with( **kwargs ) self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data), tuple(data)) + self.assertEqual(self.data, tuple(data)) def test_flavor_list_long(self): arglist = [ @@ -574,11 +626,9 @@ class TestFlavorList(TestFlavor): # Set expected values kwargs = { 'is_public': True, - 'limit': None, - 'marker': None } - self.flavors_mock.list.assert_called_with( + self.sdk_client.flavors.assert_called_with( **kwargs ) @@ -588,7 +638,7 @@ class TestFlavorList(TestFlavor): class TestFlavorSet(TestFlavor): - # Return value of self.flavors_mock.find(). + # Return value of self.sdk_client.find_flavor(). flavor = compute_fakes.FakeFlavor.create_one_flavor( attrs={'os-flavor-access:is_public': False}) project = identity_fakes.FakeProject.create_one_project() @@ -596,8 +646,7 @@ class TestFlavorSet(TestFlavor): def setUp(self): super(TestFlavorSet, self).setUp() - self.flavors_mock.find.return_value = self.flavor - self.flavors_mock.get.side_effect = exceptions.NotFound(None) + self.sdk_client.find_flavor.return_value = self.flavor # Return a project self.projects_mock.get.return_value = self.project self.cmd = flavor.SetFlavor(self.app, None) @@ -614,9 +663,14 @@ class TestFlavorSet(TestFlavor): parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.flavors_mock.find.assert_called_with(name=parsed_args.flavor, - is_public=None) - self.flavor.set_keys.assert_called_with({'FOO': '"B A R"'}) + self.sdk_client.find_flavor.assert_called_with( + parsed_args.flavor, + get_extra_specs=True, + ignore_missing=False + ) + self.sdk_client.create_flavor_extra_specs.assert_called_with( + self.flavor.id, + {'FOO': '"B A R"'}) self.assertIsNone(result) def test_flavor_set_no_property(self): @@ -631,9 +685,13 @@ class TestFlavorSet(TestFlavor): parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.flavors_mock.find.assert_called_with(name=parsed_args.flavor, - is_public=None) - self.flavor.unset_keys.assert_called_with(['property']) + self.sdk_client.find_flavor.assert_called_with( + parsed_args.flavor, + get_extra_specs=True, + ignore_missing=False + ) + self.sdk_client.delete_flavor_extra_specs_property.assert_called_with( + self.flavor.id, 'property') self.assertIsNone(result) def test_flavor_set_project(self): @@ -649,13 +707,16 @@ class TestFlavorSet(TestFlavor): result = self.cmd.take_action(parsed_args) - self.flavors_mock.find.assert_called_with(name=parsed_args.flavor, - is_public=None) - self.flavor_access_mock.add_tenant_access.assert_called_with( + self.sdk_client.find_flavor.assert_called_with( + parsed_args.flavor, + get_extra_specs=True, + ignore_missing=False + ) + self.sdk_client.flavor_add_tenant_access.assert_called_with( self.flavor.id, self.project.id, ) - self.flavor.set_keys.assert_not_called() + self.sdk_client.create_flavor_extra_specs.assert_not_called() self.assertIsNone(result) def test_flavor_set_no_project(self): @@ -681,8 +742,9 @@ class TestFlavorSet(TestFlavor): self.cmd, arglist, verifylist) def test_flavor_set_with_unexist_flavor(self): - self.flavors_mock.get.side_effect = exceptions.NotFound(None) - self.flavors_mock.find.side_effect = exceptions.NotFound(None) + self.sdk_client.find_flavor.side_effect = [ + sdk_exceptions.ResourceNotFound() + ] arglist = [ '--project', self.project.id, @@ -708,9 +770,12 @@ class TestFlavorSet(TestFlavor): parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.flavors_mock.find.assert_called_with(name=parsed_args.flavor, - is_public=None) - self.flavor_access_mock.add_tenant_access.assert_not_called() + self.sdk_client.find_flavor.assert_called_with( + parsed_args.flavor, + get_extra_specs=True, + ignore_missing=False + ) + self.sdk_client.flavor_add_tenant_access.assert_not_called() self.assertIsNone(result) def test_flavor_set_description_api_newer(self): @@ -724,11 +789,11 @@ class TestFlavorSet(TestFlavor): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.app.client_manager.compute.api_version = 2.55 - with mock.patch.object(novaclient.api_versions, - 'APIVersion', - return_value=2.55): + with mock.patch.object(sdk_utils, + 'supports_microversion', + return_value=True): result = self.cmd.take_action(parsed_args) - self.flavors_mock.update.assert_called_with( + self.sdk_client.update_flavor.assert_called_with( flavor=self.flavor.id, description='description') self.assertIsNone(result) @@ -743,9 +808,9 @@ class TestFlavorSet(TestFlavor): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.app.client_manager.compute.api_version = 2.54 - with mock.patch.object(novaclient.api_versions, - 'APIVersion', - return_value=2.55): + with mock.patch.object(sdk_utils, + 'supports_microversion', + return_value=False): self.assertRaises(exceptions.CommandError, self.cmd.take_action, parsed_args) @@ -760,11 +825,12 @@ class TestFlavorSet(TestFlavor): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.app.client_manager.compute.api_version = 2.55 - with mock.patch.object(novaclient.api_versions, - 'APIVersion', - return_value=2.55): + + with mock.patch.object(sdk_utils, + 'supports_microversion', + return_value=True): result = self.cmd.take_action(parsed_args) - self.flavors_mock.update.assert_called_with( + self.sdk_client.update_flavor.assert_called_with( flavor=self.flavor.id, description='description') self.assertIsNone(result) @@ -779,16 +845,17 @@ class TestFlavorSet(TestFlavor): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.app.client_manager.compute.api_version = 2.54 - with mock.patch.object(novaclient.api_versions, - 'APIVersion', - return_value=2.55): + + with mock.patch.object(sdk_utils, + 'supports_microversion', + return_value=False): self.assertRaises(exceptions.CommandError, self.cmd.take_action, parsed_args) class TestFlavorShow(TestFlavor): - # Return value of self.flavors_mock.find(). + # Return value of self.sdk_client.find_flavor(). flavor_access = compute_fakes.FakeFlavorAccess.create_one_flavor_access() flavor = compute_fakes.FakeFlavor.create_one_flavor() @@ -805,11 +872,11 @@ class TestFlavorShow(TestFlavor): 'ram', 'rxtx_factor', 'swap', - 'vcpus', + 'vcpus' ) data = ( - flavor.disabled, + flavor.is_disabled, flavor.ephemeral, None, flavor.description, @@ -817,7 +884,7 @@ class TestFlavorShow(TestFlavor): flavor.id, flavor.name, flavor.is_public, - format_columns.DictColumn(flavor.get_keys()), + format_columns.DictColumn(flavor.extra_specs), flavor.ram, flavor.rxtx_factor, flavor.swap, @@ -828,9 +895,8 @@ class TestFlavorShow(TestFlavor): super(TestFlavorShow, self).setUp() # Return value of _find_resource() - self.flavors_mock.find.return_value = self.flavor - self.flavors_mock.get.side_effect = exceptions.NotFound(None) - self.flavor_access_mock.list.return_value = [self.flavor_access] + self.sdk_client.find_flavor.return_value = self.flavor + self.sdk_client.get_flavor_access.return_value = [self.flavor_access] self.cmd = flavor.ShowFlavor(self.app, None) def test_show_no_options(self): @@ -862,7 +928,7 @@ class TestFlavorShow(TestFlavor): 'os-flavor-access:is_public': False, } ) - self.flavors_mock.find.return_value = private_flavor + self.sdk_client.find_flavor.return_value = private_flavor arglist = [ private_flavor.name, @@ -872,7 +938,7 @@ class TestFlavorShow(TestFlavor): ] data_with_project = ( - private_flavor.disabled, + private_flavor.is_disabled, private_flavor.ephemeral, [self.flavor_access.tenant_id], private_flavor.description, @@ -880,7 +946,7 @@ class TestFlavorShow(TestFlavor): private_flavor.id, private_flavor.name, private_flavor.is_public, - format_columns.DictColumn(private_flavor.get_keys()), + format_columns.DictColumn(private_flavor.extra_specs), private_flavor.ram, private_flavor.rxtx_factor, private_flavor.swap, @@ -891,7 +957,7 @@ class TestFlavorShow(TestFlavor): columns, data = self.cmd.take_action(parsed_args) - self.flavor_access_mock.list.assert_called_with( + self.sdk_client.get_flavor_access.assert_called_with( flavor=private_flavor.id) self.assertEqual(self.columns, columns) self.assertItemEqual(data_with_project, data) @@ -899,7 +965,7 @@ class TestFlavorShow(TestFlavor): class TestFlavorUnset(TestFlavor): - # Return value of self.flavors_mock.find(). + # Return value of self.sdk_client.find_flavor(). flavor = compute_fakes.FakeFlavor.create_one_flavor( attrs={'os-flavor-access:is_public': False}) project = identity_fakes.FakeProject.create_one_project() @@ -907,12 +973,13 @@ class TestFlavorUnset(TestFlavor): def setUp(self): super(TestFlavorUnset, self).setUp() - self.flavors_mock.find.return_value = self.flavor - self.flavors_mock.get.side_effect = exceptions.NotFound(None) + self.sdk_client.find_flavor.return_value = self.flavor # Return a project self.projects_mock.get.return_value = self.project self.cmd = flavor.UnsetFlavor(self.app, None) + self.mock_shortcut = self.sdk_client.delete_flavor_extra_specs_property + def test_flavor_unset_property(self): arglist = [ '--property', 'property', @@ -925,12 +992,49 @@ class TestFlavorUnset(TestFlavor): parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.flavors_mock.find.assert_called_with(name=parsed_args.flavor, - is_public=None) - self.flavor.unset_keys.assert_called_with(['property']) - self.flavor_access_mock.remove_tenant_access.assert_not_called() + self.sdk_client.find_flavor.assert_called_with( + parsed_args.flavor, + get_extra_specs=True, + ignore_missing=False) + self.mock_shortcut.assert_called_with( + self.flavor.id, 'property') + self.sdk_client.flavor_remove_tenant_access.assert_not_called() self.assertIsNone(result) + def test_flavor_unset_properties(self): + arglist = [ + '--property', 'property1', + '--property', 'property2', + 'baremetal' + ] + verifylist = [ + ('property', ['property1', 'property2']), + ('flavor', 'baremetal'), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.cmd.take_action(parsed_args) + self.sdk_client.find_flavor.assert_called_with( + parsed_args.flavor, + get_extra_specs=True, + ignore_missing=False) + calls = [ + mock.call(self.flavor.id, 'property1'), + mock.call(self.flavor.id, 'property2') + ] + self.mock_shortcut.assert_has_calls( + calls) + + # A bit tricky way to ensure we do not unset other properties + calls.append(mock.call(self.flavor.id, 'property')) + self.assertRaises( + AssertionError, + self.mock_shortcut.assert_has_calls, + calls + ) + + self.sdk_client.flavor_remove_tenant_access.assert_not_called() + def test_flavor_unset_project(self): arglist = [ '--project', self.project.id, @@ -945,13 +1049,14 @@ class TestFlavorUnset(TestFlavor): result = self.cmd.take_action(parsed_args) self.assertIsNone(result) - self.flavors_mock.find.assert_called_with(name=parsed_args.flavor, - is_public=None) - self.flavor_access_mock.remove_tenant_access.assert_called_with( + self.sdk_client.find_flavor.assert_called_with( + parsed_args.flavor, get_extra_specs=True, + ignore_missing=False) + self.sdk_client.flavor_remove_tenant_access.assert_called_with( self.flavor.id, self.project.id, ) - self.flavor.unset_keys.assert_not_called() + self.sdk_client.delete_flavor_extra_specs_proerty.assert_not_called() self.assertIsNone(result) def test_flavor_unset_no_project(self): @@ -977,8 +1082,9 @@ class TestFlavorUnset(TestFlavor): self.cmd, arglist, verifylist) def test_flavor_unset_with_unexist_flavor(self): - self.flavors_mock.get.side_effect = exceptions.NotFound(None) - self.flavors_mock.find.side_effect = exceptions.NotFound(None) + self.sdk_client.find_flavor.side_effect = [ + sdk_exceptions.ResourceNotFound + ] arglist = [ '--project', self.project.id, @@ -1004,4 +1110,4 @@ class TestFlavorUnset(TestFlavor): result = self.cmd.take_action(parsed_args) self.assertIsNone(result) - self.flavor_access_mock.remove_tenant_access.assert_not_called() + self.sdk_client.flavor_remove_tenant_access.assert_not_called() diff --git a/releasenotes/notes/switch-flavor-to-sdk-b874a3c39559815e.yaml b/releasenotes/notes/switch-flavor-to-sdk-b874a3c39559815e.yaml new file mode 100644 index 000000000..7863c3238 --- /dev/null +++ b/releasenotes/notes/switch-flavor-to-sdk-b874a3c39559815e.yaml @@ -0,0 +1,4 @@ +--- +features: + - Switch compute.flavor operations from direct API calls (novaclient) to + OpenStackSDK. diff --git a/requirements.txt b/requirements.txt index 9430a7fde..b167628b5 100644 --- a/requirements.txt +++ b/requirements.txt @@ -5,7 +5,7 @@ pbr!=2.1.0,>=2.0.0 # Apache-2.0 cliff>=3.4.0 # Apache-2.0 iso8601>=0.1.11 # MIT -openstacksdk>=0.51.0 # Apache-2.0 +openstacksdk>=0.52.0 # Apache-2.0 osc-lib>=2.2.0 # Apache-2.0 oslo.i18n>=3.15.3 # Apache-2.0 python-keystoneclient>=3.22.0 # Apache-2.0