Add support of refereshing the resource info in local cache

The local resource caching mechanism may cause legacy resource info
resided in the local cache and never be cleaned, uless the service
being restarted. e.g. an instance has been migrated to another host, the
instance info will reside in the cache on the ceilometer-polling
service running on the old compute host.

Change-Id: I55f12ea9f6f6104de6a38b98ad935d458a7b0cc8
Closes-Bug: #1594285
Closes-Bug: #1656166

ceilometer/compute/discovery.py

@@ -34,7 +34,18 @@ OPTS = [
                     "the instance list to poll will be updated based "
                     "on this option's interval. Measurements relating "
                     "to the instances will match intervals "
-                    "defined in pipeline.")
+                    "defined in pipeline."),
+    cfg.IntOpt('resource_cache_expiry',
+               default=3600,
+               min=0,
+               help="The expiry to totally refresh the instances resource "
+                    "cache, since the instance may be migrated to another "
+                    "host, we need to clean the legacy instances info in "
+                    "local cache by totally refreshing the local cache. "
+                    "The minimum should be the value of the config option "
+                    "of resource_update_interval. This option is only used "
+                    "for agent polling to Nova API, so it will works only "
+                    "when 'instance_discovery_method' was set to 'naive'.")
 ]
 cfg.CONF.register_opts(OPTS, group='compute')
 
@@ -46,21 +57,32 @@ class InstanceDiscovery(plugin_base.DiscoveryBase):
         self.last_run = None
         self.instances = {}
         self.expiration_time = cfg.CONF.compute.resource_update_interval
+        self.cache_expiry = cfg.CONF.compute.resource_cache_expiry
+        self.last_cache_expire = None
 
     def discover(self, manager, param=None):
         """Discover resources to monitor."""
         secs_from_last_update = 0
+        utc_now = timeutils.utcnow(True)
+        secs_from_last_expire = 0
         if self.last_run:
             secs_from_last_update = timeutils.delta_seconds(
-                self.last_run, timeutils.utcnow(True))
+                self.last_run, utc_now)
+        if self.last_cache_expire:
+            secs_from_last_expire = timeutils.delta_seconds(
+                self.last_cache_expire, utc_now)
 
         instances = []
         # NOTE(ityaptin) we update make a nova request only if
         # it's a first discovery or resources expired
         if not self.last_run or secs_from_last_update >= self.expiration_time:
             try:
-                utc_now = timeutils.utcnow(True)
-                since = self.last_run.isoformat() if self.last_run else None
+                if secs_from_last_expire < self.cache_expiry and self.last_run:
+                    since = self.last_run.isoformat()
+                else:
+                    since = None
+                    self.instances.clear()
+                    self.last_cache_expire = utc_now
                 instances = self.nova_cli.instance_get_all_by_host(
                     cfg.CONF.host, since)
                 self.last_run = utc_now

ceilometer/tests/unit/compute/test_discovery.py

@@ -18,6 +18,7 @@ from oslo_config import fixture as fixture_config
 from oslotest import mockpatch
 
 from ceilometer.compute import discovery
+from ceilometer import service
 import ceilometer.tests.base as base
 
 
@@ -59,6 +60,7 @@ class TestDiscovery(base.BaseTestCase):
         self.useFixture(patch_timeutils)
 
         self.CONF = self.useFixture(fixture_config.Config()).conf
+        service.prepare_service([], [])
         self.CONF.set_override('host', 'test')
 
     def test_normal_discovery(self):
@@ -97,3 +99,28 @@ class TestDiscovery(base.BaseTestCase):
         self.assertEqual(1, list(resources)[0].id)
         self.client.instance_get_all_by_host.assert_called_once_with(
             self.CONF.host, "2016-01-01T00:00:00+00:00")
+
+    def test_discovery_with_legacy_resource_cache_cleanup(self):
+        self.CONF.set_override("resource_update_interval", 600,
+                               group="compute")
+        self.CONF.set_override("resource_cache_expiry", 1800,
+                               group="compute")
+        dsc = discovery.InstanceDiscovery()
+        resources = dsc.discover(mock.MagicMock())
+        self.assertEqual(1, len(resources))
+
+        self.utc_now.return_value = datetime.datetime(
+            2016, 1, 1, minute=20, tzinfo=iso8601.iso8601.UTC)
+        resources = dsc.discover(mock.MagicMock())
+        self.assertEqual(1, len(resources))
+
+        self.utc_now.return_value = datetime.datetime(
+            2016, 1, 1, minute=31, tzinfo=iso8601.iso8601.UTC)
+        resources = dsc.discover(mock.MagicMock())
+        self.assertEqual(1, len(resources))
+
+        expected_calls = [mock.call('test', None),
+                          mock.call('test', '2016-01-01T00:00:00+00:00'),
+                          mock.call('test', None)]
+        self.assertEqual(expected_calls,
+                         self.client.instance_get_all_by_host.call_args_list)

releasenotes/notes/refresh-legacy-cache-e4dbbd3e2eeca70b.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    A local cache is used when polling instance metrics to minimise calls Nova
+    API. A new option is added `resource_cache_expiry` to configure a time to
+    live for cache before it expires. This resolves issue where migrated
+    instances are not removed from cache.