-
Notifications
You must be signed in to change notification settings - Fork 159
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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) | ||
) | ||
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
---|---|---|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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.