[RHELC-884] Disable RHEL repos when performing checks #1174

hosekadam · 2024-04-05T15:57:28Z

We need to disable RHEL repos. If user registers the system to satellite which provides Centos and RHEL repos and the RHEL repos would be enabled, it would cause a fail. The recommended way is to disable the rhel repos prior the conversion, but is expected it might be forgotten. This helps avoiding the problems caused by wrong setup of the repos.

Jira Issues: https://issues.redhat.com/browse/RHELC-884

Checklist

PR has been tested manually in a VM (either author or reviewer)
Jira issue has been made public if possible
[RHELC-] is part of the PR title
GitHub label has been added to help with Release notes
PR title explains the change from the user's point of view
Code and tests are documented properly
The commits are squashed to as few commits as possible (without losing data)
When merged: Jira issue has been updated to Release Pending if relevant

codecov · 2024-04-05T16:01:10Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.44%. Comparing base (63caca2) to head (63656f8).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1174      +/-   ##
==========================================
+ Coverage   95.42%   95.44%   +0.02%     
==========================================
  Files          54       54              
  Lines        4699     4720      +21     
  Branches      829      833       +4     
==========================================
+ Hits         4484     4505      +21     
  Misses        132      132              
  Partials       83       83

Flag	Coverage Δ
centos-linux-7	`90.59% <90.00%> (-0.05%)`	⬇️
centos-linux-8	`91.53% <92.50%> (-0.03%)`	⬇️
centos-linux-9	`91.58% <92.50%> (-0.03%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

r0x0d · 2024-04-08T12:22:45Z

@hosekadam, looking at how we are disabling the repos, maybe we should use rhel* instead of rhel-*?

I'm asking this more because if RHEL introduces a new repo, or modifies the name at some point to have something like: rhel9-something-foo-bar, and this repository is enabled, then we will miss this repository by the time we are running our tools.

hosekadam · 2024-04-08T12:27:47Z

@r0x0d Thanks for taking a look! I used rhel-* based on the names of the repos, But rhel* might be more generic and future-proof. I'll switch to that.

convert2rhel/pkghandler.py

convert2rhel/actions/system_checks/is_loaded_kernel_latest.py

r0x0d · 2024-04-17T19:00:32Z

convert2rhel/unit_tests/pkghandler_test.py

+    # Import the YUM inside the test. If YUM is not present the test isn't run
+    import yum


Why not using pkgmanager for this?

I'll take a look, there is one more problem with mocking the doPackageList - the tests failed. It was just a test if that yum.repos.Repository works :D

Now it's working, the tests test_get_packages_to_update_yum_rhel and test_get_packages_to_update_dnf_rhel are ready @r0x0d

hosekadam · 2024-04-19T17:22:04Z

So from the last time:

unit tests for check if system is up-to-date were added for both yum and dnf
changed the disable pattern from rhel-* to rhel*
added the --disablerepo=rhel* into the unit tests for latest kernel
@bocekm found there can be problem during backup, when the RHEL repos are enabled and the pkg to download has the same NEVRA in original vendor and rhel repos. There is possibility the RHEL one would be downloaded as backup of some pkg we need to backup (we are backuping just the special packages we are removing).
I was looking also at the RestorablePackageSet but I don't think we want to disable RHEL repos here, I think they are needed in this place.

I'm not sure if there is requirement to add unit tests for RestorablePackage.enable() (right now we are just checking if utils.download_pkg was called) and for IsLoadedKernelLatest.run() (there I added the argument to the list of expected args).

Venefilyn

LGTM, but confused about the skip if index 0 part

Venefilyn · 2024-04-26T11:28:37Z

convert2rhel/unit_tests/pkghandler_test.py

+    assert rhel_repo_id in enabled_repos
+
+    # Choose one of the enabled repos on the system
+    # Avoid getting the rhel_repo_id if on 0 index, then get the repo id from 1


I'm trying to disable one of the repos available on the system to test repo provided through the tool_opts (monkeypatch.setattr(tool_opts, "enablerepo", repo_for_disable)). I want to have something different than the added RHEL repo but still present on the system.

So I'm getting it in this way - I know if on the 0 index is rhel repo, on the 1 will be something different I can use

I'd write that out a bit clearer in the text, avoid getting something is clear but not super clear in the comments of why you do it

Comment about it added.

convert2rhel/unit_tests/pkghandler_test.py

convert2rhel/pkghandler.py

r0x0d

Looks good!

hosekadam · 2024-05-02T16:26:24Z

@pr-watson needs to try something. The PR is now WIP

convert2rhel/backup/packages.py

convert2rhel/pkghandler.py

hosekadam · 2024-05-13T16:00:03Z

created repo.get_rhel_repos_to_disable() to reduce duplicity
moved preparation of the disablerepo list higher in the chain in many places to reduce duplicity

TBD:

one place waiting to be solved in _get_package_repositories, the repoquery should contain the "--disablerepo=rhel*" (pushing the rest for having backup), tested with it, needs unit tests and better implementation
solve the conflicts :(
consider disabling also toolopts.disablerepo

hosekadam · 2024-05-14T08:28:25Z

rebased

hosekadam · 2024-05-14T09:23:03Z

_get_package_repositories, the repoquery now contains the "--disablerepo=rhel*"

convert2rhel/unit_tests/backup/packages_test.py

hosekadam · 2024-05-14T15:39:13Z

convert2rhel/pkghandler.py

@@ -393,8 +393,11 @@ def _get_package_repositories(pkgs):
    if system_info.version.major >= 8:
        query_format = "C2R %{NAME}-%{EPOCH}:%{VERSION}-%{RELEASE}.%{ARCH}&%{REPOID}\n"

+    # RHELC-884 disable the RHEL repos to avoid downloading pkgs from them.
+    disable_repo_command = repo.get_rhel_disable_repos_command()


This is part of discussion. Will be changed to be sent via argument from list third party pkgs to be more future-proof. Will do tomorrow

EDIT: During testing and final checking the output found another :( repoquery in RemoveSpecialPackages, will handle similarly as in the ListThirdPartyPackages

hosekadam · 2024-05-16T11:04:28Z

refactor how the disable_repos is passed
disabled RHEL repos when retrieving repo info from pkgs in RemoveSpecialPkgs and ListThirdPartyPackages (caused by the refactor - in the first attempt were the rhel repos disabled whenever the function was called. Now the caller can affect if the repos (and which) should be disabled)
improved comments

About the toolopts.disablerepo @r0x0d - discussed it with @bocekm, and we found that it's not needed. In fact, the option is maybe something what can be removed in the future.

Related issue: [RHELC-884] We need to disable RHEL repos when performing checks. If user registers the system to satellite which provides Centos and RHEL repos and the RHEL repos would be enabled, it would cause a fail. The recommended way is to disable the rhel repos prior the conversion, but is expected it might be forgotten. This helps avoiding the problems caused by wrong setup of the repos. RHEL repos are disabled when: * trying to download backup of package present on original system * when checking if kernel and packages are up-to-date * when retrieving repository info from packages (RemoveSpecialPkgs and ListThirdParty packages)

has-bot · 2024-05-17T10:21:47Z

/packit test --labels tier0

Comment generated by an automation.

Log | Bot Usage

r0x0d

Looks good to me. Test failure is not related

hosekadam force-pushed the disable-rhel-repos-when-performing-checks branch from 3787342 to dcaa034 Compare April 5, 2024 15:59

r0x0d reviewed Apr 10, 2024

View reviewed changes

convert2rhel/pkghandler.py Outdated Show resolved Hide resolved

r0x0d reviewed Apr 10, 2024

View reviewed changes

convert2rhel/pkghandler.py Outdated Show resolved Hide resolved

r0x0d reviewed Apr 10, 2024

View reviewed changes

convert2rhel/actions/system_checks/is_loaded_kernel_latest.py Outdated Show resolved Hide resolved

hosekadam force-pushed the disable-rhel-repos-when-performing-checks branch 4 times, most recently from a5c22e2 to 6e4d131 Compare April 17, 2024 15:48

r0x0d reviewed Apr 17, 2024

View reviewed changes

hosekadam force-pushed the disable-rhel-repos-when-performing-checks branch 2 times, most recently from 612e7d0 to e6db3b0 Compare April 19, 2024 17:10

hosekadam requested review from r0x0d and Venefilyn April 19, 2024 17:22

hosekadam marked this pull request as ready for review April 19, 2024 17:23

Venefilyn reviewed Apr 26, 2024

View reviewed changes

r0x0d reviewed Apr 26, 2024

View reviewed changes

convert2rhel/pkghandler.py Outdated Show resolved Hide resolved

r0x0d approved these changes Apr 26, 2024

View reviewed changes

hosekadam force-pushed the disable-rhel-repos-when-performing-checks branch from e6db3b0 to 0a84a44 Compare April 30, 2024 10:07

hosekadam marked this pull request as draft May 2, 2024 16:25

r0x0d reviewed May 13, 2024

View reviewed changes

convert2rhel/backup/packages.py Outdated Show resolved Hide resolved

r0x0d reviewed May 13, 2024

View reviewed changes

convert2rhel/pkghandler.py Outdated Show resolved Hide resolved

hosekadam force-pushed the disable-rhel-repos-when-performing-checks branch from 4bcd581 to d6f2a90 Compare May 13, 2024 15:59

hosekadam force-pushed the disable-rhel-repos-when-performing-checks branch from d6f2a90 to 8139107 Compare May 14, 2024 08:28

hosekadam force-pushed the disable-rhel-repos-when-performing-checks branch from 62767d7 to 960c86f Compare May 14, 2024 09:22

hosekadam marked this pull request as ready for review May 14, 2024 09:23

github-advanced-security bot found potential problems May 14, 2024

View reviewed changes

convert2rhel/unit_tests/backup/packages_test.py Fixed Show fixed Hide fixed

hosekadam force-pushed the disable-rhel-repos-when-performing-checks branch 2 times, most recently from 28ea6f0 to 737e36b Compare May 14, 2024 09:44

hosekadam commented May 14, 2024

View reviewed changes

hosekadam force-pushed the disable-rhel-repos-when-performing-checks branch from 737e36b to 45901ba Compare May 16, 2024 11:02

hosekadam force-pushed the disable-rhel-repos-when-performing-checks branch from 45901ba to 63656f8 Compare May 16, 2024 11:04

hosekadam requested review from Venefilyn and r0x0d May 16, 2024 11:06

hosekadam added kind/feature New feature or request tests/tier0 PR ready to run the essential test suit. Equivalent to `/packit test --labels tier0`. labels May 17, 2024

r0x0d approved these changes May 20, 2024

View reviewed changes

Venefilyn merged commit e23b48b into oamg:main May 21, 2024
38 of 41 checks passed

hosekadam mentioned this pull request May 27, 2024

Bump version to 2.0.0 #1243

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RHELC-884] Disable RHEL repos when performing checks #1174

[RHELC-884] Disable RHEL repos when performing checks #1174

hosekadam commented Apr 5, 2024 •

edited

Loading

codecov bot commented Apr 5, 2024 •

edited

Loading

r0x0d commented Apr 8, 2024

hosekadam commented Apr 8, 2024

r0x0d Apr 17, 2024

hosekadam Apr 18, 2024

hosekadam Apr 18, 2024

hosekadam commented Apr 19, 2024 •

edited

Loading

Venefilyn left a comment

Venefilyn Apr 26, 2024

hosekadam Apr 26, 2024 •

edited

Loading

Venefilyn Apr 26, 2024

hosekadam May 14, 2024

r0x0d left a comment

hosekadam commented May 2, 2024

hosekadam commented May 13, 2024 •

edited

Loading

hosekadam commented May 14, 2024

hosekadam commented May 14, 2024 •

edited

Loading

hosekadam May 14, 2024 •

edited

Loading

hosekadam commented May 16, 2024 •

edited

Loading

has-bot commented May 17, 2024

r0x0d left a comment

		# Import the YUM inside the test. If YUM is not present the test isn't run
		import yum

[RHELC-884] Disable RHEL repos when performing checks #1174

[RHELC-884] Disable RHEL repos when performing checks #1174

Conversation

hosekadam commented Apr 5, 2024 • edited Loading

codecov bot commented Apr 5, 2024 • edited Loading

Codecov Report

r0x0d commented Apr 8, 2024

hosekadam commented Apr 8, 2024

r0x0d Apr 17, 2024

Choose a reason for hiding this comment

hosekadam Apr 18, 2024

Choose a reason for hiding this comment

hosekadam Apr 18, 2024

Choose a reason for hiding this comment

hosekadam commented Apr 19, 2024 • edited Loading

Venefilyn left a comment

Choose a reason for hiding this comment

Venefilyn Apr 26, 2024

Choose a reason for hiding this comment

hosekadam Apr 26, 2024 • edited Loading

Choose a reason for hiding this comment

Venefilyn Apr 26, 2024

Choose a reason for hiding this comment

hosekadam May 14, 2024

Choose a reason for hiding this comment

r0x0d left a comment

Choose a reason for hiding this comment

hosekadam commented May 2, 2024

hosekadam commented May 13, 2024 • edited Loading

hosekadam commented May 14, 2024

hosekadam commented May 14, 2024 • edited Loading

hosekadam May 14, 2024 • edited Loading

Choose a reason for hiding this comment

hosekadam commented May 16, 2024 • edited Loading

has-bot commented May 17, 2024

r0x0d left a comment

Choose a reason for hiding this comment

hosekadam commented Apr 5, 2024 •

edited

Loading

codecov bot commented Apr 5, 2024 •

edited

Loading

hosekadam commented Apr 19, 2024 •

edited

Loading

hosekadam Apr 26, 2024 •

edited

Loading

hosekadam commented May 13, 2024 •

edited

Loading

hosekadam commented May 14, 2024 •

edited

Loading

hosekadam May 14, 2024 •

edited

Loading

hosekadam commented May 16, 2024 •

edited

Loading