[RHELC-16] Do not disable RHSM repos before pkg backup #627

hosekadam · 2022-10-10T19:00:44Z

There is a problem with backing up the packages on systems which use satellite server. During the conversion is system unregistered from previous subscription - in that step we lose access to the CentOS repositories. And then we need to backup packages, which could come from CentOS.

So the step with backup and remove the package is moved before the unregistration. With that show up another problem - the backed up and then removed package remove os-release file too. The file is needed for subscribing the system, so it is restored from backup and then removed (if it won't be removed, the file would stay there after conversion as os-release.rpmorig)

Jira Issue: RHELC-16

Checklist

PR meets acceptance criteria specified in the Jira issue
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
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

codecov · 2022-10-10T19:03:07Z

Codecov Report

Base: 93.47% // Head: 93.57% // Increases project coverage by +0.09% 🎉

Coverage data is based on head (994d3dc) compared to base (8bd5beb).
Patch coverage: 90.47% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #627      +/-   ##
==========================================
+ Coverage   93.47%   93.57%   +0.09%     
==========================================
  Files          22       22              
  Lines        3188     3222      +34     
  Branches      568      575       +7     
==========================================
+ Hits         2980     3015      +35     
+ Misses        146      142       -4     
- Partials       62       65       +3

Flag	Coverage Δ
centos-linux-6	`89.94% <83.33%> (+0.10%)`	⬆️
centos-linux-7	`89.42% <83.33%> (+0.11%)`	⬆️
centos-linux-8	`89.58% <83.33%> (+0.11%)`	⬆️

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

Impacted Files	Coverage Δ
convert2rhel/main.py	`91.82% <66.66%> (-0.59%)`	⬇️
convert2rhel/backup.py	`98.98% <84.61%> (-1.02%)`	⬇️
convert2rhel/subscription.py	`94.10% <93.33%> (+1.05%)`	⬆️
convert2rhel/systeminfo.py	`97.23% <100.00%> (+0.14%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

convert2rhel/subscription.py

convert2rhel/main.py

convert2rhel/backup.py

hosekadam · 2022-11-03T15:43:23Z

Rebased, conflict solved

convert2rhel/backup.py

hosekadam · 2022-11-11T16:15:16Z

Heh, I used probably a wrong command and I see there is a bit mess...

hosekadam · 2022-11-14T17:19:17Z

There are few things left that needs to be done to get the integration test running properly:

Merge this merge request
In the satellite repo for CentOS7 is the old version of kernel, so we need to wait to sync the repositories

Otherwise the test works just for the CentOS 7 so I think I should limit that just for this system and not to run it with others. The second variant is to ask satellite guys to provide servers that we need for another systems. What do you think @SpyTec or @r0x0d?

r0x0d · 2022-11-14T17:42:33Z

There are few things left that needs to be done to get the integration test running properly:
1. Merge this [merge request](https://gitlab.cee.redhat.com/oamg/convert2rhel/convert2rhel-secrets/-/merge_requests/9)

2. In the satellite repo for CentOS7 is the old version of kernel, so we need to wait to sync the repositories
Otherwise the test works just for the CentOS 7 so I think I should limit that just for this system and not to run it with others. The second variant is to ask satellite guys to provide servers that we need for another systems. What do you think @SpyTec or @r0x0d?

@hosekadam ping @kokesak to merge your PR. I don't have write access to that repository.

I don't know how the satellite guys would react if we ask them for more servers for the other distros we use… Furthermore, I don't know if @SpyTec or @bocekm knows how that works, if so, can any of you help us here?

As far as limiting the test to CentOS 7, I think it's a good idea if your test is only related to CentOS 7 and not anything else... But looking at your PR, I see that you changed the preparation of one test that runs under all distros to use the activation key for CentOS 7 you created, is that supposed to like that?

hosekadam · 2022-11-14T17:51:02Z

@r0x0d I had a discussion with @kokesak and we agreed on, that this test is the case we need. As you can see, I've removed the variables for unsupported conversion and the reproducer for this task is the same as the case, we were testing there.

kokesak · 2022-11-14T22:37:44Z

tests/integration/conftest.py

@@ -26,7 +26,7 @@
 logging.basicConfig(level="DEBUG" if env.str("DEBUG") else "INFO", stream=sys.stderr)
 logger = logging.getLogger(__name__)

-SATELLITE_URL = "satellite.sat.engineering.redhat.com"


We are actually trying to use the new satellite URL (https://issues.redhat.com/browse/RHELC-732). The dogfood is going to be removed.

Yeah, I see. But if you register the system using subscription-manager to get the CentOS 7 satellite server, there are still urls to the dogfood in /etc/yum.repos.d/redhat.repo. Maybe adding both of them, so once it would be finally changed, nothing breaks and just be needed to remove the dogfood url from /etc/dnsmasq.conf?

kokesak · 2022-11-14T22:51:53Z

The test was running for all distros so I would keep that.

IIUC, it's the offline-conversion test in which we currently use the env variables to omit bug this PR fixes. However, we need centos and oracle content in the satellite server, currently, we have just centos7 right?. Is this fix going to be in the next release?

I would rather have it tested on all distros before merging.

hosekadam · 2022-11-15T12:35:46Z

@kokesak yes, I see there that there is available satellite just for CentOS 7 (and Centos 8 Stream, but it's useless for us I think).

And this PR should be in the next release (from Jira).

danmyway · 2022-11-18T13:59:54Z

/packit test

abadger · 2022-11-18T19:52:39Z

convert2rhel/subscription.py

+            # Need to remove the file, if it would stay there would be leftover /etc/os-release.rpmorig
+            # after conversion
+            # RHELC-16
+            os_release_file.remove()


This part feels a little unclean but we can fix it in the next release. I've written up what's unclean about it and how we can fix it later here: https://issues.redhat.com/browse/RHELC-809

abadger · 2022-11-18T19:55:34Z

convert2rhel/subscription.py

+            # The file /etc/os-release is needed for subscribing the system and is being removed with
+            # <system-name>-release package in one of the steps before
+            # RHELC-16


Suggested change

# The file /etc/os-release is needed for subscribing the system and is being removed with

# <system-name>-release package in one of the steps before

# RHELC-16

# The file /etc/os-release is needed for subscribing the system and is being removed with

# <system-name>-release package in one of the previous steps:

# Fixes RHELC-16

#

# This code should be cleaned up in the future. See https://issues.redhat.com/browse/RHELC-809

# for what should be done.

danmyway · 2022-11-18T20:47:21Z

/packit test

kokesak · 2022-11-21T09:19:37Z

On centos7 it seems that the satellite repos do not have the kernel we update to during the update system phase:

22:17:29         out: CRITICAL - The version of the loaded kernel is different from the latest version in the enabled system repositories.
22:17:29         out:  Latest kernel version available in Satellite_Engineering_CentOS_7_Updates_x86_64: 3.10.0-1160.76.1.el7
22:17:29         out:  Loaded kernel version: 3.10.0-1160.80.1.el7
22:17:29         out: 
22:17:29         out: To proceed with the conversion, update the kernel version by executing the following step:
22:17:29         out: 
22:17:29         out: 1. yum install kernel-3.10.0-1160.76.1.el7 -y
22:17:29         out: 2. reboot
22:17:29         out: [11/18/2022 17:17:29] DEBUG - Traceback (most recent call last):
22:17:29         out:   File "/usr/lib/python2.7/site-packages/convert2rhel/main.py", line 101, in main
22:17:29         out:     checks.perform_system_checks()
22:17:29         out:   File "/usr/lib/python2.7/site-packages/convert2rhel/checks.py", line 127, in perform_system_checks
22:17:29         out:     is_loaded_kernel_latest()
22:17:29         out:   File "/usr/lib/python2.7/site-packages/convert2rhel/checks.py", line 780, in is_loaded_kernel_latest
22:17:29         out:     "2. reboot" % (repos_message, repoid, latest_kernel, loaded_kernel, package_to_check, latest_kernel)
22:17:29         out:   File "/usr/lib/python2.7/site-packages/convert2rhel/logger.py", line 195, in _critical
22:17:29         out:     sys.exit(msg)
22:17:29         out: SystemExit: The version of the loaded kernel is different from the latest version in the enabled system repositories.
22:17:29         out:  Latest kernel version available in Satellite_Engineering_CentOS_7_Updates_x86_64: 3.10.0-1160.76.1.el7
22:17:29         out:  Loaded kernel version: 3.10.0-1160.80.1.el7
22:17:29         out: 
22:17:29         out: To proceed with the conversion, update the kernel version by executing the following step:
22:17:29         out: 
22:17:29         out: 1. yum install kernel-3.10.0-1160.76.1.el7 -y
22:17:29         out: 2. reboot

hosekadam · 2022-11-21T13:18:17Z

@kokesak Yes, I've mentioned this problem in the comment :D Do you know somebody who we can contact to help us with that? I thought, they (who prepared the satellite server) have some automated way to stay updated with the original repositories.

When I was testing that, I used the older version of vagrant box and removed the update step from ansible setup.

kokesak · 2022-11-24T15:23:59Z

Hmm, seem that passing the the env var CONVERT2RHEL_UNSUPPORTED_SKIP_KERNEL_CURRENCY_CHECK did not work. I think I will need to make some other workaround, though it's weird that this envar is ommited

Trying to revert the merge commit that was previously created. Signed-off-by: Rodolfo Olivieri <[email protected]>

convert2rhel/subscription.py

convert2rhel/systeminfo.py

We had to do a workaround around the get_avail_subs function to place a releasever file in /etc/dnf/vars for it to work again. Signed-off-by: Rodolfo Olivieri <[email protected]>

convert2rhel/unit_tests/systeminfo_test.py

@@ -26,11 +26,11 @@
 import pytest
 import six

-from convert2rhel import logger, systeminfo, unit_tests, utils  # Imports unit_tests/__init__.py
+from convert2rhel import logger, pkgmanager, systeminfo, unit_tests, utils  # Imports unit_tests/__init__.py


kokesak · 2022-11-28T08:22:43Z

/packit test

kokesak · 2022-11-28T08:23:12Z

/packit build

r0x0d · 2022-11-28T12:27:17Z

/packit build

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

kokesak · 2022-11-28T23:29:42Z

/packit build

danmyway

Just a couple of minor changes, we can sort this out with another PR.

danmyway · 2022-11-29T15:39:27Z

plans/tier1.fmf

+  - name: workaround_kernel
+    how: shell
+    script: "yum install kernel-3.10.0-1160.76.1.el7 -y && yum remove kernel-3.10.0-1160.80.1.el7 -y"
+  - name: reboot machine
+    how: ansible
+    playbook: tests/ansible_collections/roles/reboot/main.yml


Do we need to reboot?
Wouldn't grubby --set-default-kernel be sufficient?
I would probably add some comment about the kernel workaround being temporary.
Or a #TODO to remove it, when the repository gets synced over at Satellite.
But I guess for the sake of saving some time, we can also keep it as is.

The reboot is needed I believe as the c2r check is done against the running kernel which changes only after reboot.

danmyway · 2022-11-29T15:41:03Z

tests/integration/tier1/convert-offline-systems/main.fmf

+summary: |
+    Convert systems that have no access to the Internet. This test require having the repositories of original
+    distro available in the satellite server. As of now we only have CentOS7 repositories available.


Summary should be shorter, 50 characters if possible.
Description can be longer.
But we can fix this whenever.

I created a Jira issue for this so we can tackle all the missing stuff in future fix and not block current PR.
Link to the Jira: https://issues.redhat.com/browse/RHELC-824

convert2rhel/backup.py

Co-authored-by: Rodolfo Olivieri <[email protected]> Co-authored-by: Martin Litwora <[email protected]>

Venefilyn requested changes Oct 19, 2022

View reviewed changes

convert2rhel/subscription.py Outdated Show resolved Hide resolved

convert2rhel/main.py Outdated Show resolved Hide resolved

convert2rhel/backup.py Show resolved Hide resolved

hosekadam force-pushed the do-not-disable-rhsm-repos-before-pkg-backup branch 2 times, most recently from d4290ed to 436b8a8 Compare October 26, 2022 15:47

hosekadam force-pushed the do-not-disable-rhsm-repos-before-pkg-backup branch from 436b8a8 to ad30c15 Compare November 3, 2022 15:42

r0x0d reviewed Nov 3, 2022

View reviewed changes

convert2rhel/backup.py Outdated Show resolved Hide resolved

r0x0d reviewed Nov 3, 2022

View reviewed changes

convert2rhel/backup.py Outdated Show resolved Hide resolved

hosekadam force-pushed the do-not-disable-rhsm-repos-before-pkg-backup branch from ad30c15 to 0fd2d2c Compare November 4, 2022 14:19

Venefilyn previously approved these changes Nov 9, 2022

View reviewed changes

hosekadam dismissed Venefilyn’s stale review via 328d550 November 11, 2022 16:12

r0x0d force-pushed the do-not-disable-rhsm-repos-before-pkg-backup branch from 328d550 to 8bda952 Compare November 11, 2022 17:07

kokesak reviewed Nov 14, 2022

View reviewed changes

abadger reviewed Nov 18, 2022

View reviewed changes

hosekadam force-pushed the do-not-disable-rhsm-repos-before-pkg-backup branch 2 times, most recently from f974d0b to 42a80d7 Compare November 23, 2022 15:42

kokesak force-pushed the do-not-disable-rhsm-repos-before-pkg-backup branch from 42a80d7 to d1eef72 Compare November 24, 2022 10:05

Restore&remove the os-release

d3c47fd

r0x0d and others added 4 commits November 25, 2022 10:04

Revert merge commit from previous rebase

ceafde4

Trying to revert the merge commit that was previously created. Signed-off-by: Rodolfo Olivieri <[email protected]>

Typo + change of satellite url

9fe8c5c

Rebase and woraround to non latest kernel in sat repo

63747af

Try different workaround for cent7

e683f85

kokesak force-pushed the do-not-disable-rhsm-repos-before-pkg-backup branch from 5e90827 to e683f85 Compare November 25, 2022 09:04

github-advanced-security bot found potential problems Nov 25, 2022

View reviewed changes

convert2rhel/subscription.py Fixed Show fixed Hide fixed

convert2rhel/systeminfo.py Fixed Show fixed Hide fixed

Add workaround for get_avail_subs

cc7cc06

We had to do a workaround around the get_avail_subs function to place a releasever file in /etc/dnf/vars for it to work again. Signed-off-by: Rodolfo Olivieri <[email protected]>

r0x0d force-pushed the do-not-disable-rhsm-repos-before-pkg-backup branch from ff72fd4 to cc7cc06 Compare November 25, 2022 18:25

github-advanced-security bot found potential problems Nov 25, 2022

View reviewed changes

r0x0d and others added 2 commits November 28, 2022 09:39

Avoid cyclic dependency import

7583c30

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

Address issues regarding the RHELC-51

994d3dc

kokesak requested review from r0x0d and danmyway November 29, 2022 14:08

danmyway approved these changes Nov 29, 2022

View reviewed changes

r0x0d reviewed Nov 29, 2022

View reviewed changes

convert2rhel/backup.py Show resolved Hide resolved

r0x0d approved these changes Nov 29, 2022

View reviewed changes

Venefilyn merged commit 46a2cf9 into oamg:main Nov 30, 2022

Venefilyn mentioned this pull request Nov 30, 2022

Bump version to 1.1 #675

Merged

abadger pushed a commit to Andrew-ang9/convert2rhel that referenced this pull request Dec 7, 2022

[RHELC-16] Do not disable RHSM repos before pkg backup (oamg#627)

9049187

Co-authored-by: Rodolfo Olivieri <[email protected]> Co-authored-by: Martin Litwora <[email protected]>

Venefilyn pushed a commit that referenced this pull request Jun 19, 2023

[RHELC-16] Do not disable RHSM repos before pkg backup (#627)

38bcfe4

Co-authored-by: Rodolfo Olivieri <[email protected]> Co-authored-by: Martin Litwora <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RHELC-16] Do not disable RHSM repos before pkg backup #627

[RHELC-16] Do not disable RHSM repos before pkg backup #627

hosekadam commented Oct 10, 2022

codecov bot commented Oct 10, 2022 •

edited

Loading

hosekadam commented Nov 3, 2022

hosekadam commented Nov 11, 2022

hosekadam commented Nov 14, 2022 •

edited

Loading

r0x0d commented Nov 14, 2022 •

edited

Loading

hosekadam commented Nov 14, 2022

kokesak Nov 14, 2022

hosekadam Nov 15, 2022

kokesak commented Nov 14, 2022

hosekadam commented Nov 15, 2022

danmyway commented Nov 18, 2022

abadger Nov 18, 2022

abadger Nov 18, 2022

danmyway commented Nov 18, 2022

kokesak commented Nov 21, 2022

hosekadam commented Nov 21, 2022

kokesak commented Nov 24, 2022

kokesak commented Nov 28, 2022

kokesak commented Nov 28, 2022

r0x0d commented Nov 28, 2022

kokesak commented Nov 28, 2022

danmyway left a comment

danmyway Nov 29, 2022

kokesak Nov 29, 2022 •

edited

Loading

danmyway Nov 29, 2022

kokesak Nov 29, 2022

[RHELC-16] Do not disable RHSM repos before pkg backup #627

[RHELC-16] Do not disable RHSM repos before pkg backup #627

Conversation

hosekadam commented Oct 10, 2022

codecov bot commented Oct 10, 2022 • edited Loading

Codecov Report

hosekadam commented Nov 3, 2022

hosekadam commented Nov 11, 2022

hosekadam commented Nov 14, 2022 • edited Loading

r0x0d commented Nov 14, 2022 • edited Loading

hosekadam commented Nov 14, 2022

kokesak Nov 14, 2022

Choose a reason for hiding this comment

hosekadam Nov 15, 2022

Choose a reason for hiding this comment

kokesak commented Nov 14, 2022

hosekadam commented Nov 15, 2022

danmyway commented Nov 18, 2022

abadger Nov 18, 2022

Choose a reason for hiding this comment

abadger Nov 18, 2022

Choose a reason for hiding this comment

danmyway commented Nov 18, 2022

kokesak commented Nov 21, 2022

hosekadam commented Nov 21, 2022

kokesak commented Nov 24, 2022

kokesak commented Nov 28, 2022

kokesak commented Nov 28, 2022

r0x0d commented Nov 28, 2022

kokesak commented Nov 28, 2022

danmyway left a comment

Choose a reason for hiding this comment

danmyway Nov 29, 2022

Choose a reason for hiding this comment

kokesak Nov 29, 2022 • edited Loading

Choose a reason for hiding this comment

danmyway Nov 29, 2022

Choose a reason for hiding this comment

kokesak Nov 29, 2022

Choose a reason for hiding this comment

codecov bot commented Oct 10, 2022 •

edited

Loading

hosekadam commented Nov 14, 2022 •

edited

Loading

r0x0d commented Nov 14, 2022 •

edited

Loading

kokesak Nov 29, 2022 •

edited

Loading