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.

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

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 18 additions & 3 deletions convert2rhel/backup.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,10 +298,13 @@ def backup(self):
else:
loggerinst.info("Can't find %s.", self.filepath)

def restore(self):
def restore(self, rollback=True):
"""Restore a previously backed up file"""
backup_filepath = os.path.join(BACKUP_DIR, os.path.basename(self.filepath))
loggerinst.task("Rollback: Restoring %s from backup" % self.filepath)
if rollback:
loggerinst.task("Rollback: Restoring %s from backup" % self.filepath)
hosekadam marked this conversation as resolved.
Show resolved Hide resolved
else:
loggerinst.info("Restoring %s from backup" % self.filepath)

if not os.path.isfile(backup_filepath):
loggerinst.info("%s hasn't been backed up." % self.filepath)
Expand All @@ -314,7 +317,19 @@ def restore(self):
# IOError for py2 and OSError for py3
loggerinst.warning("Error(%s): %s" % (err.errno, err.strerror))
return
loggerinst.info("File %s restored." % self.filepath)

if rollback:
loggerinst.info("File %s restored." % self.filepath)
else:
loggerinst.debug("File %s restored." % self.filepath)
r0x0d marked this conversation as resolved.
Show resolved Hide resolved

def remove(self):
"""Remove restored file from original place, backup isn't removed"""
try:
os.remove(self.filepath)
loggerinst.debug("File %s removed." % self.filepath)
except (OSError, IOError):
loggerinst.debug("Couldn't remove restored file %s" % self.filepath)


class RestorablePackage(object):
Expand Down
12 changes: 8 additions & 4 deletions convert2rhel/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,14 @@ def pre_ponr_conversion():
loggerinst.task("Convert: Install RHEL certificates for RHSM")
system_cert = cert.SystemCert()
system_cert.install()

# remove non-RHEL packages containing repofiles or affecting variables in the repofiles
# This needs to be before attempt to unregister the system because after unregistration can be lost
# access to repositories needed for backuping removed packages
loggerinst.task("Convert: Remove packages containing .repo files")
pkghandler.remove_repofile_pkgs()

if not toolopts.tool_opts.no_rhsm:
loggerinst.task("Convert: Subscription Manager - Subscribe system")
subscription.subscribe_system()
loggerinst.task("Convert: Get RHEL repository IDs")
Expand All @@ -231,10 +239,6 @@ def pre_ponr_conversion():
loggerinst.task("Convert: Subscription Manager - Disable all repositories")
subscription.disable_repos()

# remove non-RHEL packages containing repofiles or affecting variables in the repofiles
loggerinst.task("Convert: Remove packages containing .repo files")
pkghandler.remove_repofile_pkgs()

# we need to enable repos after removing repofile pkgs, otherwise we don't get backups
# to restore from on a rollback
if not toolopts.tool_opts.no_rhsm:
Expand Down
64 changes: 57 additions & 7 deletions convert2rhel/subscription.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
from six.moves import urllib

from convert2rhel import backup, i18n, pkghandler, utils
from convert2rhel.redhatrelease import os_release_file
from convert2rhel.systeminfo import system_info
from convert2rhel.toolopts import tool_opts

Expand Down Expand Up @@ -88,6 +89,9 @@

_SUBMGR_PKG_REMOVED_IN_CL_85 = "subscription-manager-initial-setup-addon"

# Path to the releasever variable file for dnf.
DNF_RELEASEVER_FILE = "/etc/yum/vars/releasever"


class UnregisterError(Exception):
"""Raised with problems unregistering a system."""
Expand Down Expand Up @@ -185,7 +189,15 @@ def register_system():
registration_cmd = RegistrationCommand.from_tool_opts(tool_opts)

try:
# 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
Comment on lines +192 to +194
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
# 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.

os_release_file.restore(rollback=False)
registration_cmd()
# 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()
Copy link
Member

Choose a reason for hiding this comment

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

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

