Remove database access from agent pollsters

Fixes bug #1012242.

Patch stolen from John Tran <jhtran@att.com>

Change-Id: Iab59eb752199e0cd3c8134a29e05b53356a30d75

ceilometer/central/manager.py

@@ -17,7 +17,6 @@
 # under the License.
 
 from ceilometer import extension_manager
-from ceilometer.compute import resources
 from ceilometer.openstack.common import cfg
 from ceilometer.openstack.common import log
 from ceilometer import publish
@@ -40,7 +39,6 @@ class AgentManager(object):
     def __init__(self, host=None):
         super(AgentManager, self).__init__()
         self.host = host
-        self.resources = resources.Resources()
         self.ext_manager = extension_manager.ActivatedExtensionManager(
             namespace=PLUGIN_NAMESPACE,
             disabled_names=cfg.CONF.disabled_central_pollsters,

ceilometer/compute/instance.py

@@ -20,7 +20,6 @@
 
 INSTANCE_PROPERTIES = [
     # Identity properties
-    'display_name',
     'reservation_id',
     # Type properties
     'architecture',
@@ -41,16 +40,16 @@ INSTANCE_PROPERTIES = [
     ]
 
 
-def get_metadata_from_dbobject(instance):
+def get_metadata_from_object(instance):
     """Return a metadata dictionary for the instance.
     """
     metadata = {
-        'display_name': instance.display_name,
+        'display_name': instance.name,
-        'instance_type': (instance.instance_type.flavorid
+        'name': getattr(instance, 'OS-EXT-SRV-ATTR:instance_name', u''),
-                          if instance.instance_type
+        'instance_type': (instance.flavor['id'] if instance.flavor else None),
-                          else None),
+        'host': instance.hostId,
-        'host': instance.host,
         }
+
     for name in INSTANCE_PROPERTIES:
-        metadata[name] = instance.get(name, u'')
+        metadata[name] = getattr(instance, name, u'')
     return metadata

ceilometer/compute/libvirt.py

@@ -34,6 +34,11 @@ from ceilometer.openstack.common import timeutils
 FLAGS = flags.FLAGS
 
 
+def _instance_name(instance):
+    """Shortcut to get instance name"""
+    return getattr(instance, 'OS-EXT-SRV-ATTR:instance_name', None)
+
+
 def get_libvirt_connection():
     """Return an open connection for talking to libvirt."""
     # The direct-import implementation only works with Folsom because
@@ -53,11 +58,10 @@ def make_counter_from_instance(instance, name, type, volume):
         type=type,
         volume=volume,
         user_id=instance.user_id,
-        project_id=instance.project_id,
+        project_id=instance.tenant_id,
-        resource_id=instance.uuid,
+        resource_id=instance.id,
         timestamp=timeutils.isotime(),
-        resource_metadata=compute_instance.get_metadata_from_dbobject(
+        resource_metadata=compute_instance.get_metadata_from_object(instance),
-            instance),
         )
 
 
@@ -110,11 +114,12 @@ class DiskIOPollster(LibVirtPollster):
         conn = get_libvirt_connection()
         # TODO(jd) This does not work see bug#998089
         # for disk in conn.get_disks(instance.name):
+        instance_name = _instance_name(instance)
         try:
-            disks = self._get_disks(conn, instance.name)
+            disks = self._get_disks(conn, instance_name)
         except Exception as err:
             self.LOG.warning('Ignoring instance %s: %s',
-                             instance.name, err)
+                             instance_name, err)
             self.LOG.exception(err)
         else:
             r_bytes = 0
@@ -122,7 +127,7 @@ class DiskIOPollster(LibVirtPollster):
             w_bytes = 0
             w_requests = 0
             for disk in disks:
-                stats = conn.block_stats(instance.name, disk)
+                stats = conn.block_stats(instance_name, disk)
                 self.LOG.info(self.DISKIO_USAGE_MESSAGE,
                               instance, disk, stats[0], stats[1],
                               stats[2], stats[3], stats[4])
@@ -159,14 +164,14 @@ class CPUPollster(LibVirtPollster):
     utilization_map = {}
 
     def get_cpu_util(self, instance, cpu_info):
