From 6f179bef525b57e049ce4b2ac97853c46a3dc9db Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Fri, 7 Oct 2011 11:06:50 -0400 Subject: don't cache collections by hostname; this could lead to bad data if a host changes OS --- src/lib/Server/Plugins/Packages/Collection.py | 18 ++++++++--------- src/lib/Server/Plugins/Packages/__init__.py | 29 ++++++++------------------- 2 files changed, 17 insertions(+), 30 deletions(-) (limited to 'src/lib/Server/Plugins') diff --git a/src/lib/Server/Plugins/Packages/Collection.py b/src/lib/Server/Plugins/Packages/Collection.py index 69e38134e..aed85fe77 100644 --- a/src/lib/Server/Plugins/Packages/Collection.py +++ b/src/lib/Server/Plugins/Packages/Collection.py @@ -8,7 +8,7 @@ except ImportError: logger = logging.getLogger("Packages") -_collections = dict() +collections = dict() class Collection(object): def __init__(self, metadata, sources, basepath): @@ -278,16 +278,16 @@ class Collection(object): self.sources.sort(cmp, key, reverse) def clear_cache(): - global _collections - _collections = dict() + global collections + collections = dict() def factory(metadata, sources, basepath): - global _collections + global collections if not sources.loaded: # if sources.xml has not received a FAM event yet, defer; # instantiate a dummy Collection object, but do not cache it - # in _collections + # in collections return Collection(metadata, [], basepath) sclasses = set() @@ -298,13 +298,13 @@ def factory(metadata, sources, basepath): relevant.append(source) sclasses.update([source.__class__]) - # _collections is a cache dict of Collection objects that is keyed + # collections is a cache dict of Collection objects that is keyed # off of the set of source urls that apply to each Collection ckeydata = set() for source in relevant: ckeydata.update(source.urls) ckey = tuple(sorted(list(ckeydata))) - if ckey not in _collections: + if ckey not in collections: if len(sclasses) > 1: logger.warning("Multiple source types found for %s: %s" % ",".join([s.__name__ for s in sclasses])) @@ -332,5 +332,5 @@ def factory(metadata, sources, basepath): collection = cclass(metadata, relevant, basepath) # reverse so that file order determines precedence collection.reverse() - _collections[ckey] = collection - return _collections[ckey] + collections[ckey] = collection + return collections[ckey] diff --git a/src/lib/Server/Plugins/Packages/__init__.py b/src/lib/Server/Plugins/Packages/__init__.py index 7dd5d25db..1132543f1 100644 --- a/src/lib/Server/Plugins/Packages/__init__.py +++ b/src/lib/Server/Plugins/Packages/__init__.py @@ -31,7 +31,6 @@ class Packages(Bcfg2.Server.Plugin.Plugin, Bcfg2.Server.Plugin.Connector.__init__(self) Bcfg2.Server.Plugin.Probing.__init__(self) - self.collections = dict() self.sentinels = set() self.cachepath = os.path.join(self.data, 'cache') self.keypath = os.path.join(self.data, 'keys') @@ -68,14 +67,14 @@ class Packages(Bcfg2.Server.Plugin.Plugin, 'type': 'file', 'perms': '0644'} - collection = self._get_collection(metadata) + collection = Collection.factory(metadata, self.sources, self.data) entry.text = collection.get_config() for (key, value) in list(attrib.items()): entry.attrib.__setitem__(key, value) def HandleEntry(self, entry, metadata): if entry.tag == 'Package': - collection = self._get_collection(metadata) + collection = Collection.factory(metadata, self.sources, self.data) entry.set('version', 'auto') entry.set('type', collection.ptype) elif entry.tag == 'Path': @@ -90,7 +89,7 @@ class Packages(Bcfg2.Server.Plugin.Plugin, def HandlesEntry(self, entry, metadata): if entry.tag == 'Package': - collection = self._get_collection(metadata) + collection = Collection.factory(metadata, self.sources, self.data) if collection.magic_groups_match(): return True elif entry.tag == 'Path': @@ -110,7 +109,7 @@ class Packages(Bcfg2.Server.Plugin.Plugin, metadata - client metadata instance structures - a list of structure-stage entry combinations ''' - collection = self._get_collection(metadata) + collection = Collection.factory(metadata, self.sources, self.data) indep = lxml.etree.Element('Independent') self._build_packages(metadata, indep, structures, collection=collection) @@ -126,7 +125,7 @@ class Packages(Bcfg2.Server.Plugin.Plugin, return if collection is None: - collection = self._get_collection(metadata) + collection = Collection.factory(metadata, self.sources, self.data) initial = set() to_remove = [] for struct in structures: @@ -174,7 +173,6 @@ class Packages(Bcfg2.Server.Plugin.Plugin, Keyword args: force_update Force downloading repo data ''' - Collection.clear_cache() self._load_sources(force_update) self._load_gpg_keys(force_update) @@ -183,13 +181,13 @@ class Packages(Bcfg2.Server.Plugin.Plugin, self.sentinels = set() cachefiles = [] - for hostname, collection in list(self.collections.items()): + for collection in list(Collection.collections.values()): cachefiles.extend(collection.cachefiles) if not self.disableMetaData: collection.setup_data(force_update) self.sentinels.update(collection.basegroups) - self.collections = dict() + Collection.clear_cache() for cfile in glob.glob(os.path.join(self.cachepath, "cache-*")): if cfile not in cachefiles: @@ -223,17 +221,6 @@ class Packages(Bcfg2.Server.Plugin.Plugin, if kfile not in keyfiles: os.unlink(kfile) - def _get_collection(self, metadata): - if not self.sources.loaded: - # do not cache a collection object instantiated before - # sources have been loaded - return Collection.factory(metadata, self.sources, self.data) - - if metadata.hostname not in self.collections: - self.collections[metadata.hostname] = \ - Collection.factory(metadata, self.sources, self.data) - return self.collections[metadata.hostname] - def get_additional_data(self, metadata): - collection = self._get_collection(metadata) + collection = Collection.factory(metadata, self.sources, self.data) return dict(sources=collection.get_additional_data()) -- cgit v1.2.3-1-g7c22