From 3c06347a93343850409a243b3c7e1f11eeb4db3f Mon Sep 17 00:00:00 2001 From: rabi Date: Fri, 23 Mar 2018 11:10:55 +0530 Subject: [PATCH] Refactor resource plugins for microversion usage This changes the way nova client plugin is used in some nova resource plugins. Change-Id: Ibc41e0387a3056b3fde7b7690bf924e7493a7a7e --- heat/engine/clients/os/nova.py | 10 +-- .../resources/openstack/nova/keypair.py | 31 ++++---- .../engine/resources/openstack/nova/server.py | 75 ++++++------------- .../resources/openstack/nova/server_group.py | 22 ++++-- heat/tests/openstack/nova/test_keypair.py | 27 +++---- heat/tests/openstack/nova/test_server.py | 22 +++--- 6 files changed, 74 insertions(+), 113 deletions(-) diff --git a/heat/engine/clients/os/nova.py b/heat/engine/clients/os/nova.py index 08afc26474..031fffa8f9 100644 --- a/heat/engine/clients/os/nova.py +++ b/heat/engine/clients/os/nova.py @@ -63,13 +63,6 @@ class NovaClientPlugin(microversion_mixin.MicroversionMixin, NOVA_API_VERSION = '2.1' - # TODO(ramishra) Remove these constants - validate_versions = [ - V2_2, V2_8, V2_10, V2_15, V2_26, V2_37, V2_42 - ] = [ - '2.2', '2.8', '2.10', '2.15', '2.26', '2.37', '2.42' - ] - max_microversion = None service_types = [COMPUTE] = ['compute'] @@ -602,7 +595,6 @@ echo -e '%s\tALL=(ALL)\tNOPASSWD: ALL' >> /etc/sudoers The actual console url is lazily resolved on access. """ nc = self.client - mks_version = self.V2_8 class ConsoleUrls(collections.Mapping): def __init__(self, server): @@ -616,7 +608,7 @@ echo -e '%s\tALL=(ALL)\tNOPASSWD: ALL' >> /etc/sudoers if key not in self.support_console_types: raise exceptions.UnsupportedConsoleType(key) if key == 'webmks': - data = nc(mks_version).servers.get_console_url( + data = nc().servers.get_console_url( server, key) else: data = self.console_method(key) diff --git a/heat/engine/resources/openstack/nova/keypair.py b/heat/engine/resources/openstack/nova/keypair.py index 24df156d85..bff7c7ba64 100644 --- a/heat/engine/resources/openstack/nova/keypair.py +++ b/heat/engine/resources/openstack/nova/keypair.py @@ -22,6 +22,10 @@ from heat.engine import support from heat.engine import translation +NOVA_MICROVERSIONS = (MICROVERSION_KEY_TYPE, + MICROVERSION_USER) = ('2.2', '2.10') + + class KeyPair(resource.Resource): """A resource for creating Nova key pairs. @@ -148,22 +152,17 @@ class KeyPair(resource.Resource): key_type = self.properties[self.KEY_TYPE] user = self.properties[self.USER] - nc_version = None validate_props = [] - if key_type: - nc_version = self.client_plugin().V2_2 + c_plugin = self.client_plugin() + if key_type and not c_plugin.is_version_supported( + MICROVERSION_KEY_TYPE): validate_props.append(self.KEY_TYPE) - if user: - nc_version = self.client_plugin().V2_10 + if user and not c_plugin.is_version_supported(MICROVERSION_USER): validate_props.append(self.USER) - if nc_version: - try: - self.client(version=nc_version) - except exception.InvalidServiceVersion as ex: - msg = (_('Cannot use "%(prop)s" properties - nova does not ' - 'support: %(error)s') % - {'error': six.text_type(ex), 'prop': validate_props}) - raise exception.StackValidationFailed(message=msg) + if validate_props: + msg = (_('Cannot use "%s" properties - nova does not ' + 'support required api microversion.') % validate_props) + raise exception.StackValidationFailed(message=msg) def handle_create(self): pub_key = self.properties[self.PUBLIC_KEY] or None @@ -175,16 +174,12 @@ class KeyPair(resource.Resource): 'public_key': pub_key } - nc_version = None if key_type: - nc_version = self.client_plugin().V2_2 create_kwargs[self.KEY_TYPE] = key_type if user_id: - nc_version = self.client_plugin().V2_10 create_kwargs['user_id'] = user_id - nc = self.client(version=nc_version) - new_keypair = nc.keypairs.create(**create_kwargs) + new_keypair = self.client().keypairs.create(**create_kwargs) if (self.properties[self.SAVE_PRIVATE_KEY] and hasattr(new_keypair, 'private_key')): diff --git a/heat/engine/resources/openstack/nova/server.py b/heat/engine/resources/openstack/nova/server.py index de5f487709..4e3f73241d 100644 --- a/heat/engine/resources/openstack/nova/server.py +++ b/heat/engine/resources/openstack/nova/server.py @@ -38,6 +38,9 @@ cfg.CONF.import_opt('default_user_data_format', 'heat.common.config') LOG = logging.getLogger(__name__) +NOVA_MICROVERSIONS = (MICROVERSION_TAGS, MICROVERSION_STR_NETWORK, + MICROVERSION_NIC_TAGS) = ('2.26', '2.37', '2.42') + class Server(server_base.BaseServer, sh.SchedulerHintsMixin, server_network_mixin.ServerNetworkMixin): @@ -840,19 +843,7 @@ class Server(server_base.BaseServer, sh.SchedulerHintsMixin, server = None try: - api_version = None - # if 'auto' or 'none' is specified, we get the string type - # nics after self._build_nics(), and the string network - # is supported since nova microversion 2.37 - if isinstance(nics, six.string_types): - api_version = self.client_plugin().V2_37 - - if self._is_nic_tagged(self.properties[self.NETWORKS]): - api_version = self.client_plugin().V2_42 - - nc = self.client(version=api_version) - - server = nc.servers.create( + server = self.client().servers.create( name=self._server_name(), image=image, flavor=flavor, @@ -888,8 +879,7 @@ class Server(server_base.BaseServer, sh.SchedulerHintsMixin, def _update_server_tags(self, tags): server = self.client().servers.get(self.resource_id) - self.client(version=self.client_plugin().V2_26 - ).servers.set_tags(server, tags) + self.client().servers.set_tags(server, tags) def handle_check(self): server = self.client().servers.get(self.resource_id) @@ -913,14 +903,8 @@ class Server(server_base.BaseServer, sh.SchedulerHintsMixin, name=self.name) raise - try: - tag_server = self.client( - version=self.client_plugin().V2_26 - ).server.get(self.resource_id) - except Exception as ex: - LOG.warning('Cannot resolve tags for observe reality in case of ' - 'unsupported minimal version tag support client') - else: + if self.client_plugin().is_version_supported(MICROVERSION_TAGS): + tag_server = self.client().servers.get(self.resource_id) server_data['tags'] = tag_server.tag_list() return server, server_data @@ -1181,12 +1165,9 @@ class Server(server_base.BaseServer, sh.SchedulerHintsMixin, if name == self.CONSOLE_URLS: return self.client_plugin('nova').get_console_urls(server) if name == self.TAGS_ATTR: - try: - cv = self.client( - version=self.client_plugin().V2_26) - return cv.servers.tag_list(server) - except exception.InvalidServiceVersion: - return None + if self.client_plugin().is_version_supported(MICROVERSION_TAGS): + return self.client().servers.tag_list(server) + return None def add_dependencies(self, deps): super(Server, self).add_dependencies(deps) @@ -1528,15 +1509,11 @@ class Server(server_base.BaseServer, sh.SchedulerHintsMixin, 'server.') % self.ALLOCATE_NETWORK raise exception.StackValidationFailed(message=msg) # Check if str_network is allowed to use - try: - self.client( - version=self.client_plugin().V2_37) - except exception.InvalidServiceVersion as ex: - msg = (_('Cannot use "%(prop)s" property - compute service ' + if not self.client_plugin().is_version_supported( + MICROVERSION_STR_NETWORK): + msg = (_('Cannot use "%s" property - compute service ' 'does not support the required api ' - 'microversion: %(ex)s') - % {'prop': self.ALLOCATE_NETWORK, - 'ex': six.text_type(ex)}) + 'microversion.') % self.ALLOCATE_NETWORK) raise exception.StackValidationFailed(message=msg) # record if any networks include explicit ports @@ -1550,25 +1527,19 @@ class Server(server_base.BaseServer, sh.SchedulerHintsMixin, # Check if nic tag is allowed to use if self._is_nic_tagged(networks=networks): - try: - self.client( - version=self.client_plugin().V2_42) - except exception.InvalidServiceVersion as ex: - msg = (_('Cannot use "%(tag)s" property in networks - ' - 'nova does not support it: %(error)s')) % { - 'tag': self.NIC_TAG, - 'error': six.text_type(ex) - } + if not self.client_plugin().is_version_supported( + MICROVERSION_NIC_TAGS): + msg = (_('Cannot use "%s" property in networks - ' + 'nova does not support required ' + 'api microversion.'), self.NIC_TAG) raise exception.StackValidationFailed(message=msg) # Check if tags is allowed to use if self.properties[self.TAGS]: - try: - self.client( - version=self.client_plugin().V2_26) - except exception.InvalidServiceVersion as ex: - msg = _('Cannot use "tags" property - nova does not support ' - 'it: %s') % six.text_type(ex) + if not self.client_plugin().is_version_supported( + MICROVERSION_TAGS): + msg = (_('Cannot use "%s" property - nova does not support ' + 'required api microversion.') % self.TAGS) raise exception.StackValidationFailed(message=msg) # retrieve provider's absolute limits if it will be needed diff --git a/heat/engine/resources/openstack/nova/server_group.py b/heat/engine/resources/openstack/nova/server_group.py index 06c394e41a..ccea87c3b9 100644 --- a/heat/engine/resources/openstack/nova/server_group.py +++ b/heat/engine/resources/openstack/nova/server_group.py @@ -10,12 +10,15 @@ # License for the specific language governing permissions and limitations # under the License. +from heat.common import exception from heat.common.i18n import _ from heat.engine import constraints from heat.engine import properties from heat.engine import resource from heat.engine import support +NOVA_MICROVERSIONS = (MICROVERSION_SOFT_POLICIES) = ('2.15') + class ServerGroup(resource.Resource): """A resource for managing a Nova server group. @@ -59,16 +62,21 @@ class ServerGroup(resource.Resource): ), } + def validate(self): + super(ServerGroup, self).validate() + policies = self.properties[self.POLICIES] + is_supported = self.client_plugin().is_version_supported( + MICROVERSION_SOFT_POLICIES) + if (('soft-affinity' in policies or + 'soft-anti-affinity' in policies) and not is_supported): + msg = _('Required microversion for soft policies not supported.') + raise exception.StackValidationFailed(message=msg) + def handle_create(self): name = self.physical_resource_name() policies = self.properties[self.POLICIES] - if 'soft-affinity' in policies or 'soft-anti-affinity' in policies: - client = self.client( - version=self.client_plugin().V2_15) - else: - client = self.client() - server_group = client.server_groups.create(name=name, - policies=policies) + server_group = self.client().server_groups.create(name=name, + policies=policies) self.resource_id_set(server_group.id) def physical_resource_name(self): diff --git a/heat/tests/openstack/nova/test_keypair.py b/heat/tests/openstack/nova/test_keypair.py index 5cd9565212..cc412bbd2d 100644 --- a/heat/tests/openstack/nova/test_keypair.py +++ b/heat/tests/openstack/nova/test_keypair.py @@ -117,7 +117,7 @@ class NovaKeyPairTest(common.HeatTestCase): self.assertEqual(tp_test.resource_id, created_key.name) self.fake_keypairs.create.assert_called_once_with( name=key_name, public_key=None, type='ssh') - self.cp_mock.assert_called_once_with(version='2.2') + self.cp_mock.assert_called_once_with() def test_create_key_with_user_id(self): key_name = "create_with_user_id" @@ -130,7 +130,7 @@ class NovaKeyPairTest(common.HeatTestCase): self.assertEqual(tp_test.resource_id, created_key.name) self.fake_keypairs.create.assert_called_once_with( name=key_name, public_key=None, user_id='userA_ID') - self.cp_mock.assert_called_once_with(version='2.10') + self.cp_mock.assert_called_once_with() def test_create_key_with_user_and_type(self): key_name = "create_with_user_id_and_type" @@ -145,7 +145,7 @@ class NovaKeyPairTest(common.HeatTestCase): self.fake_keypairs.create.assert_called_once_with( name=key_name, public_key=None, user_id='userA_ID', type='x509') - self.cp_mock.assert_called_once_with(version='2.10') + self.cp_mock.assert_called_once_with() def test_create_key_empty_name(self): """Test creation of a keypair whose name is of length zero.""" @@ -175,7 +175,7 @@ class NovaKeyPairTest(common.HeatTestCase): self.assertIn("kp.properties.name: length (256) is out of " "range (min: 1, max: 255)", six.text_type(error)) - def _test_validate(self, key_type=None, user=None, nc_version=None): + def _test_validate(self, key_type=None, user=None): template = copy.deepcopy(self.kp_template) validate_props = [] if key_type: @@ -187,26 +187,23 @@ class NovaKeyPairTest(common.HeatTestCase): stack = utils.parse_stack(template) definition = stack.t.resource_definitions(stack)['kp'] kp_res = keypair.KeyPair('kp', definition, stack) - self.patchobject(nova.NovaClientPlugin, 'client', - side_effect=exception.InvalidServiceVersion( - service='compute', - version=nc_version - )) - error = self.assertRaises(exception.StackValidationFailed, kp_res.validate) - msg = (('Cannot use "%(prop)s" properties - nova does not support: ' - 'Invalid service compute version %(ver)s') % - {'prop': validate_props, 'ver': nc_version}) + msg = (('Cannot use "%s" properties - nova does not support ' + 'required api microversion.') % validate_props) self.assertIn(msg, six.text_type(error)) def test_validate_key_type(self): - self._test_validate(key_type='x509', nc_version='2.2') + self.patchobject(nova.NovaClientPlugin, 'get_max_microversion', + return_value='2.1') + self._test_validate(key_type='x509') def test_validate_user(self): self.patchobject(keystone.KeystoneClientPlugin, 'get_user_id', return_value='user_A') - self._test_validate(user='user_A', nc_version='2.10') + self.patchobject(nova.NovaClientPlugin, 'get_max_microversion', + return_value='2.1') + self._test_validate(user='user_A') def test_check_key(self): res = self._get_test_resource(self.kp_template) diff --git a/heat/tests/openstack/nova/test_server.py b/heat/tests/openstack/nova/test_server.py index bf80c8a0e2..6e73827e28 100644 --- a/heat/tests/openstack/nova/test_server.py +++ b/heat/tests/openstack/nova/test_server.py @@ -417,6 +417,8 @@ class ServersTest(common.HeatTestCase): server_name = 'test_server_create' stack_name = '%s_s' % server_name server = self._create_test_server(return_server, server_name) + self.patchobject(nova.NovaClientPlugin, 'is_version_supported', + return_value=True) # this makes sure the auto increment worked on server creation self.assertGreater(server.id, 0) @@ -484,9 +486,8 @@ class ServersTest(common.HeatTestCase): self.assertEqual(expected_name, server.FnGetAtt('name')) self.assertEqual(['test'], server.FnGetAtt('tags')) # test with unsupported version - server.client = mock.Mock(side_effect=[ - self.fc, - exception.InvalidServiceVersion(service='a', version='0')]) + self.patchobject(nova.NovaClientPlugin, 'is_version_supported', + return_value=False) if server.attributes._resolved_values.get('tags'): del server.attributes._resolved_values['tags'] self.assertIsNone(server.FnGetAtt('tags')) @@ -576,9 +577,7 @@ class ServersTest(common.HeatTestCase): create_mock = self.patchobject(self.fc.servers, 'create', return_value=return_server) scheduler.TaskRunner(server.create)() - mock_nc.assert_has_calls([mock.call(), - mock.call(version='2.37'), - mock.call()]) + mock_nc.assert_called_with() self.assertEqual(3, mock_nc.call_count) self.assertEqual('none', create_mock.call_args[1]['nics']) @@ -1982,7 +1981,8 @@ class ServersTest(common.HeatTestCase): def test_server_get_live_state(self): return_server = self.fc.servers.list()[1] return_server.id = '5678' - + self.patchobject(nova.NovaClientPlugin, 'is_version_supported', + return_value=False) server = self._create_test_server(return_server, 'get_live_state_stack') @@ -3112,10 +3112,8 @@ class ServersTest(common.HeatTestCase): props['tags'] = ['a'] # no need test with key_name props.pop('key_name') - self.patchobject(nova.NovaClientPlugin, 'client', - side_effect=[ - exception.InvalidServiceVersion(service='a', - version='2.26')]) + self.patchobject(nova.NovaClientPlugin, 'is_version_supported', + return_value=False) resource_defns = tmpl.resource_definitions(stack) server = servers.Server('server_create_image_err', resource_defns['WebServer'], stack) @@ -3127,7 +3125,7 @@ class ServersTest(common.HeatTestCase): exc = self.assertRaises(exception.StackValidationFailed, server.validate) self.assertEqual('Cannot use "tags" property - nova does not support ' - 'it: Invalid service a version 2.26', + 'required api microversion.', six.text_type(exc)) def test_server_validate_too_many_personality(self):