Use configparser to read configuration file

With previous solution, provided parameters like --debug or
--insecure requires additional boolean parameter, which normally
it should not be required.

Change-Id: I2ecedc9d9c3610b08a6d3b9a4e5a5727a3e6e3dd

ansible/roles/logscraper/tasks/service.yml

@@ -15,16 +15,21 @@
     mode: '0755'
   register: _start_script
 
-- name: Generate logscraper download file list
+- name: Generate logscraper config file
   template:
-    src: config.yaml.j2
+    src: config.j2
-    dest: "{{ logscraper_dir }}/logscraper-{{ item.tenant }}.yaml"
+    dest: "{{ logscraper_dir }}/logscraper-{{ item.tenant }}.config"
     owner: "{{ logscraper_user }}"
     group: "{{ logscraper_group }}"
     mode: '0644'
   register: _config_file
 
-- name: Copy logscraper config file
+- name: Delete old logscraper config file
+  file:
+    path: "{{ logscraper_dir }}/logscraper-{{ item.tenant }}.yaml"
+    state: absent
+
+- name: Generate logscraper download file list
   template:
     src: download-list.yaml.j2
     dest: "{{ logscraper_dir }}/download-list-{{ item.tenant }}.yaml"

ansible/roles/logscraper/templates/config.j2

@@ -1,4 +1,4 @@
----
+[DEFAULT]
 zuul_api_url: {{ item['zuul_api_url'].split(', ') }}
 follow: {{ item['follow'] | default(true) }}
 checkpoint_file: {{ item['checkpoint_file'] | default(logscraper_dir + '/checkpoint') }}

ansible/roles/logscraper/templates/logscraper.sh.j2

@@ -15,4 +15,4 @@
     --volume {{ item.download_dir }}:{{ item.download_dir }}:z \
     {% endif %}
     {{ container_images['logscraper'] }} \
-    /usr/local/bin/logscraper --config {{ logscraper_dir }}/logscraper-{{ item['tenant'] }}.yaml
+    /usr/local/bin/logscraper --config {{ logscraper_dir }}/logscraper-{{ item['tenant'] }}.config

ansible/roles/logsender/tasks/service.yml

@@ -8,11 +8,16 @@
 
 - name: Generate logsender configuration file
   template:
-    src: config.yaml.j2
+    src: config.j2
-    dest: "{{ logscraper_dir }}/logsender-{{ item.tenant }}.yaml"
+    dest: "{{ logscraper_dir }}/logsender-{{ item.tenant }}.config"
     mode: '0644'
   register: _config_file
 
+- name: Remove old logsender configuration file
+  file:
+    path: "{{ logscraper_dir }}/logsender-{{ item.tenant }}.yaml"
+    state: absent
+
 - name: Generate systemd unit
   template:
     src: logsender.service.j2

ansible/roles/logsender/templates/config.j2

@@ -1,4 +1,4 @@
----
+[DEFAULT]
 host: {{ item['es_host'] | default('localhost') }}
 port: {{ item['es_port'] | default(9200) }}
 username: {{ item['es_username'] | default('logstash') }}

ansible/roles/logsender/templates/logsender.sh.j2

@@ -13,4 +13,4 @@
     --volume {{ item['logsender_custom_ca_crt'] }}:{{ item['logsender_custom_ca_crt'] }}:z \
     {% endif %}
     {{ container_images['logsender'] }} \
-    /usr/local/bin/logsender --config {{ logscraper_dir }}/logsender-{{ item['tenant'] }}.yaml
+    /usr/local/bin/logsender --config {{ logscraper_dir }}/logsender-{{ item['tenant'] }}.config

logscraper/logscraper.conf.sample

@@ -1,20 +1,22 @@
----
+[DEFAULT]
-# logscraper
 zuul_api_url: ['https://zuul.opendev.org/api/tenant/openstack']
 job_name: []
-follow: true
+follow: True
 checkpoint_file: /tmp/logscraper-checkpoint
 workers: 1
 max_skipped: 1000
-debug: false
+debug: False
-download: true
+download: True
 file_list: logscraper/download-list.yaml.sample
 directory: /tmp/logscraper
 wait_time: 120
