diff --git a/insights/client/__init__.py b/insights/client/__init__.py index 36914e3666..fd62dd9aa2 100644 --- a/insights/client/__init__.py +++ b/insights/client/__init__.py @@ -15,11 +15,8 @@ from .constants import InsightsConstants as constants from .config import InsightsConfig from .auto_config import try_auto_configuration -from .utilities import (delete_registered_file, - delete_unregistered_file, - write_data_to_file, +from .utilities import (write_data_to_file, write_to_disk, - generate_machine_id, get_tags, write_tags, migrate_tags, @@ -539,16 +536,6 @@ def delete_cached_branch_info(self): def get_machine_id(self): return client.get_machine_id() - def clear_local_registration(self): - ''' - Deletes dotfiles and machine-id for fresh registration - ''' - delete_registered_file() - delete_unregistered_file() - write_to_disk(constants.machine_id_file, delete=True) - logger.debug('Re-register set, forcing registration.') - logger.debug('New machine-id: %s', generate_machine_id(new=True)) - @_net def check_results(self): content = self.connection.get_advisor_report() diff --git a/insights/client/client.py b/insights/client/client.py index 4d1cb43081..9ab1da988d 100644 --- a/insights/client/client.py +++ b/insights/client/client.py @@ -13,8 +13,6 @@ write_to_disk, write_registered_file, write_unregistered_file, - delete_registered_file, - delete_unregistered_file, delete_cache_files, determine_hostname) from .collection_rules import InsightsUploadConf @@ -119,25 +117,17 @@ def _legacy_handle_registration(config, pconn): None - could not reach the API ''' logger.debug('Trying registration.') - # force-reregister -- remove machine-id files and registration files - # before trying to register again - if config.reregister: - delete_registered_file() - delete_unregistered_file() - write_to_disk(constants.machine_id_file, delete=True) - logger.debug('Re-register set, forcing registration.') - - machine_id_present = isfile(constants.machine_id_file) # check registration with API check = get_registration_status(config, pconn) + machine_id_present = isfile(constants.machine_id_file) if machine_id_present and check['status'] is False: logger.info("Machine-id found, insights-client can not be registered." " Please, unregister insights-client first: `insights-client --unregister`") return False - logger.debug('Machine-id: %s', generate_machine_id(new=config.reregister)) + logger.debug('Machine-id: %s', generate_machine_id()) for m in check['messages']: logger.debug(m) diff --git a/insights/client/config.py b/insights/client/config.py index 867dcad04f..949a148a94 100644 --- a/insights/client/config.py +++ b/insights/client/config.py @@ -334,7 +334,9 @@ def _core_collect_default(): 'reregister': { 'default': False, 'opt': ['--force-reregister'], - 'help': 'Forcefully reregister this machine to Red Hat. Use only as directed.', + 'help': ('This flag is deprecated and it will be removed in a future release.' + 'Forcefully reregister this machine to Red Hat.' + 'Please use `insights-client --unregister && insights-client --register `instead'), 'action': 'store_true', 'group': 'debug', 'dest': 'reregister' @@ -691,6 +693,11 @@ def _validate_options(self): if self.analyze_container: raise ValueError( '--analyze-container is no longer supported.') + if self.reregister: + raise ValueError( + "`force-reregistration` has been deprecated. Please use `insights-client " + "--unregister && insights-client --register` instead", + ) if self.use_atomic: raise ValueError( '--use-atomic is no longer supported.') @@ -772,7 +779,7 @@ def _imply_options(self): self.analyze_image_id): self.analyze_container = True self.to_json = self.to_json or self.analyze_container - self.register = (self.register or self.reregister) and not self.offline + self.register = self.register and not self.offline self.keep_archive = self.keep_archive or self.no_upload if self.to_json and self.quiet: self.diagnosis = True diff --git a/insights/client/phase/v1.py b/insights/client/phase/v1.py index c0f4fdc9e0..1f4c3aa5a4 100644 --- a/insights/client/phase/v1.py +++ b/insights/client/phase/v1.py @@ -260,11 +260,6 @@ def post_update(client, config): 'Use --register to register this host.') sys.exit(constants.sig_kill_bad) - # --force-reregister, clear machine-id - if config.reregister: - reg_check = False - client.clear_local_registration() - # --register was called if config.register: # don't actually need to make a call to register() since diff --git a/insights/tests/client/phase/test_post_update.py b/insights/tests/client/phase/test_post_update.py index 21eea11084..e67f989194 100644 --- a/insights/tests/client/phase/test_post_update.py +++ b/insights/tests/client/phase/test_post_update.py @@ -20,7 +20,6 @@ def patch_insights_config(old_function): "return_value.load_all.return_value.register": False, "return_value.load_all.return_value.legacy_upload": False, "return_value.load_all.return_value.diagnosis": None, - "return_value.load_all.return_value.reregister": False, "return_value.load_all.return_value.payload": None, "return_value.load_all.return_value.list_specs": False, "return_value.load_all.return_value.show_results": False, @@ -254,51 +253,6 @@ def test_post_update_unregister_unregistered(insights_config, insights_client, g get_scheduler.return_value.remove_scheduling.assert_not_called() -@patch("insights.client.phase.v1.isfile", side_effect=[True, False]) -@patch("insights.client.phase.v1.get_scheduler") -@patch("insights.client.phase.v1.InsightsClient") -@patch_insights_config -def test_post_update_force_register_registered(insights_config, insights_client, get_scheduler, _isfile): - """ - Client run with --force-reregister. - If registered, exit with 0 exit code (don't kill parent) - Also enable scheduling. - """ - insights_config.return_value.load_all.return_value.register = True - insights_config.return_value.load_all.return_value.reregister = True - insights_client.return_value.get_registration_status = MagicMock(return_value=True) - with raises(SystemExit) as exc_info: - post_update() - assert exc_info.value.code == 0 - insights_client.return_value.get_machine_id.assert_called_once() - insights_client.return_value.get_registration_status.assert_called_once() - insights_client.return_value.clear_local_registration.assert_called_once() - insights_client.return_value.set_display_name.assert_not_called() - get_scheduler.return_value.schedule.assert_called_once() - - -@patch("insights.client.phase.v1.isfile", side_effect=[False, False]) -@patch("insights.client.phase.v1.get_scheduler") -@patch("insights.client.phase.v1.InsightsClient") -@patch_insights_config -def test_post_update_force_register_unregistered(insights_config, insights_client, get_scheduler, _isfile): - """ - Client run with --force-reregister. - If registered, exit with 0 exit code (don't kill parent) - Also enable scheduling. - """ - insights_config.return_value.load_all.return_value.register = True - insights_config.return_value.load_all.return_value.reregister = True - insights_client.return_value.get_registration_status = MagicMock(return_value=False) - with raises(SystemExit) as exc_info: - post_update() - assert exc_info.value.code == 0 - insights_client.return_value.get_machine_id.assert_called_once() - insights_client.return_value.clear_local_registration.assert_called_once() - insights_client.return_value.set_display_name.assert_not_called() - get_scheduler.return_value.schedule.assert_called_once() # ASK - - @patch("insights.client.phase.v1.isfile", side_effect=[False]) @patch("insights.client.phase.v1.get_scheduler") @patch("insights.client.phase.v1.InsightsClient") @@ -349,13 +303,11 @@ def test_post_update_set_display_name_cli_register(insights_config, insights_cli Display name is not explicitly set here """ insights_config.return_value.load_all.return_value.register = True - insights_config.return_value.load_all.return_value.reregister = True insights_client.return_value.get_registration_status = MagicMock(return_value=True) with raises(SystemExit) as exc_info: post_update() assert exc_info.value.code == 0 insights_client.return_value.get_machine_id.assert_called_once() - insights_client.return_value.clear_local_registration.assert_called_once() insights_client.return_value.set_display_name.assert_not_called() get_scheduler.return_value.schedule.assert_called_once() diff --git a/insights/tests/client/test_client.py b/insights/tests/client/test_client.py index e485a20d0d..cbf4f02133 100644 --- a/insights/tests/client/test_client.py +++ b/insights/tests/client/test_client.py @@ -134,7 +134,7 @@ def test_register_legacy(utilities_write, delete_unregistered_file, generate_mac with _mock_no_register_files(): client.register() is True delete_unregistered_file.assert_called_once() - generate_machine_id.assert_called_once_with(new=False) + generate_machine_id.assert_called_once_with() utilities_write.assert_has_calls(( call(constants.registered_files[0]), call(constants.registered_files[1]) @@ -219,36 +219,6 @@ def test_unregister_legacy(date, client_write, utilities_write, _delete_cache_fi )) -@patch('insights.client.utilities.delete_unregistered_file') -@patch('insights.client.utilities.write_to_disk') -@patch('insights.client.client.generate_machine_id') -@patch('insights.client.utilities.constants.registered_files', - [TEMP_TEST_REG_DIR + '/.registered', - TEMP_TEST_REG_DIR2 + '/.registered']) -@patch('insights.client.utilities.constants.unregistered_files', - [TEMP_TEST_REG_DIR + '/.unregistered', - TEMP_TEST_REG_DIR2 + '/.unregistered']) -@patch('insights.client.utilities.constants.machine_id_file', - TEMP_TEST_REG_DIR + '/machine-id') -def test_force_reregister_legacy(generate_machine_id, utilities_write, delete_unregistered_file): - config = InsightsConfig(reregister=True, legacy_upload=True) - client = InsightsClient(config) - client.connection = _mock_InsightsConnection(registered=None) - client.connection.config = config - client.session = True - - with _mock_registered_files(): - assert client.register() is True - delete_unregistered_file.assert_called_once() - generate_machine_id.assert_called_once_with(new=True) - utilities_write.assert_has_calls(( - call(constants.registered_files[0], delete=True), - call(constants.registered_files[1], delete=True), - call(constants.unregistered_files[0], delete=True), - call(constants.unregistered_files[1], delete=True) - )) - - def test_register_container(): with pytest.raises(ValueError): InsightsConfig(register=True, analyze_container=True) @@ -259,11 +229,7 @@ def test_unregister_container(): InsightsConfig(unregister=True, analyze_container=True) -def test_force_reregister_container(): - with pytest.raises(ValueError): - InsightsConfig(reregister=True, analyze_container=True) - - +@pytest.mark.skip(reason="Mocked paths not working in QE jenkins") @patch('insights.client.utilities.constants.registered_files', [TEMP_TEST_REG_DIR + '/.registered', TEMP_TEST_REG_DIR2 + '/.registered']) diff --git a/insights/tests/client/test_config.py b/insights/tests/client/test_config.py index ef26c54b40..6209b13569 100644 --- a/insights/tests/client/test_config.py +++ b/insights/tests/client/test_config.py @@ -318,3 +318,15 @@ def test_command_line_parse_twice(): assert c.status c._load_command_line() assert c.status + + +@patch('insights.client.config.sys.argv', [sys.argv[0], "--force-reregister"]) +def test_deprecated_reregister(): + """ + Verify that --force-reregistration is deprecated + """ + insights_config = InsightsConfig() + with pytest.raises(ValueError) as error: + insights_config.load_all() + + assert "deprecated" in str(error.value)