diff --git a/openstack_dashboard/api/network.py b/openstack_dashboard/api/network.py index e514677132..18f60209b7 100644 --- a/openstack_dashboard/api/network.py +++ b/openstack_dashboard/api/network.py @@ -74,14 +74,14 @@ def floating_ip_target_list(request): return NetworkClient(request).floating_ips.list_targets() -def floating_ip_target_get_by_instance(request, instance_id): +def floating_ip_target_get_by_instance(request, instance_id, cache=None): return NetworkClient(request).floating_ips.get_target_id_by_instance( - instance_id) + instance_id, cache) -def floating_ip_target_list_by_instance(request, instance_id): +def floating_ip_target_list_by_instance(request, instance_id, cache=None): floating_ips = NetworkClient(request).floating_ips - return floating_ips.list_target_id_by_instance(instance_id) + return floating_ips.list_target_id_by_instance(instance_id, cache) def floating_ip_simple_associate_supported(request): diff --git a/openstack_dashboard/api/network_base.py b/openstack_dashboard/api/network_base.py index b3caf6cdd3..a71462615d 100644 --- a/openstack_dashboard/api/network_base.py +++ b/openstack_dashboard/api/network_base.py @@ -111,18 +111,30 @@ class FloatingIpManager(object): pass @abc.abstractmethod - def get_target_id_by_instance(self, instance_id): + def get_target_id_by_instance(self, instance_id, target_list=None): """Returns a target ID of floating IP association. Based on a backend implementation. + + :param instance_id: ID of target VM instance + :param target_list: (optional) a list returned by list_targets(). + If specified, looking up is done against the specified list + to save extra API calls to a back-end. Otherwise a target + information is retrieved from a back-end inside the method. """ pass @abc.abstractmethod - def list_target_id_by_instance(self, instance_id): + def list_target_id_by_instance(self, instance_id, target_list=None): """Returns a list of instance's target IDs of floating IP association. Based on the backend implementation + + :param instance_id: ID of target VM instance + :param target_list: (optional) a list returned by list_targets(). + If specified, looking up is done against the specified list + to save extra API calls to a back-end. Otherwise target list + is retrieved from a back-end inside the method. """ pass diff --git a/openstack_dashboard/api/neutron.py b/openstack_dashboard/api/neutron.py index 1295f76e33..4dccc6fb0b 100644 --- a/openstack_dashboard/api/neutron.py +++ b/openstack_dashboard/api/neutron.py @@ -401,11 +401,26 @@ class FloatingIpManager(network_base.FloatingIpManager): self.client.update_floatingip(floating_ip_id, {'floatingip': update_dict}) + def _get_reachable_subnets(self, ports): + # Retrieve subnet list reachable from external network + ext_net_ids = [ext_net.id for ext_net in self.list_pools()] + gw_routers = [r.id for r in router_list(self.request) + 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 == + 'network:router_interface') + and (p.device_id in gw_routers))]) + return reachable_subnets + def list_targets(self): tenant_id = self.request.user.tenant_id ports = port_list(self.request, tenant_id=tenant_id) servers, has_more = nova.server_list(self.request) server_dict = SortedDict([(s.id, s.name) for s in servers]) + reachable_subnets = self._get_reachable_subnets(ports) + targets = [] for p in ports: # Remove network ports from Floating IP targets @@ -414,8 +429,11 @@ class FloatingIpManager(network_base.FloatingIpManager): port_id = p.id server_name = server_dict.get(p.device_id) for ip in p.fixed_ips: + if ip['subnet_id'] not in reachable_subnets: + continue target = {'name': '%s: %s' % (server_name, ip['ip_address']), - 'id': '%s_%s' % (port_id, ip['ip_address'])} + 'id': '%s_%s' % (port_id, ip['ip_address']), + 'instance_id': p.device_id} targets.append(FloatingIpTarget(target)) return targets @@ -425,19 +443,30 @@ class FloatingIpManager(network_base.FloatingIpManager): search_opts = {'device_id': instance_id} return port_list(self.request, **search_opts) - def get_target_id_by_instance(self, instance_id): - # 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']) + def get_target_id_by_instance(self, instance_id, target_list=None): + if target_list is not None: + targets = [target for target in target_list + if target['instance_id'] == instance_id] + if not targets: + return None + return targets[0]['id'] + 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']) - def list_target_id_by_instance(self, instance_id): - ports = self._target_ports_by_instance(instance_id) - return ['{0}_{1}'.format(p.id, p.fixed_ips[0]['ip_address']) - for p in ports] + def list_target_id_by_instance(self, instance_id, target_list=None): + if target_list is not None: + return [target['id'] 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']) + for p in ports] def is_simple_associate_supported(self): # NOTE: There are two reason that simple association support diff --git a/openstack_dashboard/api/nova.py b/openstack_dashboard/api/nova.py index b640f6a915..e8b392b89f 100644 --- a/openstack_dashboard/api/nova.py +++ b/openstack_dashboard/api/nova.py @@ -402,10 +402,10 @@ class FloatingIpManager(network_base.FloatingIpManager): def list_targets(self): return [FloatingIpTarget(s) for s in self.client.servers.list()] - def get_target_id_by_instance(self, instance_id): + def get_target_id_by_instance(self, instance_id, target_list=None): return instance_id - def list_target_id_by_instance(self, instance_id): + def list_target_id_by_instance(self, instance_id, target_list=None): return [instance_id, ] def is_simple_associate_supported(self): diff --git a/openstack_dashboard/dashboards/project/access_and_security/floating_ips/tests.py b/openstack_dashboard/dashboards/project/access_and_security/floating_ips/tests.py index 97ce4e2631..5901eb0c9c 100644 --- a/openstack_dashboard/dashboards/project/access_and_security/floating_ips/tests.py +++ b/openstack_dashboard/dashboards/project/access_and_security/floating_ips/tests.py @@ -19,6 +19,7 @@ from django.core.urlresolvers import reverse from django import http +from django.utils.http import urlencode from mox import IsA # noqa @@ -33,13 +34,13 @@ NAMESPACE = "horizon:project:access_and_security:floating_ips" class FloatingIpViewTests(test.TestCase): + @test.create_stubs({api.network: ('floating_ip_target_list', + 'tenant_floating_ip_list',)}) def test_associate(self): - self.mox.StubOutWithMock(api.network, 'floating_ip_target_list') - self.mox.StubOutWithMock(api.network, 'tenant_floating_ip_list') api.network.floating_ip_target_list(IsA(http.HttpRequest)) \ - .AndReturn(self.servers.list()) + .AndReturn(self.servers.list()) api.network.tenant_floating_ip_list(IsA(http.HttpRequest)) \ - .AndReturn(self.floating_ips.list()) + .AndReturn(self.floating_ips.list()) self.mox.ReplayAll() url = reverse('%s:associate' % NAMESPACE) @@ -50,12 +51,35 @@ class FloatingIpViewTests(test.TestCase): # Verify that our "associated" floating IP isn't in the choices list. self.assertTrue(self.floating_ips.first() not in choices) + @test.create_stubs({api.network: ('floating_ip_target_list', + 'floating_ip_target_get_by_instance', + 'tenant_floating_ip_list',)}) + def test_associate_with_instance_id(self): + api.network.floating_ip_target_list(IsA(http.HttpRequest)) \ + .AndReturn(self.servers.list()) + api.network.floating_ip_target_get_by_instance( + IsA(http.HttpRequest), 'TEST-ID', self.servers.list()) \ + .AndReturn('TEST-ID') + api.network.tenant_floating_ip_list(IsA(http.HttpRequest)) \ + .AndReturn(self.floating_ips.list()) + self.mox.ReplayAll() + + base_url = reverse('%s:associate' % NAMESPACE) + params = urlencode({'instance_id': 'TEST-ID'}) + url = '?'.join([base_url, params]) + res = self.client.get(url) + self.assertTemplateUsed(res, views.WorkflowView.template_name) + workflow = res.context['workflow'] + choices = dict(workflow.steps[0].action.fields['ip_id'].choices) + # Verify that our "associated" floating IP isn't in the choices list. + self.assertTrue(self.floating_ips.first() not in choices) + + @test.create_stubs({api.network: ('floating_ip_associate', + 'floating_ip_target_list', + 'tenant_floating_ip_list',)}) def test_associate_post(self): floating_ip = self.floating_ips.list()[1] server = self.servers.first() - self.mox.StubOutWithMock(api.network, 'floating_ip_associate') - self.mox.StubOutWithMock(api.network, 'tenant_floating_ip_list') - self.mox.StubOutWithMock(api.network, 'floating_ip_target_list') api.network.tenant_floating_ip_list(IsA(http.HttpRequest)) \ .AndReturn(self.floating_ips.list()) @@ -72,12 +96,12 @@ class FloatingIpViewTests(test.TestCase): res = self.client.post(url, form_data) self.assertRedirectsNoFollow(res, INDEX_URL) + @test.create_stubs({api.network: ('floating_ip_associate', + 'floating_ip_target_list', + 'tenant_floating_ip_list',)}) def test_associate_post_with_redirect(self): floating_ip = self.floating_ips.list()[1] server = self.servers.first() - self.mox.StubOutWithMock(api.network, 'floating_ip_associate') - self.mox.StubOutWithMock(api.network, 'tenant_floating_ip_list') - self.mox.StubOutWithMock(api.network, 'floating_ip_target_list') api.network.tenant_floating_ip_list(IsA(http.HttpRequest)) \ .AndReturn(self.floating_ips.list()) @@ -95,12 +119,12 @@ class FloatingIpViewTests(test.TestCase): res = self.client.post("%s?next=%s" % (url, next), form_data) self.assertRedirectsNoFollow(res, next) + @test.create_stubs({api.network: ('floating_ip_associate', + 'floating_ip_target_list', + 'tenant_floating_ip_list',)}) def test_associate_post_with_exception(self): floating_ip = self.floating_ips.list()[1] server = self.servers.first() - self.mox.StubOutWithMock(api.network, 'floating_ip_associate') - self.mox.StubOutWithMock(api.network, 'tenant_floating_ip_list') - self.mox.StubOutWithMock(api.network, 'floating_ip_target_list') api.network.tenant_floating_ip_list(IsA(http.HttpRequest)) \ .AndReturn(self.floating_ips.list()) @@ -118,14 +142,14 @@ class FloatingIpViewTests(test.TestCase): res = self.client.post(url, form_data) self.assertRedirectsNoFollow(res, INDEX_URL) + @test.create_stubs({api.nova: ('server_list',), + api.network: ('floating_ip_disassociate', + 'floating_ip_supported', + 'tenant_floating_ip_get', + 'tenant_floating_ip_list',)}) def test_disassociate_post(self): floating_ip = self.floating_ips.first() server = self.servers.first() - self.mox.StubOutWithMock(api.network, 'floating_ip_supported') - self.mox.StubOutWithMock(api.network, 'tenant_floating_ip_list') - self.mox.StubOutWithMock(api.network, 'tenant_floating_ip_get') - self.mox.StubOutWithMock(api.network, 'floating_ip_disassociate') - self.mox.StubOutWithMock(api.nova, 'server_list') api.nova.server_list(IsA(http.HttpRequest)) \ .AndReturn([self.servers.list(), False]) @@ -143,14 +167,14 @@ class FloatingIpViewTests(test.TestCase): self.assertMessageCount(success=1) self.assertRedirectsNoFollow(res, INDEX_URL) + @test.create_stubs({api.nova: ('server_list',), + api.network: ('floating_ip_disassociate', + 'floating_ip_supported', + 'tenant_floating_ip_get', + 'tenant_floating_ip_list',)}) def test_disassociate_post_with_exception(self): floating_ip = self.floating_ips.first() server = self.servers.first() - self.mox.StubOutWithMock(api.network, 'floating_ip_supported') - self.mox.StubOutWithMock(api.network, 'tenant_floating_ip_list') - self.mox.StubOutWithMock(api.network, 'tenant_floating_ip_get') - self.mox.StubOutWithMock(api.network, 'floating_ip_disassociate') - self.mox.StubOutWithMock(api.nova, 'server_list') api.nova.server_list(IsA(http.HttpRequest)) \ .AndReturn([self.servers.list(), False]) diff --git a/openstack_dashboard/dashboards/project/access_and_security/floating_ips/workflows.py b/openstack_dashboard/dashboards/project/access_and_security/floating_ips/workflows.py index eb1ab5359f..40483722cd 100644 --- a/openstack_dashboard/dashboards/project/access_and_security/floating_ips/workflows.py +++ b/openstack_dashboard/dashboards/project/access_and_security/floating_ips/workflows.py @@ -19,6 +19,7 @@ from neutronclient.common import exceptions as neutron_exc from horizon import exceptions from horizon import forms +from horizon.utils import memoized from horizon import workflows from openstack_dashboard import api @@ -55,8 +56,9 @@ class AssociateIPAction(workflows.Action): # and set the initial value of instance_id ChoiceField. q_instance_id = self.request.GET.get('instance_id') if q_instance_id: + targets = self._get_target_list() target_id = api.network.floating_ip_target_get_by_instance( - self.request, q_instance_id) + self.request, q_instance_id, targets) self.initial['instance_id'] = target_id def populate_ip_id_choices(self, request, context): @@ -78,7 +80,8 @@ class AssociateIPAction(workflows.Action): return options - def populate_instance_id_choices(self, request, context): + @memoized.memoized_method + def _get_target_list(self): targets = [] try: targets = api.network.floating_ip_target_list(self.request) @@ -87,6 +90,11 @@ class AssociateIPAction(workflows.Action): exceptions.handle(self.request, _('Unable to retrieve instance list.'), redirect=redirect) + return targets + + def populate_instance_id_choices(self, request, context): + targets = self._get_target_list() + instances = [] for target in targets: instances.append((target.id, target.name)) diff --git a/openstack_dashboard/test/api_tests/network_tests.py b/openstack_dashboard/test/api_tests/network_tests.py index 1060c421c9..e15f580698 100644 --- a/openstack_dashboard/test/api_tests/network_tests.py +++ b/openstack_dashboard/test/api_tests/network_tests.py @@ -693,9 +693,14 @@ class NetworkApiNeutronFloatingIpTests(NetworkApiNeutronTestBase): def test_floating_ip_target_list(self): ports = self.api_ports.list() + # Port on the first subnet is connected to a router + # attached to external network in neutron_data. + subnet_id = self.subnets.first().id target_ports = [(self._get_target_id(p), self._get_target_name(p)) for p in ports - if not p['device_owner'].startswith('network:')] + if (not p['device_owner'].startswith('network:') and + subnet_id in [ip['subnet_id'] + for ip in p['fixed_ips']])] filters = {'tenant_id': self.request.user.tenant_id} self.qclient.list_ports(**filters).AndReturn({'ports': ports}) servers = self.servers.list() @@ -703,6 +708,15 @@ class NetworkApiNeutronFloatingIpTests(NetworkApiNeutronTestBase): novaclient.servers = self.mox.CreateMockAnything() search_opts = {'project_id': self.request.user.tenant_id} novaclient.servers.list(True, search_opts).AndReturn(servers) + + 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()}) + self.mox.ReplayAll() rets = api.network.floating_ip_target_list(self.request) @@ -732,3 +746,23 @@ class NetworkApiNeutronFloatingIpTests(NetworkApiNeutronTestBase): '1') self.assertEqual(self._get_target_id(candidates[0]), ret[0]) self.assertEqual(len(candidates), len(ret)) + + 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'}] + self.mox.ReplayAll() + + ret = api.network.floating_ip_target_get_by_instance( + self.request, 'vm2', target_list) + self.assertEqual('id21', ret) + + 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'}] + self.mox.ReplayAll() + + ret = api.network.floating_ip_target_list_by_instance( + self.request, 'vm2', target_list) + self.assertEqual(['id21', 'id22'], ret) diff --git a/openstack_dashboard/test/test_data/neutron_data.py b/openstack_dashboard/test/test_data/neutron_data.py index 90312eb41f..087a6c5211 100644 --- a/openstack_dashboard/test/test_data/neutron_data.py +++ b/openstack_dashboard/test/test_data/neutron_data.py @@ -181,6 +181,20 @@ def data(TEST): TEST.ports.add(neutron.Port(port_dict)) assoc_port = port_dict + port_dict = {'admin_state_up': True, + 'device_id': '279989f7-54bb-41d9-ba42-0d61f12fda61', + 'device_owner': 'network:router_interface', + 'fixed_ips': [{'ip_address': '10.0.0.1', + 'subnet_id': subnet_dict['id']}], + 'id': '9036eedb-e7fa-458e-bc6e-d9d06d9d1bc4', + 'mac_address': 'fa:16:3e:9c:d5:7f', + 'name': '', + 'network_id': network_dict['id'], + 'status': 'ACTIVE', + 'tenant_id': network_dict['tenant_id']} + TEST.api_ports.add(port_dict) + TEST.ports.add(neutron.Port(port_dict)) + # 2nd network. network_dict = {'admin_state_up': True, 'id': '72c3ab6c-c80f-4341-9dc5-210fa31ac6c2',