FIP associate: Show only ports from a target server
Previously when associating an FIP to a server, neutron ports which do not belong to a target server were also shown in the target list. It is clearer if only neutron ports connected to a target server are shown. This commit changes the logic to lookup FIP targets to achieve this. Closes-Bug: #1630412 Change-Id: I9901b54e125b6bf09df86537c8308b2bfb1499f2
This commit is contained in:
parent
7961c6a84f
commit
d14fbc849e
|
@ -29,6 +29,7 @@ from django.conf import settings
|
|||
from django.utils.translation import ugettext_lazy as _
|
||||
from neutronclient.common import exceptions as neutron_exc
|
||||
from neutronclient.v2_0 import client as neutron_client
|
||||
from novaclient import exceptions as nova_exc
|
||||
import six
|
||||
|
||||
from horizon import exceptions
|
||||
|
@ -625,7 +626,7 @@ class FloatingIpManager(object):
|
|||
self.client.update_floatingip(floating_ip_id,
|
||||
{'floatingip': update_dict})
|
||||
|
||||
def _get_reachable_subnets(self, ports):
|
||||
def _get_reachable_subnets(self, ports, fetch_router_ports=False):
|
||||
if not is_enabled_by_config('enable_fip_topology_check', True):
|
||||
# All subnets are reachable from external network
|
||||
return set(
|
||||
|
@ -637,10 +638,15 @@ class FloatingIpManager(object):
|
|||
if (r.external_gateway_info and
|
||||
r.external_gateway_info.get('network_id')
|
||||
in ext_net_ids)]
|
||||
reachable_subnets = set([p.fixed_ips[0]['subnet_id'] for p in ports
|
||||
if ((p.device_owner in
|
||||
ROUTER_INTERFACE_OWNERS)
|
||||
and (p.device_id in gw_routers))])
|
||||
if fetch_router_ports:
|
||||
router_ports = port_list(self.request,
|
||||
device_owner=ROUTER_INTERFACE_OWNERS)
|
||||
else:
|
||||
router_ports = [p for p in ports
|
||||
if p.device_owner in ROUTER_INTERFACE_OWNERS]
|
||||
reachable_subnets = set([p.fixed_ips[0]['subnet_id']
|
||||
for p in router_ports
|
||||
if p.device_id in gw_routers])
|
||||
# we have to include any shared subnets as well because we may not
|
||||
# have permission to see the router interface to infer connectivity
|
||||
shared = set([s.id for n in network_list(self.request, shared=True)
|
||||
|
@ -729,12 +735,21 @@ class FloatingIpManager(object):
|
|||
if target['instance_id'] == instance_id]
|
||||
else:
|
||||
ports = self._target_ports_by_instance(instance_id)
|
||||
reachable_subnets = self._get_reachable_subnets(
|
||||
ports, fetch_router_ports=True)
|
||||
name = self._get_server_name(instance_id)
|
||||
# TODO(amotoki): Avoid using p.fixed_ips[0].
|
||||
# Extract all IPv4 addresses instead
|
||||
# TODO(amotoki): Replace a label with an empty string
|
||||
# with a real server name.
|
||||
return [FloatingIpTarget(p, p.fixed_ips[0]['ip_address'], '')
|
||||
for p in ports]
|
||||
return [FloatingIpTarget(p, p.fixed_ips[0]['ip_address'], name)
|
||||
for p in ports
|
||||
if p.fixed_ips[0]['subnet_id'] in reachable_subnets]
|
||||
|
||||
def _get_server_name(self, server_id):
|
||||
try:
|
||||
server = nova.server_get(self.request, server_id)
|
||||
return server.name
|
||||
except nova_exc.NotFound:
|
||||
return ''
|
||||
|
||||
def is_simple_associate_supported(self):
|
||||
"""Returns True if the default floating IP pool is enabled."""
|
||||
|
|
|
@ -54,17 +54,14 @@ class FloatingIpViewTests(test.TestCase):
|
|||
# Verify that our "associated" floating IP isn't in the choices list.
|
||||
self.assertNotIn(self.floating_ips.first(), choices)
|
||||
|
||||
@test.create_stubs({api.neutron: ('floating_ip_target_list',
|
||||
'floating_ip_target_get_by_instance',
|
||||
@test.create_stubs({api.neutron: ('floating_ip_target_list_by_instance',
|
||||
'tenant_floating_ip_list',)})
|
||||
def test_associate_with_instance_id(self):
|
||||
targets = self._get_fip_targets()
|
||||
target = targets[0]
|
||||
api.neutron.floating_ip_target_list(IsA(http.HttpRequest)) \
|
||||
.AndReturn(targets)
|
||||
api.neutron.floating_ip_target_get_by_instance(
|
||||
IsA(http.HttpRequest), target.instance_id, targets) \
|
||||
.AndReturn(target)
|
||||
api.neutron.floating_ip_target_list_by_instance(
|
||||
IsA(http.HttpRequest), target.instance_id) \
|
||||
.AndReturn([target])
|
||||
api.neutron.tenant_floating_ip_list(IsA(http.HttpRequest)) \
|
||||
.AndReturn(self.floating_ips.list())
|
||||
self.mox.ReplayAll()
|
||||
|
|
|
@ -53,10 +53,15 @@ class AssociateIPAction(workflows.Action):
|
|||
q_instance_id = self.request.GET.get('instance_id')
|
||||
q_port_id = self.request.GET.get('port_id')
|
||||
if q_instance_id:
|
||||
targets = self._get_target_list()
|
||||
target = api.neutron.floating_ip_target_get_by_instance(
|
||||
self.request, q_instance_id, targets)
|
||||
self.initial['instance_id'] = target.id
|
||||
targets = self._get_target_list(q_instance_id)
|
||||
# Setting the initial value here is required to avoid a situation
|
||||
# where instance_id passed in the URL is used as the initial value
|
||||
# unexpectedly. (This always happens if the form is invoked from
|
||||
# the instance table.)
|
||||
if targets:
|
||||
self.initial['instance_id'] = targets[0].id
|
||||
else:
|
||||
self.initial['instance_id'] = ''
|
||||
elif q_port_id:
|
||||
targets = self._get_target_list()
|
||||
for target in targets:
|
||||
|
@ -84,10 +89,14 @@ class AssociateIPAction(workflows.Action):
|
|||
return options
|
||||
|
||||
@memoized.memoized_method
|
||||
def _get_target_list(self):
|
||||
def _get_target_list(self, instance_id=None):
|
||||
targets = []
|
||||
try:
|
||||
targets = api.neutron.floating_ip_target_list(self.request)
|
||||
if instance_id:
|
||||
targets = api.neutron.floating_ip_target_list_by_instance(
|
||||
self.request, instance_id)
|
||||
else:
|
||||
targets = api.neutron.floating_ip_target_list(self.request)
|
||||
except Exception:
|
||||
redirect = reverse('horizon:project:floating_ips:index')
|
||||
exceptions.handle(self.request,
|
||||
|
@ -97,7 +106,12 @@ class AssociateIPAction(workflows.Action):
|
|||
|
||||
# TODO(amotoki): [drop-nova-network] Rename instance_id to port_id
|
||||
def populate_instance_id_choices(self, request, context):
|
||||
targets = self._get_target_list()
|
||||
q_instance_id = self.request.GET.get('instance_id')
|
||||
# The reason of specifying an empty tuple when q_instance_id is None
|
||||
# is to make memoized_method _get_target_list work. Two calls of
|
||||
# _get_target_list from here and __init__ must have a same arguments.
|
||||
params = (q_instance_id, ) if q_instance_id else ()
|
||||
targets = self._get_target_list(*params)
|
||||
instances = sorted([(target.id, target.name) for target in targets],
|
||||
# Sort FIP targets by server name for easy browsing
|
||||
key=lambda x: x[1])
|
||||
|
|
|
@ -1246,10 +1246,39 @@ class NeutronApiFloatingIpTests(NeutronApiTestBase):
|
|||
self.assertEqual('1', ret.instance_id)
|
||||
|
||||
def test_target_floating_ip_port_by_instance(self):
|
||||
server = self.servers.first()
|
||||
ports = self.api_ports.list()
|
||||
candidates = [p for p in ports if p['device_id'] == '1']
|
||||
# _target_ports_by_instance()
|
||||
candidates = [p for p in ports if p['device_id'] == server.id]
|
||||
search_opts = {'device_id': '1'}
|
||||
self.qclient.list_ports(**search_opts).AndReturn({'ports': candidates})
|
||||
# _get_reachable_subnets()
|
||||
search_opts = {'router:external': True}
|
||||
ext_nets = [n for n in self.api_networks.list()
|
||||
if n['router:external']]
|
||||
self.qclient.list_networks(**search_opts) \
|
||||
.AndReturn({'networks': ext_nets})
|
||||
self.qclient.list_routers() \
|
||||
.AndReturn({'routers': self.api_routers.list()})
|
||||
rinfs = [p for p in ports
|
||||
if p['device_owner'] in api.neutron.ROUTER_INTERFACE_OWNERS]
|
||||
filters = {'device_owner': api.neutron.ROUTER_INTERFACE_OWNERS}
|
||||
self.qclient.list_ports(**filters).AndReturn({'ports': rinfs})
|
||||
shared_nets = [n for n in self.api_networks.list() if n['shared']]
|
||||
self.qclient.list_networks(shared=True) \
|
||||
.AndReturn({'networks': shared_nets})
|
||||
shared_subnet_ids = [s for n in shared_nets for s in n['subnets']]
|
||||
shared_subs = [s for s in self.api_subnets.list()
|
||||
if s['id'] in shared_subnet_ids]
|
||||
self.qclient.list_subnets().AndReturn({'subnets': shared_subs})
|
||||
# _get_server_name()
|
||||
novaclient = self.stub_novaclient()
|
||||
novaclient.servers = self.mox.CreateMockAnything()
|
||||
novaclient.versions = self.mox.CreateMockAnything()
|
||||
novaclient.versions.get_current().AndReturn("2.45")
|
||||
search_opts = {'project_id': self.request.user.tenant_id}
|
||||
novaclient.servers.get(server.id).AndReturn(server)
|
||||
|
||||
self.mox.ReplayAll()
|
||||
|
||||
ret = api.neutron.floating_ip_target_list_by_instance(self.request,
|
||||
|
|
Loading…
Reference in New Issue