Skip to content

Commit

Permalink
Fix bug for neutron network-name
Browse files Browse the repository at this point in the history
There may be cases when the network-name is not updated correctly
due to the requested networks not being owned by the tenant.

In the case when there is no matched network we use the details
from the used port.

A failed deletion of a port will raise an exception. If the
port is not found no exception will be raised.

Co-authored-by: Evgeny Fedoruk <EvgenyF@Radware.com>

Change-Id: Ie28e88ec9a9c7a180410ae5e57f84b1f653bbffc
Closes-Bug: #1246258
  • Loading branch information
gkotton committed Jan 19, 2014
1 parent 1e74b86 commit b12da55
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 25 deletions.
30 changes: 21 additions & 9 deletions nova/network/neutronv2/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,8 @@ def deallocate_for_instance(self, context, instance, **kwargs):
LOG.debug(_('deallocate_for_instance() for %s'),
instance['display_name'])
search_opts = {'device_id': instance['uuid']}
data = neutronv2.get_client(context).list_ports(**search_opts)
neutron = neutronv2.get_client(context)
data = neutron.list_ports(**search_opts)
ports = [port['id'] for port in data.get('ports', [])]

requested_networks = kwargs.get('requested_networks') or {}
Expand All @@ -395,10 +396,14 @@ def deallocate_for_instance(self, context, instance, **kwargs):

for port in ports:
try:
neutronv2.get_client(context).delete_port(port)
except Exception:
LOG.exception(_("Failed to delete neutron port %(portid)s")
% {'portid': port})
neutron.delete_port(port)
except neutronv2.exceptions.NeutronClientException as e:
if e.status_code == 404:
LOG.warning(_("Port %s does not exist"), port)
else:
with excutils.save_and_reraise_exception():
LOG.exception(_("Failed to delete neutron port %s"),
port)

def allocate_port_for_instance(self, context, instance, port_id,
network_id=None, requested_ip=None):
Expand Down Expand Up @@ -946,12 +951,18 @@ def _nw_info_get_subnets(self, context, port, network_IPs):
return subnets

def _nw_info_build_network(self, port, networks, subnets):
# NOTE(danms): This loop can't fail to find a network since we
# filtered ports to only the ones matching networks in our parent
network_name = None
for net in networks:
if port['network_id'] == net['id']:
network_name = net['name']
tenant_id = net['tenant_id']
break
else:
tenant_id = port['tenant_id']
LOG.warning(_("Network %(id)s not matched with the tenants "
"network! The ports tenant %(tenant_id)s will be "
"used."),
{'id': port['network_id'], 'tenant_id': tenant_id})

bridge = None
ovs_interfaceid = None
Expand All @@ -975,7 +986,7 @@ def _nw_info_build_network(self, port, networks, subnets):
bridge=bridge,
injected=CONF.flat_injected,
label=network_name,
tenant_id=net['tenant_id']
tenant_id=tenant_id
)
network['subnets'] = subnets
port_profile = port.get('binding:profile')
Expand Down Expand Up @@ -1008,7 +1019,8 @@ def _build_network_info_model(self, context, instance, networks=None):
if networks is None:
net_ids = [iface['network']['id'] for iface in ifaces]
networks = self._get_available_networks(context,
instance['project_id'])
instance['project_id'],
net_ids)

# ensure ports are in preferred network order, and filter out
# those not attached to one of the provided list of networks
Expand Down
61 changes: 45 additions & 16 deletions nova/tests/network/test_neutronv2.py
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ def _get_instance_nw_info(self, number):
self.instance['uuid'],
mox.IgnoreArg())
port_data = number == 1 and self.port_data1 or self.port_data2

