From 8239b3f655a3ac3c63888b0df6f143d7ff50512a Mon Sep 17 00:00:00 2001 From: huangtianhua Date: Tue, 16 Sep 2014 17:45:29 +0800 Subject: [PATCH] Pass correct 'security_groups' value for port operations Make sure pass the correct value of 'security_groups' property to neutronClient for OS::Neutron::Port resource operations. Change-Id: I84fa2c149c18b0b85be7cf1ff5a8ab2641a5c276 Closes-bug: #1369933 --- heat/engine/resources/neutron/port.py | 16 +++-- heat/tests/test_neutron.py | 94 ++++++++++++++++++++++----- 2 files changed, 88 insertions(+), 22 deletions(-) diff --git a/heat/engine/resources/neutron/port.py b/heat/engine/resources/neutron/port.py index 4fc887ee0..eb383dbbd 100644 --- a/heat/engine/resources/neutron/port.py +++ b/heat/engine/resources/neutron/port.py @@ -127,7 +127,6 @@ class Port(neutron.NeutronResource): SECURITY_GROUPS: properties.Schema( properties.Schema.LIST, _('Security group IDs to associate with this port.'), - default=[], update_allowed=True ), ALLOWED_ADDRESS_PAIRS: properties.Schema( @@ -245,7 +244,7 @@ class Port(neutron.NeutronResource): port = self.neutron().create_port({'port': props})['port'] self.resource_id_set(port['id']) - def _prepare_port_properties(self, props): + def _prepare_port_properties(self, props, prepare_for_update=False): for fixed_ip in props.get(self.FIXED_IPS, []): for key, value in fixed_ip.items(): if value is None: @@ -260,11 +259,18 @@ class Port(neutron.NeutronResource): pair[self.ALLOWED_ADDRESS_PAIR_MAC_ADDRESS] is None): del pair[self.ALLOWED_ADDRESS_PAIR_MAC_ADDRESS] - if props.get(self.SECURITY_GROUPS): + # if without 'security_groups', don't set the 'security_groups' + # property when creating, neutron will create the port with the + # 'default' securityGroup. If has the 'security_groups' and the + # value is [], which means to create the port without securityGroup. + if props.get(self.SECURITY_GROUPS) is not None: props[self.SECURITY_GROUPS] = self.client_plugin().\ get_secgroup_uuids(props.get(self.SECURITY_GROUPS)) else: - props.pop(self.SECURITY_GROUPS, None) + # And the update should has the same behavior. + if prepare_for_update: + props[self.SECURITY_GROUPS] = self.client_plugin().\ + get_secgroup_uuids(['default']) if not props[self.FIXED_IPS]: del(props[self.FIXED_IPS]) @@ -316,7 +322,7 @@ class Port(neutron.NeutronResource): def handle_update(self, json_snippet, tmpl_diff, prop_diff): props = self.prepare_update_properties(json_snippet) - self._prepare_port_properties(props) + self._prepare_port_properties(props, prepare_for_update=True) LOG.debug('updating port with %s' % props) self.neutron().update_port(self.resource_id, {'port': props}) diff --git a/heat/tests/test_neutron.py b/heat/tests/test_neutron.py index a8d6362a4..077034776 100644 --- a/heat/tests/test_neutron.py +++ b/heat/tests/test_neutron.py @@ -2055,7 +2055,9 @@ class NeutronFloatingIPTest(HeatTestCase): 'admin_state_up': True, 'name': 'test_port', 'device_id': 'd6b4d3a5-c700-476f-b609-1493dd9dadc2', - 'device_owner': 'network:floatingip' + 'device_owner': 'network:floatingip', + 'security_groups': [ + '8a2f582a-e1cd-480f-b85d-b02631c10656'] } } ).AndReturn(None) @@ -2089,7 +2091,8 @@ class NeutronFloatingIPTest(HeatTestCase): }], "name": "test_port", "device_id": "d6b4d3a5-c700-476f-b609-1493dd9dadc2", - 'device_owner': 'network:floatingip' + 'device_owner': 'network:floatingip', + 'security_groups': ['8a2f582a-e1cd-480f-b85d-b02631c10656'] } update_snippet = rsrc_defn.ResourceDefinition(p.name, p.type(), props) @@ -2605,7 +2608,7 @@ class NeutronPortTest(HeatTestCase): scheduler.TaskRunner(port.create)() self.m.VerifyAll() - def test_security_groups(self): + def _mock_create_with_security_groups(self, port_prop): neutronV20.find_resourceid_by_name_or_id( mox.IsA(neutronclient.Client), 'network', @@ -2616,20 +2619,10 @@ class NeutronPortTest(HeatTestCase): 'subnet', 'sub1234' ).AndReturn('sub1234') - neutronclient.Client.create_port({'port': { - 'network_id': u'net1234', - 'security_groups': ['8a2f582a-e1cd-480f-b85d-b02631c10656', - '024613dc-b489-4478-b46f-ada462738740'], - 'fixed_ips': [ - {'subnet_id': u'sub1234', 'ip_address': u'10.0.3.21'} - ], - 'name': utils.PhysName('test_stack', 'port'), - 'admin_state_up': True, - 'device_owner': u'network:dhcp'}} - ).AndReturn({'port': { - "status": "BUILD", - "id": "fc68ea2c-b60b-4b4f-bd82-94ec81110766" - }}) + neutronclient.Client.create_port({'port': port_prop}).AndReturn( + {'port': { + "status": "BUILD", + "id": "fc68ea2c-b60b-4b4f-bd82-94ec81110766"}}) neutronclient.Client.show_port( 'fc68ea2c-b60b-4b4f-bd82-94ec81110766' ).AndReturn({'port': { @@ -2639,6 +2632,20 @@ class NeutronPortTest(HeatTestCase): self.m.ReplayAll() + def test_security_groups(self): + port_prop = { + 'network_id': u'net1234', + 'security_groups': ['8a2f582a-e1cd-480f-b85d-b02631c10656', + '024613dc-b489-4478-b46f-ada462738740'], + 'fixed_ips': [ + {'subnet_id': u'sub1234', 'ip_address': u'10.0.3.21'} + ], + 'name': utils.PhysName('test_stack', 'port'), + 'admin_state_up': True, + 'device_owner': u'network:dhcp'} + + self._mock_create_with_security_groups(port_prop) + t = template_format.parse(neutron_port_template) t['Resources']['port']['Properties']['security_groups'] = [ '8a2f582a-e1cd-480f-b85d-b02631c10656', @@ -2650,6 +2657,28 @@ class NeutronPortTest(HeatTestCase): self.m.VerifyAll() + def test_security_groups_empty_list(self): + port_prop = { + 'network_id': u'net1234', + 'security_groups': [], + 'fixed_ips': [ + {'subnet_id': u'sub1234', 'ip_address': u'10.0.3.21'} + ], + 'name': utils.PhysName('test_stack', 'port'), + 'admin_state_up': True, + 'device_owner': u'network:dhcp'} + + self._mock_create_with_security_groups(port_prop) + + t = template_format.parse(neutron_port_template) + t['Resources']['port']['Properties']['security_groups'] = [] + stack = utils.parse_stack(t) + + port = stack['port'] + scheduler.TaskRunner(port.create)() + + self.m.VerifyAll() + def test_create_and_update_port(self): props = {'network_id': u'net1234', 'name': utils.PhysName('test_stack', 'port'), @@ -2657,9 +2686,17 @@ class NeutronPortTest(HeatTestCase): 'device_owner': u'network:dhcp'} new_props = props.copy() new_props['name'] = "new_name" + new_props['security_groups'] = [ + '8a2f582a-e1cd-480f-b85d-b02631c10656'] new_props_update = new_props.copy() new_props_update.pop('network_id') + new_props1 = new_props.copy() + new_props1.pop('security_groups') + new_props_update1 = new_props_update.copy() + new_props_update1['security_groups'] = [ + '0389f747-7785-4757-b7bb-2ab07e4b09c3'] + neutronV20.find_resourceid_by_name_or_id( mox.IsA(neutronclient.Client), 'network', @@ -2686,6 +2723,25 @@ class NeutronPortTest(HeatTestCase): {'port': new_props_update} ).AndReturn(None) + fake_groups_list = { + 'security_groups': [ + { + 'tenant_id': 'dc4b074874244f7693dd65583733a758', + 'id': '0389f747-7785-4757-b7bb-2ab07e4b09c3', + 'name': 'default', + 'security_group_rules': [], + 'description': 'no protocol' + } + ] + } + self.m.StubOutWithMock(neutronclient.Client, 'list_security_groups') + neutronclient.Client.list_security_groups().AndReturn( + fake_groups_list) + neutronclient.Client.update_port( + 'fc68ea2c-b60b-4b4f-bd82-94ec81110766', + {'port': new_props_update1} + ).AndReturn(None) + self.m.ReplayAll() # create port @@ -2700,6 +2756,10 @@ class NeutronPortTest(HeatTestCase): update_snippet = rsrc_defn.ResourceDefinition(port.name, port.type(), new_props) self.assertIsNone(port.handle_update(update_snippet, {}, {})) + # update again to test port without security group + update_snippet = rsrc_defn.ResourceDefinition(port.name, port.type(), + new_props1) + self.assertIsNone(port.handle_update(update_snippet, {}, {})) self.m.VerifyAll()