loggerinst.info("System registration succeeded.")
except KeyboardInterrupt:
# When the user hits Control-C to exit, we shouldn't retry
Expand Down Expand Up @@ -446,7 +458,8 @@ def __call__(self):
register_server = system_bus.get_object("com.redhat.RHSM1", "/com/redhat/RHSM1/RegisterServer")
loggerinst.debug("Starting a private DBus to talk to subscription-manager")
address = register_server.Start(
i18n.SUBSCRIPTION_MANAGER_LOCALE, dbus_interface="com.redhat.RHSM1.RegisterServer"
i18n.SUBSCRIPTION_MANAGER_LOCALE,
dbus_interface="com.redhat.RHSM1.RegisterServer",
)

try:
Expand Down Expand Up @@ -516,7 +529,10 @@ def __call__(self):
finally:
# Always shut down the private bus
loggerinst.debug("Shutting down private DBus instance")
register_server.Stop(i18n.SUBSCRIPTION_MANAGER_LOCALE, dbus_interface="com.redhat.RHSM1.RegisterServer")
register_server.Stop(
i18n.SUBSCRIPTION_MANAGER_LOCALE,
dbus_interface="com.redhat.RHSM1.RegisterServer",
)

def _set_connection_opts_in_config(self):
"""
Expand Down Expand Up @@ -767,10 +783,33 @@ def attach_subscription():
def get_avail_subs():
"""Get list of all the subscriptions available to the user so they are
accessible by index once the user chooses one.

