diff --git a/heat/engine/resources/openstack/nova/server.py b/heat/engine/resources/openstack/nova/server.py index 680bf75ee9..c1ebfb37ad 100644 --- a/heat/engine/resources/openstack/nova/server.py +++ b/heat/engine/resources/openstack/nova/server.py @@ -22,7 +22,6 @@ import six from heat.common import exception from heat.common.i18n import _ -from heat.common.i18n import _LI from heat.engine import attributes from heat.engine.clients import progress from heat.engine import constraints @@ -368,10 +367,10 @@ class Server(stack_user.StackUser, sh.SchedulerHintsMixin, NETWORK_SUBNET: properties.Schema( properties.Schema.STRING, _('Subnet in which to allocate the IP address for ' - 'port. Used only if port property is not specified ' - 'for creating port, based on derived properties.'), - support_status=support.SupportStatus(version='5.0.0'), - implemented=False + 'port. Used for creating port, based on derived ' + 'properties. If subnet is specified, network ' + 'property becomes optional.'), + support_status=support.SupportStatus(version='5.0.0') ) }, ), @@ -1165,43 +1164,8 @@ class Server(stack_user.StackUser, sh.SchedulerHintsMixin, return bootable_vol - def _validate_network(self, network): - if (network.get(self.NETWORK_ID) is None - and network.get(self.NETWORK_PORT) is None - and network.get(self.NETWORK_UUID) is None): - msg = _('One of the properties "%(id)s", "%(port_id)s", ' - '"%(uuid)s" should be set for the ' - 'specified network of server "%(server)s".' - '') % dict(id=self.NETWORK_ID, - port_id=self.NETWORK_PORT, - uuid=self.NETWORK_UUID, - server=self.name) - raise exception.StackValidationFailed(message=msg) - - if network.get(self.NETWORK_UUID) and network.get(self.NETWORK_ID): - msg = _('Properties "%(uuid)s" and "%(id)s" are both set ' - 'to the network "%(network)s" for the server ' - '"%(server)s". The "%(uuid)s" property is deprecated. ' - 'Use only "%(id)s" property.' - '') % dict(uuid=self.NETWORK_UUID, - id=self.NETWORK_ID, - network=network[self.NETWORK_ID], - server=self.name) - raise exception.StackValidationFailed(message=msg) - elif network.get(self.NETWORK_UUID): - LOG.info(_LI('For the server "%(server)s" the "%(uuid)s" ' - 'property is set to network "%(network)s". ' - '"%(uuid)s" property is deprecated. Use ' - '"%(id)s" property instead.'), - dict(uuid=self.NETWORK_UUID, - id=self.NETWORK_ID, - network=network[self.NETWORK_ID], - server=self.name)) - def validate(self): - ''' - Validate any of the provided params - ''' + """Validate any of the provided params.""" super(Server, self).validate() if self.user_data_software_config(): @@ -1314,6 +1278,9 @@ class Server(stack_user.StackUser, sh.SchedulerHintsMixin, self._delete_temp_url() self._delete_queue() + if self.data().get('internal_ports'): + self._delete_internal_ports() + try: self.client().servers.delete(self.resource_id) except Exception as e: diff --git a/heat/engine/resources/openstack/nova/server_network_mixin.py b/heat/engine/resources/openstack/nova/server_network_mixin.py index 0b77919ace..7e78e96029 100644 --- a/heat/engine/resources/openstack/nova/server_network_mixin.py +++ b/heat/engine/resources/openstack/nova/server_network_mixin.py @@ -13,19 +13,152 @@ import itertools +from oslo_log import log as logging +from oslo_serialization import jsonutils from oslo_utils import netutils +from heat.common import exception +from heat.common.i18n import _ +from heat.common.i18n import _LI +from heat.engine.cfn import functions as cfn_funcs + +LOG = logging.getLogger(__name__) + class ServerNetworkMixin(object): + + def _validate_network(self, network): + net_uuid = network.get(self.NETWORK_UUID) + net_id = network.get(self.NETWORK_ID) + port = network.get(self.NETWORK_PORT) + subnet = network.get(self.NETWORK_SUBNET) + + if (net_id is None and port is None + and net_uuid is None and subnet is None): + msg = _('One of the properties "%(id)s", "%(port_id)s", ' + '"%(uuid)s" or "%(subnet)s" should be set for the ' + 'specified network of server "%(server)s".' + '') % dict(id=self.NETWORK_ID, + port_id=self.NETWORK_PORT, + uuid=self.NETWORK_UUID, + subnet=self.NETWORK_SUBNET, + server=self.name) + raise exception.StackValidationFailed(message=msg) + + if net_uuid and net_id: + msg = _('Properties "%(uuid)s" and "%(id)s" are both set ' + 'to the network "%(network)s" for the server ' + '"%(server)s". The "%(uuid)s" property is deprecated. ' + 'Use only "%(id)s" property.' + '') % dict(uuid=self.NETWORK_UUID, + id=self.NETWORK_ID, + network=network[self.NETWORK_ID], + server=self.name) + raise exception.StackValidationFailed(message=msg) + elif net_uuid: + LOG.info(_LI('For the server "%(server)s" the "%(uuid)s" ' + 'property is set to network "%(network)s". ' + '"%(uuid)s" property is deprecated. Use ' + '"%(id)s" property instead.'), + dict(uuid=self.NETWORK_UUID, + id=self.NETWORK_ID, + network=network[self.NETWORK_ID], + server=self.name)) + + # If subnet and net are specified with some external resources, check + # them. Otherwise, if their are resources of current stack, skip + # validating in case of raising error and check it only during + # resource creating. + is_ref = False + for item in [subnet, net_uuid, net_id]: + if isinstance(item, (cfn_funcs.ResourceRef, cfn_funcs.GetAtt)): + is_ref = True + break + if not is_ref: + self._validate_belonging_subnet_to_net(network) + + def _validate_belonging_subnet_to_net(self, network): + if network.get(self.NETWORK_PORT) is None and self.is_using_neutron(): + net = self._get_network_id(network) + # check if there are subnet and network both specified that + # subnet belongs to specified network + if (network.get(self.NETWORK_SUBNET) is not None + and (net is not None)): + subnet_net = self.client_plugin( + 'neutron').network_id_from_subnet_id( + self._get_subnet_id(network)) + if subnet_net != net: + msg = _('Specified subnet %(subnet)s does not belongs to' + 'network %(network)s.') % { + 'subnet': subnet_net, + 'network': net} + raise exception.StackValidationFailed(message=msg) + + def _create_internal_port(self, net_data, net_number): + name = _('%(server)s-port-%(number)s') % {'server': self.name, + 'number': net_number} + + kwargs = {'network_id': self._get_network_id(net_data), + 'name': name} + fixed_ip = net_data.get(self.NETWORK_FIXED_IP) + subnet = net_data.get(self.NETWORK_SUBNET) + body = {} + if fixed_ip: + body['ip_address'] = fixed_ip + if subnet: + body['subnet_id'] = self._get_subnet_id(net_data) + # we should add fixed_ips only if subnet or ip were provided + if body: + kwargs.update({'fixed_ips': [body]}) + + port = self.client('neutron').create_port({'port': kwargs})['port'] + + # Store ids (used for floating_ip association, updating, etc.) + # in resource's data. + self._data_update_ports(port['id'], 'add') + + return port['id'] + + def _delete_internal_port(self, port_id): + try: + self.client('neutron').delete_port(port_id) + except Exception as ex: + self.client_plugin('neutron').ignore_not_found(ex) + + self._data_update_ports(port_id, 'delete') + + def _delete_internal_ports(self): + for port_data in self._data_get_ports(): + self._delete_internal_port(port_data['id']) + + self.data_delete('internal_ports') + + def _data_update_ports(self, port_id, action): + data = self._data_get_ports() + + if action == 'add': + data.append({'id': port_id}) + elif action == 'delete': + for port in data: + if port_id == port['id']: + data.remove(port) + break + + self.data_set('internal_ports', jsonutils.dumps(data)) + + def _data_get_ports(self): + data = self.data().get('internal_ports') + return jsonutils.loads(data) if data else [] + def _build_nics(self, networks): if not networks: return None nics = [] - for net in networks: - nic_info = {} - nic_info['net-id'] = self._get_network_id(net) + for idx, net in enumerate(networks): + self._validate_belonging_subnet_to_net(net) + nic_info = {'net-id': self._get_network_id(net)} if net.get(self.NETWORK_FIXED_IP): ip = net[self.NETWORK_FIXED_IP] if netutils.is_valid_ipv6(ip): @@ -34,6 +167,8 @@ class ServerNetworkMixin(object): nic_info['v4-fixed-ip'] = ip if net.get(self.NETWORK_PORT): nic_info['port-id'] = net[self.NETWORK_PORT] + elif self.is_using_neutron() and net.get(self.NETWORK_SUBNET): + nic_info['port-id'] = self._create_internal_port(net, idx) nics.append(nic_info) return nics @@ -65,13 +200,19 @@ class ServerNetworkMixin(object): else: net_id = self.client_plugin( 'nova').get_nova_network_id(net_id) + elif net.get(self.NETWORK_SUBNET): + net_id = self.client_plugin('neutron').network_id_from_subnet_id( + self._get_subnet_id(net)) return net_id + def _get_subnet_id(self, net): + return self.client_plugin('neutron').find_neutron_resource( + net, self.NETWORK_SUBNET, 'subnet') + def update_networks_matching_iface_port(self, nets, interfaces): def find_equal(port, net_id, ip, nets): for net in nets: - if (net.get('port') == port or (net.get('fixed_ip') == ip and self._get_network_id(net) == net_id)): @@ -133,6 +274,10 @@ class ServerNetworkMixin(object): for net in old_nets: if net.get(self.NETWORK_PORT): remove_ports.append(net.get(self.NETWORK_PORT)) + if self.data().get('internal_ports'): + # if we have internal port with such id, remove it + # instantly. + self._delete_internal_port(net.get(self.NETWORK_PORT)) handler_kwargs = {'port_id': None, 'net_id': None, 'fip': None} # if new_nets is None, we should attach first free port, @@ -141,13 +286,17 @@ class ServerNetworkMixin(object): add_nets.append(handler_kwargs) # attach section similar for both variants that # were mentioned above - for net in new_nets: - handler_kwargs = {'port_id': None, 'net_id': None, 'fip': None} - handler_kwargs['net_id'] = self._get_network_id(net) + for idx, net in enumerate(new_nets): + handler_kwargs = {'port_id': None, + 'net_id': self._get_network_id(net), + 'fip': None} if handler_kwargs['net_id']: handler_kwargs['fip'] = net.get('fixed_ip') if net.get(self.NETWORK_PORT): handler_kwargs['port_id'] = net.get(self.NETWORK_PORT) + elif self.is_using_neutron() and net.get(self.NETWORK_SUBNET): + handler_kwargs['port_id'] = self._create_internal_port(net, + idx) add_nets.append(handler_kwargs) diff --git a/heat/tests/nova/test_server.py b/heat/tests/nova/test_server.py index f93c870663..ee27653995 100644 --- a/heat/tests/nova/test_server.py +++ b/heat/tests/nova/test_server.py @@ -16,6 +16,8 @@ import copy import mock import mox +from neutronclient.neutron import v2_0 as neutronV20 +from neutronclient.v2_0 import client as neutronclient from novaclient import exceptions as nova_exceptions from oslo_utils import uuidutils import six @@ -30,6 +32,7 @@ from heat.engine.clients.os import nova from heat.engine.clients.os import swift from heat.engine.clients.os import zaqar from heat.engine import environment +from heat.engine import resource from heat.engine.resources.openstack.nova import server as servers from heat.engine.resources import scheduler_hints as sh from heat.engine import scheduler @@ -1317,9 +1320,9 @@ class ServersTest(common.HeatTestCase): ex = self.assertRaises(exception.StackValidationFailed, server.validate) - self.assertIn(_('One of the properties "network", "port", "uuid" ' - 'should be set for the specified network of server ' - '"%s".') % server.name, + self.assertIn(_('One of the properties "network", "port", "uuid" or ' + '"subnet" should be set for the specified network of ' + 'server "%s".') % server.name, six.text_type(ex)) self.m.VerifyAll() @@ -1359,6 +1362,12 @@ class ServersTest(common.HeatTestCase): nova.NovaClientPlugin._create().AndReturn(self.fc) self._mock_get_image_id_success('F17-x86_64-gold', 'image_id') self.stub_NetworkConstraint_validate() + + self.patchobject(neutronV20, 'find_resourceid_by_name_or_id', + return_value='12345') + self.patchobject(neutronclient.Client, 'show_network', + return_value={'network': {'subnets': ['abcd1234']}}) + self.m.ReplayAll() self.assertIsNone(server.validate()) @@ -1379,6 +1388,12 @@ class ServersTest(common.HeatTestCase): nova.NovaClientPlugin._create().AndReturn(self.fc) self._mock_get_image_id_success('F17-x86_64-gold', 'image_id') self.stub_NetworkConstraint_validate() + + self.patchobject(neutronV20, 'find_resourceid_by_name_or_id', + return_value='12345') + self.patchobject(neutronclient.Client, 'show_network', + return_value={'network': {'subnets': ['abcd1234']}}) + self.m.ReplayAll() self.assertIsNone(server.validate()) @@ -2355,6 +2370,8 @@ class ServersTest(common.HeatTestCase): server = self._create_test_server(return_server, 'test_server_create') self.patchobject(server, 'is_using_neutron', return_value=True) + self.patchobject(neutronclient.Client, 'create_port', + return_value={'port': {'id': '4815162342'}}) self.assertIsNone(server._build_nics([])) self.assertIsNone(server._build_nics(None)) @@ -3206,6 +3223,10 @@ class ServersTest(common.HeatTestCase): def test_server_update_None_networks_with_network_id(self): return_server = self.fc.servers.list()[3] return_server.id = '9102' + + self.patchobject(neutronclient.Client, 'create_port', + return_value={'port': {'id': 'abcd1234'}}) + server = self._create_test_server(return_server, 'networks_update') new_networks = [{'network': 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa', @@ -3405,6 +3426,10 @@ class ServersTest(common.HeatTestCase): def test_server_update_networks_with_uuid(self): return_server = self.fc.servers.list()[1] return_server.id = '5678' + + self.patchobject(neutronclient.Client, 'create_port', + return_value={'port': {'id': 'abcd1234'}}) + server = self._create_test_server(return_server, 'networks_update') old_networks = [ @@ -3766,3 +3791,313 @@ class ServersTest(common.HeatTestCase): '1234', utils.PhysName('snapshot_policy', 'WebServer')) delete_server.assert_not_called() + + +class ServerInternalPortTest(common.HeatTestCase): + def setUp(self): + super(ServerInternalPortTest, self).setUp() + self.resolve = self.patchobject(neutronV20, + 'find_resourceid_by_name_or_id') + self.port_create = self.patchobject(neutronclient.Client, + 'create_port') + self.port_delete = self.patchobject(neutronclient.Client, + 'delete_port') + + def _return_template_stack_and_rsrc_defn(self, stack_name, temp): + templ = template.Template(template_format.parse(temp), + env=environment.Environment( + {'key_name': 'test'})) + stack = parser.Stack(utils.dummy_context(), stack_name, templ, + stack_id=uuidutils.generate_uuid(), + stack_user_project_id='8888') + resource_defns = templ.resource_definitions(stack) + server = servers.Server('server', resource_defns['server'], + stack) + return templ, stack, server + + def test_build_nics_without_internal_port(self): + tmpl = """ + heat_template_version: 2015-10-15 + resources: + server: + type: OS::Nova::Server + properties: + flavor: m1.small + image: F17-x86_64-gold + networks: + - port: 12345 + network: 4321 + """ + t, stack, server = self._return_template_stack_and_rsrc_defn('test', + tmpl) + + create_internal_port = self.patchobject(server, + '_create_internal_port', + return_value='12345') + self.resolve.return_value = '4321' + + networks = [{'port': '12345', 'network': '4321'}] + nics = server._build_nics(networks) + self.assertEqual([{'port-id': '12345', 'net-id': '4321'}], nics) + self.assertEqual(0, create_internal_port.call_count) + + def validate_internal_port_subnet_not_this_network(self): + tmpl = """ + heat_template_version: 2015-10-15 + resources: + server: + type: OS::Nova::Server + properties: + flavor: m1.small + image: F17-x86_64-gold + networks: + - network: 4321 + subnet: 1234 + """ + t, stack, server = self._return_template_stack_and_rsrc_defn('test', + tmpl) + + self.patchobject(neutron.NeutronClientPlugin, + 'network_id_from_subnet_id', + return_value='not_this_network') + self.resolve.return_value = '4321' + + ex = self.assertRaises(exception.StackValidationFailed, + server.validate) + self.assertEqual('Specified subnet 1234 does not belongs to' + 'network 4321.', six.text_type(ex)) + + def test_build_nics_create_internal_port_all_props(self): + tmpl = """ + heat_template_version: 2015-10-15 + resources: + server: + type: OS::Nova::Server + properties: + flavor: m1.small + image: F17-x86_64-gold + networks: + - network: 4321 + subnet: 1234 + fixed_ip: 127.0.0.1 + """ + + t, stack, server = self._return_template_stack_and_rsrc_defn('test', + tmpl) + + self.resolve.side_effect = ['4321', '1234'] + self.patchobject(server, '_validate_belonging_subnet_to_net') + self.port_create.return_value = {'port': {'id': '111222'}} + data_set = self.patchobject(resource.Resource, 'data_set') + + network = [{'network': '4321', 'subnet': '1234', + 'fixed_ip': '127.0.0.1'}] + server._build_nics(network) + + self.port_create.assert_called_once_with( + {'port': {'name': 'server-port-0', + 'network_id': '4321', + 'fixed_ips': [{ + 'ip_address': '127.0.0.1', + 'subnet_id': '1234' + }]}}) + data_set.assert_called_once_with('internal_ports', + '[{"id": "111222"}]') + + def test_build_nics_do_not_create_internal_port(self): + tmpl = """ + heat_template_version: 2015-10-15 + resources: + server: + type: OS::Nova::Server + properties: + flavor: m1.small + image: F17-x86_64-gold + networks: + - network: 4321 + """ + + t, stack, server = self._return_template_stack_and_rsrc_defn('test', + tmpl) + + self.resolve.side_effect = ['4321', '1234'] + self.port_create.return_value = {'port': {'id': '111222'}} + data_set = self.patchobject(resource.Resource, 'data_set') + + network = [{'network': '4321'}] + server._build_nics(network) + + self.assertFalse(self.port_create.called) + self.assertFalse(data_set.called) + + def test_build_nics_create_internal_port_without_net(self): + tmpl = """ + heat_template_version: 2015-10-15 + resources: + server: + type: OS::Nova::Server + properties: + flavor: m1.small + image: F17-x86_64-gold + networks: + - subnet: 4321 + """ + t, stack, server = self._return_template_stack_and_rsrc_defn('test', + tmpl) + + self.patchobject(neutron.NeutronClientPlugin, + 'network_id_from_subnet_id', + return_value='1234') + self.resolve.return_value = '4321' + + net = {'subnet': '4321'} + net_id = server._get_network_id(net) + + self.assertEqual('1234', net_id) + subnet_id = server._get_subnet_id(net) + self.assertEqual('4321', subnet_id) + # check that networks doesn't changed in _get_subnet_id method. + self.assertEqual({'subnet': '4321'}, net) + + self.resolve.return_value = '4321' + self.port_create.return_value = {'port': {'id': '111222'}} + data_set = self.patchobject(resource.Resource, 'data_set') + + network = [{'subnet': '1234'}] + server._build_nics(network) + + self.port_create.assert_called_once_with( + {'port': {'name': 'server-port-0', + 'network_id': '1234', + 'fixed_ips': [{ + 'subnet_id': '4321' + }]}}) + data_set.assert_called_once_with('internal_ports', + '[{"id": "111222"}]') + + def test_calculate_networks_internal_ports(self): + tmpl = """ + heat_template_version: 2015-10-15 + resources: + server: + type: OS::Nova::Server + properties: + flavor: m1.small + image: F17-x86_64-gold + networks: + - network: 4321 + subnet: 1234 + fixed_ip: 127.0.0.1 + - network: 8765 + subnet: 5678 + fixed_ip: 127.0.0.2 + """ + + t, stack, server = self._return_template_stack_and_rsrc_defn('test', + tmpl) + + # NOTE(prazumovsky): this method update old_net and new_net with + # interfaces' ports. Because of uselessness of checking this method, + # we can afford to give port as part of calculate_networks args. + self.patchobject(server, 'update_networks_matching_iface_port') + + server._data = {'internal_ports': '[{"id": "1122"}]'} + self.port_create.return_value = {'port': {'id': '5566'}} + data_set = self.patchobject(resource.Resource, 'data_set') + self.resolve.side_effect = ['0912', '9021'] + + old_net = [{'network': '4321', + 'subnet': '1234', + 'fixed_ip': '127.0.0.1', + 'port': '1122'}, + {'network': '8765', + 'subnet': '5678', + 'fixed_ip': '127.0.0.2', + 'port': '3344'}] + + new_net = [{'network': '8765', + 'subnet': '5678', + 'fixed_ip': '127.0.0.2', + 'port': '3344'}, + {'network': '0912', + 'subnet': '9021', + 'fixed_ip': '127.0.0.1'}] + + server.calculate_networks(old_net, new_net, []) + + self.port_delete.assert_called_once_with('1122') + self.port_create.assert_called_once_with( + {'port': {'name': 'server-port-0', + 'network_id': '0912', + 'fixed_ips': [{'subnet_id': '9021', + 'ip_address': '127.0.0.1'}]}}) + + self.assertEqual(2, data_set.call_count) + data_set.assert_has_calls(( + mock.call('internal_ports', '[]'), + mock.call('internal_ports', '[{"id": "1122"}, {"id": "5566"}]'))) + + def test_delete_internal_ports(self): + tmpl = """ + heat_template_version: 2015-10-15 + resources: + server: + type: OS::Nova::Server + properties: + flavor: m1.small + image: F17-x86_64-gold + networks: + - network: 4321 + """ + t, stack, server = self._return_template_stack_and_rsrc_defn('test', + tmpl) + + get_data = [{'internal_ports': '[{"id": "1122"}, {"id": "3344"}, ' + '{"id": "5566"}]'}, + {'internal_ports': '[{"id": "1122"}, {"id": "3344"}, ' + '{"id": "5566"}]'}, + {'internal_ports': '[{"id": "3344"}, ' + '{"id": "5566"}]'}, + {'internal_ports': '[{"id": "5566"}]'}] + self.patchobject(server, 'data', side_effect=get_data) + data_set = self.patchobject(server, 'data_set') + data_delete = self.patchobject(server, 'data_delete') + + server._delete_internal_ports() + + self.assertEqual(3, self.port_delete.call_count) + self.assertEqual(('1122',), self.port_delete.call_args_list[0][0]) + self.assertEqual(('3344',), self.port_delete.call_args_list[1][0]) + self.assertEqual(('5566',), self.port_delete.call_args_list[2][0]) + + self.assertEqual(3, data_set.call_count) + data_set.assert_has_calls(( + mock.call('internal_ports', + '[{"id": "3344"}, {"id": "5566"}]'), + mock.call('internal_ports', '[{"id": "5566"}]'), + mock.call('internal_ports', '[]'))) + + data_delete.assert_called_once_with('internal_ports') + + def test_get_data_internal_ports(self): + tmpl = """ + heat_template_version: 2015-10-15 + resources: + server: + type: OS::Nova::Server + properties: + flavor: m1.small + image: F17-x86_64-gold + networks: + - network: 4321 + """ + t, stack, server = self._return_template_stack_and_rsrc_defn('test', + tmpl) + + server._data = {"internal_ports": '[{"id": "1122"}]'} + data = server._data_get_ports() + self.assertEqual([{"id": "1122"}], data) + + server._data = {"internal_ports": ''} + data = server._data_get_ports() + self.assertEqual([], data)