Rely on VM UUID to fetch metrics in libvirt

VM instance name is inconsistent between nova parent node and nova cell node.
So it is necessary to rely on VM UUID (which is an unique ID and immutable for VM
resource) rather than instance name to fetch system metrics via libvirt’s
lookupByUUIDString API.

Change-Id: I59dad915185fb191b7712024697e7b15e9e759e4
Closes-bug: #1396473

ceilometer/compute/pollsters/cpu.py

@@ -32,9 +32,8 @@ class CPUPollster(plugin.ComputePollster):
     def get_samples(self, manager, cache, resources):
         for instance in resources:
             LOG.debug(_('checking instance %s'), instance.id)
-            instance_name = util.instance_name(instance)
             try:
-                cpu_info = manager.inspector.inspect_cpus(instance_name)
+                cpu_info = manager.inspector.inspect_cpus(instance)
                 LOG.debug(_("CPUTIME USAGE: %(instance)s %(time)d"),
                           {'instance': instance.__dict__,
                            'time': cpu_info.time})

ceilometer/compute/pollsters/disk.py

@@ -61,9 +61,9 @@ class _Base(plugin.ComputePollster):
 
     CACHE_KEY_DISK = 'diskio'
 
-    def _populate_cache(self, inspector, cache, instance, instance_name):
+    def _populate_cache(self, inspector, cache, instance):
         i_cache = cache.setdefault(self.CACHE_KEY_DISK, {})
-        if instance_name not in i_cache:
+        if instance.id not in i_cache:
             r_bytes = 0
             r_requests = 0
             w_bytes = 0
@@ -72,7 +72,7 @@ class _Base(plugin.ComputePollster):
             per_device_read_requests = {}
             per_device_write_bytes = {}
             per_device_write_requests = {}
-            for disk, info in inspector.inspect_disks(instance_name):
+            for disk, info in inspector.inspect_disks(instance):
                 LOG.debug(self.DISKIO_USAGE_MESSAGE,
                           instance, disk.device, info.read_requests,
                           info.read_bytes, info.write_requests,
@@ -92,14 +92,14 @@ class _Base(plugin.ComputePollster):
                 'write_bytes': per_device_write_bytes,
                 'write_requests': per_device_write_requests,
             }
-            i_cache[instance_name] = DiskIOData(
+            i_cache[instance.id] = DiskIOData(
                 r_bytes=r_bytes,
                 r_requests=r_requests,
                 w_bytes=w_bytes,
                 w_requests=w_requests,
                 per_disk_requests=per_device_requests,
             )
-        return i_cache[instance_name]
+        return i_cache[instance.id]
 
     @abc.abstractmethod
     def _get_samples(instance, c_data):
@@ -113,7 +113,6 @@ class _Base(plugin.ComputePollster):
                     manager.inspector,
                     cache,
                     instance,
-                    instance_name,
                 )
                 for s in self._get_samples(instance, c_data):
                     yield s

ceilometer/compute/pollsters/net.py

@@ -70,8 +70,7 @@ class _Base(plugin.ComputePollster):
     CACHE_KEY_VNIC = 'vnics'
 
     def _get_vnic_info(self, inspector, instance):
-        instance_name = util.instance_name(instance)
+        return inspector.inspect_vnics(instance)
-        return inspector.inspect_vnics(instance_name)
 
     @staticmethod
     def _get_rx_info(info):
@@ -82,13 +81,12 @@ class _Base(plugin.ComputePollster):
         return info.tx_bytes
 
     def _get_vnics_for_instance(self, cache, inspector, instance):
-        instance_name = util.instance_name(instance)
         i_cache = cache.setdefault(self.CACHE_KEY_VNIC, {})