-insecure: false
+insecure: False
-ca_file: ""
+ca_file:
 monitoring_port: 9128
-#deprecated
+
-gearman_server: ""
+######################
-gearman_port: 4730
+# DEPRECATED OPTIONS #
-logstash_url: ""
+######################
+# gearman_server: localhost
+# gearman_port: 4730
+# logstash_url: https://localhost:9600

logscraper/logscraper.py

@@ -59,6 +59,7 @@ end_time.
 
 
 import argparse
+import configparser
 import datetime
 import gear
 import itertools
@@ -73,6 +74,7 @@ import sys
 import time
 import yaml
 
+from ast import literal_eval
 from concurrent.futures import ThreadPoolExecutor
 from distutils.version import StrictVersion as s_version
 from prometheus_client import Gauge
@@ -110,6 +112,19 @@ def is_zuul_host_up(url, verify=True):
         pass
 
 
+def _verify_ca(args):
+    """Return path for CA cert file otherwise boolean value
+
+    When insecure argument is set to True, certification validation
+    needs to be False.
+    """
+
+    if args.ca_file:
+        return args.ca_file
+    else:
+        return not args.insecure
+
+
 ###############################################################################
 #                                    CLI                                      #
 ###############################################################################
@@ -120,16 +135,17 @@ def get_arguments():
                         required=True)
     parser.add_argument("--file-list", help="File list to download")
     parser.add_argument("--zuul-api-url", help="URL(s) for Zuul API. Parameter"
-                        " can be set multiple times.", action='append')
+                        " can be set multiple times.", nargs='+', default=[])
     parser.add_argument("--job-name", help="CI job name(s). Parameter can be "
                         "set multiple times. If not set it would scrape "
-                        "every latest builds.", action='append')
+                        "every latest builds", nargs='+', default=[])
     parser.add_argument("--gearman-server", help="Gearman host address")
-    parser.add_argument("--gearman-port", help="Gearman listen port.")
+    parser.add_argument("--gearman-port", help="Gearman listen port.",
-    parser.add_argument("--follow", help="Keep polling zuul builds", type=bool,
+                        type=int)
-                        default=True)
+    parser.add_argument("--follow", help="Keep polling zuul builds",
+                        action="store_true")
     parser.add_argument("--insecure", help="Skip validating SSL cert",
-                        action="store_false")
+                        action="store_true")
     parser.add_argument("--checkpoint-file", help="File that will keep "
                         "information about last uuid timestamp for a job.")
     parser.add_argument("--logstash-url", help="When provided, script will "
@@ -140,53 +156,47 @@ def get_arguments():
                         type=int)
     parser.add_argument("--max-skipped", help="How many job results should be "
                         "checked until last uuid written in checkpoint file "
-                        "is founded")
+                        "is founded",
-    parser.add_argument("--debug", help="Print more information", type=bool,
+                        type=int)
-                        default=False)
+    parser.add_argument("--debug", help="Print more information",
+                        action="store_true")
     parser.add_argument("--download", help="Download logs and do not send "
-                        "to gearman service")
+                        "to gearman service",
+                        action="store_true")
     parser.add_argument("--directory", help="Directory, where the logs will "
                         "be stored.")
     parser.add_argument("--wait-time", help="Pause time for the next "
-                        "iteration", type=int)
+                        "iteration",
+                        type=int)
     parser.add_argument("--ca-file", help="Provide custom CA certificate")
     parser.add_argument("--monitoring-port", help="Expose an Prometheus "
                         "exporter to collect monitoring metrics."
-                        "NOTE: When no port set, monitoring will be disabled.",
+                        "NOTE: When no port set, monitoring will be disabled.")
-                        type=int)
     args = parser.parse_args()
+
+    defaults = {}
+    if args.config:
+        config = configparser.ConfigParser(delimiters=('=', ':'))
+        config.read(args.config)
+        defaults = config["DEFAULT"]
+        defaults = dict(defaults)
+
+    parsed_values = {}
+    for k, v in defaults.items():
+        if not v:
+            continue
+        try:
+            parsed_values[k] = literal_eval(v)
+        except (SyntaxError, ValueError):
+            pass
+
+    parser.set_defaults(**defaults)
+    parser.set_defaults(**parsed_values)
+    args = parser.parse_args()
+
     return args
 
 
