Skip to content

Copy dnf.conf to target userspace and allow a custom one #1143

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

Merged
merged 1 commit into from
Nov 16, 2023
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
19 changes: 19 additions & 0 deletions repos/system_upgrade/common/actors/applycustomdnfconf/actor.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
from leapp.actors import Actor
from leapp.libraries.actor import applycustomdnfconf
from leapp.tags import ApplicationsPhaseTag, IPUWorkflowTag


class ApplyCustomDNFConf(Actor):
"""
Move /etc/leapp/files/dnf.conf to /etc/dnf/dnf.conf if it exists

An actor in FactsPhase copies this file to the target userspace if present.
In such case we also want to use the file on the target system.
"""
name = "apply_custom_dnf_conf"
consumes = ()
produces = ()
tags = (ApplicationsPhaseTag, IPUWorkflowTag)

def process(self):
applycustomdnfconf.process()
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import os

from leapp.libraries.stdlib import api, CalledProcessError, run

CUSTOM_DNF_CONF_PATH = "/etc/leapp/files/dnf.conf"


def process():
if os.path.exists(CUSTOM_DNF_CONF_PATH):
try:
run(["mv", CUSTOM_DNF_CONF_PATH, "/etc/dnf/dnf.conf"])
except (CalledProcessError, OSError) as e:
api.current_logger().debug(
"Failed to move /etc/leapp/files/dnf.conf to /etc/dnf/dnf.conf: {}".format(e)
)
Comment on lines +13 to +15
Copy link
Member

@pirat89 pirat89 Nov 16, 2023

Choose a reason for hiding this comment

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

@matejmatuska it should be error log actually. but let's keep it as it is.

Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import os

import pytest

from leapp.libraries.actor import applycustomdnfconf


@pytest.mark.parametrize(
"exists,should_move",
[(False, False), (True, True)],
)
def test_copy_correct_dnf_conf(monkeypatch, exists, should_move):
monkeypatch.setattr(os.path, "exists", lambda _: exists)

run_called = [False]

def mocked_run(_):
run_called[0] = True

monkeypatch.setattr(applycustomdnfconf, 'run', mocked_run)

applycustomdnfconf.process()
assert run_called[0] == should_move
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
from leapp.actors import Actor
from leapp.libraries.actor import copydnfconfintotargetuserspace
from leapp.models import TargetUserSpacePreupgradeTasks
from leapp.tags import FactsPhaseTag, IPUWorkflowTag


class CopyDNFConfIntoTargetUserspace(Actor):
"""
Copy dnf.conf into target userspace

Copies /etc/leapp/files/dnf.conf to target userspace. If it isn't available
/etc/dnf/dnf.conf is copied instead. This allows specifying a different
config for the target userspace, which might be required if the source
system configuration file isn't compatible with the target one. One such
example is incompatible proxy configuration between RHEL7 and RHEL8 DNF
versions.
"""
name = "copy_dnf_conf_into_target_userspace"
consumes = ()
produces = (TargetUserSpacePreupgradeTasks,)
tags = (FactsPhaseTag, IPUWorkflowTag)

def process(self):
copydnfconfintotargetuserspace.process()
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import os

from leapp.libraries.stdlib import api
from leapp.models import CopyFile, TargetUserSpacePreupgradeTasks


def process():
src = "/etc/dnf/dnf.conf"
if os.path.exists("/etc/leapp/files/dnf.conf"):
src = "/etc/leapp/files/dnf.conf"

api.current_logger().debug(
"Copying dnf.conf at {} to the target userspace".format(src)
)
Comment on lines +12 to +14
Copy link
Member

Choose a reason for hiding this comment

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

@matejmatuska note this could be a little bit confusing as this actor is not copying anything. it just creates a msg, that will be processed by the other actor that will do the operation. same about the docstring of the actor. but it's a minor issue and we keep it as that. just saying to help you with writting of future docstrings.