-        if instance_name not in i_cache:
+        if instance.id not in i_cache:
-            i_cache[instance_name] = list(
+            i_cache[instance.id] = list(
                 self._get_vnic_info(inspector, instance)
             )
-        return i_cache[instance_name]
+        return i_cache[instance.id]
 
     def get_samples(self, manager, cache, resources):
         self._inspection_duration = self._record_poll_time()

ceilometer/compute/virt/hyperv/inspector.py

@@ -19,10 +19,12 @@
 from oslo.config import cfg
 from oslo.utils import units
 
+from ceilometer.compute.pollsters import util
 from ceilometer.compute.virt.hyperv import utilsv2
 from ceilometer.compute.virt import inspector as virt_inspector
 from ceilometer.openstack.common import log
 
+
 CONF = cfg.CONF
 LOG = log.getLogger(__name__)
 
@@ -39,7 +41,8 @@ class HyperVInspector(virt_inspector.Inspector):
                 name=element_name,
                 UUID=name)
 
-    def inspect_cpus(self, instance_name):
+    def inspect_cpus(self, instance):
+        instance_name = util.instance_name(instance)
         (cpu_clock_used,
          cpu_count, uptime) = self._utils.get_cpu_metrics(instance_name)
         host_cpu_clock, host_cpu_count = self._utils.get_host_cpu_info()
@@ -51,7 +54,8 @@ class HyperVInspector(virt_inspector.Inspector):
 
         return virt_inspector.CPUStats(number=cpu_count, time=cpu_time)
 
