From 2ac43314e580a0c9aef68ecd707221e2bc9a1f97 Mon Sep 17 00:00:00 2001 From: Jeremy Crafts Date: Wed, 30 Jan 2019 18:56:25 -0500 Subject: [PATCH] fix some registration quirks related to the platform (#1651) * fix some registration quirks related to the platform - skip registration check for --support (RHIPLAT1-748) - skip --unregister call, log message (RHIPLAT1-749) - skip --register call, log message (RHIPLAT1-750) - short-circuit --status, log message (RHIPLAT1-751) * FLAKE * move logging and checks to API functions. phase calls reg normally * fix goof --- insights/client/__init__.py | 17 ++++++- insights/client/client.py | 1 + insights/client/phase/v1.py | 4 -- insights/client/support.py | 9 ++-- .../tests/client/phase/test_post_update.py | 4 +- .../support/test_collect_support_info.py | 27 ++++++++++++ insights/tests/client/test_client.py | 44 +++++++++++++++++++ 7 files changed, 95 insertions(+), 11 deletions(-) diff --git a/insights/client/__init__.py b/insights/client/__init__.py index 6fcd1904ec..13b7ce86c6 100644 --- a/insights/client/__init__.py +++ b/insights/client/__init__.py @@ -326,13 +326,22 @@ def register(self): False - machine is unregistered None - could not reach the API """ - return client.handle_registration(self.config, self.connection) + if self.config.legacy_upload: + return client.handle_registration(self.config, self.connection) + else: + if self.config.register: + logger.info('Registration is not applicable to the platform.') + logger.debug('Platform upload. Bypassing registration.') + return True @_net def unregister(self): """ returns (bool): True success, False failure """ + if not self.config.legacy_upload: + logger.info('Registration is not applicable to the platform.') + return True return client.handle_unregistration(self.config, self.connection) @_net @@ -425,6 +434,12 @@ def get_registration_status(self): 'unreg_date': Date the machine was unregistered | None, 'unreachable': API could not be reached} """ + if not self.config.legacy_upload: + return { + 'messages': ['Registration is not applicable for platform uploads.'], + 'unreachable': False, + 'status': True + } return client.get_registration_status(self.config, self.connection) @_net diff --git a/insights/client/client.py b/insights/client/client.py index 542bf0872e..f0b256382d 100644 --- a/insights/client/client.py +++ b/insights/client/client.py @@ -103,6 +103,7 @@ def register(config, pconn): return pconn.register() +# TODO: eventually remove this function. Only valid for legacy stuff def handle_registration(config, pconn): ''' Handle the registration process diff --git a/insights/client/phase/v1.py b/insights/client/phase/v1.py index efe90515a3..d458a76f46 100644 --- a/insights/client/phase/v1.py +++ b/insights/client/phase/v1.py @@ -156,10 +156,6 @@ def post_update(client, config): else: sys.exit(constants.sig_kill_bad) - if not config.legacy_upload: - logger.debug('Platform upload. Bypassing registration.') - return - reg = client.register() if reg is None: # API unreachable diff --git a/insights/client/support.py b/insights/client/support.py index 0f035ac791..921c740caf 100644 --- a/insights/client/support.py +++ b/insights/client/support.py @@ -89,10 +89,11 @@ def _support_diag_dump(self): pconn = InsightsConnection(self.config) logger.info('Insights version: %s', get_nvr()) - reg_check = registration_check(pconn) - cfg_block.append('Registration check:') - for key in reg_check: - cfg_block.append(key + ': ' + str(reg_check[key])) + if self.config.legacy_upload: + reg_check = registration_check(pconn) + cfg_block.append('Registration check:') + for key in reg_check: + cfg_block.append(key + ': ' + str(reg_check[key])) lastupload = 'never' if os.path.isfile(constants.lastupload_file): diff --git a/insights/tests/client/phase/test_post_update.py b/insights/tests/client/phase/test_post_update.py index 7ba7e7aeab..10687753f1 100644 --- a/insights/tests/client/phase/test_post_update.py +++ b/insights/tests/client/phase/test_post_update.py @@ -23,14 +23,14 @@ def patch_insights_config(old_function): @patch_insights_config def test_post_update_legacy_upload_off(insights_config, insights_client): """ - Registration is not processed when platform upload + Registration is still called when platform upload """ insights_config.return_value.load_all.return_value.legacy_upload = False try: post_update() except SystemExit: pass - insights_client.return_value.register.assert_not_called() + insights_client.return_value.register.assert_called_once() @patch("insights.client.phase.v1.InsightsClient") diff --git a/insights/tests/client/support/test_collect_support_info.py b/insights/tests/client/support/test_collect_support_info.py index df9a4b0eab..779ff3f2fb 100644 --- a/insights/tests/client/support/test_collect_support_info.py +++ b/insights/tests/client/support/test_collect_support_info.py @@ -1,5 +1,6 @@ # -*- coding: UTF-8 -*- +from insights.client.config import InsightsConfig from insights.client.support import InsightsSupport from mock.mock import Mock, patch @@ -31,3 +32,29 @@ def test_support_diag_dump_insights_connection(insights_connection, registration support._support_diag_dump() insights_connection.assert_called_once_with(config) + + +@patch('insights.client.support.InsightsConnection') +@patch('insights.client.support.registration_check') +def test_registration_check_legacy_upload_on(registration_check, InsightsConnection): + ''' + Check registration when legacy_upload=True + ''' + config = InsightsConfig(legacy_upload=True) + support = InsightsSupport(config) + support.collect_support_info() + + registration_check.assert_called_once() + + +@patch('insights.client.support.InsightsConnection') +@patch('insights.client.support.registration_check') +def test_skip_registration_check_legacy_upload_off(registration_check, InsightsConnection): + ''' + Don't check registration when legacy_upload=False + ''' + config = InsightsConfig(legacy_upload=False) + support = InsightsSupport(config) + support.collect_support_info() + + registration_check.assert_not_called() diff --git a/insights/tests/client/test_client.py b/insights/tests/client/test_client.py index b5959ba6d0..bf569d4542 100644 --- a/insights/tests/client/test_client.py +++ b/insights/tests/client/test_client.py @@ -337,3 +337,47 @@ def test_delete_archive_internal(): _delete_archive_internal(config, arch) assert not os.path.exists(arch.tmp_dir) assert not os.path.exists(arch.archive_tmp_dir) + + +@patch('insights.client.client.handle_registration') +def test_platform_register_skip(handle_registration): + ''' + handle_registration not called when platform upload + ''' + config = InsightsConfig(legacy_upload=False) + client = InsightsClient(config) + assert client.register() # short circuits to True + handle_registration.assert_not_called() + + +@patch('insights.client.client.handle_unregistration') +def test_platform_unregister_skip(handle_unregistration): + ''' + handle_registration not called when platform upload + ''' + config = InsightsConfig(legacy_upload=False) + client = InsightsClient(config) + assert client.unregister() # short circuits to True + handle_unregistration.assert_not_called() + + +@patch('insights.client.client.handle_registration') +def test_legacy_register(handle_registration): + ''' + handle_unregistration called when legacy upload + ''' + config = InsightsConfig(legacy_upload=True) + client = InsightsClient(config) + client.register() + handle_registration.assert_called_once() + + +@patch('insights.client.client.handle_unregistration') +def test_legacy_unregister(handle_unregistration): + ''' + handle_unregistration called when legacy upload + ''' + config = InsightsConfig(legacy_upload=True) + client = InsightsClient(config) + client.unregister() + handle_unregistration.assert_called_once()