Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[RHELC-1635] State action in special pkg removal report msgs #1297

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[RHELC-1635] State action in special pkg removal report msgs #1297

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions convert2rhel/actions/pre_ponr_changes/handle_packages.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,14 +146,21 @@ def run(self):
# shows which packages were not removed, if false, all packages were removed
pkgs_not_removed = sorted(frozenset(pkghandler.get_pkg_nevras(all_pkgs)).difference(pkgs_removed))
if pkgs_not_removed:
message = "The following packages were not removed: %s" % ", ".join(pkgs_not_removed)
message = "The following packages cannot be removed: %s" % ", ".join(pkgs_not_removed)
logger.warning(message)
self.add_message(
level="WARNING",
id="SPECIAL_PACKAGES_NOT_REMOVED",
title="Special packages not removed",
description="Special packages which could not be removed",
diagnosis=message,
title="Some packages cannot be removed",
Comment on lines 153 to +154
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should also update the id? It feels strange that in the id we call them as "special" and in the title is just "some packages"

description=message,
diagnosis=(
"The packages match a pre-defined list of packages that are to be removed during the"
" conversion. This list includes packages that are known to cause a conversion failure."
),
remediations=(
"We do a conversion dry-run (yum transaction validation) later on as part of the pre-conversion"
" analysis. If the dry-run fails, try removing the packages listed above manually."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to add there also command customer can use?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first sentence in the remediations seems to be they would be a better fit in the diagnosis, I belive... Right now, it doesn't seem that they are fitting in the remediations correctly.

I tend to think that remediation would be a step-to-step on how to solve the problem, not really an explanation of why it failed.

Do we want to add there also command customer can use?

If we know what command can be used, I don't see why not. Would be useful

),
)

if pkgs_removed:
Expand All @@ -162,12 +169,13 @@ def run(self):
self.add_message(
level="INFO",
id="SPECIAL_PACKAGES_REMOVED",
title="Special packages to be removed",
description=(
title="Packages to be removed",
description=message,
diagnosis=(
"We have identified installed packages that match a pre-defined list of packages that are"
" to be removed during the conversion"
" known to cause a conversion failure."
),
diagnosis=message,
remediations=("Check that the system runs correctly without the packages after the conversion."),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bookwar, do you see a better action the user can take?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
remediations=("Check that the system runs correctly without the packages after the conversion."),
remediations="Check that the system runs correctly without the packages after the conversion.",

)

super(RemoveSpecialPackages, self).run()
Expand Down
74 changes: 36 additions & 38 deletions convert2rhel/unit_tests/actions/pre_ponr_changes/handle_packages_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,20 +166,19 @@ def test_run_no_packages_to_remove(self, monkeypatch, remove_special_packages_in
def test_run_all_removed(self, monkeypatch, remove_special_packages_instance):
pkgs_to_remove = [get_centos_logos_pkg_object()]
pkgs_removed = ["centos-logos-70.0.6-3.el7.centos.noarch"]
expected = set(
(
actions.ActionMessage(
level="INFO",
id="SPECIAL_PACKAGES_REMOVED",
title="Special packages to be removed",
description="We have identified installed packages that match a pre-defined list of packages that are"
" to be removed during the conversion",
diagnosis="The following packages will be removed during the conversion: centos-logos-70.0.6-3.el7.centos.noarch",
remediations=None,
variables={},
),
expected = {
actions.ActionMessage(
level="INFO",
id="SPECIAL_PACKAGES_REMOVED",
title="Packages to be removed",
description="The following packages will be removed during the conversion:"
" centos-logos-70.0.6-3.el7.centos.noarch",
diagnosis="We have identified installed packages that match a pre-defined list of packages that"
" are known to cause a conversion failure.",
remediations="Check that the system runs correctly without the packages after the conversion.",
variables={},
)
)
}
monkeypatch.setattr(
pkghandler, "get_packages_to_remove", GetPackagesToRemoveMocked(return_value=pkgs_to_remove)
)
Expand All @@ -199,31 +198,30 @@ def test_run_all_removed(self, monkeypatch, remove_special_packages_instance):
@centos8
def test_run_packages_not_removed(self, pretend_os, monkeypatch, remove_special_packages_instance):
pkgs_removed = ["kernel-core"]
expected = set(
(
actions.ActionMessage(
level="WARNING",
id="SPECIAL_PACKAGES_NOT_REMOVED",
title="Special packages not removed",
description="Special packages which could not be removed",
diagnosis="The following packages were not removed: gpg-pubkey-1.0.0-1.x86_64, pkg1-None-None.None, pkg2-None-None.None",
remediations=None,
variables={},
),
actions.ActionMessage(
level="INFO",
id="SPECIAL_PACKAGES_REMOVED",
title="Special packages to be removed",
description=(
"We have identified installed packages that match a pre-defined list of packages that are"
" to be removed during the conversion"
),
diagnosis="The following packages will be removed during the conversion: kernel-core",
remediations=None,
variables={},
),
)
)
expected = {
actions.ActionMessage(
level="WARNING",
id="SPECIAL_PACKAGES_NOT_REMOVED",
title="Some packages cannot be removed",
description="The following packages cannot be removed: gpg-pubkey-1.0.0-1.x86_64, pkg1-None-None.None,"
" pkg2-None-None.None",
diagnosis="The packages match a pre-defined list of packages that are to be removed during the"
" conversion. This list includes packages that are known to cause a conversion failure.",
remediations="We do a conversion dry-run (yum transaction validation) later on as part of the"
" pre-conversion analysis. If the dry-run fails, try removing the packages listed above manually.",
variables={},
),
actions.ActionMessage(
level="INFO",
id="SPECIAL_PACKAGES_REMOVED",
title="Packages to be removed",
description="The following packages will be removed during the conversion: kernel-core",
diagnosis="We have identified installed packages that match a pre-defined list of packages that"
" are known to cause a conversion failure.",
remediations="Check that the system runs correctly without the packages after the conversion.",
variables={},
),
}
monkeypatch.setattr(
pkghandler, "get_packages_to_remove", GetPackagesToRemoveMocked(pkg_selection="fingerprints")
)
Expand Down
Loading