-        prev_times = self.utilization_map.get(instance.uuid)
+        prev_times = self.utilization_map.get(instance.id)
-        self.utilization_map[instance.uuid] = (cpu_info['cpu_time'],
+        self.utilization_map[instance.id] = (cpu_info['cpu_time'],
                                              datetime.datetime.now())
         cpu_util = 0.0
         if prev_times:
             prev_cpu = prev_times[0]
             prev_timestamp = prev_times[1]
-            delta = self.utilization_map[instance.uuid][1] - prev_timestamp
+            delta = self.utilization_map[instance.id][1] - prev_timestamp
             elapsed = (delta.seconds * (10 ** 6) + delta.microseconds) * 1000
             cores_fraction = instance.vcpus * 1.0 / multiprocessing.cpu_count()
             # account for cpu_time being reset when the instance is restarted
@@ -178,9 +183,9 @@ class CPUPollster(LibVirtPollster):
 
     def get_counters(self, manager, instance):
         conn = get_libvirt_connection()
-        self.LOG.info('checking instance %s', instance.uuid)
+        self.LOG.info('checking instance %s', instance.id)
         try:
-            cpu_info = conn.get_info(instance)
+            cpu_info = conn.get_info({'name': _instance_name(instance)})
             self.LOG.info("CPUTIME USAGE: %s %d",
                           dict(instance), cpu_info['cpu_time'])
             cpu_util = self.get_cpu_util(instance, cpu_info)
@@ -203,7 +208,7 @@ class CPUPollster(LibVirtPollster):
                                              )
         except Exception as err:
             self.LOG.error('could not get CPU time for %s: %s',
-                           instance.uuid, err)
+                           instance.id, err)
             self.LOG.exception(err)
 
 
@@ -216,7 +221,7 @@ class NetPollster(LibVirtPollster):
 
     def _get_vnics(self, conn, instance):
         """Get disks of an instance, only used to bypass bug#998089."""
-        domain = conn._conn.lookupByName(instance.name)
+        domain = conn._conn.lookupByName(_instance_name(instance))
         tree = etree.fromstring(domain.XMLDesc(0))
         vnics = []
         for interface in tree.findall('devices/interface'):
@@ -232,14 +237,14 @@ class NetPollster(LibVirtPollster):
     @staticmethod
     def make_vnic_counter(instance, name, type, volume, vnic_data):
         resource_metadata = copy.copy(vnic_data)
-        resource_metadata['instance_id'] = instance.uuid
+        resource_metadata['instance_id'] = instance.id
 
         return counter.Counter(
             name=name,
             type=type,
             volume=volume,
             user_id=instance.user_id,
-            project_id=instance.project_id,
+            project_id=instance.tenant_id,
             resource_id=vnic_data['fref'],
             timestamp=timeutils.isotime(),
             resource_metadata=resource_metadata
@@ -247,20 +252,21 @@ class NetPollster(LibVirtPollster):
 
     def get_counters(self, manager, instance):
         conn = get_libvirt_connection()
-        self.LOG.info('checking instance %s', instance.uuid)
+        instance_name = _instance_name(instance)
+        self.LOG.info('checking instance %s', instance.id)
         try:
             vnics = self._get_vnics(conn, instance)
         except Exception as err:
             self.LOG.warning('Ignoring instance %s: %s',
-                             instance.name, err)
+                             instance_name, err)
             self.LOG.exception(err)
         else:
-            domain = conn._conn.lookupByName(instance.name)
+            domain = conn._conn.lookupByName(instance_name)
             for vnic in vnics:
                 rx_bytes, rx_packets, _, _, \
                     tx_bytes, tx_packets, _, _ = \
                     domain.interfaceStats(vnic['name'])
