From 6f1307e47212550c7b9f7fc6bb050ef5ae525f3e Mon Sep 17 00:00:00 2001 From: Akihiro MOTOKI Date: Fri, 20 Sep 2013 11:27:06 +0900 Subject: [PATCH] Determine security group API dynamically This commit removes enable_security_group from local_settings.py and determines which security group API should be used (nova or neutron). Closes-Bug: #1227804 As described in bug 1203413, there is a case where Nova security group with Neutron driver causes a problem. The type of 'name' attribute in add_security_group_to_instance and remove_security_group_from_instance depends on the backend, integer for nova security group driver and UUID for quantum security group driver to make it work as expected. enable_security_group config parameter produces a situation where Nova security group with Neutron driver. We can avoid this situation by removing this parameter when using Horizon. Change-Id: I713c6ad166e142929f0a708e93a8fedb0de48640 --- openstack_dashboard/api/network.py | 12 +++--- openstack_dashboard/api/neutron.py | 23 +++++++---- .../dashboards/project/instances/tables.py | 6 +-- .../dashboards/project/instances/tests.py | 35 ++++++++++++++--- .../local/local_settings.py.example | 1 - .../test/api_tests/network_tests.py | 39 +++++++++++++++++++ .../test/api_tests/neutron_tests.py | 2 +- .../test/test_data/neutron_data.py | 5 +-- 8 files changed, 96 insertions(+), 27 deletions(-) diff --git a/openstack_dashboard/api/network.py b/openstack_dashboard/api/network.py index 6f0fe33006..54bc1e393a 100644 --- a/openstack_dashboard/api/network.py +++ b/openstack_dashboard/api/network.py @@ -37,12 +37,8 @@ class NetworkClient(object): else: self.floating_ips = nova.FloatingIpManager(request) - # Not all qunantum plugins support security group, - # so we have enable_security_group configuration parameter. - neutron_sg_enabled = getattr(settings, - 'OPENSTACK_NEUTRON_NETWORK', - {}).get('enable_security_group', True) - if neutron_enabled and neutron_sg_enabled: + if (neutron_enabled and + neutron.is_security_group_extension_supported(request)): self.secgroups = neutron.SecurityGroupManager(request) else: self.secgroups = nova.SecurityGroupManager(request) @@ -87,6 +83,10 @@ def floating_ip_target_get_by_instance(request, instance_id): instance_id) +def floating_ip_simple_associate_supported(request): + return NetworkClient(request).floating_ips.is_simple_associate_supported() + + def security_group_list(request): return NetworkClient(request).secgroups.list() diff --git a/openstack_dashboard/api/neutron.py b/openstack_dashboard/api/neutron.py index 477f3f0662..ca985062f5 100644 --- a/openstack_dashboard/api/neutron.py +++ b/openstack_dashboard/api/neutron.py @@ -27,6 +27,8 @@ from django.conf import settings # noqa from django.utils.datastructures import SortedDict # noqa from django.utils.translation import ugettext_lazy as _ # noqa +from horizon.utils.memoized import memoized # noqa + from openstack_dashboard.api import base from openstack_dashboard.api import network_base from openstack_dashboard.api import nova @@ -714,6 +716,12 @@ def tenant_quota_update(request, tenant_id, **kwargs): return neutronclient(request).update_quota(tenant_id, quotas) +def agent_list(request): + agents = neutronclient(request).list_agents() + return [Agent(a) for a in agents['agents']] + + +@memoized def list_extensions(request): extensions_list = neutronclient(request).list_extensions() if 'extensions' in extensions_list: @@ -722,11 +730,7 @@ def list_extensions(request): return {} -def agent_list(request): - agents = neutronclient(request).list_agents() - return [Agent(a) for a in agents['agents']] - - +@memoized def is_extension_supported(request, extension_alias): extensions = list_extensions(request) @@ -737,15 +741,20 @@ def is_extension_supported(request, extension_alias): return False +@memoized def is_quotas_extension_supported(request): network_config = getattr(settings, 'OPENSTACK_NEUTRON_NETWORK', {}) - if network_config.get('enable_quotas', False) and \ - is_extension_supported(request, 'quotas'): + if (network_config.get('enable_quotas', False) and + is_extension_supported(request, 'quotas')): return True else: return False +def is_security_group_extension_supported(request): + return is_extension_supported(request, 'security-group') + + # Using this mechanism till a better plugin/sub-plugin detection # mechanism is available. # Using local_settings to detect if the "router" dashboard diff --git a/openstack_dashboard/dashboards/project/instances/tables.py b/openstack_dashboard/dashboards/project/instances/tables.py index ea6c9cf3e6..87ee0dccbe 100644 --- a/openstack_dashboard/dashboards/project/instances/tables.py +++ b/openstack_dashboard/dashboards/project/instances/tables.py @@ -353,8 +353,7 @@ class AssociateIP(tables.LinkAction): classes = ("ajax-modal", "btn-associate") def allowed(self, request, instance): - fip = api.network.NetworkClient(request).floating_ips - if fip.is_simple_associate_supported(): + if api.network.floating_ip_simple_associate_supported(request): return False return not is_deleting(instance) @@ -373,8 +372,7 @@ class SimpleAssociateIP(tables.Action): classes = ("btn-associate-simple",) def allowed(self, request, instance): - fip = api.network.NetworkClient(request).floating_ips - if not fip.is_simple_associate_supported(): + if not api.network.floating_ip_simple_associate_supported(request): return False return not is_deleting(instance) diff --git a/openstack_dashboard/dashboards/project/instances/tests.py b/openstack_dashboard/dashboards/project/instances/tests.py index 5ca61fd1d7..ef87e07bf8 100644 --- a/openstack_dashboard/dashboards/project/instances/tests.py +++ b/openstack_dashboard/dashboards/project/instances/tests.py @@ -49,7 +49,10 @@ class InstanceTests(test.TestCase): @test.create_stubs({api.nova: ('flavor_list', 'server_list', 'tenant_absolute_limits', - 'extension_supported',)}) + 'extension_supported',), + api.network: + ('floating_ip_simple_associate_supported',), + }) def test_index(self): api.nova.extension_supported('AdminActions', IsA(http.HttpRequest)) \ @@ -61,6 +64,8 @@ class InstanceTests(test.TestCase): .AndReturn([self.servers.list(), False]) api.nova.tenant_absolute_limits(IsA(http.HttpRequest), reserved=True) \ .MultipleTimes().AndReturn(self.limits['absolute']) + api.network.floating_ip_simple_associate_supported( + IsA(http.HttpRequest)).MultipleTimes().AndReturn(True) self.mox.ReplayAll() @@ -93,7 +98,10 @@ class InstanceTests(test.TestCase): 'server_list', 'flavor_get', 'tenant_absolute_limits', - 'extension_supported',)}) + 'extension_supported',), + api.network: + ('floating_ip_simple_associate_supported',), + }) def test_index_flavor_list_exception(self): servers = self.servers.list() flavors = self.flavors.list() @@ -111,6 +119,8 @@ class InstanceTests(test.TestCase): AndReturn(full_flavors[server.flavor["id"]]) api.nova.tenant_absolute_limits(IsA(http.HttpRequest), reserved=True) \ .MultipleTimes().AndReturn(self.limits['absolute']) + api.network.floating_ip_simple_associate_supported( + IsA(http.HttpRequest)).MultipleTimes().AndReturn(True) self.mox.ReplayAll() @@ -125,7 +135,10 @@ class InstanceTests(test.TestCase): 'server_list', 'flavor_get', 'tenant_absolute_limits', - 'extension_supported',)}) + 'extension_supported',), + api.network: + ('floating_ip_simple_associate_supported',), + }) def test_index_flavor_get_exception(self): servers = self.servers.list() flavors = self.flavors.list() @@ -146,6 +159,8 @@ class InstanceTests(test.TestCase): AndRaise(self.exceptions.nova) api.nova.tenant_absolute_limits(IsA(http.HttpRequest), reserved=True) \ .MultipleTimes().AndReturn(self.limits['absolute']) + api.network.floating_ip_simple_associate_supported( + IsA(http.HttpRequest)).MultipleTimes().AndReturn(True) self.mox.ReplayAll() @@ -1588,7 +1603,10 @@ class InstanceTests(test.TestCase): @test.create_stubs({api.nova: ('flavor_list', 'server_list', 'tenant_absolute_limits', - 'extension_supported',)}) + 'extension_supported',), + api.network: + ('floating_ip_simple_associate_supported',), + }) def test_launch_button_disabled_when_quota_exceeded(self): limits = self.limits['absolute'] limits['totalInstancesUsed'] = limits['maxTotalInstances'] @@ -1603,6 +1621,8 @@ class InstanceTests(test.TestCase): .AndReturn([self.servers.list(), False]) api.nova.tenant_absolute_limits(IsA(http.HttpRequest), reserved=True) \ .MultipleTimes().AndReturn(limits) + api.network.floating_ip_simple_associate_supported( + IsA(http.HttpRequest)).MultipleTimes().AndReturn(True) self.mox.ReplayAll() @@ -1621,7 +1641,10 @@ class InstanceTests(test.TestCase): @test.create_stubs({api.nova: ('flavor_list', 'server_list', 'tenant_absolute_limits', - 'extension_supported',)}) + 'extension_supported',), + api.network: + ('floating_ip_simple_associate_supported',), + }) def test_index_options_after_migrate(self): server = self.servers.first() server.status = "VERIFY_RESIZE" @@ -1635,6 +1658,8 @@ class InstanceTests(test.TestCase): .AndReturn([self.servers.list(), False]) api.nova.tenant_absolute_limits(IsA(http.HttpRequest), reserved=True) \ .MultipleTimes().AndReturn(self.limits['absolute']) + api.network.floating_ip_simple_associate_supported( + IsA(http.HttpRequest)).MultipleTimes().AndReturn(True) self.mox.ReplayAll() diff --git a/openstack_dashboard/local/local_settings.py.example b/openstack_dashboard/local/local_settings.py.example index 7fb7ddeebe..6d333e2a1a 100644 --- a/openstack_dashboard/local/local_settings.py.example +++ b/openstack_dashboard/local/local_settings.py.example @@ -160,7 +160,6 @@ OPENSTACK_NEUTRON_NETWORK = { 'enable_lb': False, 'enable_firewall': False, 'enable_quotas': True, - 'enable_security_group': True, 'enable_vpn': False, # The profile_support option is used to detect if an external router can be # configured via the dashboard. When using specific plugins the diff --git a/openstack_dashboard/test/api_tests/network_tests.py b/openstack_dashboard/test/api_tests/network_tests.py index 0388f7b2d0..a68f9ab530 100644 --- a/openstack_dashboard/test/api_tests/network_tests.py +++ b/openstack_dashboard/test/api_tests/network_tests.py @@ -27,6 +27,43 @@ from openstack_dashboard import api from openstack_dashboard.test import helpers as test +class NetworkClientTestCase(test.APITestCase): + def test_networkclient_no_neutron(self): + self.mox.StubOutWithMock(api.base, 'is_service_enabled') + api.base.is_service_enabled(IsA(http.HttpRequest), 'network') \ + .AndReturn(False) + self.mox.ReplayAll() + + nc = api.network.NetworkClient(self.request) + self.assertIsInstance(nc.floating_ips, api.nova.FloatingIpManager) + self.assertIsInstance(nc.secgroups, api.nova.SecurityGroupManager) + + def test_networkclient_neutron(self): + self.mox.StubOutWithMock(api.base, 'is_service_enabled') + api.base.is_service_enabled(IsA(http.HttpRequest), 'network') \ + .AndReturn(True) + self.neutronclient = self.stub_neutronclient() + self.neutronclient.list_extensions() \ + .AndReturn({'extensions': self.api_extensions.list()}) + self.mox.ReplayAll() + + nc = api.network.NetworkClient(self.request) + self.assertIsInstance(nc.floating_ips, api.neutron.FloatingIpManager) + self.assertIsInstance(nc.secgroups, api.neutron.SecurityGroupManager) + + def test_networkclient_neutron_with_nova_security_group(self): + self.mox.StubOutWithMock(api.base, 'is_service_enabled') + api.base.is_service_enabled(IsA(http.HttpRequest), 'network') \ + .AndReturn(True) + self.neutronclient = self.stub_neutronclient() + self.neutronclient.list_extensions().AndReturn({'extensions': []}) + self.mox.ReplayAll() + + nc = api.network.NetworkClient(self.request) + self.assertIsInstance(nc.floating_ips, api.neutron.FloatingIpManager) + self.assertIsInstance(nc.secgroups, api.nova.SecurityGroupManager) + + class NetworkApiNovaTestBase(test.APITestCase): def setUp(self): super(NetworkApiNovaTestBase, self).setUp() @@ -156,6 +193,8 @@ class NetworkApiNeutronTestBase(test.APITestCase): api.base.is_service_enabled(IsA(http.HttpRequest), 'network') \ .AndReturn(True) self.qclient = self.stub_neutronclient() + self.qclient.list_extensions() \ + .AndReturn({'extensions': self.api_extensions.list()}) class NetworkApiNeutronSecurityGroupTests(NetworkApiNeutronTestBase): diff --git a/openstack_dashboard/test/api_tests/neutron_tests.py b/openstack_dashboard/test/api_tests/neutron_tests.py index 10c2b4bfed..2aac4fad33 100644 --- a/openstack_dashboard/test/api_tests/neutron_tests.py +++ b/openstack_dashboard/test/api_tests/neutron_tests.py @@ -274,7 +274,7 @@ class NeutronApiTests(test.APITestCase): def test_is_extension_supported(self): neutronclient = self.stub_neutronclient() neutronclient.list_extensions().MultipleTimes() \ - .AndReturn(self.api_extensions.first()) + .AndReturn({'extensions': self.api_extensions.list()}) self.mox.ReplayAll() self.assertTrue( diff --git a/openstack_dashboard/test/test_data/neutron_data.py b/openstack_dashboard/test/test_data/neutron_data.py index 7f787035f8..b902843d9c 100644 --- a/openstack_dashboard/test/test_data/neutron_data.py +++ b/openstack_dashboard/test/test_data/neutron_data.py @@ -534,9 +534,8 @@ def data(TEST): extension_2 = {"name": "Quota management support", "alias": "quotas", "description": "Expose functions for quotas management"} - extensions = {} - extensions['extensions'] = [extension_1, extension_2] - TEST.api_extensions.add(extensions) + TEST.api_extensions.add(extension_1) + TEST.api_extensions.add(extension_2) #------------------------------------------------------------ # 1st agent