From 37a940424a32f3d9201a0f71469762487444acc7 Mon Sep 17 00:00:00 2001 From: Daniel Pawlik Date: Thu, 25 Jan 2024 10:53:39 +0100 Subject: [PATCH] Add feature: support multiple file-list arguments This update allows users to provide the download list parameter multiple times. This would helps avoid situations where only one file is modified, and users can now provide a list of files to be downloaded later by logscraper and parsed for tags by logsender. Change-Id: If9adeb5f9be1e7d79ac05076589374a045246668 --- ansible/roles/logscraper/README.rst | 5 +- ansible/roles/logscraper/defaults/main.yml | 2 +- ansible/roles/logscraper/templates/config.j2 | 2 +- .../logscraper/templates/logscraper.sh.j2 | 4 +- ansible/roles/logsender/README.rst | 5 +- ansible/roles/logsender/defaults/main.yml | 1 + ansible/roles/logsender/templates/config.j2 | 2 +- .../roles/logsender/templates/logsender.sh.j2 | 4 +- logscraper/logscraper.conf.sample | 2 +- logscraper/logscraper.py | 29 +++++++++- logscraper/logsender.conf.sample | 2 +- logscraper/logsender.py | 21 ++++--- logscraper/tests/test_logscraper.py | 55 +++++++++++++++++++ logscraper/tests/test_logsender.py | 7 ++- 14 files changed, 117 insertions(+), 24 deletions(-) diff --git a/ansible/roles/logscraper/README.rst b/ansible/roles/logscraper/README.rst index dff88f2..4563464 100644 --- a/ansible/roles/logscraper/README.rst +++ b/ansible/roles/logscraper/README.rst @@ -28,7 +28,7 @@ for example: zuul_api_url: - https://zuul.opendev.org/api/tenant/openstack insecure: false - file_list: /etc/logscraper/download-list-TENANT.yaml + file_list: ['/etc/logscraper/download-list-TENANT.yaml'] will deploy service with name: `logscraper@openstack.service`. It is because on one service we are able to deploy multiple instances @@ -68,7 +68,8 @@ and second one for getting logs from `sometenant` tenant. insecure: True download: true download_dir: /mnt/logscraper - file_list: /etc/logscraper/my-downloadlist.yaml + file_list: + - /etc/logscraper/my-downloadlist.yaml roles: - logscraper diff --git a/ansible/roles/logscraper/defaults/main.yml b/ansible/roles/logscraper/defaults/main.yml index d6f08d1..e66726e 100644 --- a/ansible/roles/logscraper/defaults/main.yml +++ b/ansible/roles/logscraper/defaults/main.yml @@ -24,7 +24,7 @@ container_images: # insecure: true # download: true # download_dir: /mnt/logscraper/sometenant -# file_list: "" +# file_list: [] # job_name: # - test # - test_new diff --git a/ansible/roles/logscraper/templates/config.j2 b/ansible/roles/logscraper/templates/config.j2 index 6618594..41fe3c6 100644 --- a/ansible/roles/logscraper/templates/config.j2 +++ b/ansible/roles/logscraper/templates/config.j2 @@ -6,7 +6,7 @@ workers: {{ item['logscraper_workers'] | default(1) }} max_skipped: {{ item['max_skipped'] | default(1000) }} debug: {{ item['debug'] | default(false) }} download: {{ item['download'] | default(true) }} -file_list: {{ item['file_list'] | default(logscraper_dir + '/download-list-' + item['tenant'] + '.yaml') }} +file_list: {{ item['file_list'] | default([logscraper_dir + '/download-list-' + item['tenant'] + '.yaml']) | list }} directory: {{ item['download_dir'] | default('/tmp/logscraper') }} wait_time: {{ item['logscraper_wait_time'] | default(120) }} insecure: {{ item['insecure'] | default(false) }} diff --git a/ansible/roles/logscraper/templates/logscraper.sh.j2 b/ansible/roles/logscraper/templates/logscraper.sh.j2 index 269934e..d030f12 100644 --- a/ansible/roles/logscraper/templates/logscraper.sh.j2 +++ b/ansible/roles/logscraper/templates/logscraper.sh.j2 @@ -8,7 +8,9 @@ --uidmap 1000:{{ logscraper_uid }}:1 \ --name logscraper-{{ item.tenant }} \ --volume {{ item.logscraper_dir | default(logscraper_dir) }}:{{ logscraper_dir }}:z \ - --volume {{ item['file_list'] | default(logscraper_dir + '/download-list-' + item['tenant'] + '.yaml') }}:{{ item['file_list'] | default(logscraper_dir + '/download-list-' + item['tenant'] + '.yaml') }}:z \ + {% for file_list in item['file_list'] | default([logscraper_dir + '/download-list-' + item['tenant'] + '.yaml']) -%} + --volume {{ file_list }}:{{ file_list }}:z \ + {% endfor -%} {% if 'logscraper_custom_ca_crt' in item %} --volume {{ item['logscraper_custom_ca_crt'] }}:{{ item['logscraper_custom_ca_crt'] }}:z \ {% endif %} diff --git a/ansible/roles/logsender/README.rst b/ansible/roles/logsender/README.rst index 0033016..d3b2b55 100644 --- a/ansible/roles/logsender/README.rst +++ b/ansible/roles/logsender/README.rst @@ -30,7 +30,7 @@ Example Ansible variables that are configuring service: es_insecure: true es_index: logstash-logscraper download_dir: /mnt/logscraper/sometenant - file_list: /etc/logsender/download-list-TENANT.yaml + file_list: ['/etc/logsender/download-list-TENANT.yaml'] That configuration will will deploy service with name: `logsender-openstack.service`. @@ -76,7 +76,8 @@ and second one for getting logs from `sometenant` tenant. es_index: "" es_index_prefix: "" download_dir: /mnt/logscraper/sometenant - file_list: /etc/logscraper/my-downloadlist.yaml + file_list: + - /etc/logscraper/my-downloadlist.yaml roles: - logsender diff --git a/ansible/roles/logsender/defaults/main.yml b/ansible/roles/logsender/defaults/main.yml index d79c298..2cbcbeb 100644 --- a/ansible/roles/logsender/defaults/main.yml +++ b/ansible/roles/logsender/defaults/main.yml @@ -20,6 +20,7 @@ container_images: # es_index: "" # es_insecure: false # download_dir: /mnt/logscraper/sometenant +# file_list: [] # doc_type: "_doc" # logsender_workers: 1 # chunk_size: 1500 diff --git a/ansible/roles/logsender/templates/config.j2 b/ansible/roles/logsender/templates/config.j2 index e487a40..d7025b7 100644 --- a/ansible/roles/logsender/templates/config.j2 +++ b/ansible/roles/logsender/templates/config.j2 @@ -16,6 +16,6 @@ follow: {{ item['follow'] | default(true) }} workers: {{ item['logsender_workers'] | default(1) }} ignore_es_status: {{ item['ignore_es_status'] | default(false) }} directory: {{ item['download_dir'] | default('/tmp/logscraper') }} -file_list: {{ item['file_list'] | default(logscraper_dir + '/download-list-' + item['tenant'] + '.yaml') }} +file_list: {{ item['file_list'] | default([logscraper_dir + '/download-list-' + item['tenant'] + '.yaml']) | list }} performance_index_prefix: {{ item['performance_index_prefix'] | default('') }} subunit_index_prefix: {{ item['subunit_index_prefix'] | default('') }} diff --git a/ansible/roles/logsender/templates/logsender.sh.j2 b/ansible/roles/logsender/templates/logsender.sh.j2 index 5060e49..f623d3b 100644 --- a/ansible/roles/logsender/templates/logsender.sh.j2 +++ b/ansible/roles/logsender/templates/logsender.sh.j2 @@ -9,7 +9,9 @@ --name logsender-{{ item.tenant }} \ --volume {{ item.download_dir }}:{{ item.download_dir }}:z \ --volume {{ item.logscraper_dir | default(logscraper_dir) }}:{{ logscraper_dir }}:z \ - --volume {{ item['file_list'] | default(logscraper_dir + '/download-list-' + item['tenant'] + '.yaml') }}:{{ item['file_list'] | default(logscraper_dir + '/download-list-' + item['tenant'] + '.yaml') }}:z \ + {% for file_list in item['file_list'] | default([logscraper_dir + '/download-list-' + item['tenant'] + '.yaml']) -%} + --volume {{ file_list }}:{{ file_list }}:z \ + {% endfor -%} {% if 'logsender_custom_ca_crt' in item %} --volume {{ item['logsender_custom_ca_crt'] }}:{{ item['logsender_custom_ca_crt'] }}:z \ {% endif %} diff --git a/logscraper/logscraper.conf.sample b/logscraper/logscraper.conf.sample index acc167d..156bcb9 100644 --- a/logscraper/logscraper.conf.sample +++ b/logscraper/logscraper.conf.sample @@ -7,7 +7,7 @@ workers: 1 max_skipped: 1000 debug: False download: True -file_list: logscraper/download-list.yaml.sample +file_list: ['logscraper/download-list.yaml.sample'] directory: /tmp/logscraper wait_time: 120 insecure: False diff --git a/logscraper/logscraper.py b/logscraper/logscraper.py index 3b666c4..7a4c4d1 100755 --- a/logscraper/logscraper.py +++ b/logscraper/logscraper.py @@ -133,7 +133,9 @@ def get_arguments(): "CI job logs into gearman.") parser.add_argument("--config", help="Logscraper config file", required=True) - parser.add_argument("--file-list", help="File list to download") + parser.add_argument("--file-list", help="File list to download. Parameter " + "can be set multiple times.", default=[], + action='append') parser.add_argument("--zuul-api-url", help="URL(s) for Zuul API. Parameter" " can be set multiple times.", nargs='+', default=[]) parser.add_argument("--job-name", help="CI job name(s). Parameter can be " @@ -496,10 +498,31 @@ def save_build_info(directory, build): yaml.dump(build, text_file) +def merge_dicts(main, additional): + merged = main.copy() + + for k, v in additional.items(): + if k in merged: + if isinstance(merged[k], list) and isinstance(v, list): + merged[k].extend(v) + elif isinstance(merged[k], dict) and isinstance(v, dict): + merged[k] = merge_dicts(merged[k], v) + else: + logging.critical("Trying to merge incompatible " + "types", type(merged[k]), type(v)) + else: + merged[k] = v + + return merged + + def load_config(config_path): try: - with open(config_path) as f: - return yaml.safe_load(f) + merged_config = {} + for yaml_file in config_path: + with open(yaml_file) as f: + merged_config = merge_dicts(merged_config, yaml.safe_load(f)) + return merged_config except PermissionError: logging.critical("Can not open config file %s" % config_path) except FileNotFoundError: diff --git a/logscraper/logsender.conf.sample b/logscraper/logsender.conf.sample index c8d9755..0c890eb 100644 --- a/logscraper/logsender.conf.sample +++ b/logscraper/logsender.conf.sample @@ -15,6 +15,6 @@ insecure: False ca_file: follow: True workers: 1 -file_list: logscraper/download-list.yaml.sample +file_list: ['logscraper/download-list.yaml.sample'] performance_index_prefix: performance- subunit_index_prefix: subunit- diff --git a/logscraper/logsender.py b/logscraper/logsender.py index 774c634..e9c9ed9 100755 --- a/logscraper/logsender.py +++ b/logscraper/logsender.py @@ -41,6 +41,11 @@ from pathlib import Path from ruamel.yaml import YAML from subunit2sql.read_subunit import ReadSubunit +try: + from logscraper import load_config as l_config +except ImportError: + from logscraper.logscraper import load_config as l_config + ############################################################################### # CLI # @@ -50,7 +55,9 @@ def get_arguments(): "and push to the Opensearch service") parser.add_argument("--config", help="Logscraper config file", required=True) - parser.add_argument("--file-list", help="File list to download") + parser.add_argument("--file-list", help="File list to download. Parameter " + "can be set multiple times.", default=[], + action='append') parser.add_argument("--directory", help="Directory, where the logs will " "be stored.") @@ -330,13 +337,11 @@ def open_file(path): def get_file_info(config, build_file): - yaml = YAML() - with open_file(config) as f: - config_files = yaml.load(f) - for f in config_files["files"]: - file_name = os.path.basename(f["name"]) - if build_file.endswith(file_name): - return f["name"], f.get('tags', []) + [file_name] + config_files = l_config(config) + for f in config_files["files"]: + file_name = os.path.basename(f["name"]) + if build_file.endswith(file_name): + return f["name"], f.get('tags', []) + [file_name] return os.path.basename(build_file), [os.path.basename(build_file)] diff --git a/logscraper/tests/test_logscraper.py b/logscraper/tests/test_logscraper.py index c38bfd2..a3a6954 100644 --- a/logscraper/tests/test_logscraper.py +++ b/logscraper/tests/test_logscraper.py @@ -552,6 +552,61 @@ class TestScraper(base.TestCase): 10) self.assertEqual(2, len(files)) + @mock.patch('yaml.safe_load') + @mock.patch('builtins.open', new_callable=mock.mock_open()) + def test_load_config(self, mock_open, mock_yaml): + config_path = ['/tmp/config_1', '/tmp/config_2'] + config_1 = {'files': [{ + 'name': 'job-output.txt', + 'tags': ['console', 'console.html'] + }, {'name': 'logs/undercloud/var/log/extra/logstash.txt', + 'tags': ['console', 'postpci']}]} + config_2 = {'files': [{ + 'name': 'new-job.txt', + 'tags': ['new-console'] + }, {'name': 'logs/some-file.log', + 'tags': ['postpci']}]} + final_config = {'files': [{ + 'name': 'job-output.txt', + 'tags': ['console', 'console.html'] + }, { + 'name': 'logs/undercloud/var/log/extra/logstash.txt', + 'tags': ['console', 'postpci'] + }, { + 'name': 'new-job.txt', 'tags': ['new-console'] + }, { + 'name': 'logs/some-file.log', 'tags': ['postpci']}]} + mock_yaml.side_effect = [config_1, config_2] + parsed_config = logscraper.load_config(config_path) + self.assertEqual(final_config, parsed_config) + + @mock.patch('yaml.safe_load') + @mock.patch('builtins.open', new_callable=mock.mock_open()) + def test_load_config_different_keys(self, mock_open, mock_yaml): + config_path = ['/tmp/config_1', '/tmp/config_2'] + config_1 = {'files': [{ + 'name': 'job-output.txt', + 'tags': ['console', 'console.html'] + }, {'name': 'logs/undercloud/var/log/extra/logstash.txt', + 'tags': ['console', 'postpci']}]} + config_2 = {'files2': [{ + 'name': 'new-job.txt', + 'tags': ['new-console'] + }, {'name': 'logs/some-file.log', + 'tags': ['postpci']}]} + final_config = {'files': [{ + 'name': 'job-output.txt', + 'tags': ['console', 'console.html'] + }, {'name': 'logs/undercloud/var/log/extra/logstash.txt', + 'tags': ['console', 'postpci']} + ], 'files2': [{ + 'name': 'new-job.txt', + 'tags': ['new-console'] + }, {'name': 'logs/some-file.log', 'tags': ['postpci']}]} + mock_yaml.side_effect = [config_1, config_2] + parsed_config = logscraper.load_config(config_path) + self.assertEqual(final_config, parsed_config) + class TestConfig(base.TestCase): @mock.patch('logscraper.logscraper.load_config') diff --git a/logscraper/tests/test_logsender.py b/logscraper/tests/test_logsender.py index be5ded9..deafec8 100755 --- a/logscraper/tests/test_logsender.py +++ b/logscraper/tests/test_logsender.py @@ -1157,9 +1157,12 @@ class TestSender(base.TestCase): got = logsender.get_timestamp(line) self.assertEqual(expected, got) - @mock.patch('ruamel.yaml.YAML.load') + @mock.patch('yaml.safe_load') + @mock.patch('builtins.open', new_callable=mock.mock_open()) + @mock.patch('logscraper.logscraper.load_config') @mock.patch('logscraper.logsender.open_file') - def test_get_file_info(self, mock_open_file, mock_yaml): + def test_get_file_info(self, mock_open_file, mock_load_config, mock_open, + mock_yaml): config = {'files': [{ 'name': 'job-output.txt', 'tags': ['console', 'console.html']