From 9a2f634e55d5a4fda326b38a1b6f2ea3b829d8fe Mon Sep 17 00:00:00 2001 From: Xiangce Liu Date: Wed, 16 Oct 2024 12:01:18 +0800 Subject: [PATCH] do not collect metadata stuff on client side - "metadata" was already collected by core collection specs - This change needs QE tests for all dependencies - Jira: RHINENG-7581 Signed-off-by: Xiangce Liu --- insights/client/core_collector.py | 124 +++--------------- insights/specs/datasources/client_metadata.py | 6 - .../core_collector/test_run_collection.py | 29 +--- .../core_collector/test_write_metadata.py | 123 ----------------- .../spec_cleaner/test_clean_file_redact.py | 8 -- 5 files changed, 23 insertions(+), 267 deletions(-) delete mode 100644 insights/tests/client/core_collector/test_write_metadata.py diff --git a/insights/client/core_collector.py b/insights/client/core_collector.py index 8af2c68762..458c895ec2 100644 --- a/insights/client/core_collector.py +++ b/insights/client/core_collector.py @@ -3,17 +3,12 @@ """ from __future__ import absolute_import -import json import logging -import os - -from itertools import chain from insights import collect from insights.client.archive import InsightsArchive from insights.client.constants import InsightsConstants as constants -from insights.client.utilities import systemd_notify_init_thread, get_version_info, get_tags -from insights.core.blacklist import BLACKLISTED_SPECS +from insights.client.utilities import systemd_notify_init_thread APP_NAME = constants.app_name logger = logging.getLogger(__name__) @@ -21,96 +16,11 @@ class CoreCollector(object): """ - Collectoer for new core-collector + Collector for core collection """ - def __init__(self, config, archive_=None, mountpoint=None, spec_conf=None): + def __init__(self, config): self.config = config - self.archive = archive_ if archive_ else InsightsArchive(config) - self.mountpoint = mountpoint if mountpoint else '/' - self.spec_conf = spec_conf if spec_conf else {} - - def _write_branch_info(self, branch_info): - logger.debug("Writing branch information to archive...") - self.archive.add_metadata_to_archive( - json.dumps(branch_info), '/branch_info') - - def _write_display_name(self): - if self.config.display_name: - logger.debug("Writing display_name to archive...") - self.archive.add_metadata_to_archive( - self.config.display_name, '/display_name') - - def _write_ansible_host(self): - if self.config.ansible_host: - logger.debug("Writing ansible_host to archive...") - self.archive.add_metadata_to_archive( - self.config.ansible_host, '/ansible_host') - - def _write_version_info(self): - logger.debug("Writing version information to archive...") - version_info = get_version_info() - self.archive.add_metadata_to_archive( - json.dumps(version_info), '/version_info') - - def _write_tags(self): - tags = get_tags() - # NOTE: - # The following code is also used by datasource 'tags' - # - insights.specs.datasources.tags - # Please keep them consistence before removing this. - if tags is not None: - def f(k, v): - if type(v) is list: - col = [] - for val in v: - col.append(f(k, val)) - return list(chain.from_iterable(col)) - elif type(v) is dict: - col = [] - for key, val in v.items(): - col.append(f(k + ":" + key, val)) - return list(chain.from_iterable(col)) - else: - return [{"key": k, "value": v, "namespace": constants.app_name}] - logger.debug("Writing tags to archive...") - t = [] - for k, v in tags.items(): - iv = f(k, v) - t.append(iv) - t = list(chain.from_iterable(t)) - self.archive.add_metadata_to_archive(json.dumps(t), '/tags.json') - - def _write_blacklist_report(self, blacklist_report): - logger.debug("Writing blacklist report to archive...") - self.archive.add_metadata_to_archive( - json.dumps(blacklist_report), '/blacklist_report') - - def _write_blacklisted_specs(self): - if BLACKLISTED_SPECS: - logger.debug("Writing blacklisted specs to archive...") - self.archive.add_metadata_to_archive( - json.dumps({"specs": BLACKLISTED_SPECS}), '/blacklisted_specs') - - def _write_egg_release(self): - logger.debug("Writing egg release to archive...") - egg_release = '' - try: - with open(constants.egg_release_file) as fil: - egg_release = fil.read() - except (IOError, MemoryError) as e: - logger.debug('Could not read the egg release file: %s', str(e)) - try: - os.remove(constants.egg_release_file) - except OSError as e: - logger.debug('Could not remove the egg release file: %s', str(e)) - - try: - self.archive.add_metadata_to_archive( - egg_release, '/egg_release') - except OSError as e: - logger.debug('Could not add the egg release file to the archive: %s', str(e)) - self.archive.add_metadata_to_archive( - '', '/egg_release') + self.archive = InsightsArchive(config) def run_collection(self, rm_conf, branch_info, blacklist_report): ''' @@ -134,17 +44,21 @@ def run_collection(self, rm_conf, branch_info, blacklist_report): logger.debug('Core collection finished.') - # collect metadata - logger.debug('Collecting metadata ...') - self._write_branch_info(branch_info) - self._write_display_name() - self._write_ansible_host() - self._write_version_info() - self._write_tags() - self._write_blacklist_report(blacklist_report) - self._write_blacklisted_specs() - self._write_egg_release() - logger.debug('Metadata collection finished.') + # About "metadata": + # The following metadata is now collected by core collection when + # available, and will be stored under the "./data" directory instead + # of the root of the archive. + # + # ansible_host = client_metadata.ansible_host + # blacklist_report = client_metadata.blacklist_report + # blacklisted_specs = client_metadata.blacklisted_specs + # branch_info = client_metadata.branch_info + # display_name = client_metadata.display_name + # egg_release = client_metadata.egg_release + # tags = client_metadata.tags + # version_info = client_metadata.version_info + # + # See `insights.specs.datasources.client_metadata` def done(self): """ diff --git a/insights/specs/datasources/client_metadata.py b/insights/specs/datasources/client_metadata.py index 18b2b9dc65..5cdd11653c 100644 --- a/insights/specs/datasources/client_metadata.py +++ b/insights/specs/datasources/client_metadata.py @@ -229,11 +229,6 @@ def tags(broker): logger.error("Invalid YAML. Unable to load '%s'" % tags_file_path) raise ContentException('ERROR: Cannot parse %s.\n\nError details: \n%s\n' % (tags_file_path, e)) - # --START-- - # NOTE: - # The following code is from the following function - # - insights.client.core_collector.CoreCollector._write_tags - # Please keep them consistence before removing that. def f(k, v): if type(v) is list: col = [] @@ -252,7 +247,6 @@ def f(k, v): iv = f(k, v) t.append(iv) t = list(chain.from_iterable(t)) - # --END-- if t: # The actual file path in archive: diff --git a/insights/tests/client/core_collector/test_run_collection.py b/insights/tests/client/core_collector/test_run_collection.py index 706ada0bd7..263dd21dfe 100644 --- a/insights/tests/client/core_collector/test_run_collection.py +++ b/insights/tests/client/core_collector/test_run_collection.py @@ -1,39 +1,18 @@ 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): +def test_run_collection(collect, create_archive_dir, systemd_notify_init_thread, logger): conf = InsightsConfig() - arch = InsightsArchive(conf) - cc = CoreCollector(conf, arch) + cc = CoreCollector(conf) cc.run_collection({}, {}, {}) + systemd_notify_init_thread.assert_called_once() create_archive_dir.assert_called_once() collect.collect.assert_called_once() - logger.debug.assert_called_with('Metadata collection finished.') + logger.debug.assert_called_with('Core collection finished.') diff --git a/insights/tests/client/core_collector/test_write_metadata.py b/insights/tests/client/core_collector/test_write_metadata.py deleted file mode 100644 index 7dcf4ba85b..0000000000 --- a/insights/tests/client/core_collector/test_write_metadata.py +++ /dev/null @@ -1,123 +0,0 @@ -import six -import mock -from insights.client.constants import InsightsConstants as constants -from insights.client.config import InsightsConfig -from insights.client.core_collector import CoreCollector -from mock.mock import call -from mock.mock import patch - - -@patch('insights.client.core_collector.os.remove') -@patch('insights.client.core_collector.InsightsArchive') -def test_egg_release_file_read_and_written(archive, remove): - ''' - Verify the egg release file is read from file and - written to the archive - ''' - if six.PY3: - open_name = 'builtins.open' - else: - open_name = '__builtin__.open' - - with patch(open_name, create=True) as mock_open: - mock_open.side_effect = [mock.mock_open(read_data='/testvalue').return_value] - c = InsightsConfig() - d = CoreCollector(c) - d._write_egg_release() - remove.assert_called_once_with(constants.egg_release_file) - d.archive.add_metadata_to_archive.assert_called_once_with('/testvalue', '/egg_release') - - -@patch('insights.client.core_collector.os.remove') -@patch('insights.client.core_collector.InsightsArchive') -def test_egg_release_file_read_and_written_no_delete(archive, remove): - ''' - Verify the egg release file is read from file and - written to the archive, even if the file cannot be deleted - ''' - if six.PY3: - open_name = 'builtins.open' - else: - open_name = '__builtin__.open' - - remove.side_effect = OSError('test') - - with patch(open_name, create=True) as mock_open: - mock_open.side_effect = [mock.mock_open(read_data='/testvalue').return_value] - c = InsightsConfig() - d = CoreCollector(c) - d._write_egg_release() - remove.assert_called_once_with(constants.egg_release_file) - d.archive.add_metadata_to_archive.assert_called_once_with('/testvalue', '/egg_release') - - -@patch('insights.client.core_collector.os.remove') -@patch('insights.client.core_collector.InsightsArchive') -def test_egg_release_file_read_and_written_no_read(archive, remove): - ''' - Verify that when the egg release file cannot be read, - a blank string is written to the archive - ''' - if six.PY3: - open_name = 'builtins.open' - else: - open_name = '__builtin__.open' - - remove.side_effect = OSError('test') - - with patch(open_name, create=True) as mock_open: - mock_open.side_effect = IOError('test') - c = InsightsConfig() - d = CoreCollector(c) - d._write_egg_release() - remove.assert_called_once_with(constants.egg_release_file) - d.archive.add_metadata_to_archive.assert_called_once_with('', '/egg_release') - - -@patch('insights.client.core_collector.os.remove') -@patch('insights.client.core_collector.InsightsArchive') -def test_egg_release_file_read_memory_error(archive, remove): - ''' - Verify that a memory error on the egg release file read is not - fatal. - ''' - if six.PY3: - open_name = 'builtins.open' - else: - open_name = '__builtin__.open' - - with patch(open_name, create=True) as mock_open: - file_mock = mock.mock_open().return_value - file_mock.read.side_effect = MemoryError() - mock_open.side_effect = [file_mock] - c = InsightsConfig() - d = CoreCollector(c) - d._write_egg_release() - remove.assert_called_once_with(constants.egg_release_file) - d.archive.add_metadata_to_archive.assert_called_once_with('', '/egg_release') - - -@patch('insights.client.core_collector.os.remove') -@patch( - 'insights.client.core_collector.InsightsArchive', - **{'return_value.add_metadata_to_archive.side_effect': [OSError('[Errno 28] No space left on device'), None]}) -def test_egg_release_file_write_os_error(archive, remove): - ''' - Verify that an OS Error (e.g. no space left) on the egg release file - write is not fatal - an empty file is written instead. - ''' - if six.PY3: - open_name = 'builtins.open' - else: - open_name = '__builtin__.open' - - with patch(open_name, create=True) as mock_open: - mock_open.side_effect = [mock.mock_open(read_data='/testvalue').return_value] - c = InsightsConfig() - d = CoreCollector(c) - d._write_egg_release() - remove.assert_called_once_with(constants.egg_release_file) - failed_call = call('/testvalue', '/egg_release') - rescue_call = call('', '/egg_release') - expected_calls = [failed_call, rescue_call] - d.archive.add_metadata_to_archive.assert_has_calls(expected_calls) diff --git a/insights/tests/core/spec_cleaner/test_clean_file_redact.py b/insights/tests/core/spec_cleaner/test_clean_file_redact.py index bb4b9df5fc..71ee7d814c 100644 --- a/insights/tests/core/spec_cleaner/test_clean_file_redact.py +++ b/insights/tests/core/spec_cleaner/test_clean_file_redact.py @@ -1,6 +1,5 @@ import os -from mock.mock import patch, Mock from pytest import mark from insights.client.archive import InsightsArchive @@ -11,13 +10,6 @@ test_file_data_sensitive = 'test\nabcd\n1234\npassword: p4ssw0rd here\npassword= p4ssw0rd here\npassword' -@patch('insights.client.archive.InsightsArchive', Mock()) -@patch('insights.client.core_collector.CoreCollector._write_branch_info', Mock()) -@patch('insights.client.core_collector.CoreCollector._write_display_name', Mock()) -@patch('insights.client.core_collector.CoreCollector._write_version_info', Mock()) -@patch('insights.client.core_collector.CoreCollector._write_tags', Mock()) -@patch('insights.client.core_collector.CoreCollector._write_blacklist_report', Mock()) -@patch('insights.client.core_collector.collect.collect', Mock(return_value=('/var/tmp/testarchive/insights-test', {}))) def test_redact_core(): conf = InsightsConfig() rm_conf = {'test': 'test'}