-                self.LOG.info(self.NET_USAGE_MESSAGE, instance.name,
+                self.LOG.info(self.NET_USAGE_MESSAGE, instance_name,
                               vnic['name'], rx_bytes, tx_bytes)
                 yield self.make_vnic_counter(instance,
                                              name='network.incoming.bytes',

ceilometer/compute/manager.py

@@ -17,9 +17,10 @@
 # under the License.
 
 from ceilometer import extension_manager
+
+from ceilometer import nova_client
 from ceilometer.openstack.common import cfg
 from ceilometer.openstack.common import log
-from ceilometer.compute import resources
 from ceilometer import publish
 
 OPTS = [
@@ -40,8 +41,6 @@ PLUGIN_NAMESPACE = 'ceilometer.poll.compute'
 class AgentManager(object):
 
     def __init__(self):
-        self.resources = resources.Resources()
-
         self.ext_manager = extension_manager.ActivatedExtensionManager(
             namespace=PLUGIN_NAMESPACE,
             disabled_names=cfg.CONF.disabled_compute_pollsters,
@@ -76,6 +75,7 @@ class AgentManager(object):
 
     def periodic_tasks(self, context, raise_on_error=False):
         """Tasks to be run at a periodic interval."""
-        for instance in self.resources.instance_get_all_by_host(context):
+        nv = nova_client.Client()
-            if instance['vm_state'] != 'error':
+        for instance in nv.instance_get_all_by_host(cfg.CONF.host):
+            if getattr(instance, 'OS-EXT-STS:vm_state', None) != 'error':
                 self.poll_instance(context, instance)

ceilometer/compute/resources.py

@@ -1,37 +0,0 @@
-#
-# 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.
-
-from ceilometer.openstack.common import cfg
-from nova.openstack.common import importutils
-
-
-db_driver_opt = cfg.StrOpt('db_driver',
-                           default='nova.db',
-                           help='driver to use for database access')
-
-
-cfg.CONF.register_opt(db_driver_opt)
-
-
-# FIXME(dhellmann): How do we get a list of instances without
-# talking directly to the database?
-class Resources(object):
-
-    def __init__(self):
-        self.db = importutils.import_module(cfg.CONF.db_driver)
-
-    def instance_get_all_by_host(self, context):
-        return self.db.instance_get_all_by_host(context, cfg.CONF.host)
-
-    def floating_ip_get_all(self, context):
-        return self.db.floating_ip_get_all(context)

ceilometer/network/floatingip.py

@@ -22,6 +22,7 @@ from ceilometer.openstack.common import log
 from ceilometer.openstack.common import timeutils
 
 from ceilometer import counter
+from ceilometer import nova_client
 from ceilometer.central import plugin
 
 
@@ -30,14 +31,8 @@ class FloatingIPPollster(plugin.CentralPollster):
     LOG = log.getLogger(__name__ + '.floatingip')
 
     def get_counters(self, manager, context):
-        try:
+        nv = nova_client.Client()
-            ips = manager.resources.floating_ip_get_all(context)
+        for ip in nv.floating_ip_get_all():
-        except exception.NoFloatingIpsDefined:
-            pass
-        except exception.FloatingIpNotFoundForHost:
-            pass
-        else:
-            for ip in ips:
             self.LOG.info("FLOATING IP USAGE: %s" % ip.address)
             yield counter.Counter(
                 name='ip.floating',

ceilometer/nova_client.py

@@ -0,0 +1,62 @@
+# -*- encoding: utf-8 -*-
+#
+# Author: John Tran <jhtran@att.com>
+#
+# 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.
+
+import os
+from functools import wraps
+from novaclient.v1_1 import client as nova_client
+
+import ceilometer.service
+from ceilometer.openstack.common import cfg, log
+
+LOG = log.getLogger(__name__)
+
+
+def logged(func):
+
+    @wraps(func)
+    def with_logging(*args, **kwargs):
+        try:
+            return func(*args, **kwargs)
+        except Exception, e:
+            LOG.exception(e)
+            raise
+
+    return with_logging
+
+
+class Client(object):
+
+    def __init__(self):
+        """Returns nova client"""
+        conf = cfg.CONF
+        tenant = conf.os_tenant_id and conf.os_tenant_id or conf.os_tenant_name
+        self.nova_client = nova_client.Client(username=cfg.CONF.os_username,
+                                              api_key=cfg.CONF.os_password,
+                                              project_id=tenant,
+                                              auth_url=cfg.CONF.os_auth_url,
+                                              no_cache=True)
+
+    @logged
+    def instance_get_all_by_host(self, hostname):
+        """Returns list of instances on particular host"""
+        search_opts = {'host': hostname, 'all_tenants': True}
+        return self.nova_client.servers.list(detailed=True,
+                                             search_opts=search_opts)
+
+    @logged
+    def floating_ip_get_all(self):
+        """Returns all floating ips"""
+        return self.nova_client.floating_ips.list()

tests/compute/test_instance.py

@@ -44,7 +44,9 @@ class FauxInstance(object):
 
 class TestLocationMetadata(unittest.TestCase):
 
-    INSTANCE_PROPERTIES = {'display_name': 'display name',
+    # Mimics an instance returned from nova api call
+    INSTANCE_PROPERTIES = {'name': 'display name',
+                           'OS-EXT-SRV-ATTR:instance_name': 'instance-000001',
                            'reservation_id': 'reservation id',
                            'architecture': 'x86_64',
                            'availability_zone': 'zone1',
@@ -58,6 +60,8 @@ class TestLocationMetadata(unittest.TestCase):
                            'memory_mb': 2048,
                            'root_gb': 3,
                            'vcpus': 1,
+                           'flavor': {'id': 1},
+                           'hostId': '1234-5678'
                            }
 
     def setUp(self):
@@ -70,9 +74,18 @@ class TestLocationMetadata(unittest.TestCase):
         self.instance.instance_type = m
 
     def test_metadata(self):
-        md = instance.get_metadata_from_dbobject(self.instance)
+        md = instance.get_metadata_from_object(self.instance)
-        for name in self.INSTANCE_PROPERTIES.keys():
+        iprops = self.INSTANCE_PROPERTIES
+        for name in md.keys():
             actual = md[name]
             print 'checking', name, actual
-            expected = self.INSTANCE_PROPERTIES[name]
+            if name == 'name':
-            assert actual == expected
+                assert actual == iprops['OS-EXT-SRV-ATTR:instance_name']
+            elif name == 'host':
+                assert actual == iprops['hostId']
+            elif name == 'display_name':
+                assert actual == iprops['name']
+            elif name == 'instance_type':
+                assert actual == iprops['flavor']['id']
+            else:
+                assert actual == iprops[name]

tests/compute/test_libvirt.py

@@ -56,6 +56,8 @@ class TestLibvirtBase(test_base.TestCase):
         self.manager = manager.AgentManager()
         self.instance = mock.MagicMock()
         self.instance.name = 'instance-00000001'
+        setattr(self.instance, 'OS-EXT-SRV-ATTR:instance_name',
+                self.instance.name)
         self.instance.id = 1
         self.instance.instance_type = mock.MagicMock()
         self.instance.instance_type.name = 'm1.small'
@@ -217,10 +219,13 @@ class TestCPUPollster(TestLibvirtBase):
         self.instance.vcpus = 1
         conn = fake_libvirt_conn(self.mox, 3)
         self.mox.StubOutWithMock(conn, 'get_info')
-        conn.get_info(self.instance).AndReturn({'cpu_time': 1 * (10 ** 6)})
+        conn.get_info({'name': self.instance.name}).AndReturn(
-        conn.get_info(self.instance).AndReturn({'cpu_time': 3 * (10 ** 6)})
+            {'cpu_time': 1 * (10 ** 6)})
+        conn.get_info({'name': self.instance.name}).AndReturn(
+            {'cpu_time': 3 * (10 ** 6)})
         # cpu_time resets on instance restart
-        conn.get_info(self.instance).AndReturn({'cpu_time': 2 * (10 ** 6)})
+        conn.get_info({'name': self.instance.name}).AndReturn(
+            {'cpu_time': 2 * (10 ** 6)})
         self.mox.ReplayAll()
 
         def _verify_cpu_metering(zero, expected_time):

tests/compute/test_manager.py

@@ -23,6 +23,7 @@ import mock
 
 from stevedore import extension
 
+from ceilometer import nova_client
 from ceilometer.compute import manager
 from ceilometer import counter
 from ceilometer import publish
@@ -77,24 +78,18 @@ class TestRunTasks(base.TestCase):
         # Set up a fake instance value to be returned by
         # instance_get_all_by_host() so when the manager gets the list
         # of instances to poll we can control the results.
-        self.instance = mock.MagicMock()
+        self.instance = {'name': 'faux',
-        self.instance.name = 'faux'
+                         'OS-EXT-STS:vm_state': 'active'}
-        self.instance.vm_state = 'active'
+        stillborn_instance = {'name': 'stillborn',
-        stillborn_instance = mock.MagicMock()
+                              'OS-EXT-STS:vm_state': 'error'}
-        stillborn_instance.name = 'stillborn'
+        self.stubs.Set(nova_client.Client, 'instance_get_all_by_host',
-        stillborn_instance.vm_state = 'error'
+                       lambda *x: [self.instance, stillborn_instance])
-        self.mox.StubOutWithMock(self.mgr.resources,
-                                 'instance_get_all_by_host')
-        self.mgr.resources.instance_get_all_by_host(
-            None
-            ).AndReturn([self.instance, stillborn_instance])
-
         self.mox.ReplayAll()
         # Invoke the periodic tasks to call the pollsters.
         self.mgr.periodic_tasks(None)
 
     def test_message(self):
-        assert len(self.Pollster.counters) == 2
+        self.assertEqual(len(self.Pollster.counters), 2)
         assert self.Pollster.counters[0][1] is self.instance
 
     def test_notifications(self):

tests/network/test_floatingip.py

@@ -19,8 +19,7 @@
 
 import mock
 
-from nova import db
+from ceilometer import nova_client
-
 from ceilometer.network import floatingip
 from ceilometer.central import manager
 from ceilometer.openstack.common import context
@@ -34,9 +33,10 @@ class TestFloatingIPPollster(base.TestCase):
         self.context = context.get_admin_context()
         self.manager = manager.AgentManager()
         self.pollster = floatingip.FloatingIPPollster()
-        self.stubs.Set(db, 'floating_ip_get_all', self.faux_get_ips)
+        self.stubs.Set(nova_client.Client, 'floating_ip_get_all',
+                       self.faux_get_ips)
 
-    def faux_get_ips(self, context):
+    def faux_get_ips(self):
         ips = []
         for i in range(1, 4):
             ip = mock.MagicMock()

tools/pip-requires

@@ -12,3 +12,4 @@ Flask==0.9
 stevedore>=0.6
 python-glanceclient
 python-cinderclient
+python-novaclient