Merge pull request #299 from AlexanderS/fix/caching

Expire metadata cache in all Connector plugins.

11 files changed, 99 insertions, 32 deletions

diff --git a/doc/development/plugins.txt b/doc/development/plugins.txt
index d292c9dd7..0d524973e 100644
--- a/doc/development/plugins.txt
+++ b/doc/development/plugins.txt
@@ -118,7 +118,9 @@ Invalidating Caches
 In Bcfg2 1.3.0, some limited :ref:`server-caching` was introduced.  If
 you are writing a :class:`Bcfg2.Server.Plugin.interfaces.Connector`
 plugin that implements
-:func:`Bcfg2.Server.Plugin.interfaces.Connector.get_additional_groups`,
+:func:`Bcfg2.Server.Plugin.interfaces.Connector.get_additional_groups`
+or
+:func:`Bcfg2.Server.Plugin.interfaces.Connector.get_additional_data`,
 then you need to be able to invalidate the server metadata cache in
 order to be compatible with the ``cautious`` or ``aggressive`` caching
 modes.
@@ -140,9 +142,9 @@ called with one string argument, it expires cached data for the named
 client.
 
 It's important, therefore, that your Connector plugin can either track
-when changes are made to the group membership it reports, and expire
-cached data appropriately when in ``cautious`` or ``aggressive`` mode;
-or prudently flag an incompatibility with those two modes.
+when changes are made to the data or group membership it reports, and
+expire cached data appropriately when in ``cautious`` or ``aggressive``
+mode; or prudently flag an incompatibility with those two modes.
 
 For examples, see:
 
