From b0116dbb22535725492f738fdf2a2d53ec62efe3 Mon Sep 17 00:00:00 2001 From: Nejc Saje Date: Tue, 9 Sep 2014 22:55:51 -0400 Subject: [PATCH] Use central agent manager's keystone token in discoveries Currently, EndpointDiscovery creates its own keystone client. This patch passes an agent-manager reference into the discoveries, enabling them to reuse the manager's keystone client and possibly other things in the future. Change-Id: I9875f15c4be25c2ac125458caa9a09927afdf3b2 Closes-bug: #1368125 --- ceilometer/agent.py | 2 +- ceilometer/central/discovery.py | 16 +------ ceilometer/compute/discovery.py | 2 +- ceilometer/hardware/discovery.py | 2 +- ceilometer/network/services/discovery.py | 16 +++---- ceilometer/plugin.py | 3 +- ceilometer/tests/agentbase.py | 4 +- ceilometer/tests/central/test_discovery.py | 46 +++++++++++++++++++ .../tests/network/services/test_fwaas.py | 5 +- .../tests/network/services/test_lbaas.py | 10 ++-- .../tests/network/services/test_vpnaas.py | 6 ++- 11 files changed, 76 insertions(+), 36 deletions(-) create mode 100644 ceilometer/tests/central/test_discovery.py diff --git a/ceilometer/agent.py b/ceilometer/agent.py index 3a60e62d5..f906e019f 100644 --- a/ceilometer/agent.py +++ b/ceilometer/agent.py @@ -204,7 +204,7 @@ class AgentManager(os_service.Service): discoverer = self._discoverer(name) if discoverer: try: - discovered = discoverer.discover(param) + discovered = discoverer.discover(self, param) partitioned = self.partition_coordinator.extract_my_subset( self._construct_group_id(discoverer.group_id), discovered) diff --git a/ceilometer/central/discovery.py b/ceilometer/central/discovery.py index f9fa45674..902f37ad2 100644 --- a/ceilometer/central/discovery.py +++ b/ceilometer/central/discovery.py @@ -15,7 +15,6 @@ # License for the specific language governing permissions and limitations # under the License. -from keystoneclient.v2_0 import client as ksclient from oslo.config import cfg from ceilometer.openstack.common.gettextutils import _LW @@ -28,22 +27,11 @@ cfg.CONF.import_group('service_credentials', 'ceilometer.service') class EndpointDiscovery(plugin.DiscoveryBase): - def __init__(self): - super(EndpointDiscovery, self).__init__() - self.keystone = ksclient.Client( - username=cfg.CONF.service_credentials.os_username, - password=cfg.CONF.service_credentials.os_password, - tenant_id=cfg.CONF.service_credentials.os_tenant_id, - tenant_name=cfg.CONF.service_credentials.os_tenant_name, - cacert=cfg.CONF.service_credentials.os_cacert, - auth_url=cfg.CONF.service_credentials.os_auth_url, - region_name=cfg.CONF.service_credentials.os_region_name, - insecure=cfg.CONF.service_credentials.insecure) - def discover(self, param=None): + def discover(self, manager, param=None): if not param: return [] - endpoints = self.keystone.service_catalog.get_urls( + endpoints = manager.keystone.service_catalog.get_urls( service_type=param, endpoint_type=cfg.CONF.service_credentials.os_endpoint_type, region_name=cfg.CONF.service_credentials.os_region_name) diff --git a/ceilometer/compute/discovery.py b/ceilometer/compute/discovery.py index 7ec17526a..01f8f083e 100644 --- a/ceilometer/compute/discovery.py +++ b/ceilometer/compute/discovery.py @@ -34,7 +34,7 @@ class InstanceDiscovery(plugin.DiscoveryBase): super(InstanceDiscovery, self).__init__() self.nova_cli = nova_client.Client() - def discover(self, param=None): + def discover(self, manager, param=None): """Discover resources to monitor.""" instances = self.nova_cli.instance_get_all_by_host(cfg.CONF.host) return [i for i in instances diff --git a/ceilometer/hardware/discovery.py b/ceilometer/hardware/discovery.py index 8848eb576..3b7863a25 100644 --- a/ceilometer/hardware/discovery.py +++ b/ceilometer/hardware/discovery.py @@ -45,7 +45,7 @@ class NodesDiscoveryTripleO(plugin.DiscoveryBase): def _address(instance, field): return instance.addresses['ctlplane'][0].get(field) - def discover(self, param=None): + def discover(self, manager, param=None): """Discover resources to monitor.""" instances = self.nova_cli.instance_get_all() diff --git a/ceilometer/network/services/discovery.py b/ceilometer/network/services/discovery.py index 710a9f041..87e646ded 100644 --- a/ceilometer/network/services/discovery.py +++ b/ceilometer/network/services/discovery.py @@ -29,7 +29,7 @@ class _BaseServicesDiscovery(base_plugin.DiscoveryBase): class LBPoolsDiscovery(_BaseServicesDiscovery): @plugin.check_keystone('network') - def discover(self, param=None): + def discover(self, manager, param=None): """Discover resources to monitor.""" pools = self.neutron_cli.pool_get_all() @@ -39,7 +39,7 @@ class LBPoolsDiscovery(_BaseServicesDiscovery): class LBVipsDiscovery(_BaseServicesDiscovery): @plugin.check_keystone('network') - def discover(self, param=None): + def discover(self, manager, param=None): """Discover resources to monitor.""" vips = self.neutron_cli.vip_get_all() @@ -49,7 +49,7 @@ class LBVipsDiscovery(_BaseServicesDiscovery): class LBMembersDiscovery(_BaseServicesDiscovery): @plugin.check_keystone('network') - def discover(self, param=None): + def discover(self, manager, param=None): """Discover resources to monitor.""" members = self.neutron_cli.member_get_all() @@ -59,7 +59,7 @@ class LBMembersDiscovery(_BaseServicesDiscovery): class LBHealthMonitorsDiscovery(_BaseServicesDiscovery): @plugin.check_keystone('network') - def discover(self, param=None): + def discover(self, manager, param=None): """Discover resources to monitor.""" probes = self.neutron_cli.health_monitor_get_all() @@ -68,7 +68,7 @@ class LBHealthMonitorsDiscovery(_BaseServicesDiscovery): class VPNServicesDiscovery(_BaseServicesDiscovery): @plugin.check_keystone('network') - def discover(self, param=None): + def discover(self, manager, param=None): """Discover resources to monitor.""" vpnservices = self.neutron_cli.vpn_get_all() @@ -78,7 +78,7 @@ class VPNServicesDiscovery(_BaseServicesDiscovery): class IPSecConnectionsDiscovery(_BaseServicesDiscovery): @plugin.check_keystone('network') - def discover(self, param=None): + def discover(self, manager, param=None): """Discover resources to monitor.""" conns = self.neutron_cli.ipsec_site_connections_get_all() @@ -87,7 +87,7 @@ class IPSecConnectionsDiscovery(_BaseServicesDiscovery): class FirewallDiscovery(_BaseServicesDiscovery): @plugin.check_keystone('network') - def discover(self, param=None): + def discover(self, manager, param=None): """Discover resources to monitor.""" fw = self.neutron_cli.firewall_get_all() @@ -97,7 +97,7 @@ class FirewallDiscovery(_BaseServicesDiscovery): class FirewallPolicyDiscovery(_BaseServicesDiscovery): @plugin.check_keystone('network') - def discover(self, param=None): + def discover(self, manager, param=None): """Discover resources to monitor.""" return self.neutron_cli.fw_policy_get_all() diff --git a/ceilometer/plugin.py b/ceilometer/plugin.py index 46481da11..a0fb9fa17 100644 --- a/ceilometer/plugin.py +++ b/ceilometer/plugin.py @@ -163,9 +163,10 @@ class PollsterBase(PluginBase): @six.add_metaclass(abc.ABCMeta) class DiscoveryBase(object): @abc.abstractmethod - def discover(self, param=None): + def discover(self, manager, param=None): """Discover resources to monitor. + :param manager: The service manager class invoking the plugin. :param param: an optional parameter to guide the discovery """ diff --git a/ceilometer/tests/agentbase.py b/ceilometer/tests/agentbase.py index bb4a6e398..a582109ec 100644 --- a/ceilometer/tests/agentbase.py +++ b/ceilometer/tests/agentbase.py @@ -95,13 +95,13 @@ class TestPollsterException(TestPollster): class TestDiscovery(plugin.DiscoveryBase): - def discover(self, param=None): + def discover(self, manager, param=None): self.params.append(param) return self.resources class TestDiscoveryException(plugin.DiscoveryBase): - def discover(self, param=None): + def discover(self, manager, param=None): self.params.append(param) raise Exception() diff --git a/ceilometer/tests/central/test_discovery.py b/ceilometer/tests/central/test_discovery.py new file mode 100644 index 000000000..eac489b5a --- /dev/null +++ b/ceilometer/tests/central/test_discovery.py @@ -0,0 +1,46 @@ +# +# Copyright 2014 Red Hat Inc. +# +# Author: Nejc Saje +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +"""Tests for ceilometer/central/manager.py +""" + +import mock +from oslo.config import fixture as fixture_config +from oslotest import base + +from ceilometer.central import discovery + + +class TestEndpointDiscovery(base.BaseTestCase): + + def setUp(self): + super(TestEndpointDiscovery, self).setUp() + self.discovery = discovery.EndpointDiscovery() + self.manager = mock.MagicMock() + self.CONF = self.useFixture(fixture_config.Config()).conf + + def test_keystone_called(self): + self.CONF.set_override('os_endpoint_type', 'test-endpoint-type', + group='service_credentials') + self.CONF.set_override('os_region_name', 'test-region-name', + group='service_credentials') + self.discovery.discover(self.manager, param='test-service-type') + expected = [mock.call(service_type='test-service-type', + endpoint_type='test-endpoint-type', + region_name='test-region-name')] + self.assertEqual(expected, + self.manager.keystone.service_catalog.get_urls + .call_args_list) \ No newline at end of file diff --git a/ceilometer/tests/network/services/test_fwaas.py b/ceilometer/tests/network/services/test_fwaas.py index ee2ad86f2..f59743d56 100644 --- a/ceilometer/tests/network/services/test_fwaas.py +++ b/ceilometer/tests/network/services/test_fwaas.py @@ -106,7 +106,7 @@ class TestFirewallPollster(_BaseTestFWPollster): set([s.name for s in samples])) def test_vpn_discovery(self): - discovered_fws = discovery.FirewallDiscovery().discover() + discovered_fws = discovery.FirewallDiscovery().discover(self.manager) self.assertEqual(3, len(discovered_fws)) for vpn in self.fake_get_fw_service(): @@ -165,6 +165,7 @@ class TestIPSecConnectionsPollster(_BaseTestFWPollster): set([s.name for s in samples])) def test_fw_policy_discovery(self): - discovered_policy = discovery.FirewallPolicyDiscovery().discover() + discovered_policy = discovery.FirewallPolicyDiscovery().discover( + self.manager) self.assertEqual(1, len(discovered_policy)) self.assertEqual(self.fake_get_fw_policy(), discovered_policy) diff --git a/ceilometer/tests/network/services/test_lbaas.py b/ceilometer/tests/network/services/test_lbaas.py index 60322e566..ed37c3485 100644 --- a/ceilometer/tests/network/services/test_lbaas.py +++ b/ceilometer/tests/network/services/test_lbaas.py @@ -153,7 +153,7 @@ class TestLBPoolPollster(_BaseTestLBPollster): set([s.name for s in samples])) def test_pool_discovery(self): - discovered_pools = discovery.LBPoolsDiscovery().discover() + discovered_pools = discovery.LBPoolsDiscovery().discover(self.manager) self.assertEqual(4, len(discovered_pools)) for pool in self.fake_get_pools(): if pool['status'] == 'error': @@ -276,7 +276,7 @@ class TestLBVipPollster(_BaseTestLBPollster): set([s.name for s in samples])) def test_vip_discovery(self): - discovered_vips = discovery.LBVipsDiscovery().discover() + discovered_vips = discovery.LBVipsDiscovery().discover(self.manager) self.assertEqual(4, len(discovered_vips)) for pool in self.fake_get_vips(): if pool['status'] == 'error': @@ -369,7 +369,8 @@ class TestLBMemberPollster(_BaseTestLBPollster): set([s.name for s in samples])) def test_members_discovery(self): - discovered_members = discovery.LBMembersDiscovery().discover() + discovered_members = discovery.LBMembersDiscovery().discover( + self.manager) self.assertEqual(4, len(discovered_members)) for pool in self.fake_get_members(): if pool['status'] == 'error': @@ -417,7 +418,8 @@ class TestLBHealthProbePollster(_BaseTestLBPollster): set([s.name for s in samples])) def test_probes_discovery(self): - discovered_probes = discovery.LBHealthMonitorsDiscovery().discover() + discovered_probes = discovery.LBHealthMonitorsDiscovery().discover( + self.manager) self.assertEqual(discovered_probes, self.fake_get_health_monitor()) diff --git a/ceilometer/tests/network/services/test_vpnaas.py b/ceilometer/tests/network/services/test_vpnaas.py index 228da739b..864a2efb3 100644 --- a/ceilometer/tests/network/services/test_vpnaas.py +++ b/ceilometer/tests/network/services/test_vpnaas.py @@ -110,7 +110,8 @@ class TestVPNServicesPollster(_BaseTestVPNPollster): set([s.name for s in samples])) def test_vpn_discovery(self): - discovered_vpns = discovery.VPNServicesDiscovery().discover() + discovered_vpns = discovery.VPNServicesDiscovery().discover( + self.manager) self.assertEqual(3, len(discovered_vpns)) for vpn in self.fake_get_vpn_service(): @@ -170,6 +171,7 @@ class TestIPSecConnectionsPollster(_BaseTestVPNPollster): set([s.name for s in samples])) def test_conns_discovery(self): - discovered_conns = discovery.IPSecConnectionsDiscovery().discover() + discovered_conns = discovery.IPSecConnectionsDiscovery().discover( + self.manager) self.assertEqual(1, len(discovered_conns)) self.assertEqual(self.fake_get_ipsec_connections(), discovered_conns)