Display only reachable IP as Floating IP association target

In Neutron deployments some VM port can be unreachable from
external network and cannot be associated with floating IP.
It is confusing if these ports are listed in Floating IP
Associate form.

Change-Id: I2d8faf0dbf4490d198b883fe1becfd950b1b4d14
Closes-Bug: #1252403

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):

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
 

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

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):

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])

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))

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)

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',