diff options
| -rw-r--r-- | src/lib/Bcfg2/Client/Tools/APK.py | 5 | ||||
| -rw-r--r-- | src/lib/Bcfg2/Client/Tools/APT.py | 2 | ||||
| -rw-r--r-- | src/lib/Bcfg2/Client/Tools/Action.py | 33 | ||||
| -rw-r--r-- | src/lib/Bcfg2/Client/Tools/Chkconfig.py | 7 | ||||
| -rw-r--r-- | src/lib/Bcfg2/Client/Tools/Encap.py | 14 | ||||
| -rw-r--r-- | src/lib/Bcfg2/Client/Tools/FreeBSDInit.py | 2 | ||||
| -rw-r--r-- | src/lib/Bcfg2/Client/Tools/FreeBSDPackage.py | 2 | ||||
| -rw-r--r-- | src/lib/Bcfg2/Client/Tools/IPS.py | 2 | ||||
| -rw-r--r-- | src/lib/Bcfg2/Client/Tools/MacPorts.py | 15 | ||||
| -rw-r--r-- | src/lib/Bcfg2/Client/Tools/Pacman.py | 11 | ||||
| -rw-r--r-- | src/lib/Bcfg2/Client/Tools/RcUpdate.py | 16 | ||||
| -rw-r--r-- | src/lib/Bcfg2/Client/Tools/SMF.py | 7 | ||||
| -rw-r--r-- | src/lib/Bcfg2/Client/Tools/Upstart.py | 18 | ||||
| -rw-r--r-- | src/lib/Bcfg2/Client/Tools/__init__.py | 188 | ||||
| -rwxr-xr-x | src/sbin/bcfg2-info | 1 | ||||
| -rw-r--r-- | testsuite/Testsrc/test_code_checks.py | 151 | ||||
| -rw-r--r-- | testsuite/pylintrc.conf | 4 |
17 files changed, 271 insertions, 207 deletions
diff --git a/src/lib/Bcfg2/Client/Tools/APK.py b/src/lib/Bcfg2/Client/Tools/APK.py index 9c7692dbb..539a0fddb 100644 --- a/src/lib/Bcfg2/Client/Tools/APK.py +++ b/src/lib/Bcfg2/Client/Tools/APK.py @@ -28,7 +28,7 @@ class APK(Bcfg2.Client.Tools.PkgTool): self.logger.debug(" version: %s" % version) self.installed[pkgname] = version - def VerifyPackage(self, entry, modlist): + def VerifyPackage(self, entry, _): """Verify Package status for entry.""" if not 'version' in entry.attrib: self.logger.info("Cannot verify unversioned package %s" % @@ -36,7 +36,8 @@ class APK(Bcfg2.Client.Tools.PkgTool): return False if entry.attrib['name'] in self.installed: - if entry.attrib['version'] in ['auto', self.installed[entry.attrib['name']]]: + if entry.attrib['version'] in \ + ['auto', self.installed[entry.attrib['name']]]: #if not self.setup['quick'] and \ # entry.get('verify', 'true') == 'true': #FIXME: Does APK have any sort of verification mechanism? diff --git a/src/lib/Bcfg2/Client/Tools/APT.py b/src/lib/Bcfg2/Client/Tools/APT.py index ce7e9701f..879d2720a 100644 --- a/src/lib/Bcfg2/Client/Tools/APT.py +++ b/src/lib/Bcfg2/Client/Tools/APT.py @@ -66,7 +66,7 @@ class APT(Bcfg2.Client.Tools.Tool): except SystemError: e = sys.exc_info()[1] self.logger.info("Failed to initialize APT cache: %s" % e) - raise Bcfg2.Client.Tools.toolInstantiationError + raise Bcfg2.Client.Tools.ToolInstantiationError self.pkg_cache.update() self.pkg_cache = apt.cache.Cache() if 'req_reinstall_pkgs' in dir(self.pkg_cache): diff --git a/src/lib/Bcfg2/Client/Tools/Action.py b/src/lib/Bcfg2/Client/Tools/Action.py index 4f4fdc3b2..7726da94c 100644 --- a/src/lib/Bcfg2/Client/Tools/Action.py +++ b/src/lib/Bcfg2/Client/Tools/Action.py @@ -2,21 +2,7 @@ import Bcfg2.Client.Tools from Bcfg2.Client.Frame import matches_white_list, passes_black_list -from Bcfg2.Compat import input - -""" -<Action timing='pre|post|both' - name='name' - command='cmd text' - when='always|modified' - status='ignore|check'/> -<PostInstall name='foo'/> - => <Action timing='post' - when='modified' - name='n' - command='foo' - status='ignore'/> -""" +from Bcfg2.Compat import input # pylint: disable=W0622 class Action(Bcfg2.Client.Tools.Tool): @@ -27,6 +13,8 @@ class Action(Bcfg2.Client.Tools.Tool): 'Action': ['name', 'timing', 'when', 'command', 'status']} def _action_allowed(self, action): + """ Return true if the given action is allowed to be run by + the whitelist or blacklist """ if self.setup['decision'] == 'whitelist' and \ not matches_white_list(action, self.setup['decision_list']): self.logger.info("In whitelist mode: suppressing Action:" + \ @@ -50,16 +38,18 @@ class Action(Bcfg2.Client.Tools.Tool): return False if self.setup['servicemode'] == 'build': if entry.get('build', 'true') == 'false': - self.logger.debug("Action: Deferring execution of %s due to build mode" % (entry.get('command'))) + self.logger.debug("Action: Deferring execution of %s due " + "to build mode" % entry.get('command')) return False self.logger.debug("Running Action %s" % (entry.get('name'))) - rc = self.cmd.run(entry.get('command'))[0] - self.logger.debug("Action: %s got rc %s" % (entry.get('command'), rc)) - entry.set('rc', str(rc)) + rv = self.cmd.run(entry.get('command'))[0] + self.logger.debug("Action: %s got return code %s" % + (entry.get('command'), rv)) + entry.set('rc', str(rv)) if entry.get('status', 'check') == 'ignore': return True else: - return rc == 0 + return rv == 0 else: self.logger.debug("In dryrun mode: not running action: %s" % (entry.get('name'))) @@ -80,6 +70,9 @@ class Action(Bcfg2.Client.Tools.Tool): return True def InstallPostInstall(self, entry): + """ Install a deprecated PostInstall entry """ + self.logger.warning("Installing deprecated PostInstall entry %s" % + entry.get("name")) return self.InstallAction(entry) def BundleUpdated(self, bundle, states): diff --git a/src/lib/Bcfg2/Client/Tools/Chkconfig.py b/src/lib/Bcfg2/Client/Tools/Chkconfig.py index b62fb9ef8..ef697091c 100644 --- a/src/lib/Bcfg2/Client/Tools/Chkconfig.py +++ b/src/lib/Bcfg2/Client/Tools/Chkconfig.py @@ -51,10 +51,8 @@ class Chkconfig(Bcfg2.Client.Tools.SvcTool): pstatus = self.check_service(entry) if entry.get('status') == 'on': status = (len(onlevels) > 0 and pstatus) - command = 'start' else: status = (len(onlevels) == 0 and not pstatus) - command = 'stop' if not status: if entry.get('status') == 'on': @@ -84,8 +82,9 @@ class Chkconfig(Bcfg2.Client.Tools.SvcTool): def FindExtra(self): """Locate extra chkconfig Services.""" - allsrv = [line.split()[0] for line in \ - self.cmd.run("/sbin/chkconfig --list 2>/dev/null|grep :on")[1]] + allsrv = [line.split()[0] + for line in self.cmd.run("/sbin/chkconfig " + "--list 2>/dev/null|grep :on")[1]] self.logger.debug('Found active services:') self.logger.debug(allsrv) specified = [srv.get('name') for srv in self.getSupportedEntries()] diff --git a/src/lib/Bcfg2/Client/Tools/Encap.py b/src/lib/Bcfg2/Client/Tools/Encap.py index fa09c3ec7..b5057786f 100644 --- a/src/lib/Bcfg2/Client/Tools/Encap.py +++ b/src/lib/Bcfg2/Client/Tools/Encap.py @@ -4,6 +4,7 @@ import glob import re import Bcfg2.Client.Tools + class Encap(Bcfg2.Client.Tools.PkgTool): """Support for Encap packages.""" name = 'Encap' @@ -14,9 +15,6 @@ class Encap(Bcfg2.Client.Tools.PkgTool): pkgtool = ("/usr/local/bin/epkg -l -f -q %s", ("%s", ["url"])) splitter = re.compile('.*/(?P<name>[\w-]+)\-(?P<version>[\w\.+-]+)') -# If you define self.pkgtool and self.pkgname it will [use] the Pkgtool.Install -# method will do the installation stuff for you - def RefreshPackages(self): """Try to find encap packages.""" self.installed = {} @@ -26,24 +24,24 @@ class Encap(Bcfg2.Client.Tools.PkgTool): self.installed[match.group('name')] = match.group('version') else: print("Failed to split name %s" % pkg) - self.logger.debug("Encap.py: RefreshPackages: self.installed.keys() are:") + self.logger.debug("Encap: RefreshPackages: self.installed.keys() are:") self.logger.debug("%s" % list(self.installed.keys())) def VerifyPackage(self, entry, _): """Verify Package status for entry.""" if not entry.get('version'): - self.logger.info("Insufficient information of Package %s; cannot Verify" % entry.get('name')) + self.logger.info("Insufficient information of Package %s; " + "cannot Verify" % entry.get('name')) return False cmdrc = self.cmd.run("/usr/local/bin/epkg -q -S -k %s-%s >/dev/null" % (entry.get('name'), entry.get('version')))[0] if cmdrc != 0: - self.logger.debug("Package %s version incorrect" % entry.get('name')) + self.logger.debug("Package %s version incorrect" % + entry.get('name')) else: return True return False - # Can use the FindExtraPackages method from Bcfg2.Client.Tools.PkgTool - def RemovePackages(self, packages): """Deal with extra configuration detected.""" names = " ".join([pkg.get('name') for pkg in packages]) diff --git a/src/lib/Bcfg2/Client/Tools/FreeBSDInit.py b/src/lib/Bcfg2/Client/Tools/FreeBSDInit.py index 10f0f2e93..8ff26d8f3 100644 --- a/src/lib/Bcfg2/Client/Tools/FreeBSDInit.py +++ b/src/lib/Bcfg2/Client/Tools/FreeBSDInit.py @@ -18,7 +18,7 @@ class FreeBSDInit(Bcfg2.Client.Tools.SvcTool): def __init__(self, logger, cfg, setup): Bcfg2.Client.Tools.Tool.__init__(self, logger, cfg, setup) if os.uname()[0] != 'FreeBSD': - raise Bcfg2.Client.Tools.toolInstantiationError + raise Bcfg2.Client.Tools.ToolInstantiationError def VerifyService(self, entry, _): return True diff --git a/src/lib/Bcfg2/Client/Tools/FreeBSDPackage.py b/src/lib/Bcfg2/Client/Tools/FreeBSDPackage.py index 3e6f2b6bb..ded84bef4 100644 --- a/src/lib/Bcfg2/Client/Tools/FreeBSDPackage.py +++ b/src/lib/Bcfg2/Client/Tools/FreeBSDPackage.py @@ -28,7 +28,7 @@ class FreeBSDPackage(Bcfg2.Client.Tools.PkgTool): version = pattern.match(pkg).group(2) self.installed[name] = version - def VerifyPackage(self, entry, modlist): + def VerifyPackage(self, entry, _): if not 'version' in entry.attrib: self.logger.info("Cannot verify unversioned package %s" % (entry.attrib['name'])) diff --git a/src/lib/Bcfg2/Client/Tools/IPS.py b/src/lib/Bcfg2/Client/Tools/IPS.py index e30bbd2a4..dc4d48235 100644 --- a/src/lib/Bcfg2/Client/Tools/IPS.py +++ b/src/lib/Bcfg2/Client/Tools/IPS.py @@ -35,7 +35,7 @@ class IPS(Bcfg2.Client.Tools.PkgTool): if pinfo['upgradable']: self.pending_upgrades.add(pname) - def VerifyPackage(self, entry, modlist): + def VerifyPackage(self, entry, _): """Verify package for entry.""" pname = entry.get('name') if not 'version' in entry.attrib: diff --git a/src/lib/Bcfg2/Client/Tools/MacPorts.py b/src/lib/Bcfg2/Client/Tools/MacPorts.py index 9724fab57..d768019a9 100644 --- a/src/lib/Bcfg2/Client/Tools/MacPorts.py +++ b/src/lib/Bcfg2/Client/Tools/MacPorts.py @@ -29,7 +29,7 @@ class MacPorts(Bcfg2.Client.Tools.PkgTool): self.logger.info(" pkgname: %s version: %s" % (pkgname, version)) self.installed[pkgname] = version - def VerifyPackage(self, entry, modlist): + def VerifyPackage(self, entry, _): """Verify Package status for entry.""" if not 'version' in entry.attrib: self.logger.info("Cannot verify unversioned package %s" % @@ -37,8 +37,8 @@ class MacPorts(Bcfg2.Client.Tools.PkgTool): return False if entry.attrib['name'] in self.installed: - if (self.installed[entry.attrib['name']] == entry.attrib['version'] or - entry.attrib['version'] == 'any'): + if (self.installed[entry.attrib['name']] == entry.attrib['version'] + or entry.attrib['version'] == 'any'): #if not self.setup['quick'] and \ # entry.get('verify', 'true') == 'true': #FIXME: We should be able to check this once @@ -46,10 +46,11 @@ class MacPorts(Bcfg2.Client.Tools.PkgTool): return True else: self.logger.info(" %s: Wrong version installed. " - "Want %s, but have %s" % (entry.get("name"), - entry.get("version"), - self.installed[entry.get("name")], - )) + "Want %s, but have %s" % + (entry.get("name"), + entry.get("version"), + self.installed[entry.get("name")], + )) entry.set('current_version', self.installed[entry.get('name')]) return False diff --git a/src/lib/Bcfg2/Client/Tools/Pacman.py b/src/lib/Bcfg2/Client/Tools/Pacman.py index 02889a2e7..75dd62ede 100644 --- a/src/lib/Bcfg2/Client/Tools/Pacman.py +++ b/src/lib/Bcfg2/Client/Tools/Pacman.py @@ -28,7 +28,7 @@ class Pacman(Bcfg2.Client.Tools.PkgTool): #self.logger.info(" pkgname: %s, version: %s" % (pkgname, version)) self.installed[pkgname] = version - def VerifyPackage(self, entry, modlist): + def VerifyPackage(self, entry, _): '''Verify Package status for entry''' self.logger.info("VerifyPackage : %s : %s" % entry.get('name'), @@ -42,7 +42,8 @@ class Pacman(Bcfg2.Client.Tools.PkgTool): if entry.attrib['name'] in self.installed: if entry.attrib['version'] == 'auto': return True - elif self.installed[entry.attrib['name']] == entry.attrib['version']: + elif self.installed[entry.attrib['name']] == \ + entry.attrib['version']: #if not self.setup['quick'] and \ # entry.get('verify', 'true') == 'true': #FIXME: need to figure out if pacman @@ -79,6 +80,6 @@ class Pacman(Bcfg2.Client.Tools.PkgTool): try: self.logger.debug("Running : %s -S %s" % (self.pkgtool, pkgline)) self.cmd.run("%s -S %s" % (self.pkgtool, pkgline)) - except Exception: - e = sys.exc_info()[1] - self.logger.error("Error occurred during installation: %s" % e) + except: # pylint: disable=W0702 + err = sys.exc_info()[1] + self.logger.error("Error occurred during installation: %s" % err) diff --git a/src/lib/Bcfg2/Client/Tools/RcUpdate.py b/src/lib/Bcfg2/Client/Tools/RcUpdate.py index a53a45251..d5cef6e34 100644 --- a/src/lib/Bcfg2/Client/Tools/RcUpdate.py +++ b/src/lib/Bcfg2/Client/Tools/RcUpdate.py @@ -23,8 +23,8 @@ class RcUpdate(Bcfg2.Client.Tools.SvcTool): # check if service is enabled cmd = '/sbin/rc-update show default | grep %s' - rc = self.cmd.run(cmd % entry.get('name'))[0] - is_enabled = (rc == 0) + rv = self.cmd.run(cmd % entry.get('name'))[0] + is_enabled = (rv == 0) # check if init script exists try: @@ -36,8 +36,8 @@ class RcUpdate(Bcfg2.Client.Tools.SvcTool): # check if service is enabled cmd = '/etc/init.d/%s status | grep started' - rc = self.cmd.run(cmd % entry.attrib['name'])[0] - is_running = (rc == 0) + rv = self.cmd.run(cmd % entry.attrib['name'])[0] + is_running = (rv == 0) if entry.get('status') == 'on' and not (is_enabled and is_running): entry.set('current_status', 'off') @@ -60,16 +60,16 @@ class RcUpdate(Bcfg2.Client.Tools.SvcTool): self.start_service(entry) # make sure it's enabled cmd = '/sbin/rc-update add %s default' - rc = self.cmd.run(cmd % entry.get('name'))[0] - return (rc == 0) + rv = self.cmd.run(cmd % entry.get('name'))[0] + return (rv == 0) elif entry.get('status') == 'off': if entry.get('current_status') == 'on': self.stop_service(entry) # make sure it's disabled cmd = '/sbin/rc-update del %s default' - rc = self.cmd.run(cmd % entry.get('name'))[0] - return (rc == 0) + rv = self.cmd.run(cmd % entry.get('name'))[0] + return (rv == 0) return False diff --git a/src/lib/Bcfg2/Client/Tools/SMF.py b/src/lib/Bcfg2/Client/Tools/SMF.py index 3e0a9da13..43e4b3bf5 100644 --- a/src/lib/Bcfg2/Client/Tools/SMF.py +++ b/src/lib/Bcfg2/Client/Tools/SMF.py @@ -104,7 +104,7 @@ class SMF(Bcfg2.Client.Tools.SvcTool): cmdrc = 1 else: srvdata = self.cmd.run("/usr/bin/svcs -H -o STA %s" % - entry.get('FMRI'))[1] [0].split() + entry.get('FMRI'))[1][0].split() if srvdata[0] == 'MNT': cmdarg = 'clear' else: @@ -126,7 +126,8 @@ class SMF(Bcfg2.Client.Tools.SvcTool): self.cmd.run("/usr/bin/svcs -a -H -o FMRI,STATE")[1]] if version != 'disabled'] - [allsrv.remove(svc.get('FMRI')) for svc in self.getSupportedEntries() \ - if svc.get("FMRI") in allsrv] + for svc in self.getSupportedEntries(): + if svc.get("FMRI") in allsrv: + allsrv.remove(svc.get('FMRI')) return [Bcfg2.Client.XML.Element("Service", type='smf', name=name) \ for name in allsrv] diff --git a/src/lib/Bcfg2/Client/Tools/Upstart.py b/src/lib/Bcfg2/Client/Tools/Upstart.py index aa5a921a6..02ed52544 100644 --- a/src/lib/Bcfg2/Client/Tools/Upstart.py +++ b/src/lib/Bcfg2/Client/Tools/Upstart.py @@ -38,14 +38,15 @@ class Upstart(Bcfg2.Client.Tools.SvcTool): params = '' try: - output = self.cmd.run('/usr/sbin/service %s status %s' % \ - ( entry.get('name'), params ))[1][0] + output = self.cmd.run('/usr/sbin/service %s status %s' % + (entry.get('name'), params))[1][0] except IndexError: - self.logger.error("Service %s not an Upstart service" % \ + self.logger.error("Service %s not an Upstart service" % entry.get('name')) return False - match = re.compile("%s( \(.*\))? (start|stop)/(running|waiting)" %entry.get('name') ).match( output ) + match = re.compile("%s( \(.*\))? (start|stop)/(running|waiting)" % + entry.get('name')).match(output) if match == None: # service does not exist entry.set('current_status', 'off') @@ -80,9 +81,8 @@ class Upstart(Bcfg2.Client.Tools.SvcTool): """Locate extra Upstart services.""" specified = [entry.get('name') for entry in self.getSupportedEntries()] extra = [] - for name in [self.svcre.match(fname).group('name') for fname in - glob.glob("/etc/init/*.conf") \ - if self.svcre.match(fname).group('name') not in specified]: - extra.append(name) - return [Bcfg2.Client.XML.Element('Service', type='upstart', name=name) \ + for fname in glob.glob("/etc/init/*.conf"): + if self.svcre.match(fname).group('name') not in specified: + extra.append(self.svcre.match(fname).group('name')) + return [Bcfg2.Client.XML.Element('Service', type='upstart', name=name) for name in extra] diff --git a/src/lib/Bcfg2/Client/Tools/__init__.py b/src/lib/Bcfg2/Client/Tools/__init__.py index c11b96ef7..8f1d277d8 100644 --- a/src/lib/Bcfg2/Client/Tools/__init__.py +++ b/src/lib/Bcfg2/Client/Tools/__init__.py @@ -1,23 +1,25 @@ """This contains all Bcfg2 Tool modules""" + import os -import sys import stat -import time from subprocess import Popen, PIPE import Bcfg2.Client.XML -from Bcfg2.Compat import input, walk_packages +from Bcfg2.Compat import input, walk_packages # pylint: disable=W0622 __all__ = [m[1] for m in walk_packages(path=__path__)] + +# pylint: disable=C0103 drivers = [item for item in __all__ if item not in ['rpmtools']] -default = [item for item in drivers if item not in ['RPM', 'Yum']] +default = drivers[:] +# pylint: enable=C0103 -class toolInstantiationError(Exception): +class ToolInstantiationError(Exception): """This error is called if the toolset cannot be instantiated.""" pass -class executor: +class Executor: """This class runs stuff for us""" def __init__(self, logger): @@ -25,19 +27,33 @@ class executor: def run(self, command): """Run a command in a pipe dealing with stdout buffer overloads.""" - p = Popen(command, shell=True, bufsize=16384, - stdin=PIPE, stdout=PIPE, close_fds=True) - output = p.communicate()[0] + proc = Popen(command, shell=True, bufsize=16384, + stdin=PIPE, stdout=PIPE, close_fds=True) + output = proc.communicate()[0] for line in output.splitlines(): self.logger.debug('< %s' % line) - return (p.returncode, output.splitlines()) + return (proc.returncode, output.splitlines()) + +class ClassName(object): + """ This very simple descriptor class exists only to get the name + of the owner class. This is used because, for historical reasons, + we expect every tool to have a ``name`` attribute that is in + almost all cases the same as the ``__class__.__name__`` attribute + of the plugin object. This makes that more dynamic so that each + plugin isn't repeating its own name.""" + def __get__(self, inst, owner): + return owner.__name__ + + +# pylint: disable=W0702 +# in the base tool class we frequently want to catch all exceptions, +# regardless of type, so disable the pylint rule that catches that. class Tool(object): - """ - All tools subclass this. It defines all interfaces that need to be defined. - """ - name = 'Tool' + """ All tools subclass this. It defines all interfaces that need + to be defined. """ + name = ClassName() __execs__ = [] __handles__ = [] __req__ = {} @@ -50,7 +66,7 @@ class Tool(object): if not hasattr(self, '__ireq__'): self.__ireq__ = self.__req__ self.config = config - self.cmd = executor(logger) + self.cmd = Executor(logger) self.modified = [] self.extra = [] self.__important__ = [] @@ -66,38 +82,38 @@ class Tool(object): try: mode = stat.S_IMODE(os.stat(filename)[stat.ST_MODE]) if mode & stat.S_IEXEC != stat.S_IEXEC: - self.logger.debug("%s: %s not executable" % \ + self.logger.debug("%s: %s not executable" % (self.name, filename)) - raise toolInstantiationError + raise ToolInstantiationError except OSError: - raise toolInstantiationError + raise ToolInstantiationError except: self.logger.debug("%s failed" % filename, exc_info=1) - raise toolInstantiationError + raise ToolInstantiationError - def BundleUpdated(self, _, states): + def BundleUpdated(self, bundle, states): # pylint: disable=W0613 """This callback is used when bundle updates occur.""" return - def BundleNotUpdated(self, _, states): + def BundleNotUpdated(self, bundle, states): # pylint: disable=W0613 """This callback is used when a bundle is not updated.""" return - def Inventory(self, states, structures=[]): + def Inventory(self, states, structures=None): """Dispatch verify calls to underlying methods.""" if not structures: structures = self.config.getchildren() mods = self.buildModlist() - for (struct, entry) in [(struct, entry) for struct in structures \ - for entry in struct.getchildren() \ - if self.canVerify(entry)]: - try: - func = getattr(self, "Verify%s" % (entry.tag)) - states[entry] = func(entry, mods) - except: - self.logger.error( - "Unexpected failure of verification method for entry type %s" \ - % (entry.tag), exc_info=1) + for struct in structures: + for entry in struct.getchildren(): + if self.canVerify(entry): + try: + func = getattr(self, "Verify%s" % entry.tag) + states[entry] = func(entry, mods) + except: + self.logger.error("Unexpected failure of verification " + "method for entry type %s" % + entry.tag, exc_info=1) self.extra = self.FindExtra() def Install(self, entries, states): @@ -109,8 +125,9 @@ class Tool(object): if states[entry]: self.modified.append(entry) except: - self.logger.error("Unexpected failure of install method for entry type %s" \ - % (entry.tag), exc_info=1) + self.logger.error("Unexpected failure of install method for " + "entry type %s" % entry.tag, + exc_info=1) def Remove(self, entries): """Remove specified extra entries""" @@ -118,26 +135,32 @@ class Tool(object): def getSupportedEntries(self): """Return a list of supported entries.""" - return [entry for struct in \ - self.config.getchildren() for entry in \ - struct.getchildren() \ - if self.handlesEntry(entry)] + rv = [] + for struct in self.config.getchildren(): + rv.extend([entry for entry in struct.getchildren() + if self.handlesEntry(entry)]) + return rv def handlesEntry(self, entry): """Return if entry is handled by this tool.""" return (entry.tag, entry.get('type')) in self.__handles__ def buildModlist(self): - '''Build a list of potentially modified POSIX paths for this entry''' - return [entry.get('name') for struct in self.config.getchildren() \ - for entry in struct.getchildren() \ - if entry.tag == 'Path'] + """ Build a list of potentially modified POSIX paths for this + entry """ + rv = [] + for struct in self.config.getchildren(): + rv.extend([entry.get('name') for entry in struct.getchildren() + if entry.tag == 'Path']) + return rv def gatherCurrentData(self, entry): """Default implementation of the information gathering routines.""" pass def missing_attrs(self, entry): + """ Return a list of attributes that were expected on entry + but not found """ required = self.__req__[entry.tag] if isinstance(required, dict): required = ["type"] @@ -155,9 +178,8 @@ class Tool(object): return False if 'failure' in entry.attrib: - self.logger.error("Entry %s:%s reports bind failure: %s" % \ - (entry.tag, - entry.get('name'), + self.logger.error("Entry %s:%s reports bind failure: %s" % + (entry.tag, entry.get('name'), entry.get('failure'))) return False @@ -190,7 +212,7 @@ class Tool(object): return False if 'failure' in entry.attrib: - self.logger.error("Cannot install entry %s:%s with bind failure" % \ + self.logger.error("Cannot install entry %s:%s with bind failure" % (entry.tag, entry.get('name'))) return False @@ -202,6 +224,7 @@ class Tool(object): ", ".join(missing))) return False return True +# pylint: enable=W0702 class PkgTool(Tool): @@ -216,8 +239,6 @@ class PkgTool(Tool): def __init__(self, logger, setup, config): Tool.__init__(self, logger, setup, config) self.installed = {} - self.Remove = self.RemovePackages - self.FindExtra = self.FindExtraPackages self.RefreshPackages() def VerifyPackage(self, dummy, _): @@ -225,32 +246,30 @@ class PkgTool(Tool): return False def Install(self, packages, states): - """ - Run a one-pass install, followed by - single pkg installs in case of failure. - """ - self.logger.info("Trying single pass package install for pkgtype %s" % \ + """ Run a one-pass install, followed by single pkg installs in + case of failure. """ + self.logger.info("Trying single pass package install for pkgtype %s" % self.pkgtype) data = [tuple([pkg.get(field) for field in self.pkgtool[1][1]]) for pkg in packages] pkgargs = " ".join([self.pkgtool[1][0] % datum for datum in data]) - self.logger.debug("Installing packages: :%s:" % pkgargs) - self.logger.debug("Running command ::%s::" % (self.pkgtool[0] % pkgargs)) + self.logger.debug("Installing packages: %s" % pkgargs) + self.logger.debug("Running command: %s" % (self.pkgtool[0] % pkgargs)) cmdrc = self.cmd.run(self.pkgtool[0] % pkgargs)[0] if cmdrc == 0: self.logger.info("Single Pass Succeded") # set all package states to true and flush workqueues pkgnames = [pkg.get('name') for pkg in packages] - for entry in [entry for entry in list(states.keys()) - if entry.tag == 'Package' - and entry.get('type') == self.pkgtype - and entry.get('name') in pkgnames]: - self.logger.debug('Setting state to true for pkg %s' % \ - (entry.get('name'))) - states[entry] = True + for entry in list(states.keys()): + if (entry.tag == 'Package' + and entry.get('type') == self.pkgtype + and entry.get('name') in pkgnames): + self.logger.debug('Setting state to true for pkg %s' % + (entry.get('name'))) + states[entry] = True self.RefreshPackages() else: self.logger.error("Single Pass Failed") @@ -259,19 +278,21 @@ class PkgTool(Tool): for pkg in packages: # handle state tracking updates if self.VerifyPackage(pkg, []): - self.logger.info("Forcing state to true for pkg %s" % \ + self.logger.info("Forcing state to true for pkg %s" % (pkg.get('name'))) states[pkg] = True else: self.logger.info("Installing pkg %s version %s" % (pkg.get('name'), pkg.get('version'))) - cmdrc = self.cmd.run(self.pkgtool[0] % - (self.pkgtool[1][0] % - tuple([pkg.get(field) for field in self.pkgtool[1][1]]))) + cmdrc = self.cmd.run( + self.pkgtool[0] % + (self.pkgtool[1][0] % + tuple([pkg.get(field) + for field in self.pkgtool[1][1]]))) if cmdrc[0] == 0: states[pkg] = True else: - self.logger.error("Failed to install package %s" % \ + self.logger.error("Failed to install package %s" % (pkg.get('name'))) self.RefreshPackages() for entry in [ent for ent in packages if states[ent]]: @@ -294,6 +315,9 @@ class PkgTool(Tool): type=self.pkgtype, version=version) \ for (name, version) in extras] + Remove = RemovePackages + FindExtra = FindExtraPackages + class SvcTool(Tool): """This class defines basic Service behavior""" @@ -308,19 +332,23 @@ class SvcTool(Tool): return '/etc/init.d/%s %s' % (service.get('name'), action) def start_service(self, service): + """ Start a service """ self.logger.debug('Starting service %s' % service.get('name')) return self.cmd.run(self.get_svc_command(service, 'start'))[0] def stop_service(self, service): + """ Stop a service """ self.logger.debug('Stopping service %s' % service.get('name')) return self.cmd.run(self.get_svc_command(service, 'stop'))[0] def restart_service(self, service): + """ Restart a service """ self.logger.debug('Restarting service %s' % service.get('name')) restart_target = service.get('target', 'restart') return self.cmd.run(self.get_svc_command(service, restart_target))[0] def check_service(self, service): + """ Get the status of a service """ return self.cmd.run(self.get_svc_command(service, 'status'))[0] == 0 def Remove(self, services): @@ -342,10 +370,10 @@ class SvcTool(Tool): not self.setup['interactive'])): continue - rc = None + rv = None if entry.get('status') == 'on': if self.setup['servicemode'] == 'build': - rc = self.stop_service(entry) + rv = self.stop_service(entry) elif entry.get('name') not in self.restarted: if self.setup['interactive']: prompt = ('Restart service %s?: (y/N): ' % @@ -353,30 +381,26 @@ class SvcTool(Tool): ans = input(prompt) if ans not in ['y', 'Y']: continue - rc = self.restart_service(entry) - if not rc: + rv = self.restart_service(entry) + if not rv: self.restarted.append(entry.get('name')) else: - rc = self.stop_service(entry) - if rc: + rv = self.stop_service(entry) + if rv: self.logger.error("Failed to manipulate service %s" % (entry.get('name'))) def Install(self, entries, states): """Install all entries in sublist.""" + install_entries = [] for entry in entries: if entry.get('install', 'true').lower() == 'false': self.logger.info("Service %s installation is false. Skipping " "installation." % (entry.get('name'))) - continue - try: - func = getattr(self, "Install%s" % (entry.tag)) - states[entry] = func(entry) - if states[entry]: - self.modified.append(entry) - except: - self.logger.error("Unexpected failure of install method for entry type %s" - % (entry.tag), exc_info=1) + else: + install_entries.append(entry) + return Tool.Install(self, install_entries, states) def InstallService(self, entry): + """ Install a single service entry """ raise NotImplementedError diff --git a/src/sbin/bcfg2-info b/src/sbin/bcfg2-info index f67850587..3d1e8bc76 100755 --- a/src/sbin/bcfg2-info +++ b/src/sbin/bcfg2-info @@ -701,7 +701,6 @@ class infoCore(cmd.Cmd, Bcfg2.Server.Core.BaseCore): if __name__ == '__main__': optinfo = dict(profile=Bcfg2.Options.CORE_PROFILE, - mconnect=Bcfg2.Options.SERVER_MCONNECT, interactive=Bcfg2.Options.INTERACTIVE, interpreter=Bcfg2.Options.INTERPRETER) optinfo.update(Bcfg2.Options.CLI_COMMON_OPTIONS) diff --git a/testsuite/Testsrc/test_code_checks.py b/testsuite/Testsrc/test_code_checks.py index 5f956cf3b..0436063e1 100644 --- a/testsuite/Testsrc/test_code_checks.py +++ b/testsuite/Testsrc/test_code_checks.py @@ -15,12 +15,6 @@ while _path != '/': _path = os.path.dirname(_path) from common import can_skip, skip, skipIf, skipUnless, Bcfg2TestCase -try: - import django - HAS_DJANGO = True -except ImportError: - HAS_DJANGO = False - # path to Bcfg2 src directory srcpath = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..", "src")) @@ -37,32 +31,34 @@ except OSError: HAS_PYLINT = False -# perform a full range of code checks on the listed files. -full_checks = { - "lib/Bcfg2": ["*.py"], - "lib/Bcfg2/Server": ["Lint", - "Plugin", - "FileMonitor", - "*.py"], - "lib/Bcfg2/Server/Plugins": ["Cfg", "Packages", "*.py"], - "lib/Bcfg2/Client": ["*.py"], - "lib/Bcfg2/Client/Tools": ["POSIX", "SELinux.py"], +# perform error checks only on the listed executables +sbin_error_checks = { + "sbin": ["bcfg2", "bcfg2-build-reports", "bcfg2-info", "bcfg2-admin", + "bcfg2-reports"] } -# perform full code checks on the listed executables -sbin_checks = { - "sbin": ["bcfg2-server", "bcfg2-yum-helper", "bcfg2-crypt", "bcfg2-test", - "bcfg2-lint"] - } - -# perform limited, django-safe checks on the listed files -django_checks = { - "lib/Bcfg2/Server": ["Reports", "models.py"] - } +# perform checks on the listed files only if the module listed in the +# keys can be imported +contingent_checks = dict( + django={"lib/Bcfg2/Server": ["Reports", "SchemaUpdater", "models.py"]}, + pyinotify={"lib/Bcfg2/Server/FileMonitor": ["Inotify.py"]}, + yum={"lib/Bcfg2/Client/Tools": ["YUM*"]} + ) # perform only error checking on the listed files error_checks = { "lib/Bcfg2": ["Proxy.py", "SSLServer.py"], + "lib/Bcfg2/Server": ["Admin", "Reports", "SchemaUpdater"], + "lib/Bcfg2/Client/Tools": ["launchd.py", + "OpenCSW.py", + "Blast.py", + "SYSV.py", + "FreeBSDInit.py", + "DebInit.py", + "RcUpdate.py", + "VCS.py", + "YUM.py", + "YUM24.py"], "lib/Bcfg2/Server/Plugins": ["Decisions.py", "Deps.py", "Ldap.py", @@ -73,7 +69,7 @@ error_checks = { # perform no checks at all on the listed files no_checks = { - "lib/Bcfg2/Client/Tools": ["APT.py", "RPMng.py", "rpmtools.py"], + "lib/Bcfg2/Client/Tools": ["APT.py", "RPM.py", "rpmtools.py"], "lib/Bcfg2/Server": ["Snapshots", "Hostbase"], "lib/Bcfg2": ["manage.py"], "lib/Bcfg2/Server/Reports": ["manage.py"], @@ -88,6 +84,17 @@ no_checks = { } +try: + any +except NameError: + def any(iterable): + """ implementation of builtin any() for python 2.4 """ + for element in iterable: + if element: + return True + return False + + def expand_path_dict(pathdict): """ given a path dict as above, return a list of all the paths """ rv = [] @@ -97,6 +104,33 @@ def expand_path_dict(pathdict): return rv +def whitelist_filter(filelist, whitelist): + rv = [] + for fpath in filelist: + if fpath in whitelist: + rv.append(fpath) + continue + # check if the path is in any directories that are in the + # whitelist + if any(fpath.startswith(wpath + "/") for wpath in whitelist): + rv.append(fpath) + continue + return rv + + +def blacklist_filter(filelist, blacklist): + rv = [] + for fpath in filelist: + if fpath in blacklist: + continue + # check that the path isn't in any directories that are in + # the blacklist + if any(fpath.startswith(bpath + "/") for bpath in blacklist): + continue + rv.append(fpath) + return rv + + class TestPylint(Bcfg2TestCase): pylint_cmd = ["pylint", "--rcfile", rcfile, "--init-hook", "import sys;sys.path.append('%s')" % @@ -108,33 +142,44 @@ class TestPylint(Bcfg2TestCase): # build the blacklist blacklist = expand_path_dict(no_checks) - def _get_paths(self, pathdict): - return list(set(expand_path_dict(pathdict)) - set(self.blacklist)) - @skipIf(not os.path.exists(srcpath), "%s does not exist" % srcpath) @skipIf(not os.path.exists(rcfile), "%s does not exist" % rcfile) @skipUnless(HAS_PYLINT, "pylint not found, skipping") def test_lib_full(self): - full_list = list((set(self._get_paths(full_checks)) - - set(expand_path_dict(error_checks))) - - set(expand_path_dict(django_checks))) + blacklist = expand_path_dict(error_checks) + self.blacklist + for filedict in contingent_checks.values(): + blacklist += expand_path_dict(filedict) + full_list = [] + for root, _, files in os.walk(os.path.join(srcpath, "lib")): + full_list.extend(blacklist_filter([os.path.join(root, f) + for f in files + if f.endswith(".py")], + blacklist)) self._pylint_full(full_list) - @skipUnless(HAS_DJANGO, "Django not found, skipping") @skipIf(not os.path.exists(srcpath), "%s does not exist" % srcpath) @skipIf(not os.path.exists(rcfile), "%s does not exist" % rcfile) @skipUnless(HAS_PYLINT, "pylint not found, skipping") - def test_django_full(self): - test_list = list(set(self._get_paths(full_checks)) & - set(expand_path_dict(django_checks))) - return self._pylint_errors(test_list, - extra_args=["-d", "E1101"]) + def test_contingent_full(self): + blacklist = set(expand_path_dict(error_checks) + self.blacklist) + for (mod, filedict) in contingent_checks.items(): + try: + __import__(mod) + except ImportError: + continue + self._pylint_full(blacklist_filter(expand_path_dict(filedict), + blacklist)) @skipIf(not os.path.exists(srcpath), "%s does not exist" % srcpath) @skipIf(not os.path.exists(rcfile), "%s does not exist" % rcfile) @skipUnless(HAS_PYLINT, "pylint not found, skipping") def test_sbin_full(self): - self._pylint_full(self._get_paths(sbin_checks), + all_sbin = [os.path.join(srcpath, "sbin", f) + for f in glob.glob(os.path.join(srcpath, "sbin", "*"))] + sbin_list = blacklist_filter([f for f in all_sbin + if not os.path.islink(f)], + expand_path_dict(sbin_error_checks)) + self._pylint_full(sbin_list, extra_args=["--module-rgx", "[a-z_-][a-z0-9_-]*$"]) @@ -152,27 +197,29 @@ class TestPylint(Bcfg2TestCase): @skipIf(not os.path.exists(rcfile), "%s does not exist" % rcfile) @skipUnless(HAS_PYLINT, "pylint not found, skipping") def test_sbin_errors(self): - flist = list(set(os.path.join(srcpath, p) - for p in glob.glob("sbin/*")) - set(self.blacklist)) - return self._pylint_errors(flist) + return self._pylint_errors(expand_path_dict(sbin_error_checks)) - @skipUnless(HAS_DJANGO, "Django not found, skipping") @skipIf(not os.path.exists(srcpath), "%s does not exist" % srcpath) @skipIf(not os.path.exists(rcfile), "%s does not exist" % rcfile) @skipUnless(HAS_PYLINT, "pylint not found, skipping") - def test_django_errors(self): - return self._pylint_errors(self._get_paths(django_checks), - extra_args=["-d", "E1101"]) + def test_contingent_errors(self): + whitelist = expand_path_dict(error_checks) + for (mod, filedict) in contingent_checks.items(): + try: + __import__(mod) + except ImportError: + continue + flist = \ + blacklist_filter(whitelist_filter(expand_path_dict(filedict), + whitelist), + self.blacklist) + self._pylint_errors(flist) @skipIf(not os.path.exists(srcpath), "%s does not exist" % srcpath) @skipIf(not os.path.exists(rcfile), "%s does not exist" % rcfile) @skipUnless(HAS_PYLINT, "pylint not found, skipping") def test_lib_errors(self): - ignore = [] - for fname_list in django_checks.values() + no_checks.values(): - ignore.extend(fname_list) - return self._pylint_errors(["lib/Bcfg2"], - extra_args=["--ignore", ",".join(ignore)]) + return self._pylint_errors(expand_path_dict(error_checks)) def _pylint_errors(self, paths, extra_args=None): """ test all files for fatals and errors """ diff --git a/testsuite/pylintrc.conf b/testsuite/pylintrc.conf index 250929163..63cc8eed8 100644 --- a/testsuite/pylintrc.conf +++ b/testsuite/pylintrc.conf @@ -33,7 +33,7 @@ load-plugins= # can either give multiple identifier separated by comma (,) or put this option # multiple time (only on the command line, not in the configuration file where # it should appear only once). -disable=F0401,W0142,W0511,W0603,W1201,R0201,R0901,R0902,R0903,R0904,R0921,R0922,C0302,I0011 +disable=F0401,W0142,W0511,W0603,W1201,R0201,R0801,R0901,R0902,R0903,R0904,R0921,R0922,C0302,I0011 [REPORTS] @@ -112,7 +112,7 @@ ignore-mixin-members=yes # List of classes names for which member attributes should not be checked # (useful for classes with attributes dynamically set). -ignored-classes=SQLObject +ignored-classes=ForeignKey,Interaction # When zope mode is activated, add a predefined set of Zope acquired attributes # to generated-members. |