-def get_config_args(config_path):
-    config_file = load_config(config_path)
-    if config_file:
-        return config_file
-
-
-def parse_args(app_args, config_args):
-    if not config_args:
-        logging.warning("Can not get information from config files")
-
-    if not config_args:
-        print("The config file is necessary to provide!")
-        sys.exit(1)
-
-    # NOTE: When insecure flag is set as an argument, the value is False,
-    # so if insecure is set to True in config file, it should also be False.
-    if not getattr(app_args, 'insecure') or (
-            'insecure' in config_args and config_args['insecure']):
-        setattr(app_args, 'insecure', False)
-
-    for k, v in config_args.items():
-        # Arguments provided via CLI should have higher priority than
-        # provided in config.
-        if getattr(app_args, k, None) is None:
-            setattr(app_args, k, v)
-
-    return app_args
-
-
 ###############################################################################
 #                      Configuration of this process                          #
 ###############################################################################
@@ -666,8 +676,10 @@ def run_build(build):
             logging.critical("Exception occurred %s on creating dir %s" % (
                 e, directory))
 
+        validate_ca = _verify_ca(args)
+
         if is_job_with_result(build):
-            check_specified_files(build, args.insecure, directory)
+            check_specified_files(build, validate_ca, directory)
         else:
             # NOTE: if build result is "ABORTED" or "NODE_FAILURE, there is
             # no any job result files to parse, but we would like to have that
@@ -684,8 +696,9 @@ def run_build(build):
         # NOTE: As it was earlier, logs that contains status other than
         # "SUCCESS" or "FAILURE" will be parsed by Gearman service.
         logging.debug("Parsing content for gearman service")
+        validate_ca = _verify_ca(args)
         results = dict(files=[], jobs=[], invocation={})
