openstack
/
horizon
Code Issues Proposed changes

Refactor FIP code to use FloatingIpTarget properly 

FloatingIpTarget was introduced but it was used partially.
This commit clean up the remaining logic of FIP target ID calculation.

Related-Bug: #1725657
Change-Id: I4b93b9a7a3977b042bb9c3200923c5c2fa84476d
This commit is contained in:
Akihiro Motoki 2017-10-21 08:18:09 +00:00
parent 00cfce3b2d
commit 7961c6a84f
6 changed files with 63 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
openstack_dashboard/api/neutron.py
View File

@ -479,7 +479,8 @@ class FloatingIpTarget(base.APIDictWrapper):
"""
def __init__(self, port, ip_address, label):
target = {'name': '%s: %s' % (label, ip_address),
name = '%s: %s' % (label, ip_address) if label else ip_address
target = {'name': name,
'id': '%s_%s' % (port.id, ip_address),
'port_id': port.id,
'instance_id': port.device_id}
@ -606,8 +607,8 @@ class FloatingIpManager(object):
``port_id`` represents a VNIC of an instance.
``port_id`` argument is different from a normal neutron port ID.
A value passed as ``port_id`` must be one of target_id returned by
``list_targets``, ``get_target_id_by_instance`` or
``list_target_id_by_instance`` method.
``list_targets``, ``get_target_by_instance`` or
``list_targets_by_instance`` method.
"""
# NOTE: In Neutron Horizon floating IP support, port_id is
# "<port_id>_<ip_address>" format to identify multiple ports.
@ -687,8 +688,8 @@ class FloatingIpManager(object):
return port_list(self.request, **search_opts)
@profiler.trace
def get_target_id_by_instance(self, instance_id, target_list=None):
"""Returns a target ID of floating IP association.
def get_target_by_instance(self, instance_id, target_list=None):
"""Returns a FloatingIpTarget object of floating IP association.
:param instance_id: ID of target VM instance
:param target_list: (optional) a list returned by list_targets().
@ -701,19 +702,21 @@ class FloatingIpManager(object):
if target['instance_id'] == instance_id]
if not targets:
return None
return targets[0]['id']
return targets[0]
else:
# In Neutron one port can have multiple ip addresses, so this
# method picks up the first one and generate target id.
ports = self._target_ports_by_instance(instance_id)
if not ports:
return None
return '{0}_{1}'.format(ports[0].id,
ports[0].fixed_ips[0]['ip_address'])
# TODO(amotoki): Avoid using p.fixed_ips[0].
# Extract all IPv4 addresses instead
return FloatingIpTarget(
ports[0], ports[0].fixed_ips[0]['ip_address'], '')
@profiler.trace
def list_target_id_by_instance(self, instance_id, target_list=None):
"""Returns a list of instance's target IDs of floating IP association.
def list_targets_by_instance(self, instance_id, target_list=None):
"""Returns a list of FloatingIpTarget objects of FIP association.
:param instance_id: ID of target VM instance
:param target_list: (optional) a list returned by list_targets().
@ -722,11 +725,15 @@ class FloatingIpManager(object):
is retrieved from a back-end inside the method.
"""
if target_list is not None:
return [target['id'] for target in target_list
return [target for target in target_list
if target['instance_id'] == instance_id]
else:
ports = self._target_ports_by_instance(instance_id)
return ['{0}_{1}'.format(p.id, p.fixed_ips[0]['ip_address'])
# 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]
def is_simple_associate_supported(self):
@ -1407,12 +1414,12 @@ def floating_ip_target_list(request):
def floating_ip_target_get_by_instance(request, instance_id, cache=None):
return FloatingIpManager(request).get_target_id_by_instance(
return FloatingIpManager(request).get_target_by_instance(
instance_id, cache)
def floating_ip_target_list_by_instance(request, instance_id, cache=None):
return FloatingIpManager(request).list_target_id_by_instance(
return FloatingIpManager(request).list_targets_by_instance(
instance_id, cache)

2
openstack_dashboard/dashboards/project/floating_ips/tests.py
View File

@ -64,7 +64,7 @@ class FloatingIpViewTests(test.TestCase):
.AndReturn(targets)
api.neutron.floating_ip_target_get_by_instance(
IsA(http.HttpRequest), target.instance_id, targets) \
.AndReturn(target.id)
.AndReturn(target)
api.neutron.tenant_floating_ip_list(IsA(http.HttpRequest)) \
.AndReturn(self.floating_ips.list())
self.mox.ReplayAll()

18
openstack_dashboard/dashboards/project/floating_ips/workflows.py
View File

@ -54,14 +54,13 @@ class AssociateIPAction(workflows.Action):
q_port_id = self.request.GET.get('port_id')
if q_instance_id:
targets = self._get_target_list()
target_id = api.neutron.floating_ip_target_get_by_instance(
target = api.neutron.floating_ip_target_get_by_instance(
self.request, q_instance_id, targets)
self.initial['instance_id'] = target_id
self.initial['instance_id'] = target.id
elif q_port_id:
targets = self._get_target_list()
for target in targets:
if (hasattr(target, 'port_id') and
target.port_id == q_port_id):
if target.port_id == q_port_id:
self.initial['instance_id'] = target.id
break
@ -99,14 +98,9 @@ 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()
instances = []
for target in targets:
instances.append((target.id, target.name))
# Sort instances for easy browsing
instances = sorted(instances, key=lambda x: x[1])
instances = sorted([(target.id, target.name) for target in targets],
# Sort FIP targets by server name for easy browsing
key=lambda x: x[1])
if instances:
instances.insert(0, ("", _("Select a port")))
else:

4
openstack_dashboard/dashboards/project/instances/tables.py
View File

@ -681,12 +681,10 @@ class SimpleDisassociateIP(policy.PolicyTargetMixin, tables.Action):
def single(self, table, request, instance_id):
try:
# target_id is port_id for Neutron and instance_id for Nova Network
# (Neutron API wrapper returns a 'portid_fixedip' string)
targets = api.neutron.floating_ip_target_list_by_instance(
request, instance_id)
target_ids = [t.split('_')[0] for t in targets]
target_ids = [t.port_id for t in targets]
fips = [fip for fip in api.neutron.tenant_floating_ip_list(request)
if fip.port_id in target_ids]

7
openstack_dashboard/dashboards/project/instances/tests.py
View File

@ -3634,8 +3634,11 @@ class InstanceTests(helpers.ResetImageAPIVersionMixin, helpers.TestCase):
def test_disassociate_floating_ip(self):
servers = self.servers.list()
server = servers[0]
port = [p for p in self.ports.list() if p.device_id == server.id][0]
fip_target = api.neutron.FloatingIpTarget(
port, port['fixed_ips'][0]['ip_address'], server.name)
fip = self.floating_ips.first()
fip.port_id = server.id
fip.port_id = port.id
search_opts = {'marker': None, 'paginate': True}
api.nova.server_list(IsA(http.HttpRequest), search_opts=search_opts) \
@ -3646,7 +3649,7 @@ class InstanceTests(helpers.ResetImageAPIVersionMixin, helpers.TestCase):
.AndReturn((self.images.list(), False, False))
api.neutron.floating_ip_target_list_by_instance(
IsA(http.HttpRequest),
server.id).AndReturn([server.id, ])
server.id).AndReturn([fip_target])
api.neutron.tenant_floating_ip_list(
IsA(http.HttpRequest)).AndReturn([fip])
api.neutron.floating_ip_disassociate(

41
openstack_dashboard/test/api_tests/neutron_tests.py
View File

@ -1241,7 +1241,9 @@ class NeutronApiFloatingIpTests(NeutronApiTestBase):
self.mox.ReplayAll()
ret = api.neutron.floating_ip_target_get_by_instance(self.request, '1')
self.assertEqual(self._get_target_id(candidates[0]), ret)
self.assertEqual(self._get_target_id(candidates[0]), ret.id)
self.assertEqual(candidates[0]['id'], ret.port_id)
self.assertEqual('1', ret.instance_id)
def test_target_floating_ip_port_by_instance(self):
ports = self.api_ports.list()
@ -1252,25 +1254,40 @@ class NeutronApiFloatingIpTests(NeutronApiTestBase):
ret = api.neutron.floating_ip_target_list_by_instance(self.request,
'1')
self.assertEqual(self._get_target_id(candidates[0]), ret[0])
ret_val = ret[0]
self.assertEqual(self._get_target_id(candidates[0]), ret_val.id)
self.assertEqual(candidates[0]['id'], ret_val.port_id)
self.assertEqual(candidates[0]['device_id'], ret_val.instance_id)
self.assertEqual(len(candidates), len(ret))
def _get_preloaded_targets(self):
return [
api.neutron.FloatingIpTarget(
api.neutron.Port({'name': 'name11', 'id': 'id11',
'device_id': 'id-vm1'}),
'192.168.1.1', 'vm1'),
api.neutron.FloatingIpTarget(
api.neutron.Port({'name': 'name21', 'id': 'id21',
'device_id': 'id-vm2'}),
'172.16.1.1', 'vm2'),
api.neutron.FloatingIpTarget(
api.neutron.Port({'name': 'name22', 'id': 'id22',
'device_id': 'id-vm2'}),
'10.11.12.13', 'vm3'),
]
def test_floating_ip_target_get_by_instance_with_preloaded_target(self):
target_list = [{'name': 'name11', 'id': 'id11', 'instance_id': 'vm1'},
{'name': 'name21', 'id': 'id21', 'instance_id': 'vm2'},
{'name': 'name22', 'id': 'id22', 'instance_id': 'vm2'}]
target_list = self._get_preloaded_targets()
self.mox.ReplayAll()
ret = api.neutron.floating_ip_target_get_by_instance(
self.request, 'vm2', target_list)
self.assertEqual('id21', ret)
self.request, 'id-vm2', target_list)
self.assertEqual('id21', ret.port_id)
def test_target_floating_ip_port_by_instance_with_preloaded_target(self):
target_list = [{'name': 'name11', 'id': 'id11', 'instance_id': 'vm1'},
{'name': 'name21', 'id': 'id21', 'instance_id': 'vm2'},
{'name': 'name22', 'id': 'id22', 'instance_id': 'vm2'}]
target_list = self._get_preloaded_targets()
self.mox.ReplayAll()
ret = api.neutron.floating_ip_target_list_by_instance(
self.request, 'vm2', target_list)
self.assertEqual(['id21', 'id22'], ret)
self.request, 'id-vm2', target_list)
self.assertEqual(['id21', 'id22'], [r.port_id for r in ret])