Remove usage of reregistration and deprecate cli-option (#3522)

Signed-off-by: ahitacat <[email protected]>

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()

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)

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

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

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()
 

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

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)