diff --git a/src/lib/Bcfg2/Server/Plugins/AWSTags.py b/src/lib/Bcfg2/Server/Plugins/AWSTags.py
index 0d6eefaaa..556805bde 100644
--- a/src/lib/Bcfg2/Server/Plugins/AWSTags.py
+++ b/src/lib/Bcfg2/Server/Plugins/AWSTags.py
@@ -172,6 +172,11 @@ class AWSTags(Bcfg2.Server.Plugin.Plugin,
 
     def start_client_run(self, metadata):
         self.expire_cache(key=metadata.hostname)
+        if self.core.metadata_cache_mode == 'aggressive':
+            self.logger.warning("AWSTags is incompatible with aggressive "
+                                "client metadata caching, try 'cautious' "
+                                "or 'initial'")
+            self.core.metadata_cache.expire(metadata.hostname)
 
     def get_additional_data(self, metadata):
         return self.get_tags(metadata)
diff --git a/src/lib/Bcfg2/Server/Plugins/GroupLogic.py b/src/lib/Bcfg2/Server/Plugins/GroupLogic.py
index b60f60e65..184b362f9 100644
--- a/src/lib/Bcfg2/Server/Plugins/GroupLogic.py
+++ b/src/lib/Bcfg2/Server/Plugins/GroupLogic.py
@@ -13,6 +13,17 @@ class GroupLogicConfig(Bcfg2.Server.Plugin.StructFile):
     create = lxml.etree.Element("GroupLogic",
                                 nsmap=dict(py="http://genshi.edgewall.org/"))
 
+    def __init__(self, filename, core):
+        Bcfg2.Server.Plugin.StructFile.__init__(self, filename,
+                                                should_monitor=True)
+        self.core = core
+
+    def Index(self):
+        Bcfg2.Server.Plugin.StructFile.Index(self)
+
+        if self.core.metadata_cache_mode in ['cautious', 'aggressive']:
+            self.core.metadata_cache.expire()
+
     def _match(self, item, metadata, *args):
         if item.tag == 'Group' and not len(item.getchildren()):
             return [item]
@@ -39,7 +50,7 @@ class GroupLogic(Bcfg2.Server.Plugin.Plugin,
         Bcfg2.Server.Plugin.Plugin.__init__(self, core)
         Bcfg2.Server.Plugin.Connector.__init__(self)
         self.config = GroupLogicConfig(os.path.join(self.data, "groups.xml"),
-                                       should_monitor=True)
+                                       core=core)
         self._local = local()
 
     def get_additional_groups(self, metadata):
diff --git a/src/lib/Bcfg2/Server/Plugins/Ohai.py b/src/lib/Bcfg2/Server/Plugins/Ohai.py
index 461be9ba8..b314e60a0 100644
--- a/src/lib/Bcfg2/Server/Plugins/Ohai.py
+++ b/src/lib/Bcfg2/Server/Plugins/Ohai.py
@@ -94,7 +94,12 @@ class Ohai(Bcfg2.Server.Plugin.Plugin,
         return [self.probe]
 
     def ReceiveData(self, meta, datalist):
-        self.cache[meta.hostname] = datalist[0].text
+        if meta.hostname not in self.cache or \
+           self.cache[meta.hostname] != datalist[0].text:
+            self.cache[meta.hostname] = datalist[0].text
+
+            if self.core.metadata_cache_mode in ['cautious', 'aggressive']:
+                self.core.metadata_cache.expire(meta.hostname)
 
     def get_additional_data(self, meta):
         if meta.hostname in self.cache:
diff --git a/src/lib/Bcfg2/Server/Plugins/Probes.py b/src/lib/Bcfg2/Server/Plugins/Probes.py
index 76aab69b5..573c9af71 100644
--- a/src/lib/Bcfg2/Server/Plugins/Probes.py
+++ b/src/lib/Bcfg2/Server/Plugins/Probes.py
@@ -74,6 +74,7 @@ class ProbeStore(Debuggable):
 
     def __init__(self, core, datadir):  # pylint: disable=W0613
         Debuggable.__init__(self)
+        self.core = core
         self._groupcache = Bcfg2.Server.Cache.Cache("Probes", "probegroups")
         self._datacache = Bcfg2.Server.Cache.Cache("Probes", "probedata")
 
@@ -134,7 +135,7 @@ class DBProbeStore(ProbeStore, Bcfg2.Server.Plugin.DatabaseBacked):
         Bcfg2.Server.Cache.expire("Probes", "probegroups", hostname)
         groupdata = ProbesGroupsModel.objects.filter(hostname=hostname)
         self._groupcache[hostname] = list(set(r.group for r in groupdata))
-        Bcfg2.Server.Cache.expire("Metadata", hostname)
+        self.core.metadata_cache.expire(hostname)
 
     @Bcfg2.Server.Plugin.DatabaseBacked.get_db_lock
     def set_groups(self, hostname, groups):
@@ -155,7 +156,7 @@ class DBProbeStore(ProbeStore, Bcfg2.Server.Plugin.DatabaseBacked):
         ProbesGroupsModel.objects.filter(
             hostname=hostname).exclude(group__in=groups).delete()
         if olddata != groups:
-            Bcfg2.Server.Cache.expire("Metadata", hostname)
+            self.core.metadata_cache.expire(hostname)
 
     def _load_data(self, hostname):
         Bcfg2.Server.Cache.expire("Probes", "probegroups", hostname)
@@ -168,7 +169,7 @@ class DBProbeStore(ProbeStore, Bcfg2.Server.Plugin.DatabaseBacked):
                     time.mktime(pdata.timestamp.timetuple())
                 ts_set = True
             self._datacache[hostname][pdata.probe] = ProbeData(pdata.data)
-        Bcfg2.Server.Cache.expire("Metadata", hostname)
+        self.core.metadata_cache.expire(hostname)
 
     @Bcfg2.Server.Plugin.DatabaseBacked.get_db_lock
     def set_data(self, hostname, data):
@@ -198,7 +199,7 @@ class DBProbeStore(ProbeStore, Bcfg2.Server.Plugin.DatabaseBacked):
             qset.delete()
             expire_metadata = True
         if expire_metadata:
-            Bcfg2.Server.Cache.expire("Metadata", hostname)
+            self.core.metadata_cache.expire(hostname)
 
 
 class XMLProbeStore(ProbeStore):
@@ -234,7 +235,7 @@ class XMLProbeStore(ProbeStore):
                     self._groupcache[client.get('name')].append(
                         pdata.get('name'))
 
-        Bcfg2.Server.Cache.expire("Metadata")
+        self.core.metadata_cache.expire()
 
     def _load_groups(self, hostname):
         self._load_data(hostname)
@@ -274,7 +275,7 @@ class XMLProbeStore(ProbeStore):
         olddata = self._groupcache.get(hostname, [])
         self._groupcache[hostname] = groups
         if olddata != groups:
-            Bcfg2.Server.Cache.expire("Metadata", hostname)
+            self.core.metadata_cache.expire(hostname)
 
     def set_data(self, hostname, data):
         Bcfg2.Server.Cache.expire("Probes", "probedata", hostname)
@@ -285,7 +286,7 @@ class XMLProbeStore(ProbeStore):
             self._datacache[hostname][probe] = pdata
             expire_metadata |= olddata != data
         if expire_metadata:
-            Bcfg2.Server.Cache.expire("Metadata", hostname)
+            self.core.metadata_cache.expire(hostname)
 
 
 class ClientProbeDataSet(dict):
diff --git a/src/lib/Bcfg2/Server/Plugins/Properties.py b/src/lib/Bcfg2/Server/Plugins/Properties.py
index c4dd75e60..e6549b714 100644
--- a/src/lib/Bcfg2/Server/Plugins/Properties.py
+++ b/src/lib/Bcfg2/Server/Plugins/Properties.py
@@ -35,13 +35,17 @@ LOGGER = logging.getLogger(__name__)
 class PropertyFile(object):
     """ Base Properties file handler """
 
-    def __init__(self, name):
+    def __init__(self, name, core):
         """
         :param name: The filename of this properties file.
+        :type name: string
+        :param core: The Bcfg2.Server.Core initializing the Properties plugin
+        :type core: Bcfg2.Server.Core
 
         .. automethod:: _write
         """
         self.name = name
+        self.core = core
 
     def write(self):
         """ Write the data in this data structure back to the property
@@ -69,6 +73,12 @@ class PropertyFile(object):
         file. """
         raise NotImplementedError
 
+    def _expire_metadata_cache(self):
+        """ Expires the metadata cache, if it is required by the caching
+        mode. """
+        if self.core.metadata_cache_mode in ['cautious', 'aggressive']:
+            self.core.metadata_cache.expire()
+
     def validate_data(self):
         """ Verify that the data in this file is valid. """
         raise NotImplementedError
@@ -81,9 +91,9 @@ class PropertyFile(object):
 class JSONPropertyFile(Bcfg2.Server.Plugin.FileBacked, PropertyFile):
     """Handle JSON Properties files."""
 
-    def __init__(self, name):
+    def __init__(self, name, core):
         Bcfg2.Server.Plugin.FileBacked.__init__(self, name)
-        PropertyFile.__init__(self, name)
+        PropertyFile.__init__(self, name, core)
         self.json = None
 
     def Index(self):
@@ -93,10 +103,13 @@ class JSONPropertyFile(Bcfg2.Server.Plugin.FileBacked, PropertyFile):
             err = sys.exc_info()[1]
             raise PluginExecutionError("Could not load JSON data from %s: %s" %
                                        (self.name, err))
+        self._expire_metadata_cache()
+    Index.__doc__ = Bcfg2.Server.Plugin.FileBacked.Index.__doc__
 
     def _write(self):
         json.dump(self.json, open(self.name, 'wb'))
         return True
+    _write.__doc__ = PropertyFile._write.__doc__
 
     def validate_data(self):
         try:
@@ -105,6 +118,7 @@ class JSONPropertyFile(Bcfg2.Server.Plugin.FileBacked, PropertyFile):
             err = sys.exc_info()[1]
             raise PluginExecutionError("Data for %s cannot be dumped to JSON: "
                                        "%s" % (self.name, err))
+    validate_data.__doc__ = PropertyFile.validate_data.__doc__
 
     def __str__(self):
         return str(self.json)
@@ -116,11 +130,10 @@ class JSONPropertyFile(Bcfg2.Server.Plugin.FileBacked, PropertyFile):
 class YAMLPropertyFile(Bcfg2.Server.Plugin.FileBacked, PropertyFile):
     """ Handle YAML Properties files. """
 
-    def __init__(self, name):
+    def __init__(self, name, core):
         Bcfg2.Server.Plugin.FileBacked.__init__(self, name)
-        PropertyFile.__init__(self, name)
+        PropertyFile.__init__(self, name, core)
         self.yaml = None
-    __init__.__doc__ = Bcfg2.Server.Plugin.FileBacked.__init__.__doc__
 
     def Index(self):
         try:
@@ -129,6 +142,7 @@ class YAMLPropertyFile(Bcfg2.Server.Plugin.FileBacked, PropertyFile):
             err = sys.exc_info()[1]
             raise PluginExecutionError("Could not load YAML data from %s: %s" %
                                        (self.name, err))
+        self._expire_metadata_cache()
     Index.__doc__ = Bcfg2.Server.Plugin.FileBacked.Index.__doc__
 
     def _write(self):
@@ -155,10 +169,15 @@ class YAMLPropertyFile(Bcfg2.Server.Plugin.FileBacked, PropertyFile):
 class XMLPropertyFile(Bcfg2.Server.Plugin.StructFile, PropertyFile):
     """ Handle XML Properties files. """
 
-    def __init__(self, name, should_monitor=False):
+    def __init__(self, name, core, should_monitor=False):
         Bcfg2.Server.Plugin.StructFile.__init__(self, name,
                                                 should_monitor=should_monitor)
-        PropertyFile.__init__(self, name)
+        PropertyFile.__init__(self, name, core)
+
+    def Index(self):
+        Bcfg2.Server.Plugin.StructFile.Index(self)
+        self._expire_metadata_cache()
+    Index.__doc__ = Bcfg2.Server.Plugin.StructFile.Index.__doc__
 
     def _write(self):
         open(self.name, "wb").write(
@@ -258,11 +277,11 @@ class Properties(Bcfg2.Server.Plugin.Plugin,
                   :class:`PropertyFile`
         """
         if fname.endswith(".xml"):
-            return XMLPropertyFile(fname)
+            return XMLPropertyFile(fname, self.core)
         elif HAS_JSON and fname.endswith(".json"):
-            return JSONPropertyFile(fname)
+            return JSONPropertyFile(fname, self.core)
         elif HAS_YAML and (fname.endswith(".yaml") or fname.endswith(".yml")):
-            return YAMLPropertyFile(fname)
+            return YAMLPropertyFile(fname, self.core)
         else:
             raise Bcfg2.Server.Plugin.PluginExecutionError(
                 "Properties: Unknown extension %s" % fname)
diff --git a/src/lib/Bcfg2/Server/Plugins/PuppetENC.py b/src/lib/Bcfg2/Server/Plugins/PuppetENC.py
index 59fbe6f03..e2d8a058f 100644
--- a/src/lib/Bcfg2/Server/Plugins/PuppetENC.py
+++ b/src/lib/Bcfg2/Server/Plugins/PuppetENC.py
@@ -117,7 +117,7 @@ class PuppetENC(Bcfg2.Server.Plugin.Plugin,
             self.logger.warning("PuppetENC is incompatible with aggressive "
                                 "client metadata caching, try 'cautious' or "
                                 "'initial' instead")
-            self.core.expire_caches_by_type(Bcfg2.Server.Plugin.Metadata)
+            self.core.metadata_cache.expire()
 
     def end_statistics(self, metadata):
         self.end_client_run(self, metadata)
diff --git a/src/lib/Bcfg2/Server/Plugins/SSHbase.py b/src/lib/Bcfg2/Server/Plugins/SSHbase.py
index 7736bd050..7f20e72eb 100644
--- a/src/lib/Bcfg2/Server/Plugins/SSHbase.py
+++ b/src/lib/Bcfg2/Server/Plugins/SSHbase.py
@@ -286,6 +286,10 @@ class SSHbase(Bcfg2.Server.Plugin.Plugin,
                     self.debug_log("New public key %s; invalidating "
                                    "ssh_known_hosts cache" % event.filename)
                     self.skn = False
+
+                    if self.core.metadata_cache_mode in ['cautious',
+                                                         'aggressive']:
+                        self.core.metadata_cache.expire()
                 return
 
         if event.filename == 'info.xml':
diff --git a/src/lib/Bcfg2/Server/Plugins/TemplateHelper.py b/src/lib/Bcfg2/Server/Plugins/TemplateHelper.py
index b0b8d0061..ff67571fa 100644
--- a/src/lib/Bcfg2/Server/Plugins/TemplateHelper.py
+++ b/src/lib/Bcfg2/Server/Plugins/TemplateHelper.py
@@ -15,9 +15,10 @@ MODULE_RE = re.compile(r'(?P<filename>(?P<module>[^\/]+)\.py)$')
 class HelperModule(Debuggable):
     """ Representation of a TemplateHelper module """
 
-    def __init__(self, name):
+    def __init__(self, name, core):
         Debuggable.__init__(self)
         self.name = name
+        self.core = core
 
         #: The name of the module as used by get_additional_data().
         #: the name of the file with .py stripped off.
@@ -44,6 +45,10 @@ class HelperModule(Debuggable):
         if event and event.code2str() not in ['exists', 'changed', 'created']:
             return
 
+        # expire the metadata cache, because the module might have changed
+        if self.core.metadata_cache_mode in ['cautious', 'aggressive']:
+            self.core.metadata_cache.expire()
+
         try:
             module = imp.load_source(
                 safe_module_name('TemplateHelper', self._module_name),
@@ -101,7 +106,6 @@ class TemplateHelper(Plugin, Connector, DirectoryBacked, TemplateDataProvider):
     __author__ = 'chris.a.st.pierre@gmail.com'
     ignore = re.compile(r'^(\.#.*|.*~|\..*\.(sw[px])|.*\.py[co])$')
     patterns = MODULE_RE
-    __child__ = HelperModule
 
     def __init__(self, core):
         Plugin.__init__(self, core)
@@ -109,6 +113,10 @@ class TemplateHelper(Plugin, Connector, DirectoryBacked, TemplateDataProvider):
         DirectoryBacked.__init__(self, self.data)
         TemplateDataProvider.__init__(self)
 
+        # The HelperModule needs access to the core, so we have to construct
+        # it manually and add the custom argument.
+        self.__child__ = lambda fname: HelperModule(fname, core)
+
     def get_additional_data(self, _):
         return dict([(h._module_name, h)  # pylint: disable=W0212
                      for h in self.entries.values()])
diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestProperties.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestProperties.py
index 36baee899..8c7f3c5d7 100644
--- a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestProperties.py
+++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestProperties.py
@@ -29,11 +29,14 @@ class TestPropertyFile(Bcfg2TestCase):
     test_obj = PropertyFile
     path = os.path.join(datastore, "test")
 
-    def get_obj(self, path=None):
+    def get_obj(self, path=None, core=None, *args, **kwargs):
         set_setup_default("writes_enabled", False)
         if path is None:
             path = self.path
-        return self.test_obj(path)
+        if core is None:
+            core = Mock()
+            core.metadata_cache_mode = 'none'
+        return self.test_obj(path, core, *args, **kwargs)
 
     def test_write(self):
         pf = self.get_obj()
@@ -97,6 +100,9 @@ class TestJSONPropertyFile(TestFileBacked, TestPropertyFile):
         TestFileBacked.setUp(self)
         TestPropertyFile.setUp(self)
 
+    def get_obj(self, *args, **kwargs):
+        return TestPropertyFile.get_obj(self, *args, **kwargs)
+
     @patch("%s.loads" % JSON)
     def test_Index(self, mock_loads):
         pf = self.get_obj()
@@ -137,6 +143,9 @@ class TestYAMLPropertyFile(TestFileBacked, TestPropertyFile):
         TestFileBacked.setUp(self)
         TestPropertyFile.setUp(self)
 
+    def get_obj(self, *args, **kwargs):
+        return TestPropertyFile.get_obj(self, *args, **kwargs)
+
     @patch("yaml.load")
     def test_Index(self, mock_load):
         pf = self.get_obj()
@@ -179,7 +188,7 @@ class TestXMLPropertyFile(TestPropertyFile, TestStructFile):
         set_setup_default("automatch", False)
 
     def get_obj(self, *args, **kwargs):
-        return TestStructFile.get_obj(self, *args, **kwargs)
+        return TestPropertyFile.get_obj(self, *args, **kwargs)
 
     @patch("%s.open" % builtins)
     def test__write(self, mock_open):
diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestTemplateHelper.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestTemplateHelper.py
index 1c2fa23a2..a470dca67 100644
--- a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestTemplateHelper.py
+++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestTemplateHelper.py
@@ -23,10 +23,13 @@ class TestHelperModule(Bcfg2TestCase):
     test_obj = HelperModule
     path = os.path.join(datastore, "test.py")
 
-    def get_obj(self, path=None):
+    def get_obj(self, path=None, core=None):
         if path is None:
             path = self.path
-        return self.test_obj(path)
+        if core is None:
+            core = Mock()
+            core.metadata_cache_mode = 'none'
+        return self.test_obj(path, core)
 
     def test__init(self):
         hm = self.get_obj()