Merge "Display only reachable IP as Floating IP association target"

8 files changed, 169 insertions, 48 deletions

diff --git a/openstack_dashboard/api/network.py b/openstack_dashboard/api/network.py
index e51467713..18f60209b 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 b3caf6cdd..a71462615 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 1295f76e3..4dccc6fb0 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 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 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, 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 66df52895..f52472f54 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 97ce4e263..5901eb0c9 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 eb1ab5359..40483722c 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 1060c421c..e15f58069 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 90312eb41..087a6c521 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',