.. note::
Get multiline string holding all the subscriptions available to the
logged-in user
"""
# Get multiline string holding all the subscriptions available to the
# logged-in user
# With the current changes introduced in PR#627, we needed to remove the
# non-RHEL packages that contain repofiles before we do any call to
# subscription-manager, and since we are doing that, this function starts to
# fail because it doesn't have the package that sets the releasever anymore.
# For some reason, subscription-manager needs to call libdnf internally to
# read the repositories on the system, and since libdnf needs to do
# substitutions on those repofiles, mainly the releasever and any other file
# placed in /etc/dnf/vars, this command call fails as it cannot replace the
# releasever variable anymore.
releaver_created = False
if system_info.version.major >= 8:
if not os.path.exists(DNF_RELEASEVER_FILE):
with open(DNF_RELEASEVER_FILE, "w") as handler:
handler.write(system_info.original_releasever)

releaver_created = True

subs_raw, ret_code = utils.run_subprocess(["subscription-manager", "list", "--available"], print_output=False)

if releaver_created:
os.remove(DNF_RELEASEVER_FILE)

if ret_code != 0:
loggerinst.critical("Unable to get list of available subscriptions:\n%s" % subs_raw)
return list(get_sub(subs_raw))
Expand Down Expand Up @@ -996,7 +1035,12 @@ def _log_rhsm_download_directory_contents(directory, when_message):
pkgs = ["<download directory does not exist>"]
if os.path.isdir(SUBMGR_RPMS_DIR):
pkgs = os.listdir(SUBMGR_RPMS_DIR)
loggerinst.debug("Contents of %s directory %s:\n%s", SUBMGR_RPMS_DIR, when_message, "\n".join(pkgs))
loggerinst.debug(
"Contents of %s directory %s:\n%s",
SUBMGR_RPMS_DIR,
when_message,
"\n".join(pkgs),
)


def exit_on_failed_download(paths):
Expand Down Expand Up @@ -1027,7 +1071,11 @@ def lock_releasever_in_rhel_repositories():
"Updating /etc/yum.repos.d/rehat.repo to point to RHEL %s instead of the default latest minor version."
% system_info.releasever
)
cmd = ["subscription-manager", "release", "--set=%s" % system_info.releasever]
cmd = [
"subscription-manager",
"release",
"--set=%s" % system_info.releasever,
]

output, ret_code = utils.run_subprocess(cmd, print_output=False)
if ret_code != 0:
Expand Down Expand Up @@ -1056,7 +1104,9 @@ def update_rhsm_custom_facts():

if ret_code != 0:
loggerinst.warning(
"Failed to update the RHSM custom facts with return code '%s' and output '%s'.", ret_code, output
"Failed to update the RHSM custom facts with return code '%s' and output '%s'.",
ret_code,
output,
)
else:
loggerinst.info("RHSM custom facts uploaded successfully.")
Expand Down
17 changes: 17 additions & 0 deletions convert2rhel/systeminfo.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ def __init__(self):
self.kmods_to_ignore = []
# Booted kernel VRA (version, release, architecture), e.g. "4.18.0-240.22.1.el8_3.x86_64"
self.booted_kernel = ""
self.original_releasever = ""

def resolve_system_info(self):
self.logger = logging.getLogger(__name__)
Expand All @@ -125,6 +126,7 @@ def resolve_system_info(self):
self.booted_kernel = self._get_booted_kernel()
self.has_internet_access = self._check_internet_access()
self.dbus_running = self._is_dbus_running()
self.original_releasever = _get_original_releasever()

def print_system_information(self):
"""Print system related information."""
Expand Down Expand Up @@ -468,6 +470,21 @@ def get_system_release_info(self, system_release_content=None):
return release_info


def _get_original_releasever():
"""Get the original value for releasever using either YUM or DNF."""
from convert2rhel import pkgmanager

original_releasever = ""
if pkgmanager.TYPE == "yum":
yb = pkgmanager.YumBase()
original_releasever = yb.conf.yumvar["releasever"]
else:
db = pkgmanager.Base()
original_releasever = db.conf.releasever

return str(original_releasever)


def _is_sysv_managed_dbus_running():
"""Get DBus status from SysVinit compatible systems."""
# None means the status should be retried because we weren't sure if it is turned off.
Expand Down
22 changes: 22 additions & 0 deletions convert2rhel/unit_tests/backup_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -662,3 +662,25 @@ def test_restore_previously_installed(self, run_subprocess_with_empty_rpmdb, rpm
)
def test_remove_epoch_from_yum_nevra_notation(pkg_nevra, nvra_without_epoch):
assert backup.remove_epoch_from_yum_nevra_notation(pkg_nevra) == nvra_without_epoch


@pytest.mark.parametrize(
("file", "filepath", "message"),
(
(False, "/invalid/path", "Couldn't remove restored file /invalid/path"),
(True, "filename", "File %s removed."),
),
)
def test_restorable_file_remove(tmpdir, caplog, file, filepath, message):
if file:
path = tmpdir.join(filepath)
path.write("content")
path = str(path)
message = message % path
else:
path = filepath

restorable_file = backup.RestorableFile(path)
restorable_file.remove()

assert message in caplog.text
10 changes: 5 additions & 5 deletions convert2rhel/unit_tests/main_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,11 +209,12 @@ def test_pre_ponr_conversion_order_with_rhsm(self):
intended_call_order["replace_subscription_manager"] = 1
intended_call_order["verify_rhsm_installed"] = 1
intended_call_order["install"] = 1
intended_call_order["remove_repofile_pkgs"] = 1
intended_call_order["subscribe_system"] = 1
intended_call_order["get_rhel_repoids"] = 1
intended_call_order["check_needed_repos_availability"] = 1
intended_call_order["disable_repos"] = 1
intended_call_order["remove_repofile_pkgs"] = 1

intended_call_order["enable_repos"] = 1
intended_call_order["perform_pre_ponr_checks"] = 1
intended_call_order["perform_system_checks"] = 1
Expand Down Expand Up @@ -313,15 +314,14 @@ def test_pre_ponr_conversion_order_without_rhsm(self):
intended_call_order["replace_subscription_manager"] = 0
intended_call_order["verify_rhsm_installed"] = 0
intended_call_order["install"] = 0
intended_call_order["remove_repofile_pkgs"] = 1

# Do not expect this one to be called - related to RHSM
intended_call_order["subscribe_system"] = 0
intended_call_order["get_rhel_repoids"] = 0
intended_call_order["check_needed_repos_availability"] = 0
intended_call_order["disable_repos"] = 0

intended_call_order["remove_repofile_pkgs"] = 1

intended_call_order["enable_repos"] = 0

intended_call_order["perform_pre_ponr_checks"] = 1

# Merge the two together like a zipper, creates a tuple which we can assert with - including method call order!
Expand Down
82 changes: 81 additions & 1 deletion convert2rhel/unit_tests/subscription_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import dbus
import dbus.connection
import dbus.exceptions
import pexpect
import pytest
import six

Expand Down Expand Up @@ -1573,3 +1572,84 @@ def test_update_rhsm_custom_facts_no_rhsm(global_tool_opts, caplog, monkeypatch)

subscription.update_rhsm_custom_facts()
assert "Skipping updating RHSM custom facts." in caplog.records[-1].message


MOCK_LIST_AVAILABLE_SUBS_OUTPUT = """\
Subscription Name: Red Hat\n
Provides: Test\n
SKU: RHTEST\n
Contract: 123456\n
Pool ID: 096fbb526ce611ed-97076c9466263bdf\n
Provides Management: No\n
Available: Unlimited\n
Suggested: 1\n
Service Type: L1-L3\n
Roles: \n
Service Level: Self-Support\n
Usage: \n
Add-ons: \n
Subscription Type: Standard\n
Starts: 03/29/2000\n
Ends: 03/29/2003\n
Entitlement Type: Physical\n
"""


@centos8
def test_get_avail_subs(pretend_os, tmpdir, monkeypatch):
return_value = (MOCK_LIST_AVAILABLE_SUBS_OUTPUT, 0)
cmd = ["subscription-manager", "list", "--available"]
tmpdir.join("releasever").write("8")
dnf_releasever_file = str(tmpdir)
run_subprocess_mock = mock.Mock(
side_effect=unit_tests.run_subprocess_side_effect((cmd, return_value)),
)
monkeypatch.setattr(
utils,
"run_subprocess",
value=run_subprocess_mock,
)
monkeypatch.setattr(subscription, "DNF_RELEASEVER_FILE", dnf_releasever_file)

result = subscription.get_avail_subs()
assert result[0].pool_id == "096fbb526ce611ed-97076c9466263bdf"


@centos8
def test_get_avail_subs_failed_command(pretend_os, tmpdir, monkeypatch, caplog):
return_value = (MOCK_LIST_AVAILABLE_SUBS_OUTPUT, 1)
cmd = ["subscription-manager", "list", "--available"]
tmpdir.join("releasever").write("8")
dnf_releasever_file = str(tmpdir)
run_subprocess_mock = mock.Mock(
side_effect=unit_tests.run_subprocess_side_effect((cmd, return_value)),
)
monkeypatch.setattr(
utils,
"run_subprocess",
value=run_subprocess_mock,
)
monkeypatch.setattr(subscription, "DNF_RELEASEVER_FILE", dnf_releasever_file)

with pytest.raises(SystemExit):
subscription.get_avail_subs()
assert "Unable to get list of available subscriptions:" in caplog.records[-1].message


@centos8
def test_get_avail_subs_no_releasever_file(pretend_os, tmpdir, monkeypatch):
return_value = (MOCK_LIST_AVAILABLE_SUBS_OUTPUT, 0)
cmd = ["subscription-manager", "list", "--available"]
dnf_releasever_file = str(tmpdir.join("releasever"))
run_subprocess_mock = mock.Mock(
side_effect=unit_tests.run_subprocess_side_effect((cmd, return_value)),
)
monkeypatch.setattr(
utils,
"run_subprocess",
value=run_subprocess_mock,
)
monkeypatch.setattr(subscription, "DNF_RELEASEVER_FILE", dnf_releasever_file)

result = subscription.get_avail_subs()
assert result[0].pool_id == "096fbb526ce611ed-97076c9466263bdf"
4 changes: 2 additions & 2 deletions convert2rhel/unit_tests/systeminfo_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Check notice

Code scanning / CodeQL

Unused import

Import of 'pkgmanager' is not used.
from convert2rhel.systeminfo import RELEASE_VER_MAPPING, Version, system_info
from convert2rhel.toolopts import tool_opts
from convert2rhel.unit_tests import is_rpm_based_os
from convert2rhel.unit_tests.conftest import all_systems, centos8
from convert2rhel.unit_tests.conftest import all_systems, centos7, centos8


six.add_move(six.MovedModule("mock", "mock", "unittest.mock"))
Expand Down
Loading