Clear yum cache before any yum call #398

r0x0d · 2022-01-10T17:15:41Z

This change intends to call the check_custom_repos_are_valid function
as the first thing before any other checks. The reason for that is we
can guarantee that we will work with the most recent cache for
yum/dnf.

Jira reference (If any): RHELC-251
Bugzilla reference (If any):

r0x0d · 2022-01-10T17:25:29Z

Looking through the code we don't seem to mess with anything related to repos or pkg manager before the checks start to take place. I think that just moving the function at the top of the checks should do the trick.

@SpyTec, do you think we should rewrite this function to not be a check, but rather, do this as a special operation of cleaning/making cache? Then this check_custom_repos_are_valid could be rewritten in another way to just check against custom repos, and not deal with cache generation.

Something like this (this is just an example): https://gist.github.com/r0x0d/f645a1f7b005ee96e9421954e27528db

Venefilyn · 2022-01-11T15:48:47Z

@SpyTec, do you think we should rewrite this function to not be a check, but rather, do this as a special operation of cleaning/making cache? Then this check_custom_repos_are_valid could be rewritten in another way to just check against custom repos, and not deal with cache generation.

Something like this (this is just an example): gist.github.com/r0x0d/f645a1f7b005ee96e9421954e27528db

@r0x0d That would be out of scope for the issue and can instead be tackled in OAMG-5570. For this PR we should just move it to be in the beginning. We also need to do more verification to make sure using repolist will work for us and be faster

r0x0d · 2022-01-11T16:10:18Z

@SpyTec, do you think we should rewrite this function to not be a check, but rather, do this as a special operation of cleaning/making cache? Then this check_custom_repos_are_valid could be rewritten in another way to just check against custom repos, and not deal with cache generation.
Something like this (this is just an example): gist.github.com/r0x0d/f645a1f7b005ee96e9421954e27528db

@r0x0d That would be out of scope for the issue and can instead be tackled in OAMG-5570. For this PR we should just move it to be in the beginning. We also need to do more verification to make sure using repolist will work for us and be faster

Gotcha! I will upload this gist to OAMG-5570 in case anyone in the future wants to work on that and they can take a look at this prototype of implementation.

bocekm

The main goal of this change should be making sure that the yum cache is up-to-date before we start doing some checks - especially the check whether the installed packages including kernel are up-to-date (handled bycheck_package_updates()). We can do that by clearing the yum cache. That's what we do already in the function for checking whether custom repos used by the customer are accessible: https://github.com/oamg/convert2rhel/blob/v0.25/convert2rhel/checks.py#L165.

However this cache cleaning happens only when custom repos are used (--no-rhsm option is used). Yet we need to clean the cache every time - because the check_package_updates() is called every time.

There's a disadvantage of performing the cache cleaning every time though - rebuilding the cache takes long (minutes). But we don't currently have a better/faster way of making sure the yum cash is up-to-date. We can try speeding it up in a subsequent PR.

r0x0d · 2022-01-26T17:38:44Z

@bocekm you think that this draft I made for the PR could solve the issue? https://gist.github.com/r0x0d/f645a1f7b005ee96e9421954e27528db

We could also include a command (or environment var) like --skip-yum-cache if the user guarantees to us that the yum metadata/cache is new.

What do you guys think?

bocekm · 2022-02-08T12:15:20Z

@bocekm you think that this draft I made for the PR could solve the issue? https://gist.github.com/r0x0d/f645a1f7b005ee96e9421954e27528db

Feel free to introduce the change from the gist to this PR, so that we can do the review using the github tools like code line comments and suggestions.
Based on what did you choose yum repolist as an alternative to yum makecache? I remember that repolist may not be what we're looking for #196 (comment). But maybe with a clean cache it is the right command, hence my question.
Also, maybe the makecache is not necessary to run in the generate_latest_yum_cache(). What we can do is just clean the cache there and let the first next yum call build the cache with just the data it needs for the executed yum command. I have a feeling that makecache downloads way too many data, more than we need.

We could also include a command (or environment var) like --skip-yum-cache if the user guarantees to us that the yum metadata/cache is new.

Yeah, it'd be useful to have for development purposes. But only for that. We can't get any guarantee on that from the user. For that reason I'd introduce env var like CONVERT2RHEL_UNSUPPORTED_DONT_CLEAN_YUM_CACHE. Not in this PR but in a separate one though. We may find out that not running makecache makes the subsequent yum calls faster.

Ccing @abadger.

convert2rhel/checks.py

convert2rhel/main.py

convert2rhel/pkghandler.py

convert2rhel/unit_tests/pkghandler_test.py

bocekm

Approved. Waiting on an integration test.

danmyway · 2022-03-08T08:31:37Z

/packit test

danmyway · 2022-03-28T12:54:08Z

/packit test

r0x0d · 2022-03-28T12:59:38Z

Fixing the conflicts

codecov · 2022-03-28T13:04:56Z

Codecov Report

Merging #398 (ca4594f) into main (38d8621) will increase coverage by 1.58%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #398      +/-   ##
==========================================
+ Coverage   87.67%   89.25%   +1.58%     
==========================================
  Files          17       17              
  Lines        2458     2466       +8     
  Branches      425      426       +1     
