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 <[email protected]>

rh-pre-commit.version: 2.3.1
rh-pre-commit.check-secrets: ENABLED

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,

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:

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.')

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']}])