api.produce(
TargetUserSpacePreupgradeTasks(
copy_files=[CopyFile(src=src, dst="/etc/dnf/dnf.conf")]
)
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import os

import pytest

from leapp.libraries.actor import copydnfconfintotargetuserspace
from leapp.libraries.common.testutils import logger_mocked, produce_mocked


@pytest.mark.parametrize(
"userspace_conf_exists,expected",
[(False, "/etc/dnf/dnf.conf"), (True, "/etc/leapp/files/dnf.conf")],
)
def test_copy_correct_dnf_conf(monkeypatch, userspace_conf_exists, expected):
monkeypatch.setattr(os.path, "exists", lambda _: userspace_conf_exists)

mocked_produce = produce_mocked()
monkeypatch.setattr(copydnfconfintotargetuserspace.api, 'produce', mocked_produce)
monkeypatch.setattr(copydnfconfintotargetuserspace.api, 'current_logger', logger_mocked())

copydnfconfintotargetuserspace.process()

assert mocked_produce.called == 1
assert len(mocked_produce.model_instances) == 1
assert len(mocked_produce.model_instances[0].copy_files) == 1
assert mocked_produce.model_instances[0].copy_files[0].src == expected
assert mocked_produce.model_instances[0].copy_files[0].dst == "/etc/dnf/dnf.conf"
Original file line number Diff line number Diff line change
Expand Up @@ -269,15 +269,25 @@ def prepare_target_userspace(context, userspace_dir, enabled_repos, packages):
# failed since leapp does not support updates behind proxy yet.
for manager_info in api.consume(PkgManagerInfo):
if manager_info.configured_proxies:
details['details'] = ("DNF failed to install userspace packages, likely due to the proxy "
"configuration detected in the YUM/DNF configuration file.")
details['details'] = (
"DNF failed to install userspace packages, likely due to the proxy "
"configuration detected in the YUM/DNF configuration file. "
"Make sure the proxy is properly configured in /etc/dnf/dnf.conf. "
"It's also possible the proxy settings in the DNF configuration file are "
"incompatible with the target system. A compatible configuration can be "
"placed in /etc/leapp/files/dnf.conf which, if present, will be used during "
"the upgrade instead of /etc/dnf/dnf.conf. "
"In such case the configuration will also be applied to the target system."
Comment on lines +273 to +280
Copy link
Member

Choose a reason for hiding this comment

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

I missed to update this msg. possibly we will update it as a follow up. the text of the msg has no effect on the functionality, so I am in favor to merge it now to allow the testing. The problem I see with this msg is that people could thing they need to configure correctly just /etc/leapp/files/dnf.conf file, but the truth is that /etc/leapp/files needs to be still configured properly on the src system as well.

)

# Similarly if a proxy was set specifically for one of the repositories.
for repo_facts in api.consume(RepositoriesFacts):
for repo_file in repo_facts.repositories:
if any(repo_data.proxy and repo_data.enabled for repo_data in repo_file.data):
details['details'] = ("DNF failed to install userspace packages, likely due to the proxy "
"configuration detected in a repository configuration file.")
details['details'] = (
"DNF failed to install userspace packages, likely due to the proxy "
"configuration detected in a repository configuration file."
)

raise StopActorExecutionError(message=message, details=details)

Expand Down
24 changes: 23 additions & 1 deletion repos/system_upgrade/common/libraries/dnfplugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,30 @@ def _handle_transaction_err_msg(stage, xfs_info, err, is_container=False):
return # not needed actually as the above function raises error, but for visibility
NO_SPACE_STR = 'more space needed on the'
message = 'DNF execution failed with non zero exit code.'
details = {'STDOUT': err.stdout, 'STDERR': err.stderr}
if NO_SPACE_STR not in err.stderr:
# if there was a problem reaching repos and proxy is configured in DNF/YUM configs, the
# proxy is likely the problem.
# NOTE(mmatuska): We can't consistently detect there was a problem reaching some repos,
# because it isn't clear what are all the possible DNF error messages we can encounter,
# such as: "Failed to synchronize cache for repo ..." or "Errors during downloading
# metadata for # repository" or "No more mirrors to try - All mirrors were already tried
# without success"
# NOTE(mmatuska): We could check PkgManagerInfo to detect if proxy is indeed configured,
# however it would be pretty ugly to pass it all the way down here
proxy_hint = (
"If there was a problem reaching remote content (see stderr output) and proxy is "
"configured in the YUM/DNF configuration file, the proxy configuration is likely "
"causing this error. "
"Make sure the proxy is properly configured in /etc/dnf/dnf.conf. "
"It's also possible the proxy settings in the DNF configuration file are "
"incompatible with the target system. A compatible configuration can be "
"placed in /etc/leapp/files/dnf.conf which, if present, it will be used during "
"some parts of the upgrade instead of original /etc/dnf/dnf.conf. "
"In such case the configuration will also be applied to the target system. "
"Note that /etc/dnf/dnf.conf needs to be still configured correctly "
"for your current system to pass the early phases of the upgrade process."
)
details = {'STDOUT': err.stdout, 'STDERR': err.stderr, 'hint': proxy_hint}
raise StopActorExecutionError(message=message, details=details)

# Disk Requirements:
Expand Down