code review;

more stable config sharing, configurator always shares it config readers now;

fail2ban/client/configparserinc.py

@@ -62,6 +62,7 @@ else: # pragma: no cover
 
 # Gets the instance of the logger.
 logSys = getLogger(__name__)
+logLevel = 7
 
 __all__ = ['SafeConfigParserWithIncludes']
 
@@ -126,8 +127,8 @@ after = 1.conf
 				cfg = SCPWI(share_config=self._cfg_share)
 				i = cfg.read(filename, get_includes=False)
 				self._cfg_share[hashv] = (cfg, i)
-			else:
-				logSys.debug("    Shared file: %s", filename)
+			elif logSys.getEffectiveLevel() <= logLevel:
+				logSys.log(logLevel, "    Shared file: %s", filename)
 		else:
 			# don't have sharing:
 			cfg = SCPWI()
@@ -207,10 +208,9 @@ after = 1.conf
 		if not fileNamesFull:
 			return []
 
-		if self._cfg_share is not None:
-			logSys.debug("  Sharing files: %s", fileNamesFull)
-		else:
-			logSys.debug("  Reading files: %s", fileNamesFull)
+		if logSys.getEffectiveLevel() <= logLevel:
+			logSys.log(logLevel, ("  Sharing files: %s" if self._cfg_share is not None else \
+			                       "  Reading files: %s"), fileNamesFull)
 
 		if len(fileNamesFull) > 1:
 			# read multiple configs:
@@ -237,10 +237,10 @@ after = 1.conf
 			return ret
 
 		# read one config :
-		logSys.debug("  Reading file: %s", fileNamesFull[0])
+		if logSys.getEffectiveLevel() <= logLevel:
+			logSys.log(logLevel, "  Reading file: %s", fileNamesFull[0])
 		# read file(s) :
 		if sys.version_info >= (3,2): # pragma: no cover
 			return SafeConfigParser.read(self, fileNamesFull, encoding='utf-8')
 		else:
 			return SafeConfigParser.read(self, fileNamesFull)
-

fail2ban/client/configreader.py

@@ -27,12 +27,11 @@ __license__ = "GPL"
 import glob, os
 from ConfigParser import NoOptionError, NoSectionError
 
-from .configparserinc import SafeConfigParserWithIncludes
+from .configparserinc import SafeConfigParserWithIncludes, logLevel
 from ..helpers import getLogger
 
 # Gets the instance of the logger.
 logSys = getLogger(__name__)
-_logLevel = 6
 
 class ConfigReader():
 	"""Generic config reader class.
@@ -72,6 +71,22 @@ class ConfigReader():
 
 	def read(self, name, once=True):
 		# shared ?
+		if not self._cfg:
+			self.touch(name)
+		# performance feature - read once if using shared config reader:
+		if once and self._cfg.read_cfg_files is not None:
+			return self._cfg.read_cfg_files
+
+		# read:
+		if self._cfg_share is not None:
+			logSys.info("Sharing configs for %s under %s ", name, self._cfg.getBaseDir())
+		ret = self._cfg.read(name)
+
+		# save already read and return:
+		self._cfg.read_cfg_files = ret
+		return ret
+
+	def touch(self, name = ''):
 		if not self._cfg and self._cfg_share is not None:
 			self._cfg = self._cfg_share.get(name)
 			if not self._cfg:
@@ -79,36 +94,33 @@ class ConfigReader():
 				if self._cfg_share_basedir is not None:
 					self._cfg.setBaseDir(self._cfg_share_basedir)
 				self._cfg_share[name] = self._cfg
-		# performance feature - read once if using shared config reader:
-		rc = self._cfg.read_cfg_files
-		if once and rc.get(name) is not None:
-			return rc.get(name)
-
-		# read:
-		if self._cfg_share is not None:
-			logSys.info("Sharing configs for %s under %s ", name, self._cfg.getBaseDir())
-		ret = self._cfg.read(name)
-
-		# save already read:
-		if once:
-			rc[name] = ret
-		return ret
+		else:
+			self._cfg = ConfigReaderUnshared(**self._cfg_share_kwargs)
 
 	def sections(self):
-		return self._cfg.sections()
+		if self._cfg is not None:
+			return self._cfg.sections()
+		return []
 
 	def has_section(self, sec):
-		return self._cfg.has_section(sec)
+		if self._cfg is not None:
+			return self._cfg.has_section(sec)
+		return False
 
 	def options(self, *args):
-		return self._cfg.options(*args)
+		if self._cfg is not None:
+			return self._cfg.options(*args)
+		return {}
 
 	def get(self, sec, opt):
-		return self._cfg.get(sec, opt)
+		if self._cfg is not None:
+			return self._cfg.get(sec, opt)
+		return None
 
 	def getOptions(self, *args, **kwargs):
-		return self._cfg.getOptions(*args, **kwargs)
-
+		if self._cfg is not None:
+			return self._cfg.getOptions(*args, **kwargs)
+		return {}
 
 class ConfigReaderUnshared(SafeConfigParserWithIncludes):
 	"""Unshared config reader (previously ConfigReader).
