Add intrinsic creating port for server

If neutron is used and networks list port_id property is not specified
and subnet_id or some port related extra_properties specified,
create internal port with using nic info. Store list of dictionaries
with id of created ports in resource data as json data.

Also move validate_network method to mixin class.

implements bp rich-network-prop

Change-Id: Iaeda645cd019e02e24ab5418eaa86f631a51cd74

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 '
+                          'port. Used for creating port, based on derived '
-                          'for creating port, based on derived properties.'),
+                          'properties. If subnet is specified, network '
-                        support_status=support.SupportStatus(version='5.0.0'),
+                          'property becomes optional.'),
-                        implemented=False
+                        support_status=support.SupportStatus(version='5.0.0')
                     )
                 },
             ),
@@ -1163,43 +1162,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():
@@ -1312,6 +1276,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:

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:
+        for idx, net in enumerate(networks):
-            nic_info = {}
+            self._validate_belonging_subnet_to_net(net)
-            nic_info['net-id'] = self._get_network_id(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:
+        for idx, net in enumerate(new_nets):
-            handler_kwargs = {'port_id': None, 'net_id': None, 'fip': None}
+            handler_kwargs = {'port_id': None,
-            handler_kwargs['net_id'] = self._get_network_id(net)
+                              '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)
 

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" '
+        self.assertIn(_('One of the properties "network", "port", "uuid" or '
-                        'should be set for the specified network of server '
+                        '"subnet" should be set for the specified network of '
-                        '"%s".') % server.name,
+                        '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)