From d14250e948af744e5273f12aa8e300f71e9d3df8 Mon Sep 17 00:00:00 2001 From: Xiangce Liu Date: Tue, 12 Nov 2024 11:44:45 +0800 Subject: [PATCH] fix: do not import 'insights.client.constants' in spec_cleaner - it causes circular import issue when triggering data collection via 'insights-collect' command - instead of importing the constants from insights.client, this change sets the "rhsm_facts_file" to insights_config Signed-off-by: Xiangce Liu rh-pre-commit.version: 2.3.1 rh-pre-commit.check-secrets: ENABLED --- insights/client/core_collector.py | 1 + insights/core/spec_cleaner.py | 12 +++--- .../core_collector/test_run_collection.py | 39 +++++++++++++++++++ .../tests/core/spec_cleaner/test_reports.py | 12 +++--- 4 files changed, 53 insertions(+), 11 deletions(-) create mode 100644 insights/tests/client/core_collector/test_run_collection.py diff --git a/insights/client/core_collector.py b/insights/client/core_collector.py index a659dd5d96..8af2c68762 100644 --- a/insights/client/core_collector.py +++ b/insights/client/core_collector.py @@ -124,6 +124,7 @@ def run_collection(self, rm_conf, branch_info, blacklist_report): logger.debug('Beginning to run core collection ...') + self.config.rhsm_facts_file = constants.rhsm_facts_file collect.collect( tmp_path=self.archive.tmp_dir, archive_name=self.archive.archive_name, diff --git a/insights/core/spec_cleaner.py b/insights/core/spec_cleaner.py index d50c03569b..82247c5143 100644 --- a/insights/core/spec_cleaner.py +++ b/insights/core/spec_cleaner.py @@ -21,7 +21,6 @@ import socket import struct -from insights.client.constants import InsightsConstants as constants from insights.util.hostname import determine_hostname from insights.util.posix_regex import replace_posix @@ -55,6 +54,9 @@ def write_report(report, report_file, mode=0o644): class Cleaner(object): def __init__(self, config, rm_conf, fqdn=None): self.report_dir = '/tmp' + self.rhsm_facts_file = getattr( + config, 'rhsm_facts_file', + os.path.join(self.report_dir, 'insights-client.facts')) # Obfuscation - set: ip and hostname only self.obfuscate = set() self.obfuscate.add('ip') if config and config.obfuscate else None @@ -423,7 +425,7 @@ def clean_file(self, _file, no_obfuscate=None, no_redact=False): raise Exception("Error: Cannot Write to File: %s" % _file) def generate_rhsm_facts(self): - logger.info('Writing RHSM facts to %s ...', constants.rhsm_facts_file) + logger.info('Writing RHSM facts to %s ...', self.rhsm_facts_file) hn_block = [] for k, v in self.hn_db.items(): @@ -450,7 +452,7 @@ def generate_rhsm_facts(self): 'insights_client.keywords': json.dumps(kw_block), } - write_report(facts, constants.rhsm_facts_file) + write_report(facts, self.rhsm_facts_file) def generate_ip_report(self, archive_name): try: @@ -501,8 +503,8 @@ def generate_kw_report(self, archive_name): logger.info('Completed Keyword Report.') def generate_report(self, archive_name): - # Always generate the rhsm.facts files - self.generate_rhsm_facts() + if self.rhsm_facts_file: + self.generate_rhsm_facts() if 'ip' in self.obfuscate: self.generate_ip_report(archive_name) if 'hostname' in self.obfuscate: diff --git a/insights/tests/client/core_collector/test_run_collection.py b/insights/tests/client/core_collector/test_run_collection.py new file mode 100644 index 0000000000..706ada0bd7 --- /dev/null +++ b/insights/tests/client/core_collector/test_run_collection.py @@ -0,0 +1,39 @@ +from mock.mock import patch + +from insights.client.archive import InsightsArchive +from insights.client.config import InsightsConfig +from insights.client.core_collector import CoreCollector + + +@patch('insights.client.core_collector.logger') +@patch('insights.client.core_collector.CoreCollector._write_egg_release', return_value=None) +@patch('insights.client.core_collector.CoreCollector._write_blacklisted_specs', return_value=None) +@patch('insights.client.core_collector.CoreCollector._write_blacklist_report', return_value=None) +@patch('insights.client.core_collector.CoreCollector._write_tags', return_value=None) +@patch('insights.client.core_collector.CoreCollector._write_version_info', return_value=None) +@patch('insights.client.core_collector.CoreCollector._write_ansible_host', return_value=None) +@patch('insights.client.core_collector.CoreCollector._write_display_name', return_value=None) +@patch('insights.client.core_collector.CoreCollector._write_branch_info', return_value=None) +@patch('insights.client.core_collector.systemd_notify_init_thread', return_value=None) +@patch('insights.client.core_collector.InsightsArchive.create_archive_dir', return_value=None) +@patch('insights.client.core_collector.collect', return_value=None) +def test_run_collection( + collect, + create_archive_dir, + systemd_notify_init_thread, + _write_branch_info, + _write_display_name, + _write_ansible_host, + _write_version_info, + _write_tags, + _write_blacklist_report, + _write_blacklisted_specs, + _write_egg_release, + logger): + conf = InsightsConfig() + arch = InsightsArchive(conf) + cc = CoreCollector(conf, arch) + cc.run_collection({}, {}, {}) + create_archive_dir.assert_called_once() + collect.collect.assert_called_once() + logger.debug.assert_called_with('Metadata collection finished.') diff --git a/insights/tests/core/spec_cleaner/test_reports.py b/insights/tests/core/spec_cleaner/test_reports.py index c9cf067243..ac54c17663 100644 --- a/insights/tests/core/spec_cleaner/test_reports.py +++ b/insights/tests/core/spec_cleaner/test_reports.py @@ -7,7 +7,6 @@ from insights.client.archive import InsightsArchive from insights.client.config import InsightsConfig -from insights.client.constants import InsightsConstants as constants from insights.core.spec_cleaner import Cleaner hostname = "report.test.com" @@ -22,9 +21,10 @@ ) @mark.parametrize("test_umask", [0o000, 0o022]) def test_rhsm_facts(test_umask, obfuscate, obfuscate_hostname): - constants.rhsm_facts_file = '/tmp/insights_test_rhsm.facts' + rhsm_facts_file = '/tmp/insights_test_rhsm.facts' conf = InsightsConfig(obfuscate=obfuscate, obfuscate_hostname=obfuscate_hostname) + conf.rhsm_facts_file = rhsm_facts_file arch = InsightsArchive(conf) arch.create_archive_dir() @@ -41,11 +41,11 @@ def test_rhsm_facts(test_umask, obfuscate, obfuscate_hostname): umask_after_test = os.umask(old_umask) - assert os.path.isfile(constants.rhsm_facts_file) - st = os.stat(constants.rhsm_facts_file) + assert os.path.isfile(rhsm_facts_file) + st = os.stat(rhsm_facts_file) assert st.st_mode & 0o777 == 0o644 & ~test_umask assert umask_after_test == test_umask # umask was not changed by Cleaner - with open(constants.rhsm_facts_file, 'r') as fp: + with open(rhsm_facts_file, 'r') as fp: facts = json.load(fp) # hostname assert facts['insights_client.hostname'] == hostname @@ -69,7 +69,7 @@ def test_rhsm_facts(test_umask, obfuscate, obfuscate_hostname): assert kws[0]['original'] == 'testword' assert kws[0]['obfuscated'] == 'keyword0' - os.unlink(constants.rhsm_facts_file) + os.unlink(rhsm_facts_file) @mark.parametrize("rm_conf", [{}, {'keywords': ['testword']}])