-    def inspect_vnics(self, instance_name):
+    def inspect_vnics(self, instance):
+        instance_name = util.instance_name(instance)
         for vnic_metrics in self._utils.get_vnic_metrics(instance_name):
             interface = virt_inspector.Interface(
                 name=vnic_metrics["element_name"],
@@ -67,7 +71,8 @@ class HyperVInspector(virt_inspector.Inspector):
 
             yield (interface, stats)
 
-    def inspect_disks(self, instance_name):
+    def inspect_disks(self, instance):
+        instance_name = util.instance_name(instance)
         for disk_metrics in self._utils.get_disk_metrics(instance_name):
             disk = virt_inspector.Disk(device=disk_metrics['instance_id'])
             stats = virt_inspector.DiskStats(

ceilometer/compute/virt/inspector.py

@@ -151,10 +151,10 @@ class Inspector(object):
         """List the instances on the current host."""
         raise ceilometer.NotImplementedError
 
-    def inspect_cpus(self, instance_name):
+    def inspect_cpus(self, instance):
         """Inspect the CPU statistics for an instance.
 
-        :param instance_name: the name of the target instance
+        :param instance: the target instance
         :return: the number of CPUs and cumulative CPU time
         """
         raise ceilometer.NotImplementedError
@@ -169,10 +169,10 @@ class Inspector(object):
         """
         raise ceilometer.NotImplementedError
 
-    def inspect_vnics(self, instance_name):
+    def inspect_vnics(self, instance):
         """Inspect the vNIC statistics for an instance.
 
-        :param instance_name: the name of the target instance
+        :param instance: the target instance
         :return: for each vNIC, the number of bytes & packets
                  received and transmitted
         """
@@ -189,10 +189,10 @@ class Inspector(object):
         """
         raise ceilometer.NotImplementedError
 
-    def inspect_disks(self, instance_name):
+    def inspect_disks(self, instance):
         """Inspect the disk statistics for an instance.
 
-        :param instance_name: the name of the target instance
+        :param instance: the target instance
         :return: for each disk, the number of bytes & operations
                  read and written, and the error count
         """

ceilometer/compute/virt/libvirt/inspector.py

@@ -84,9 +84,10 @@ class LibvirtInspector(virt_inspector.Inspector):
         return self.connection
 
     @retry_on_disconnect
-    def _lookup_by_name(self, instance_name):
+    def _lookup_by_uuid(self, instance):
+        instance_name = util.instance_name(instance)
         try:
-            return self._get_connection().lookupByName(instance_name)
+            return self._get_connection().lookupByUUIDString(instance.id)
         except Exception as ex:
             if not libvirt or not isinstance(ex, libvirt.libvirtError):
                 raise virt_inspector.InspectorException(six.text_type(ex))
@@ -95,9 +96,11 @@ class LibvirtInspector(virt_inspector.Inspector):
                 ex.get_error_domain() in (libvirt.VIR_FROM_REMOTE,
                                           libvirt.VIR_FROM_RPC)):
                 raise
-            msg = ("Error from libvirt while looking up %(instance_name)s: "
+            msg = ("Error from libvirt while looking up instance Name "
+                   "%(instance_name)s UUID %(instance_uuid)s: "
                    "[Error Code %(error_code)s] "
                    "%(ex)s" % {'instance_name': instance_name,
+                               'instance_uuid': instance.id,
                                'error_code': error_code,
                                'ex': ex})
             raise virt_inspector.InstanceNotFoundException(msg)
@@ -119,18 +122,21 @@ class LibvirtInspector(virt_inspector.Inspector):
                         # Instance was deleted while listing... ignore it
                         pass
 
-    def inspect_cpus(self, instance_name):
+    def inspect_cpus(self, instance):
-        domain = self._lookup_by_name(instance_name)
+        domain = self._lookup_by_uuid(instance)
         dom_info = domain.info()
         return virt_inspector.CPUStats(number=dom_info[3], time=dom_info[4])
 
-    def inspect_vnics(self, instance_name):
+    def inspect_vnics(self, instance):
-        domain = self._lookup_by_name(instance_name)
+        instance_name = util.instance_name(instance)
+        domain = self._lookup_by_uuid(instance)
         state = domain.info()[0]
         if state == libvirt.VIR_DOMAIN_SHUTOFF:
-            LOG.warn(_('Failed to inspect vnics of %(instance_name)s, '
+            LOG.warn(_('Failed to inspect vnics of instance Name '
+                       '%(instance_name)s UUID %(instance_uuid)s, '
                        'domain is in state of SHUTOFF'),
-                     {'instance_name': instance_name})
+                     {'instance_name': instance_name,
+                      'instance_uuid': instance.id})
             return
         tree = etree.fromstring(domain.XMLDesc(0))
         for iface in tree.findall('devices/interface'):
@@ -159,13 +165,16 @@ class LibvirtInspector(virt_inspector.Inspector):
                                                   tx_packets=dom_stats[5])
             yield (interface, stats)
 
-    def inspect_disks(self, instance_name):
+    def inspect_disks(self, instance):
-        domain = self._lookup_by_name(instance_name)
+        instance_name = util.instance_name(instance)
+        domain = self._lookup_by_uuid(instance)
         state = domain.info()[0]
         if state == libvirt.VIR_DOMAIN_SHUTOFF:
-            LOG.warn(_('Failed to inspect disks of %(instance_name)s, '
+            LOG.warn(_('Failed to inspect disks of instance Name '
+                       '%(instance_name)s UUID %(instance_uuid)s, '
                        'domain is in state of SHUTOFF'),
-                     {'instance_name': instance_name})
+                     {'instance_name': instance_name,
+                      'instance_uuid': instance.id})
             return
         tree = etree.fromstring(domain.XMLDesc(0))
         for device in filter(
@@ -183,12 +192,14 @@ class LibvirtInspector(virt_inspector.Inspector):
 
     def inspect_memory_usage(self, instance, duration=None):
         instance_name = util.instance_name(instance)
-        domain = self._lookup_by_name(instance_name)
+        domain = self._lookup_by_uuid(instance)
         state = domain.info()[0]
         if state == libvirt.VIR_DOMAIN_SHUTOFF:
-            LOG.warn(_('Failed to inspect memory usage of %(instance_name)s, '
+            LOG.warn(_('Failed to inspect memory usage of instance Name '
+                       '%(instance_name)s UUID %(instance_uuid)s, '
                        'domain is in state of SHUTOFF'),
-                     {'instance_name': instance_name})
+                     {'instance_name': instance_name,
+                      'instance_uuid': instance.id})
             return
 
         try:
@@ -202,13 +213,15 @@ class LibvirtInspector(virt_inspector.Inspector):
                 memory_used = memory_used / units.Ki
                 return virt_inspector.MemoryUsageStats(usage=memory_used)
             else:
-                LOG.warn(_('Failed to inspect memory usage of '
+                LOG.warn(_('Failed to inspect memory usage of instance Name '
-                           '%(instance_name)s, can not get info from libvirt'),
+                           '%(instance_name)s UUID %(instance_uuid)s, '
-                         {'instance_name': instance_name})
+                           'can not get info from libvirt'),
+                         {'instance_name': instance_name,
+                          'instance_uuid': instance.id})
         # memoryStats might launch an exception if the method
         # is not supported by the underlying hypervisor being
         # used by libvirt
         except libvirt.libvirtError as e:
-            LOG.warn(_('Failed to inspect memory usage of %(instance_name)s, '
+            LOG.warn(_('Failed to inspect memory usage of %(instance_uuid)s, '
                        'can not get info from libvirt: %(error)s'),
-                     {'instance_name': instance_name, 'error': e})
+                     {'instance_uuid': instance.id, 'error': e})

ceilometer/tests/compute/pollsters/test_diskio.py

@@ -116,7 +116,7 @@ class TestDiskPollsters(TestBaseDiskIO):
         self.assertIsNotEmpty(samples)
         self.assertIn(pollster.CACHE_KEY_DISK, cache)
         for instance in self.instance:
-            self.assertIn(instance.name, cache[pollster.CACHE_KEY_DISK])
+            self.assertIn(instance.id, cache[pollster.CACHE_KEY_DISK])
         self.assertEqual(set([name]), set([s.name for s in samples]))
 
         match = [s for s in samples if s.name == name]

ceilometer/tests/compute/pollsters/test_net.py

@@ -215,7 +215,7 @@ class TestNetPollsterCache(base.TestPollsterBase):
         pollster = factory()
         cache = {
             pollster.CACHE_KEY_VNIC: {
-                self.instance.name: vnics,
+                self.instance.id: vnics,
             },
         }
         samples = list(pollster.get_samples(mgr, cache, [self.instance]))

ceilometer/tests/compute/virt/libvirt/test_inspector.py

@@ -33,7 +33,11 @@ class TestLibvirtInspection(base.BaseTestCase):
 
     def setUp(self):
         super(TestLibvirtInspection, self).setUp()
-        self.instance_name = 'instance-00000001'
+
+        class VMInstance:
+            id = 'ff58e738-12f4-4c58-acde-77617b68da56'
+            name = 'instance-00000001'
+        self.instance = VMInstance
         self.inspector = libvirt_inspector.LibvirtInspector()
         self.inspector.connection = mock.Mock()
         libvirt_inspector.libvirt = mock.Mock()
@@ -65,12 +69,12 @@ class TestLibvirtInspection(base.BaseTestCase):
 
     def test_inspect_cpus(self):
         with contextlib.nested(mock.patch.object(self.inspector.connection,
-                                                 'lookupByName',
+                                                 'lookupByUUIDString',
                                                  return_value=self.domain),
                                mock.patch.object(self.domain, 'info',
                                                  return_value=(0L, 0L, 0L,
                                                                2L, 999999L))):
-                cpu_info = self.inspector.inspect_cpus(self.instance_name)
+                cpu_info = self.inspector.inspect_cpus(self.instance)
                 self.assertEqual(2L, cpu_info.number)
                 self.assertEqual(999999L, cpu_info.time)
 
@@ -140,7 +144,8 @@ class TestLibvirtInspection(base.BaseTestCase):
         interfaceStats = interface_stats.__getitem__
 
         connection = self.inspector.connection
-        with contextlib.nested(mock.patch.object(connection, 'lookupByName',
+        with contextlib.nested(mock.patch.object(connection,
+                                                 'lookupByUUIDString',
                                                  return_value=self.domain),
                                mock.patch.object(self.domain, 'XMLDesc',
                                                  return_value=dom_xml),
@@ -150,7 +155,7 @@ class TestLibvirtInspection(base.BaseTestCase):
                                mock.patch.object(self.domain, 'info',
                                                  return_value=(0L, 0L, 0L,
                                                                2L, 999999L))):
-            interfaces = list(self.inspector.inspect_vnics(self.instance_name))
+            interfaces = list(self.inspector.inspect_vnics(self.instance))
 
             self.assertEqual(3, len(interfaces))
             vnic0, info0 = interfaces[0]
@@ -191,12 +196,13 @@ class TestLibvirtInspection(base.BaseTestCase):
 
     def test_inspect_vnics_with_domain_shutoff(self):
         connection = self.inspector.connection
-        with contextlib.nested(mock.patch.object(connection, 'lookupByName',
+        with contextlib.nested(mock.patch.object(connection,
+                                                 'lookupByUUIDString',
                                                  return_value=self.domain),
                                mock.patch.object(self.domain, 'info',
                                                  return_value=(5L, 0L, 0L,
                                                                2L, 999999L))):
-            interfaces = list(self.inspector.inspect_vnics(self.instance_name))
+            interfaces = list(self.inspector.inspect_vnics(self.instance))
             self.assertEqual([], interfaces)
 
     def test_inspect_disks(self):
@@ -216,7 +222,7 @@ class TestLibvirtInspection(base.BaseTestCase):
         """
 
         with contextlib.nested(mock.patch.object(self.inspector.connection,
-                                                 'lookupByName',
+                                                 'lookupByUUIDString',
                                                  return_value=self.domain),
                                mock.patch.object(self.domain, 'XMLDesc',
                                                  return_value=dom_xml),
@@ -226,7 +232,7 @@ class TestLibvirtInspection(base.BaseTestCase):
                                mock.patch.object(self.domain, 'info',
                                                  return_value=(0L, 0L, 0L,
                                                                2L, 999999L))):
-                disks = list(self.inspector.inspect_disks(self.instance_name))
+                disks = list(self.inspector.inspect_disks(self.instance))
 
                 self.assertEqual(1, len(disks))
                 disk0, info0 = disks[0]
@@ -238,18 +244,19 @@ class TestLibvirtInspection(base.BaseTestCase):
 
     def test_inspect_disks_with_domain_shutoff(self):
         connection = self.inspector.connection
-        with contextlib.nested(mock.patch.object(connection, 'lookupByName',
+        with contextlib.nested(mock.patch.object(connection,
+                                                 'lookupByUUIDString',
                                                  return_value=self.domain),
                                mock.patch.object(self.domain, 'info',
                                                  return_value=(5L, 0L, 0L,
                                                                2L, 999999L))):
-            disks = list(self.inspector.inspect_disks(self.instance_name))
+            disks = list(self.inspector.inspect_disks(self.instance))
             self.assertEqual([], disks)
 
     def test_inspect_memory_usage(self):
         fake_memory_stats = {'available': 51200L, 'unused': 25600L}
         connection = self.inspector.connection
-        with mock.patch.object(connection, 'lookupByName',
+        with mock.patch.object(connection, 'lookupByUUIDString',
                                return_value=self.domain):
             with mock.patch.object(self.domain, 'info',
                                    return_value=(0L, 0L, 51200L,
@@ -257,23 +264,23 @@ class TestLibvirtInspection(base.BaseTestCase):
                 with mock.patch.object(self.domain, 'memoryStats',
                                        return_value=fake_memory_stats):
                     memory = self.inspector.inspect_memory_usage(
-                        self.instance_name)
+                        self.instance)
                     self.assertEqual(25600L / units.Ki, memory.usage)
 
     def test_inspect_memory_usage_with_domain_shutoff(self):
         connection = self.inspector.connection
-        with mock.patch.object(connection, 'lookupByName',
+        with mock.patch.object(connection, 'lookupByUUIDString',
                                return_value=self.domain):
             with mock.patch.object(self.domain, 'info',
                                    return_value=(5L, 0L, 0L,
                                                  2L, 999999L)):
                 memory = self.inspector.inspect_memory_usage(
-                    self.instance_name)
+                    self.instance)
                 self.assertIsNone(memory)
 
     def test_inspect_memory_usage_with_empty_stats(self):
         connection = self.inspector.connection
-        with mock.patch.object(connection, 'lookupByName',
+        with mock.patch.object(connection, 'lookupByUUIDString',
                                return_value=self.domain):
             with mock.patch.object(self.domain, 'info',
                                    return_value=(0L, 0L, 51200L,
@@ -281,7 +288,7 @@ class TestLibvirtInspection(base.BaseTestCase):
                 with mock.patch.object(self.domain, 'memoryStats',
                                        return_value={}):
                     memory = self.inspector.inspect_memory_usage(
-                        self.instance_name)
+                        self.instance)
                     self.assertIsNone(memory)