@@ -121,7 +133,7 @@ class ConfigReaderUnshared(SafeConfigParserWithIncludes):
 	
 	def __init__(self, basedir=None, *args, **kwargs):
 		SafeConfigParserWithIncludes.__init__(self, *args, **kwargs)
-		self.read_cfg_files = dict()
+		self.read_cfg_files = None
 		self.setBaseDir(basedir)
 	
 	def setBaseDir(self, basedir):
@@ -137,7 +149,7 @@ class ConfigReaderUnshared(SafeConfigParserWithIncludes):
 			raise ValueError("Base configuration directory %s does not exist "
 							  % self._basedir)
 		basename = os.path.join(self._basedir, filename)
-		logSys.info("Reading configs for %s under %s " , filename, self._basedir)
+		logSys.debug("Reading configs for %s under %s " , filename, self._basedir)
 		config_files = [ basename + ".conf" ]
 
 		# possible further customizations under a .conf.d directory
@@ -203,8 +215,8 @@ class ConfigReaderUnshared(SafeConfigParserWithIncludes):
 					logSys.warning("'%s' not defined in '%s'. Using default one: %r"
 								% (option[1], sec, option[2]))
 					values[option[1]] = option[2]
-				elif logSys.getEffectiveLevel() <= _logLevel:
-					logSys.log(_logLevel, "Non essential option '%s' not defined in '%s'.", option[1], sec)
+				elif logSys.getEffectiveLevel() <= logLevel:
+					logSys.log(logLevel, "Non essential option '%s' not defined in '%s'.", option[1], sec)
 			except ValueError:
 				logSys.warning("Wrong value for '" + option[1] + "' in '" + sec +
 							"'. Using default one: '" + `option[2]` + "'")

fail2ban/client/configurator.py

@@ -36,6 +36,9 @@ class Configurator:
 	def __init__(self, force_enable=False, share_config=None):
 		self.__settings = dict()
 		self.__streams = dict()
+		# always share all config readers:
+		if share_config is None:
+			share_config = dict()
 		self.__fail2ban = Fail2banReader(share_config=share_config)
 		self.__jails = JailsReader(force_enable=force_enable, share_config=share_config)
 	

fail2ban/client/fail2banreader.py

@@ -33,7 +33,6 @@ logSys = getLogger(__name__)
 class Fail2banReader(ConfigReader):
 	
 	def __init__(self, **kwargs):
-		self.__opts = None
 		ConfigReader.__init__(self, **kwargs)
 	
 	def read(self):

fail2ban/client/jailreader.py

@@ -42,7 +42,6 @@ class JailReader(ConfigReader):
 		r'([\w\-_\.]+)=(?:"([^"]*)"|\'([^\']*)\'|([^,]*))(?:,|$)')
 	
 	def __init__(self, name, force_enable=False, **kwargs):
-		# use shared config if possible:
 		ConfigReader.__init__(self, **kwargs)
 		self.__name = name
 		self.__filter = None

fail2ban/client/jailsreader.py

@@ -41,7 +41,6 @@ class JailsReader(ConfigReader):
 		  Passed to JailReader to force enable the jails.
 		  It is for internal use
 		"""
-		# use shared config if possible:
 		ConfigReader.__init__(self, **kwargs)
 		self.__jails = list()
 		self.__force_enable = force_enable

fail2ban/tests/clientreadertestcase.py

@@ -21,9 +21,9 @@ __author__ = "Cyril Jaquier, Yaroslav Halchenko"
 __copyright__ = "Copyright (c) 2004 Cyril Jaquier, 2011-2013 Yaroslav Halchenko"
 __license__ = "GPL"
 
-import os, glob, shutil, tempfile, unittest, re
-
+import os, glob, shutil, tempfile, unittest, re, logging
 from ..client.configreader import ConfigReaderUnshared
+from ..client import configparserinc
 from ..client.jailreader import JailReader
 from ..client.filterreader import FilterReader
 from ..client.jailsreader import JailsReader
@@ -355,6 +355,8 @@ class JailsReaderTestCache(LogCaptureTestCase):
 		return cnt
 
 	def testTestJailConfCache(self):
+		saved_ll = configparserinc.logLevel
+		configparserinc.logLevel = logging.DEBUG
 		basedir = tempfile.mkdtemp("fail2ban_conf")
 		try:
 			shutil.rmtree(basedir)
@@ -388,6 +390,7 @@ class JailsReaderTestCache(LogCaptureTestCase):
 			self.assertTrue(cnt == 1, "Unexpected count by reading of action files, cnt = %s" % cnt)
 		finally:
 			shutil.rmtree(basedir)
+			configparserinc.logLevel = saved_ll
 
 
 class JailsReaderTest(LogCaptureTestCase):