pes_events_scanner: fix processing of RpmTransactionTasks

repos/system_upgrade/common/actors/peseventsscanner/libraries/pes_events_scanner.py

@@ -78,19 +78,22 @@ def get_installed_pkgs():
 
 def get_transaction_configuration():
     """
-    Get pkgs to install, keep and remove from the user configuration files in /etc/leapp/transaction/.
+    Get pkgs to install, keep and remove from RpmTransactionTasks messages.
 
-    These configuration files have higher priority than PES data.
-    :return: RpmTransactionTasks model instance
+    Note these messages reflects inputs from various actors and configuration
+    files in /etc/leapp/transaction/. As these are explicit instruction, they
+    have higher priority than instructions from PES data.
+
+    :return: TransactionConfiguration
     """
-    transaction_configuration = TransactionConfiguration(to_install=[], to_remove=[], to_keep=[])
+    transaction_configuration = TransactionConfiguration(to_install=set(), to_remove=set(), to_keep=set())
 
     _Pkg = partial(Package, repository=None, modulestream=None)
 
     for tasks in api.consume(RpmTransactionTasks):
-        transaction_configuration.to_install.extend(_Pkg(name=pkg_name) for pkg_name in tasks.to_install)
-        transaction_configuration.to_remove.extend(_Pkg(name=pkg_name) for pkg_name in tasks.to_remove)
-        transaction_configuration.to_keep.extend(_Pkg(name=pkg_name) for pkg_name in tasks.to_keep)
+        transaction_configuration.to_install.update(_Pkg(name=pkg_name) for pkg_name in tasks.to_install)
+        transaction_configuration.to_remove.update(_Pkg(name=pkg_name) for pkg_name in tasks.to_remove)
+        transaction_configuration.to_keep.update(_Pkg(name=pkg_name) for pkg_name in tasks.to_keep)
     return transaction_configuration
 
 
@@ -467,9 +470,8 @@ def replace_pesids_with_repoids_in_packages(packages, source_pkgs_repoids):
     return packages_with_repoid.union(packages_without_pesid)
 
 
-def apply_transaction_configuration(source_pkgs):
+def apply_transaction_configuration(source_pkgs, transaction_configuration):
     source_pkgs_with_conf_applied = set(source_pkgs)
-    transaction_configuration = get_transaction_configuration()
 
     source_pkgs_with_conf_applied = source_pkgs.union(transaction_configuration.to_install)
 
@@ -501,38 +503,87 @@ def remove_leapp_related_events(events):
     return res
 
 
+def include_instructions_from_transaction_configuration(rpm_tasks, transaction_configuration, installed_pkgs):
+    """
+    Extend current rpm_tasks applying data from transaction_configuration
+
+    :param PESRpmTransactionTasks rpm_tasks: Currently calculated rpm tasks based on PES data.
+    :param TransactionConfiguration transaction_configuration: Tasked configured by user manually.
+    :param set(str) installed_pkgs: Set of distribution signed packages installed on the system.
+    :returns: updated tasks respecting configuration changes made by user
+    :rtype: PESRpmTransactionTasks
+    """
+    to_install_from_rpm_tasks = set() if not rpm_tasks else set(rpm_tasks.to_install)
+    to_remove_from_rpm_tasks = set() if not rpm_tasks else set(rpm_tasks.to_remove)
+    to_keep_from_rpm_tasks = set() if not rpm_tasks else set(rpm_tasks.to_keep)
+
+    # We don't want to try removing packages that are not installed - include only installed ones
+    installed_pkgs_requested_to_be_removed = transaction_configuration.to_remove.intersection(installed_pkgs)
+    pkgs_names_to_extend_to_remove_with = set(pkg.name for pkg in installed_pkgs_requested_to_be_removed)
+
+    # Add packages to 'to_install' only if they are not already requested to be installed by rpm_tasks
+    pkgs_names_requested_to_be_installed = set(pkg.name for pkg in transaction_configuration.to_install)
+    to_install_pkgs_names_missing_from_tasks = pkgs_names_requested_to_be_installed - to_install_from_rpm_tasks
+
+    pkg_names_user_wants_to_keep = {pkg.name for pkg in transaction_configuration.to_keep}
+
+    # Remove packages that were requested by rpm_tasks or by user, but exclude those that should be kept
+    new_to_remove_set = (to_remove_from_rpm_tasks | pkgs_names_to_extend_to_remove_with) - pkg_names_user_wants_to_keep
+    new_to_remove_list = sorted(new_to_remove_set)
+
+    new_to_install_list = sorted(to_install_from_rpm_tasks | to_install_pkgs_names_missing_from_tasks)
+    new_to_keep_list = sorted(to_keep_from_rpm_tasks | pkg_names_user_wants_to_keep)
+
+    if not any((new_to_remove_list, new_to_keep_list, new_to_install_list)):  # Are all empty?
+        return rpm_tasks  # We do not modify the original tasks
+
+    modules_to_enable = rpm_tasks.modules_to_enable if rpm_tasks else []
+    modules_to_reset = rpm_tasks.modules_to_reset if rpm_tasks else []
+
+    return PESRpmTransactionTasks(to_install=new_to_install_list,
+                                  to_remove=new_to_remove_list,
+                                  to_keep=new_to_keep_list,
+                                  modules_to_enable=modules_to_enable,
+                                  modules_to_reset=modules_to_reset)
+
+
 def process():
     # Retrieve data - installed_pkgs, transaction configuration, pes events
     events = get_pes_events('/etc/leapp/files', 'pes-events.json')
     if not events:
         return
 
     releases = get_relevant_releases(events)