==========================================
+ Hits         2155     2201      +46     
+ Misses        241      203      -38     
  Partials       62       62

Impacted Files	Coverage Δ
convert2rhel/checks.py	`95.68% <100.00%> (-0.02%)`	⬇️
convert2rhel/main.py	`85.23% <100.00%> (+25.37%)`	⬆️
convert2rhel/pkghandler.py	`91.98% <100.00%> (+0.12%)`	⬆️
convert2rhel/utils.py	`80.00% <0.00%> (+0.43%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 38d8621...ca4594f. Read the comment docs.

danmyway · 2022-03-29T08:15:59Z

Hi @r0x0d !
The tests are failing mainly on this:

  File "/usr/lib/python3.6/site-packages/convert2rhel/main.py", line 84, in main
    pkghandler.clean_yum_metadata()
  File "/usr/lib/python3.6/site-packages/convert2rhel/pkghandler.py", line 980, in clean_yum_metadata
    output, ret_code = call_yum_cmd(command="clean", args=["metadata"], print_output=False)
  File "/usr/lib/python3.6/site-packages/convert2rhel/pkghandler.py", line 151, in call_yum_cmd
    if system_info.version.major == 8:
AttributeError: 'NoneType' object has no attribute 'major'

Could you please look into it?

r0x0d · 2022-07-13T15:19:38Z

Rebased.

r0x0d · 2022-07-14T00:17:48Z

Fixed the unit_test for centos7

kokesak · 2022-07-14T17:54:09Z

The failing integration tests are caused by incorrect cleanup in one of our tests. The fix for that is introduced here: 41fbb29

This change intends to call the `check_custom_repos_are_valid` function as the first thing before any other checks. The reason for that is we can guarantee that we will work with the most recent cache for `yum`/`dnf`. Jira reference (If any): https://issues.redhat.com/browse/OAMG-6294 Bugzilla reference (If any):

Signed-off-by: Rodolfo Olivieri <[email protected]>

for more information, see https://pre-commit.ci

Signed-off-by: Rodolfo Olivieri <[email protected]>

bocekm

One little nitpick otherwise it works and looks good.

convert2rhel/main.py

Co-authored-by: Michal Bocek <[email protected]>

danmyway · 2022-07-25T07:50:42Z

/packit retest-failed

tests/integration/tier0/basic-sanity-checks/test_basic_sanity_checks.py

danmyway · 2022-07-26T11:05:31Z

I have run basic_sanity_checks with fixes from 41fbb29 and #398 (comment)
All of them have passed.
We can either wait for #447 to be merged and re-run the CI with rebased fixes or merge this PR after #398 (comment) is addressed.

…checks.py Co-authored-by: Daniel Diblik <[email protected]>

danmyway

LGTM! Thank you @kokesak

r0x0d self-assigned this Jan 10, 2022

r0x0d requested review from bocekm and Venefilyn January 10, 2022 17:15

r0x0d marked this pull request as draft January 10, 2022 17:31

Venefilyn approved these changes Jan 11, 2022

View reviewed changes

Venefilyn previously approved these changes Jan 26, 2022

View reviewed changes

bocekm requested changes Jan 26, 2022

View reviewed changes

r0x0d dismissed Venefilyn’s stale review via 8d67280 February 10, 2022 17:46

r0x0d force-pushed the move-cache-generation-to-beginning-of-conversion branch from 877cb22 to 8d67280 Compare February 10, 2022 17:46

r0x0d marked this pull request as ready for review February 10, 2022 18:00

bocekm requested changes Feb 22, 2022

View reviewed changes

r0x0d added the need-integration-tests label Feb 22, 2022

r0x0d force-pushed the move-cache-generation-to-beginning-of-conversion branch 2 times, most recently from d30ffab to eee495a Compare February 23, 2022 16:33

bocekm requested changes Feb 23, 2022

View reviewed changes

convert2rhel/unit_tests/pkghandler_test.py Outdated Show resolved Hide resolved

r0x0d force-pushed the move-cache-generation-to-beginning-of-conversion branch from eee495a to 0760283 Compare February 23, 2022 17:39

bocekm previously approved these changes Feb 23, 2022

View reviewed changes

r0x0d dismissed bocekm’s stale review via 52141e1 March 28, 2022 13:02

r0x0d force-pushed the move-cache-generation-to-beginning-of-conversion branch from 0760283 to 52141e1 Compare March 28, 2022 13:02

r0x0d force-pushed the move-cache-generation-to-beginning-of-conversion branch from 730eaab to 6ea9ff0 Compare March 28, 2022 13:06

r0x0d force-pushed the move-cache-generation-to-beginning-of-conversion branch from 9a9b860 to d299bc9 Compare July 13, 2022 15:18

bocekm changed the title ~~Call check_custom_repos_are_valid before any other checks~~ Clear yum cache before any yum call Jul 13, 2022

r0x0d mentioned this pull request Jul 19, 2022

[RHELC-576, RHELC-596] Merge yum transactions into a single one #528

Merged

18 tasks

r0x0d and others added 10 commits July 19, 2022 16:28

Fix position for clean_yum_metadata

3ea1f69

Signed-off-by: Rodolfo Olivieri <[email protected]>

Use run_subprocess instead of call_yum_cmd

f0abce6

Signed-off-by: Rodolfo Olivieri <[email protected]>

Fix unit test

6ce031b

Signed-off-by: Rodolfo Olivieri <[email protected]>

Add mocked unit test for main

f5c1b6e

Signed-off-by: Rodolfo Olivieri <[email protected]>

Fix lint and rebase

5014695

Signed-off-by: Rodolfo Olivieri <[email protected]>

[pre-commit.ci] auto fixes from pre-commit.com hooks

5b006af

for more information, see https://pre-commit.ci

Int test for the yum cache

b31a4e1

[pre-commit.ci] auto fixes from pre-commit.com hooks

f077eac

for more information, see https://pre-commit.ci

Fix main_test.py

421b930

Signed-off-by: Rodolfo Olivieri <[email protected]>

r0x0d force-pushed the move-cache-generation-to-beginning-of-conversion branch from b57a65d to 421b930 Compare July 19, 2022 19:28

bocekm previously approved these changes Jul 20, 2022

View reviewed changes

convert2rhel/main.py Outdated Show resolved Hide resolved

Update convert2rhel/main.py

6af724d

Co-authored-by: Michal Bocek <[email protected]>

r0x0d dismissed bocekm’s stale review via 6af724d July 21, 2022 12:07

bocekm previously approved these changes Jul 21, 2022

View reviewed changes

danmyway requested changes Jul 26, 2022

View reviewed changes

tests/integration/tier0/basic-sanity-checks/test_basic_sanity_checks.py Outdated Show resolved Hide resolved

tests/integration/tier0/basic-sanity-checks/test_basic_sanity_checks.py Outdated Show resolved Hide resolved

Update tests/integration/tier0/basic-sanity-checks/test_basic_sanity_…

2981cd1

…checks.py Co-authored-by: Daniel Diblik <[email protected]>

r0x0d dismissed bocekm’s stale review via 2981cd1 July 26, 2022 12:07

Update tests/integration/tier0/basic-sanity-checks/test_basic_sanity_…

ca4594f

…checks.py Co-authored-by: Daniel Diblik <[email protected]>

danmyway approved these changes Jul 26, 2022

View reviewed changes

danmyway merged commit 510555d into oamg:main Jul 26, 2022

bocekm mentioned this pull request Aug 19, 2022

Bump version to 1.0 #576

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clear yum cache before any yum call #398

Clear yum cache before any yum call #398

r0x0d commented Jan 10, 2022 •

edited by Venefilyn

Loading

r0x0d commented Jan 10, 2022 •

edited

Loading

Venefilyn commented Jan 11, 2022 •

edited

Loading

r0x0d commented Jan 11, 2022

bocekm left a comment

r0x0d commented Jan 26, 2022 •

edited

Loading

bocekm commented Feb 8, 2022 •

edited

Loading

bocekm left a comment

danmyway commented Mar 8, 2022

danmyway commented Mar 28, 2022

r0x0d commented Mar 28, 2022

codecov bot commented Mar 28, 2022 •

edited

Loading

danmyway commented Mar 29, 2022

r0x0d commented Jul 13, 2022

r0x0d commented Jul 14, 2022

kokesak commented Jul 14, 2022

bocekm left a comment

danmyway commented Jul 25, 2022

danmyway commented Jul 26, 2022

danmyway left a comment

Clear yum cache before any yum call #398

Clear yum cache before any yum call #398

Conversation

r0x0d commented Jan 10, 2022 • edited by Venefilyn Loading

r0x0d commented Jan 10, 2022 • edited Loading

Venefilyn commented Jan 11, 2022 • edited Loading

r0x0d commented Jan 11, 2022

bocekm left a comment

Choose a reason for hiding this comment

r0x0d commented Jan 26, 2022 • edited Loading

bocekm commented Feb 8, 2022 • edited Loading

bocekm left a comment

Choose a reason for hiding this comment

danmyway commented Mar 8, 2022

danmyway commented Mar 28, 2022

r0x0d commented Mar 28, 2022

codecov bot commented Mar 28, 2022 • edited Loading

Codecov Report

danmyway commented Mar 29, 2022

r0x0d commented Jul 13, 2022

r0x0d commented Jul 14, 2022

kokesak commented Jul 14, 2022

bocekm left a comment

Choose a reason for hiding this comment

danmyway commented Jul 25, 2022

danmyway commented Jul 26, 2022

danmyway left a comment

Choose a reason for hiding this comment

r0x0d commented Jan 10, 2022 •

edited by Venefilyn

Loading

r0x0d commented Jan 10, 2022 •

edited

Loading

Venefilyn commented Jan 11, 2022 •

edited

Loading

r0x0d commented Jan 26, 2022 •

edited

Loading

bocekm commented Feb 8, 2022 •

edited

Loading

codecov bot commented Mar 28, 2022 •

edited

Loading