[RHELC-1494, RHELC-1289] Restore disabled repos during conversion in …

…rollback (#1212)

* Restore disabled repos during conversion in rollback

In a satellite analysis/conversion, we are disabling all repositories to
not interfer with the rest of the execution.

With this commit, we are introducing a way of checking what repositories
are enabled prior to disable them, and back the repositories names up.
In the rollback phase, we enable the repositories back on the system to
leave everything the way it was.

* Improve unit tests to increase coverage

* Add integration tests

This test will perform the following operations:
    - Collect the enabled repositories prior to the analysis start
    - Run the analysis and assert that we successfully enabled the RHSM repositories
    - Collect the enabled repositories after the tool run to compare with the repositories prior to the analysis

* Apply suggestions from code review and few modifications

In this commit, we are including the following modications that are
worth mentioning:

- We are dropping the filter for rhel repositories, so if the user has
  any rhel repositories enabled before the analysis, we should re-enable
  them after the analysis is done (alongside any other repository that
  rhsm reports that is enabled)
- Updates to the integration tests to strip the white spaces and compare
  correctly the list of repositories enabled

* Apply suggestions from code review

Co-authored-by: Martin "kokesak" Litwora <mlitwora@redhat.com>

* Enable centos repos before the analysis

---------

Co-authored-by: Martin "kokesak" Litwora <mlitwora@redhat.com>

convert2rhel/actions/pre_ponr_changes/subscription.py

@@ -20,6 +20,11 @@
 
 from convert2rhel import actions, backup, exceptions, pkghandler, repo, subscription, toolopts, utils
 from convert2rhel.backup.certs import RestorablePEMCert
+from convert2rhel.backup.subscription import (
+    RestorableAutoAttachmentSubscription,
+    RestorableDisableRepositories,
+    RestorableSystemSubscription,
+)
 
 
 logger = logging.getLogger(__name__)
@@ -184,7 +189,7 @@ def run(self):
                     "The system is registered with an RHSM account that has Simple Content Access (SCA) disabled but no subscription is attached. Without enabled SCA or an attached subscription the system can't access RHEL repositories. We'll try to auto-attach a subscription."
                 )
                 try:
-                    backup.backup_control.push(subscription.RestorableAutoAttachmentSubscription())
+                    backup.backup_control.push(RestorableAutoAttachmentSubscription())
                 except subscription.SubscriptionAutoAttachmentError:
                     self.set_result(
                         level="ERROR",
@@ -202,15 +207,15 @@ def run(self):
             # have to disentangle the exception handling when we do that.
             if subscription.should_subscribe():
                 logger.task("Prepare: Subscription Manager - Subscribe system")
-                restorable_subscription = subscription.RestorableSystemSubscription()
+                restorable_subscription = RestorableSystemSubscription()
                 backup.backup_control.push(restorable_subscription)
 
+            logger.task("Prepare: Subscription Manager - Disable all repositories")
+            backup.backup_control.push(RestorableDisableRepositories())
+
             logger.task("Prepare: Get RHEL repository IDs")
             rhel_repoids = repo.get_rhel_repoids()
 
-            logger.task("Prepare: Subscription Manager - Disable all repositories")
-            subscription.disable_repos()
-
             # we need to enable repos after removing repofile pkgs, otherwise
             # we don't get backups to restore from on a rollback
             logger.task("Prepare: Subscription Manager - Enable RHEL repositories")

convert2rhel/backup/subscription.py

@@ -0,0 +1,140 @@
+# -*- coding: utf-8 -*-
+#
+# Copyright(C) 2016 Red Hat, Inc.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+__metaclass__ = type
+
+import logging
+import re
+
+from convert2rhel import subscription, utils
+from convert2rhel.backup import RestorableChange
+
+
+loggerinst = logging.getLogger(__name__)
+
+
+class RestorableSystemSubscription(RestorableChange):
+    """
+    Register with RHSM in a fashion that can be reverted.
+    """
+
+    # We need this __init__ because it is an abstractmethod in the base class
+    def __init__(self):  # pylint: disable=useless-parent-delegation
+        super(RestorableSystemSubscription, self).__init__()
+
+    def enable(self):
+        """Register and attach a specific subscription to OS."""
+        if self.enabled:
+            return
+
+        subscription.register_system()
+        subscription.attach_subscription()
+
+        super(RestorableSystemSubscription, self).enable()
+
+    def restore(self):
+        """Rollback subscription related changes"""
+        loggerinst.task("Rollback: RHSM-related actions")
+
+        if self.enabled:
+            try:
+                subscription.unregister_system()
+            except subscription.UnregisterError as e:
+                loggerinst.warning(str(e))
+            except OSError:
+                loggerinst.warning("subscription-manager not installed, skipping")
+
+        super(RestorableSystemSubscription, self).restore()
+
+
+class RestorableAutoAttachmentSubscription(RestorableChange):
+    """
+    Auto attach subscriptions with RHSM in a fashion that can be reverted.
+    """
+
+    def __init__(self):
+        super(RestorableAutoAttachmentSubscription, self).__init__()
+        self._is_attached = False
+
+    def enable(self):
+        self._is_attached = subscription.auto_attach_subscription()
+        super(RestorableAutoAttachmentSubscription, self).enable()
+
+    def restore(self):
+        if self._is_attached:
+            subscription.remove_subscription()
+            super(RestorableAutoAttachmentSubscription, self).restore()
+
+
+class RestorableDisableRepositories(RestorableChange):
+    """
+    Gather repositories enabled on the system before we disable them and enable
+    them in the rollback.
+    """
+
+    # Look for the `Repo ID` key in the subscription-manager output, if there
+    # is a match, we save it in the named group `repo_id`. This will find all
+    # occurrences of the Repo ID in the output.
+    ENABLED_REPOS_PATTERN = re.compile(r"Repo ID:\s+(?P<repo_id>\S+)")
+
+    def __init__(self):
+        super(RestorableDisableRepositories, self).__init__()
+        self._repos_to_enable = []
+
+    def _get_enabled_repositories(self):
+        """Get repositories that were enabled prior to the conversion.
+
+        :returns list[str]: List of repositories enabled prior the conversion.
+            If no repositories were enabled or match the ignored rhel
+            repositories, defaults to an empty list.
+        """
+        cmd = ["subscription-manager", "repos", "--list-enabled"]
+        output, _ = utils.run_subprocess(cmd, print_output=False)
+
+        repositories = []
+        matches = re.finditer(self.ENABLED_REPOS_PATTERN, output)
+        if matches:
+            repositories = [match.group("repo_id") for match in matches if match.group("repo_id")]
+
+        return repositories
+
+    def enable(self):
+        repositories = self._get_enabled_repositories()
+
+        if repositories:
+            self._repos_to_enable = repositories
+            loggerinst.debug("Repositories enabled in the system prior to the conversion: %s" % ",".join(repositories))
+
+        subscription.disable_repos()
+        super(RestorableDisableRepositories, self).enable()
+
+    def restore(self):
+        if not self.enabled:
+            return
+
+        loggerinst.task("Rollback: Restoring state of the repositories")
+
+        if self._repos_to_enable:
+            loggerinst.debug("Repositories to enable: %s" % ",".join(self._repos_to_enable))
+
+            # This is not the ideal state. We should really have a generic
+            # class for enabling/disabling the repositories we have touched for
+            # RHSM. Jira issue: https://issues.redhat.com/browse/RHELC-1560
+            subscription.disable_repos()
+            subscription.submgr_enable_repos(self._repos_to_enable)
+
+        super(RestorableDisableRepositories, self).restore()

convert2rhel/subscription.py

@@ -96,59 +96,6 @@ class SubscriptionAutoAttachmentError(Exception):
     """Raised when there is a failure in auto attaching a subscription via subscription-manager."""
 
 
-class RestorableSystemSubscription(backup.RestorableChange):
-    """
-    Register with RHSM in a fashion that can be reverted.
-    """
-
-    # We need this __init__ because it is an abstractmethod in the base class
-    def __init__(self):  # pylint: disable=useless-parent-delegation
-        super(RestorableSystemSubscription, self).__init__()
-
-    def enable(self):
-        """Register and attach a specific subscription to OS."""
-        if self.enabled:
-            return
-
-        register_system()
-        attach_subscription()
-
-        super(RestorableSystemSubscription, self).enable()
-
-    def restore(self):
-        """Rollback subscription related changes"""
-        loggerinst.task("Rollback: RHSM-related actions")
-
-        if self.enabled:
-            try:
-                unregister_system()
-            except UnregisterError as e:
-                loggerinst.warning(str(e))
-            except OSError:
-                loggerinst.warning("subscription-manager not installed, skipping")
-
-        super(RestorableSystemSubscription, self).restore()
-
-
-class RestorableAutoAttachmentSubscription(backup.RestorableChange):
-    """
-    Auto attach subscriptions with RHSM in a fashion that can be reverted.
-    """
-
-    def __init__(self):
-        super(RestorableAutoAttachmentSubscription, self).__init__()
-        self._is_attached = False
-
-    def enable(self):
-        self._is_attached = auto_attach_subscription()
-        super(RestorableAutoAttachmentSubscription, self).enable()
-
-    def restore(self):
-        if self._is_attached:
-            remove_subscription()
-            super(RestorableAutoAttachmentSubscription, self).restore()
-
-
 def remove_subscription():
     """Remove all subscriptions added from auto attachment"""
     loggerinst.info("Removing auto attached subscriptions.")
@@ -797,18 +744,11 @@ def disable_repos():
     """Before enabling specific repositories, all repositories should be
     disabled. This can be overriden by the --disablerepo option.
     """
-    disable_cmd = ["subscription-manager", "repos"]
-    disable_repos = []
-    for repo in tool_opts.disablerepo:
-        disable_repos.append("--disable=%s" % repo)
-
-    if not disable_repos:
-        # Default is to disable all repos to make clean environment for
-        # enabling repos later
-        disable_repos.append("--disable=*")
-
-    disable_cmd.extend(disable_repos)
-    output, ret_code = utils.run_subprocess(disable_cmd, print_output=False)
+    cmd = ["subscription-manager", "repos"]
+    disabled_repos = ["*"] if not tool_opts.disablerepo else tool_opts.disablerepo
+    disable_cmd = ["".join("--disable=" + repo) for repo in disabled_repos]
+    cmd.extend(disable_cmd)
+    output, ret_code = utils.run_subprocess(cmd, print_output=False)
     if ret_code != 0:
         loggerinst.critical_no_exit("Could not disable subscription-manager repositories:\n%s" % output)
         raise exceptions.CriticalError(
@@ -817,8 +757,8 @@ def disable_repos():
             description="As part of the conversion process, convert2rhel disables all current subscription-manager repositories and enables only repositories required for the conversion. convert2rhel was unable to disable these repositories, and the conversion is unable to proceed.",
             diagnosis="Failed to disable repositories: %s." % (output),
         )
+
     loggerinst.info("Repositories disabled.")
-    return
 
 
 def enable_repos(rhel_repoids):
@@ -853,24 +793,24 @@ def enable_repos(rhel_repoids):
             # Try first if it's possible to enable EUS repoids. Otherwise try
             # enabling the default RHSM repoids. Otherwise, if it raiess an
             # exception, try to enable the default rhsm-repos
-            _submgr_enable_repos(repos_to_enable)
+            submgr_enable_repos(repos_to_enable)
         except SystemExit:
             loggerinst.info(
                 "The RHEL EUS repositories are not possible to enable.\n"
                 "Trying to enable standard RHEL repositories as a fallback."
             )
             # Fallback to the default_rhsm_repoids
             repos_to_enable = system_info.default_rhsm_repoids
-            _submgr_enable_repos(repos_to_enable)
+            submgr_enable_repos(repos_to_enable)
     else:
         # This could be either the default_rhsm repos or any user specific
         # repoids
-        _submgr_enable_repos(repos_to_enable)
+        submgr_enable_repos(repos_to_enable)
 
     system_info.submgr_enabled_repos = repos_to_enable
 
 
-def _submgr_enable_repos(repos_to_enable):
+def submgr_enable_repos(repos_to_enable):
     """Go through subscription manager repos and try to enable them through subscription-manager."""
     enable_cmd = ["subscription-manager", "repos"]
     for repo in repos_to_enable:

convert2rhel/unit_tests/actions/pre_ponr_changes/subscription_test.py

@@ -28,6 +28,7 @@
 from convert2rhel.actions import STATUS_CODE
 from convert2rhel.actions.pre_ponr_changes import subscription as appc_subscription
 from convert2rhel.actions.pre_ponr_changes.subscription import PreSubscription, SubscribeSystem
+from convert2rhel.backup.subscription import RestorableDisableRepositories, RestorableSystemSubscription
 from convert2rhel.subscription import RefreshSubscriptionManagerError, SubscriptionAutoAttachmentError
 from convert2rhel.unit_tests import AutoAttachSubscriptionMocked, RefreshSubscriptionManagerMocked, RunSubprocessMocked
 
@@ -222,15 +223,15 @@ def test_subscribe_system_dependency_order(self, subscribe_system_instance):
 
         assert expected_dependencies == subscribe_system_instance.dependencies
 
-    def test_subscribe_system_do_not_subscribe(self, global_tool_opts, subscribe_system_instance, monkeypatch, caplog):
+    def test_subscribe_system_do_not_subscribe(self, global_tool_opts, subscribe_system_instance, monkeypatch):
         global_tool_opts.no_rhsm = False
         # partial saves the real copy of tool_opts to use with
         # _should_subscribe so we have to monkeypatch with the mocked version
         # of tool_opts.
         monkeypatch.setattr(subscription, "should_subscribe", partial(toolopts._should_subscribe, global_tool_opts))
-        monkeypatch.setattr(subscription.RestorableSystemSubscription, "enable", mock.Mock())
+        monkeypatch.setattr(RestorableSystemSubscription, "enable", mock.Mock())
         monkeypatch.setattr(repo, "get_rhel_repoids", mock.Mock())
-        monkeypatch.setattr(subscription, "disable_repos", mock.Mock())
+        monkeypatch.setattr(RestorableDisableRepositories, "enable", mock.Mock())
         monkeypatch.setattr(subscription, "enable_repos", mock.Mock())
         monkeypatch.setattr(utils, "run_subprocess", RunSubprocessMocked())
 
@@ -291,17 +292,17 @@ def test_subscribe_system_not_registered(self, global_tool_opts, subscribe_syste
 
     def test_subscribe_system_run(self, subscribe_system_instance, monkeypatch):
         monkeypatch.setattr(subscription, "should_subscribe", lambda: True)
-        monkeypatch.setattr(subscription.RestorableSystemSubscription, "enable", mock.Mock())
+        monkeypatch.setattr(RestorableSystemSubscription, "enable", mock.Mock())
         monkeypatch.setattr(repo, "get_rhel_repoids", mock.Mock())
-        monkeypatch.setattr(subscription, "disable_repos", mock.Mock())
+        monkeypatch.setattr(RestorableDisableRepositories, "enable", mock.Mock())
         monkeypatch.setattr(subscription, "enable_repos", mock.Mock())
 
         subscribe_system_instance.run()
 
         assert subscribe_system_instance.result.level == STATUS_CODE["SUCCESS"]
-        assert subscription.RestorableSystemSubscription.enable.call_count == 1
+        assert RestorableSystemSubscription.enable.call_count == 1
         assert repo.get_rhel_repoids.call_count == 1
-        assert subscription.disable_repos.call_count == 1
+        assert RestorableDisableRepositories.enable.call_count == 1
         assert subscription.enable_repos.call_count == 1
 
     def test_subscribe_no_access_to_rhel_repos(self, subscribe_system_instance, monkeypatch):
@@ -316,7 +317,6 @@ def test_subscribe_no_access_to_rhel_repos(self, subscribe_system_instance, monk
         monkeypatch.setattr(utils, "run_subprocess", RunSubprocessMocked())
 
         subscribe_system_instance.run()
-        print(subscribe_system_instance.result)
         unit_tests.assert_actions_result(
             subscribe_system_instance,
             level="ERROR",
@@ -361,7 +361,7 @@ def test_subscribe_system_exceptions(self, exception, expected_level, subscribe_
         # In the actual code, the exceptions can happen at different stages, but
         # since it is a unit test, it doesn't matter what function will raise the
         # exception we want.
-        monkeypatch.setattr(subscription.RestorableSystemSubscription, "enable", mock.Mock(side_effect=exception))
+        monkeypatch.setattr(RestorableSystemSubscription, "enable", mock.Mock(side_effect=exception))
 
         subscribe_system_instance.run()