-    source_pkgs = get_installed_pkgs()
-    source_pkgs = apply_transaction_configuration(source_pkgs)
+    installed_pkgs = get_installed_pkgs()
+    transaction_configuration = get_transaction_configuration()
+    pkgs_to_begin_computation_with = apply_transaction_configuration(installed_pkgs, transaction_configuration)
 
     # Keep track of what repoids have the source packages to be able to determine what are the PESIDs of the computed
     # packages of the target system, so we can distinguish what needs to be repomapped
-    repoids_of_source_pkgs = {pkg.repository for pkg in source_pkgs}
+    repoids_of_source_pkgs = {pkg.repository for pkg in pkgs_to_begin_computation_with}
 
     events = remove_leapp_related_events(events)
     events = remove_undesired_events(events, releases)
 
     # Apply events - compute what packages should the target system have
-    target_pkgs, pkgs_to_demodularize = compute_packages_on_target_system(source_pkgs, events, releases)
+    target_pkgs, pkgs_to_demodularize = compute_packages_on_target_system(pkgs_to_begin_computation_with,
+                                                                          events, releases)
 
     # Packages coming out of the events have PESID as their repository, however, we need real repoid
     target_pkgs = replace_pesids_with_repoids_in_packages(target_pkgs, repoids_of_source_pkgs)
 
     # Apply the desired repository blacklisting
-    blacklisted_repoids, target_pkgs = remove_new_packages_from_blacklisted_repos(source_pkgs, target_pkgs)
+    blacklisted_repoids, target_pkgs = remove_new_packages_from_blacklisted_repos(pkgs_to_begin_computation_with,
+                                                                                  target_pkgs)
 
     # Look at the target packages and determine what repositories to enable
     target_repoids = sorted(set(p.repository for p in target_pkgs) - blacklisted_repoids - repoids_of_source_pkgs)
     repos_to_enable = RepositoriesSetupTasks(to_enable=target_repoids)
     api.produce(repos_to_enable)
 
     # Compare the packages on source system and the computed packages on target system and determine what to install
-    rpm_tasks = compute_rpm_tasks_from_pkg_set_diff(source_pkgs, target_pkgs, pkgs_to_demodularize)
+    rpm_tasks = compute_rpm_tasks_from_pkg_set_diff(pkgs_to_begin_computation_with, target_pkgs, pkgs_to_demodularize)
+    rpm_tasks = include_instructions_from_transaction_configuration(rpm_tasks, transaction_configuration,
+                                                                    installed_pkgs)
     if rpm_tasks:
         api.produce(rpm_tasks)

repos/system_upgrade/common/actors/peseventsscanner/tests/test_pes_event_scanner.py

@@ -9,9 +9,7 @@
     api,
     compute_packages_on_target_system,
     compute_rpm_tasks_from_pkg_set_diff,
-    get_installed_pkgs,
     Package,
-    process,
     reporting,
     TransactionConfiguration
 )
@@ -27,8 +25,8 @@
     RepositoriesSetupTasks,
     RepositoryData,
     RepositoryFile,
-    RHUIInfo,
-    RPM
+    RPM,
+    RpmTransactionTasks
 )
 
 
@@ -286,17 +284,14 @@ def test_actor_performs(monkeypatch):
 def test_transaction_configuration_has_effect(monkeypatch):
     _Pkg = partial(Package, repository=None, modulestream=None)
 
-    def mocked_transaction_conf():
-        return TransactionConfiguration(
-            to_install=[_Pkg('pkg-a'), _Pkg('pkg-b')],
-            to_remove=[_Pkg('pkg-c'), _Pkg('pkg-d')],
-            to_keep=[]
-        )
-
-    monkeypatch.setattr(pes_events_scanner, 'get_transaction_configuration', mocked_transaction_conf)
+    transaction_cfg = TransactionConfiguration(
+        to_install=[_Pkg('pkg-a'), _Pkg('pkg-b')],
+        to_remove=[_Pkg('pkg-c'), _Pkg('pkg-d')],
+        to_keep=[]
+    )
 
     packages = {_Pkg('pkg-a'), _Pkg('pkg-c')}