nets = number == 1 and self.nets1 or self.nets2
net_info_cache = []
for port in port_data:
net_info_cache.append({"network": {"id": port['network_id']}})
Expand All @@ -428,12 +428,10 @@ def _get_instance_nw_info(self, number):
tenant_id=self.instance['project_id'],
device_id=self.instance['uuid']).AndReturn(
{'ports': port_data})
net_ids = [port['network_id'] for port in port_data]
nets = number == 1 and self.nets1 or self.nets2
self.moxed_client.list_networks(
tenant_id=self.instance['project_id'],
shared=False).AndReturn({'networks': nets})
self.moxed_client.list_networks(
shared=True).AndReturn({'networks': []})
id=net_ids).AndReturn({'networks': nets})
for i in xrange(1, number + 1):
float_data = number == 1 and self.float_data1 or self.float_data2
for ip in port_data[i - 1]['fixed_ips']:
Expand Down Expand Up @@ -580,11 +578,8 @@ def test_get_instance_nw_info_without_subnet(self):
device_id=self.instance['uuid']).AndReturn(
{'ports': self.port_data3})
self.moxed_client.list_networks(
shared=False,
tenant_id=self.instance['project_id']).AndReturn(
id=[self.port_data1[0]['network_id']]).AndReturn(
{'networks': self.nets1})
self.moxed_client.list_networks(
shared=True).AndReturn({'networks': []})
neutronv2.get_client(mox.IgnoreArg(),
admin=True).MultipleTimes().AndReturn(
self.moxed_client)
Expand Down Expand Up @@ -884,7 +879,7 @@ def _deallocate_for_instance(self, number):
self.moxed_client.list_ports(
device_id=self.instance['uuid']).AndReturn(
{'ports': port_data})
for port in port_data:
for port in reversed(port_data):
self.moxed_client.delete_port(port['id'])

self.mox.ReplayAll()
Expand All @@ -900,8 +895,25 @@ def test_deallocate_for_instance_2(self):
# Test to deallocate in two ports env.
self._deallocate_for_instance(2)

def test_deallocate_for_instance_port_not_found(self):
port_data = self.port_data1
self.moxed_client.list_ports(
device_id=self.instance['uuid']).AndReturn(
{'ports': port_data})

NeutronNotFound = neutronv2.exceptions.NeutronClientException(
status_code=404)
for port in reversed(port_data):
self.moxed_client.delete_port(port['id']).AndRaise(
NeutronNotFound)
self.mox.ReplayAll()

api = neutronapi.API()
api.deallocate_for_instance(self.context, self.instance)

def _test_deallocate_port_for_instance(self, number):
port_data = number == 1 and self.port_data1 or self.port_data2
nets = number == 1 and self.nets1 or self.nets2
self.moxed_client.delete_port(port_data[0]['id'])

net_info_cache = []
Expand All @@ -920,12 +932,9 @@ def _test_deallocate_port_for_instance(self, number):
{'ports': port_data[1:]})
neutronv2.get_client(mox.IgnoreArg()).MultipleTimes().AndReturn(
self.moxed_client)
self.moxed_client.list_networks(
tenant_id=self.instance['project_id'],
shared=False).AndReturn(
{'networks': [self.nets2[1]]})
self.moxed_client.list_networks(shared=True).AndReturn(
{'networks': []})
net_ids = [port['network_id'] for port in port_data]
self.moxed_client.list_networks(id=net_ids).AndReturn(
{'networks': nets})
float_data = number == 1 and self.float_data1 or self.float_data2
for data in port_data[1:]:
for ip in data['fixed_ips']:
Expand Down Expand Up @@ -1755,6 +1764,26 @@ def test_nw_info_build_network_other(self):
self.assertNotIn('should_create_bridge', net)
self.assertIsNone(iid)

def test_nw_info_build_no_match(self):
fake_port = {
'fixed_ips': [{'ip_address': '1.1.1.1'}],
'id': 'port-id',
'network_id': 'net-id1',
'tenant_id': 'tenant',
'binding:vif_type': model.VIF_TYPE_OVS,
}
fake_subnets = [model.Subnet(cidr='1.0.0.0/8')]
fake_nets = [{'id': 'net-id2', 'name': 'foo', 'tenant_id': 'tenant'}]
api = neutronapi.API()
self.mox.ReplayAll()
neutronv2.get_client('fake')
net, iid = api._nw_info_build_network(fake_port, fake_nets,
fake_subnets)
self.assertEqual(fake_subnets, net['subnets'])
self.assertEqual('net-id1', net['id'])
self.assertEqual('net-id1', net['id'])
self.assertEqual('tenant', net['meta']['tenant_id'])

def test_build_network_info_model(self):
api = neutronapi.API()
fake_inst = {'project_id': 'fake', 'uuid': 'uuid',
Expand Down

0 comments on commit b12da55

Please sign in to comment.