Sandro Bonazzola has uploaded a new change for review. Change subject: tools: fixed command line parser setup ......................................................................
tools: fixed command line parser setup The previous command line parser setup caused the rewrite of the database connection parameters read from .pgpass with the hardcoded defaults each time the command line was parsed. This caused a failure while collecting PostgreSQL database configuration when using a remote setup Now the script check if we have root permission, then read .pgpass setting the default values for the command line parser. In order to figure out what was happening, I added some logging to the __main__ module and to the posgresql sos plugin. Change-Id: I284554888143ad27bca0630cdbc26e91cf735ab9 Bug-Url: https://bugzilla.redhat.com/885421 Signed-off-by: Sandro Bonazzola <[email protected]> --- M src/__main__.py M src/sos/plugins/postgresql.py 2 files changed, 95 insertions(+), 70 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-log-collector refs/changes/29/12729/1 diff --git a/src/__main__.py b/src/__main__.py index 634e61d..62e03a2 100755 --- a/src/__main__.py +++ b/src/__main__.py @@ -57,6 +57,59 @@ t = gettext.translation('logcollector', fallback=True) _ = t.ugettext + +def get_pg_var(dbconf_param, user=None): + ''' + Provides a mechanism to extract information from .pgpass. + ''' + field = {'pass': 4, 'admin': 3, 'host': 0, 'port': 1} + if dbconf_param not in field.keys(): + raise Exception("Error: unknown value type '%s' was requested" % \ + dbconf_param) + inDbAdminSection = False + if (os.path.exists(FILE_PG_PASS)): + logging.debug("found existing pgpass file, fetching DB %s value" % \ + dbconf_param) + with open(FILE_PG_PASS) as pgPassFile: + for line in pgPassFile: + + # find the line with "DB ADMIN" + if PGPASS_FILE_ADMIN_LINE in line: + inDbAdminSection = True + continue + + if inDbAdminSection and not line.startswith("#"): + # Means we're on DB ADMIN line, as it's for all DBs + dbcreds = line.split(":", 4) + return str(dbcreds[field[dbconf_param]]).strip() + + # Fetch the password if needed + if dbconf_param == "pass" and user \ + and not line.startswith("#"): + dbcreds = line.split(":", 4) + if dbcreds[3] == user: + return dbcreds[field[dbconf_param]] + return None + + +def setup_pg_defaults(): + """ + Set defaults value to those read from .pgpass + """ + global pg_user + global pg_pass + global pg_dbhost + global pg_dbport + try: + pg_user = get_pg_var('admin') or pg_user + pg_pass = get_pg_var('pass', pg_user) or pg_pass + pg_dbhost = get_pg_var('host') or pg_dbhost + pg_dbport = get_pg_var('port') or pg_dbport + except Exception as e: + print "Warning: error while reading .pgpass configuration: %s" % str(e) + ExitCodes.exit_code = ExitCodes.WARN + + def multilog(logger, msg): for line in str(msg).splitlines(): logger(line) @@ -96,6 +149,7 @@ def call(self, cmds): """Uses the configuration to fork a subprocess and run cmds.""" _cmds = self.prep(cmds) + logging.debug("calling(%s)" % _cmds) proc = subprocess.Popen(_cmds, stdout=subprocess.PIPE, stderr=subprocess.PIPE) @@ -137,9 +191,6 @@ raise Exception("Configuration requires a parser") self.options, self.args = self.parser.parse_args() - if os.geteuid() != 0: - raise Exception("This tool requires root permissions to run.") - # At this point we know enough about the command line options # to test for verbose and if it is set we should re-initialize # the logger to DEBUG. This will have the effect of printing @@ -147,7 +198,6 @@ if getattr(self.options, "verbose"): self.__initLogger(logging.DEBUG) - self.set_pg_db_attrs() self.load_config_file() if self.options: @@ -172,58 +222,6 @@ self.__initLogger(level, self.options.quiet, self.options.log_file) def __missing__(self, key): - return None - - def set_pg_db_attrs(self): - ''' - This method will check for /etc/ovirt-engin/.pgpass and set the - values for the admin user, admin user pw, pg host, and pg port - in the configuration dictionary. - - If values not found, use defaults. - ''' - - try: - self['pg_user'] = self._get_pg_var('admin') or pg_user - self['pg_pass'] = self._get_pg_var('pass', self['pg_user']) or pg_pass - self['pg_dbhost'] = self._get_pg_var('host') or pg_dbhost - self['pg_dbport'] = self._get_pg_var('port') or pg_dbport - except Exception, e: - logging.debug(str(e)) - - def _get_pg_var(self, dbconf_param, user=None): - ''' - Provides a mechanism to extract information from .pgpass. - ''' - field = {'pass': 4, 'admin': 3, 'host': 0, 'port': 1} - if dbconf_param not in field.keys(): - raise Exception("Error: unknown value type '%s' was requested" % \ - dbconf_param) - - inDbAdminSection = False - if (os.path.exists(FILE_PG_PASS)): - logging.debug("found existing pgpass file, fetching DB %s value" % \ - dbconf_param) - with open(FILE_PG_PASS) as pgPassFile: - for line in pgPassFile: - - # find the line with "DB ADMIN" - if PGPASS_FILE_ADMIN_LINE in line: - inDbAdminSection = True - continue - - if inDbAdminSection and not line.startswith("#"): - # Means we're on DB ADMIN line, as it's for all DBs - dbcreds = line.split(":", 4) - return str(dbcreds[field[dbconf_param]]).strip() - - # Fetch the password if needed - if dbconf_param == "pass" and user \ - and not line.startswith("#"): - dbcreds = line.split(":", 4) - if dbcreds[3] == user: - return dbcreds[field[dbconf_param]] - return None def load_config_file(self): @@ -641,7 +639,7 @@ def sosreport(self): opt = "" - if self.hostname == "localhost": + if self.configuration.get("pg_dbhost") == "localhost": if self.configuration.get("pg_pass"): opt = '-k postgresql.dbname=%(pg_dbname)s -k postgresql.username=%(pg_user)s -k postgresql.password=%(pg_pass)s' @@ -890,6 +888,11 @@ setattr(parser.values, option.dest, value) if __name__ == '__main__': + if os.geteuid() != 0: + print "This tool requires root permissions to run." + sys.exit(ExitCodes.CRITICAL) + + setup_pg_defaults() def comma_separated_list(option, opt_str, value, parser): setattr(parser.values, option.dest, [v.strip() for v in value.split(",")]) diff --git a/src/sos/plugins/postgresql.py b/src/sos/plugins/postgresql.py index 1088aaf..7befbb3 100644 --- a/src/sos/plugins/postgresql.py +++ b/src/sos/plugins/postgresql.py @@ -44,28 +44,35 @@ old_env_pgpassword = os.environ.get("PGPASSWORD") os.environ["PGPASSWORD"] = "%s" % (self.getOption("password")) if self.getOption("dbhost"): - (status, output, rtime) = self.callExtProg("pg_dump -U %s -h %s -p %s -w -f %s -F t %s" % - (self.__username, - self.getOption("dbhost"), - self.__dbport, - dest_file, - self.getOption("dbname"))) + cmd = "pg_dump -U %s -h %s -p %s -w -f %s -F t %s" % ( + self.__username, + self.getOption("dbhost"), + self.__dbport, + dest_file, + self.getOption("dbname") + ) else: - (status, output, rtime) = self.callExtProg("pg_dump -C -U %s -w -f %s -F t %s " % - (self.__username, - dest_file, - self.getOption("dbname"))) - + cmd = "pg_dump -C -U %s -w -f %s -F t %s " % ( + self.__username, + dest_file, + self.getOption("dbname") + ) + self.soslog.debug("calling %s" % cmd) + (status, output, rtime) = self.callExtProg(cmd) if old_env_pgpassword is not None: os.environ["PGPASSWORD"] = str(old_env_pgpassword) if (status == 0): self.addCopySpec(dest_file) else: + self.soslog.error( + "Unable to execute pg_dump. Error(%s)" % (output) + ) self.addAlert("ERROR: Unable to execute pg_dump. Error(%s)" % (output)) def setup(self): if self.getOption("pghome"): self.__pghome = self.getOption("pghome") + self.soslog.debug("using pghome=%s" % self.__pghome) if self.getOption("dbname"): if self.getOption("password"): @@ -76,7 +83,20 @@ self.tmp_dir = tempfile.mkdtemp() self.pg_dump() else: - self.addAlert("WARN: password must be supplied to dump a database.") + self.soslog.warning( + "password must be supplied to dump a database." + ) + self.addAlert( + "WARN: password must be supplied to dump a database." + ) + else: + self.soslog.warning( + "dbname must be supplied to dump a database." + ) + self.addAlert( + "WARN: dbname must be supplied to dump a database." + ) + # Copy PostgreSQL log files. for file in find("*.log", self.__pghome): @@ -88,10 +108,12 @@ self.addCopySpec(os.path.join(self.__pghome, "data" , "PG_VERSION")) self.addCopySpec(os.path.join(self.__pghome, "data" , "postmaster.opts")) - def postproc(self): import shutil try: shutil.rmtree(self.tmp_dir) except: + self.soslog.exception( + "Unable to remove %s." % (self.tmp_dir) + ) self.addAlert("ERROR: Unable to remove %s." % (self.tmp_dir)) -- To view, visit http://gerrit.ovirt.org/12729 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I284554888143ad27bca0630cdbc26e91cf735ab9 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-log-collector Gerrit-Branch: master Gerrit-Owner: Sandro Bonazzola <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