-    _result = pes_events_scanner.apply_transaction_configuration(packages)
+    _result = pes_events_scanner.apply_transaction_configuration(packages, transaction_cfg)
     result = {(p.name, p.repository, p.modulestream) for p in _result}
     expected = {('pkg-a', None, None), ('pkg-b', None, None)}
 
@@ -340,7 +335,7 @@ def test_blacklisted_repoid_is_not_produced(monkeypatch):
 
     monkeypatch.setattr(pes_events_scanner, 'get_installed_pkgs', lambda: installed_pkgs)
     monkeypatch.setattr(pes_events_scanner, 'get_pes_events', lambda folder, filename: events)
-    monkeypatch.setattr(pes_events_scanner, 'apply_transaction_configuration', lambda pkgs: pkgs)
+    monkeypatch.setattr(pes_events_scanner, 'apply_transaction_configuration', lambda pkgs, transaction_cfg: pkgs)
     monkeypatch.setattr(pes_events_scanner, 'get_blacklisted_repoids', lambda: {'blacklisted-rhel8'})
     monkeypatch.setattr(pes_events_scanner, 'replace_pesids_with_repoids_in_packages',
                         lambda pkgs, src_pkgs_repoids: pkgs)
@@ -475,3 +470,58 @@ def test_remove_leapp_related_events(monkeypatch):
 
     out_events = pes_events_scanner.remove_leapp_related_events(in_events)
     assert out_events == expected_out_events
+
+
+def test_transaction_configuration_is_applied(monkeypatch):
+    installed_pkgs = {
+         Package(name='moved-in', repository='rhel7-base', modulestream=None),
+         Package(name='split-in', repository='rhel7-base', modulestream=None),
+         Package(name='pkg-not-in-events', repository='rhel7-base', modulestream=None),
+    }
+    monkeypatch.setattr(pes_events_scanner, 'get_installed_pkgs', lambda *args, **kwags: installed_pkgs)
+
+    Pkg = partial(Package, modulestream=None)
+    events = [
+        Event(1, Action.SPLIT,
+              {Pkg('split-in', 'rhel7-base')},
+              {Pkg('split-out0', 'rhel8-BaseOS'), Pkg('split-out1', 'rhel8-BaseOS')},
+              (7, 9), (8, 0), []),
+        Event(3, Action.MOVED,
+              {Pkg('moved-in', 'rhel7-base')}, {Pkg('moved-out', 'rhel8-BaseOS')},
+              (7, 9), (8, 0), []),
+    ]
+    monkeypatch.setattr(pes_events_scanner, 'get_pes_events', lambda *args, **kwargs: events)
+    monkeypatch.setattr(pes_events_scanner, 'remove_leapp_related_events', lambda events: events)
+    monkeypatch.setattr(pes_events_scanner, 'remove_undesired_events', lambda events, releases: events)
+    monkeypatch.setattr(pes_events_scanner, '_get_enabled_modules', lambda *args: [])
+    monkeypatch.setattr(pes_events_scanner, 'replace_pesids_with_repoids_in_packages',
+                        lambda target_pkgs, repoids_of_source_pkgs: target_pkgs)
+    monkeypatch.setattr(pes_events_scanner,
+                        'remove_new_packages_from_blacklisted_repos',
+                        lambda source_pkgs, target_pkgs: (set(), target_pkgs))
+
+    msgs = [
+        RpmTransactionTasks(to_remove=['pkg-not-in-events']),
+        RpmTransactionTasks(to_remove=['pkg-not-in-events', 'pkg-not-in-events']),
+        RpmTransactionTasks(to_install=['pkg-to-install']),
+        RpmTransactionTasks(to_keep=['keep-me']),
+    ]
+    mocked_actor = CurrentActorMocked(arch='x86_64', src_ver='7.9', dst_ver='8.8', msgs=msgs)
+    monkeypatch.setattr(api, 'current_actor', mocked_actor)
+
+    monkeypatch.setattr(api, 'produce', produce_mocked())
+
+    pes_events_scanner.process()
+
+    assert api.produce.called == 2
+
+    produced_rpm_transaction_tasks = [
+        msg for msg in api.produce.model_instances if isinstance(msg, PESRpmTransactionTasks)
+    ]
+
+    assert len(produced_rpm_transaction_tasks) == 1
+    rpm_transaction_tasks = produced_rpm_transaction_tasks[0]
+    # It is important to see 'pkg-not-in-events' in the list - if the user says remove pkg A, we really remove it
+    assert sorted(rpm_transaction_tasks.to_remove) == ['moved-in', 'pkg-not-in-events', 'split-in']
+    assert sorted(rpm_transaction_tasks.to_install) == ['moved-out', 'pkg-to-install', 'split-out0', 'split-out1']
+    assert sorted(rpm_transaction_tasks.to_keep) == ['keep-me']