From 3d1411f3a194a66e0000797d023a6de588fd1a4c Mon Sep 17 00:00:00 2001 From: Sorin Sbarnea Date: Wed, 19 Aug 2020 12:09:56 +0100 Subject: [PATCH] Enable configuration via environment variables Refactors configuration loading in order to simplify it and to allow overriding defaults using environment variables. This behavior is similar to other tools like pip or ansible, which can load any configurable option from env. This step ease migration towards containerized use, where we do not want to keep any secrets inside containers and we may want to avoid volume mounting, especially when testing. Change-Id: I0d3a9f19b0ba8d1604d0ca63db01296a3219fb47 --- elastic_recheck/bot.py | 2 +- elastic_recheck/config.py | 132 +++++++++++-------------- elastic_recheck/tests/unit/test_bot.py | 3 +- 3 files changed, 62 insertions(+), 75 deletions(-) diff --git a/elastic_recheck/bot.py b/elastic_recheck/bot.py index b4b9cec2..eca798d0 100755 --- a/elastic_recheck/bot.py +++ b/elastic_recheck/bot.py @@ -67,7 +67,7 @@ class RecheckWatchBot(irc.bot.SingleServerIRCBot): def __init__(self, channels, config): super(RecheckWatchBot, self).__init__( [(config.ircbot_server, - config.ircbot_port, + int(config.ircbot_port), config.ircbot_server_password)], config.ircbot_nick, config.ircbot_nick) diff --git a/elastic_recheck/config.py b/elastic_recheck/config.py index 4eba2acb..7d8fed48 100644 --- a/elastic_recheck/config.py +++ b/elastic_recheck/config.py @@ -16,19 +16,20 @@ import os import re import configparser -DEFAULT_INDEX_FORMAT = 'logstash-%Y.%m.%d' - -ES_URL = 'http://logstash.openstack.org:80/elasticsearch' -LS_URL = 'http://logstash.openstack.org' -DB_URI = 'mysql+pymysql://query:query@logstash.openstack.org/subunit2sql' - -JOBS_RE = 'dsvm' -CI_USERNAME = 'jenkins' - -GERRIT_QUERY_FILE = 'queries' -GERRIT_HOST = 'review.opendev.org' - -PID_FN = '/var/run/elastic-recheck/elastic-recheck.pid' +# Can be overriden by defining environment variables with same name +DEFAULTS = { + 'ES_URL': 'http://logstash.openstack.org:80/elasticsearch', + 'LS_URL': 'http://logstash.openstack.org', + 'DB_URI': 'mysql+pymysql://query:query@logstash.openstack.org/subunit2sql', + 'server_password': '', + 'CI_USERNAME': 'jenkins', + 'JOBS_RE': 'dsvm', + 'PID_FN': '/var/run/elastic-recheck/elastic-recheck.pid', + 'INDEX_FORMAT': r'logstash-%Y.%m.%d', + 'GERRIT_QUERY_FILE': 'queries', + 'GERRIT_HOST': 'review.opendev.org', + 'IRC_LOG_CONFIG': None +} # Not all teams actively used elastic recheck for categorizing their # work, so to keep the uncategorized page more meaningful, we exclude @@ -82,76 +83,61 @@ class Config(object): uncat_search_size=None, gerrit_query_file=None): - self.es_url = es_url or ES_URL - self.ls_url = ls_url or LS_URL - self.db_uri = db_uri or DB_URI - self.jobs_re = jobs_re or JOBS_RE - self.ci_username = ci_username or CI_USERNAME - self.es_index_format = es_index_format or DEFAULT_INDEX_FORMAT - self.pid_fn = pid_fn or PID_FN + # override defaults with environment variables + for key, val in os.environ.items(): + if key in DEFAULTS: + DEFAULTS[key] = val + + self.es_url = es_url or DEFAULTS['ES_URL'] + self.ls_url = ls_url or DEFAULTS['LS_URL'] + self.db_uri = db_uri or DEFAULTS['DB_URI'] + self.jobs_re = jobs_re or DEFAULTS['JOBS_RE'] + self.ci_username = ci_username or DEFAULTS['CI_USERNAME'] + self.es_index_format = es_index_format or DEFAULTS['INDEX_FORMAT'] + self.pid_fn = pid_fn or DEFAULTS['PID_FN'] self.ircbot_channel_config = None - self.irc_log_config = None + self.irc_log_config = DEFAULTS['IRC_LOG_CONFIG'] self.all_fails_query = all_fails_query or ALL_FAILS_QUERY self.excluded_jobs_regex = excluded_jobs_regex or EXCLUDED_JOBS_REGEX self.included_projects_regex = \ included_projects_regex or INCLUDED_PROJECTS_REGEX self.uncat_search_size = uncat_search_size or UNCAT_MAX_SEARCH_SIZE - self.gerrit_query_file = gerrit_query_file or GERRIT_QUERY_FILE + self.gerrit_query_file = (gerrit_query_file or + DEFAULTS['GERRIT_QUERY_FILE']) + self.gerrit_user = None + self.gerrit_host = None + self.gerrit_host_key = None if config_file or config_obj: if config_obj: config = config_obj else: - config = configparser.ConfigParser( - {'es_url': ES_URL, - 'ls_url': LS_URL, - 'db_uri': DB_URI, - 'server_password': '', - 'ci_username': CI_USERNAME, - 'jobs_re': JOBS_RE, - 'pidfile': PID_FN, - 'index_format': DEFAULT_INDEX_FORMAT, - 'query_file': GERRIT_QUERY_FILE - } - ) + config = configparser.ConfigParser() config.read(config_file) - - if config.has_section('data_source'): - self.es_url = config.get('data_source', 'es_url') - self.ls_url = config.get('data_source', 'ls_url') - self.db_uri = config.get('data_source', 'db_uri') - self.es_index_format = config.get('data_source', - 'index_format') - - if config.has_section('recheckwatch'): - self.ci_username = config.get('recheckwatch', 'ci_username') - self.jobs_re = config.get('recheckwatch', 'jobs_re') - - if config.has_section('gerrit'): - self.gerrit_user = config.get('gerrit', 'user') - self.gerrit_query_file = config.get('gerrit', 'query_file') - # workaround for python api change: - # https://docs.python.org/3/library/configparser.html#fallback-values + cfg_map = { + 'db_uri': ('data_source', 'db_uri'), + 'es_url': ('data_source', 'es_url'), + 'gerrit_host': ('gerrit', 'host'), + 'gerrit_host_key': ('gerrit', 'key'), + 'gerrit_query_file': ('gerrit', 'query_file'), + 'gerrit_user': ('gerrit', 'user'), + 'index_format': ('data_source', 'index_format'), + 'irc_log_config': ('ircbot', 'log_config'), + 'ircbot_channel_config': ('ircbot', 'channel_config'), + 'ircbot_server': ('ircbot', 'server_password'), + 'ircbot_sever_password': ('ircbot', 'port'), + 'jobs_re': ('recheckwatch', 'jobs_re'), + 'ls_url': ('data_source', 'ls_url'), + 'nick': ('ircbot', 'nick'), + 'pass': ('ircbot', 'pass'), + 'pid_fn': ('ircbot', 'pidfile'), + 'recheckwatch': ('recheckwatch', 'ci_username'), + } + for k, v in cfg_map.items(): try: - self.gerrit_host = config.get('gerrit', - 'host', - fallback=GERRIT_HOST) - except TypeError: - self.gerrit_host = config.get('gerrit', - 'host', - GERRIT_HOST) - self.gerrit_host_key = config.get('gerrit', 'key') - - if config.has_section('ircbot'): - self.pid_fn = os.path.expanduser(config.get('ircbot', - 'pidfile')) - self.ircbot_nick = config.get('ircbot', 'nick') - self.ircbot_pass = config.get('ircbot', 'pass') - self.ircbot_server = config.get('ircbot', 'server') - self.ircbot_port = config.getint('ircbot', 'port') - self.ircbot_server_password = config.get('ircbot', - 'server_password') - self.ircbot_channel_config = config.get('ircbot', - 'channel_config') - if config.has_option('ircbot', 'log_config'): - self.irc_log_config = config.get('ircbot', 'log_config') + if hasattr(self, k) and getattr(self, k) is None: + setattr(self, k, config.get(v[0], v[1])) + except ( + configparser.NoOptionError, + configparser.NoSectionError): + pass diff --git a/elastic_recheck/tests/unit/test_bot.py b/elastic_recheck/tests/unit/test_bot.py index 653cb809..bfc8cafd 100644 --- a/elastic_recheck/tests/unit/test_bot.py +++ b/elastic_recheck/tests/unit/test_bot.py @@ -30,7 +30,7 @@ def _set_fake_config(fake_config): fake_config.add_section('ircbot') fake_config.add_section('gerrit') # Set fake ircbot config - fake_config.set('ircbot', 'pidfile', er_conf.PID_FN) + fake_config.set('ircbot', 'pidfile', er_conf.DEFAULTS['PID_FN']) fake_config.set('ircbot', 'nick', 'Fake_User') fake_config.set('ircbot', 'pass', '') fake_config.set('ircbot', 'server', 'irc.fake.net') @@ -47,6 +47,7 @@ def _set_fake_config(fake_config): # NOTE(mtreinish) Using unittest here because testtools TestCase.assertRaises # doesn't support using it as a context manager class TestBot(unittest.TestCase): + def setUp(self): super(TestBot, self).setUp() self.fake_config = configparser.ConfigParser(