-        files = check_specified_files(build, args.insecure)
+        files = check_specified_files(build, validate_ca)
 
         results["files"] = files
         lmc = LogMatcher(
@@ -720,7 +733,8 @@ def run_scraping(args, zuul_api_url, job_name=None, monitoring=None):
     config = Config(args, zuul_api_url, job_name)
 
     builds = []
-    for build in get_last_job_results(zuul_api_url, args.insecure,
+    validate_ca = _verify_ca(args)
+    for build in get_last_job_results(zuul_api_url, validate_ca,
                                       args.max_skipped, config.build_cache,
                                       job_name):
         logging.debug("Working on build %s" % build['uuid'])
@@ -750,10 +764,7 @@ def run_scraping(args, zuul_api_url, job_name=None, monitoring=None):
 
 
 def run(args, monitoring):
-    if args.ca_file:
+    validate_ca = _verify_ca(args)
-        validate_ca = args.ca_file
-    else:
-        validate_ca = args.insecure
 
     for zuul_api_url in args.zuul_api_url:
 
@@ -779,10 +790,7 @@ def run(args, monitoring):
 
 
 def main():
-    app_args = get_arguments()
+    args = get_arguments()
-    config_args = get_config_args(app_args.config)
-    args = parse_args(app_args, config_args)
-
     setup_logging(args.debug)
 
     monitoring = None

logscraper/logsender.conf.sample

@@ -1,20 +1,20 @@
----
+[DEFAULT]
 directory: /tmp/logscraper
 host: localhost
 port: 9200
 username: logstash
-password: ""
+password:
-index_prefix: "logstash-"
+index_prefix: logstash-
-index: ""
+index:
 chunk_size: 1500
 skip_debug: true
 keep: false
 debug: false
 wait_time: 120
 insecure: false
-ca_file: ""
+ca_file:
 follow: true
 workers: 1
 file_list: logscraper/download-list.yaml.sample
-performance_index_prefix: ""
+performance_index_prefix: performance-
-subunit_index_prefix: ""
+subunit_index_prefix: subunit-

logscraper/logsender.py

@@ -20,6 +20,7 @@ The goal is to get content from build uuid directory and send to Opensearch
 """
 
 import argparse
+import configparser
 import copy
 import datetime
 import itertools
@@ -32,14 +33,7 @@ import shutil
 import sys
 import time
 
-# FIXME: discover why stestr in tox env can not import base lib
+from ast import literal_eval
-try:
-    from logscraper import get_config_args
-    from logscraper import parse_args
-except ImportError:
-    from logscraper.logscraper import get_config_args
-    from logscraper.logscraper import parse_args
-
 from opensearchpy import exceptions as opensearch_exceptions
 from opensearchpy import helpers
 from opensearchpy import OpenSearch
@@ -63,31 +57,56 @@ def get_arguments():
     parser.add_argument("--port", help="Opensearch port", type=int)
     parser.add_argument("--username", help="Opensearch username")
     parser.add_argument("--password", help="Opensearch user password")
-    parser.add_argument("--index-prefix", help="Prefix for the index.")
+    parser.add_argument("--index-prefix", help="Prefix for the index.",
+                        default="logstash-")
     parser.add_argument("--index", help="Opensearch index")
     parser.add_argument("--performance-index-prefix", help="Prefix for the"
                         "index that will proceed performance.json file"
-                        "NOTE: it will use same opensearch user credentials")
+                        "NOTE: it will use same opensearch user credentials",
+                        default="performance-")
     parser.add_argument("--subunit-index-prefix", help="Prefix for the"
                         "index that will proceed testrepository.subunit file"
-                        "NOTE: it will use same opensearch user credentials")
+                        "NOTE: it will use same opensearch user credentials",
+                        default="subunit-")
     parser.add_argument("--insecure", help="Skip validating SSL cert",
-                        action="store_false")
+                        action="store_true")
-    parser.add_argument("--follow", help="Keep sending CI logs", type=bool,
+    parser.add_argument("--follow", help="Keep sending CI logs",
-                        default=True)
+                        action="store_true")
     parser.add_argument("--workers", help="Worker processes for logsender",
                         type=int)
     parser.add_argument("--chunk-size", help="The bulk chunk size", type=int)
     parser.add_argument("--skip-debug", help="Skip messages that contain: "
-                        "DEBUG word", type=bool, default=True)
+                        "DEBUG word",
+                        action="store_true")
     parser.add_argument("--keep", help="Do not remove log directory after",
                         type=bool)
-    parser.add_argument("--debug", help="Be more verbose", type=bool,
+    parser.add_argument("--debug", help="Be more verbose",
-                        default=False)
+                        action="store_true")
     parser.add_argument("--wait-time", help="Pause time for the next "
                         "iteration", type=int)
     parser.add_argument("--ca-file", help="Provide custom CA certificate")
     args = parser.parse_args()
+
+    defaults = {}
+    if args.config:
+        config = configparser.ConfigParser(delimiters=('=', ':'))
+        config.read(args.config)
+        defaults = config["DEFAULT"]
+        defaults = dict(defaults)
+
+    parsed_values = {}
+    for k, v in defaults.items():
+        if not v:
+            continue
+        try:
+            parsed_values[k] = literal_eval(v)
+        except (SyntaxError, ValueError):
+            pass
+
+    parser.set_defaults(**defaults)
+    parser.set_defaults(**parsed_values)
+    args = parser.parse_args()
+
     return args
 
 
@@ -548,8 +567,8 @@ def get_es_client(args):
             "port": args.port,
             "http_compress": True,
             "use_ssl": True,
-            "verify_certs": args.insecure,
+            "verify_certs": not args.insecure,
-            "ssl_show_warn": args.insecure,
+            "ssl_show_warn": not args.insecure,
         }
 
     if args.username and args.password:
@@ -572,9 +591,7 @@ def run(args):
 
 
 def main():
-    app_args = get_arguments()
+    args = get_arguments()
-    config_args = get_config_args(app_args.config)
-    args = parse_args(app_args, config_args)
     setup_logging(args.debug)
     while True:
         run(args)

logscraper/opensearch_dashboards_backup.py

@@ -57,7 +57,7 @@ def get_arguments():
                              default=os.path.dirname(
                                  os.path.realpath(__file__)))
     args_parser.add_argument('--no-resolve-conflicts',
-                             default=False,
+                             action="store_true",
                              help='Resolve conflicts by removing index '
                              'id reference in backup file')
     args_parser.add_argument('--overwrite-index-pattern',
@@ -65,7 +65,7 @@ def get_arguments():
                              help='WARNING: Use that option if you want'
                              'to restart also index pattern')
     args_parser.add_argument('--insecure',
-                             action='store_false',
+                             action='store_true',
                              help='Use that option to ignore if SSL cert '
                              'has been verified by root CA')
     args_parser.add_argument('--tenant',
@@ -90,8 +90,7 @@ def get_arguments():
     args_parser.add_argument('--ca-file',
                              help='Custom CA certificate file')
     args_parser.add_argument("--debug", help="Print more information",
-                             type=bool,
+                             action="store_true")
-                             default=False)
     return args_parser.parse_args()
 
 

logscraper/tests/test_logscraper.py

@@ -150,7 +150,7 @@ class FakeArgs(object):
                  logstash_url=None, workers=None, max_skipped=None,
                  job_name=None, download=None, directory=None,
                  config=None, wait_time=None, ca_file=None,
-                 file_list=None, monitoring_port=None):
+                 file_list=None, monitoring_port=None, debug=None):
 
         self.zuul_api_url = zuul_api_url
         self.gearman_server = gearman_server
@@ -170,6 +170,7 @@ class FakeArgs(object):
         self.ca_file = ca_file
         self.file_list = file_list
         self.monitoring_port = monitoring_port
+        self.debug = debug
 
 
 class TestScraper(base.TestCase):
@@ -183,6 +184,22 @@ class TestScraper(base.TestCase):
             }]
         }
 
+    @mock.patch('argparse.ArgumentParser.parse_args')
+    def test_get_arguments(self, mock_args):
+        mock_args.return_value = FakeArgs(
+            zuul_api_url='somehost.com',
+            debug=True,
+            insecure=False,
+            config='/tmp/somefile.conf'
+        )
+        m = mock.mock_open(read_data="[DEFAULT]\ndebug: False\n"
+                           "insecure: True")
+        with mock.patch('builtins.open', m) as mocked_open:
+            args = logscraper.get_arguments()
+            self.assertEqual(True, args.debug)
+            self.assertEqual(False, args.insecure)
+            mocked_open.assert_called_once()
+
     def test_parse_version(self):
         ver1 = logscraper.parse_version('4.6.0-1.el7')
         ver2 = logscraper.parse_version('4.10.2.dev6-22f04be1')

logscraper/tests/test_logsender.py

@@ -304,6 +304,27 @@ class FakeArgs(object):
 
 class TestSender(base.TestCase):
 
+    @mock.patch('argparse.ArgumentParser.parse_args')
+    def test_get_arguments(self, mock_args):
+        mock_args.return_value = FakeArgs(
+            host='somehost.com',
+            debug=True,
+            insecure=False,
+            config='/tmp/somefile.conf',
+            port=9200,
+            subunit_index_prefix='test-'
+        )
+        m = mock.mock_open(read_data="[DEFAULT]\ndebug: False\n"
+                           "insecure: True\nport: 9000\n"
+                           "subunit_index_prefix: subunit-")
+        with mock.patch('builtins.open', m) as mocked_open:
+            args = logsender.get_arguments()
+            self.assertEqual(True, args.debug)
+            self.assertEqual(False, args.insecure)
+            self.assertEqual(9200, args.port)
+            self.assertEqual('test-', args.subunit_index_prefix)
+            mocked_open.assert_called_once()
+
     @mock.patch('logscraper.logsender.get_file_info')
     @mock.patch('logscraper.logsender.remove_directory')
     @mock.patch('logscraper.logsender.send_to_es')

setup.py

@@ -19,6 +19,6 @@ setuptools.setup(
     pbr=True,
     include_package_data=True,
     package_data={'logscraper': ['download-list.yaml.sample',
-                                 'logscraper.yaml.sample',
+                                 'logscraper.conf.sample',
-                                 'logsender.yaml.sample']}
+                                 'logsender.conf.sample']}
 )