Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1656019 - dnf doesn't complain on conflict in modulemd defaults
Summary: dnf doesn't complain on conflict in modulemd defaults
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Red Hat Enterprise Linux 8
Classification: Red Hat
Component: libdnf
Version: 8.0
Hardware: Unspecified
OS: Unspecified
high
high
Target Milestone: rc
: 8.0
Assignee: Jaroslav Mracek
QA Contact: Karel Srot
URL:
Whiteboard:
Depends On: 1666871
Blocks: 1649352
TreeView+ depends on / blocked
 
Reported: 2018-12-04 13:28 UTC by Karel Srot
Modified: 2021-02-16 14:10 UTC (History)
7 users (show)

Fixed In Version: libdnf-0.22.4-1.el8
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2019-06-14 01:21:01 UTC
Type: Bug
Target Upstream Version:


Attachments (Terms of Use)
reproducer (12.95 KB, application/x-gzip)
2018-12-04 13:29 UTC, Karel Srot
no flags Details

Description Karel Srot 2018-12-04 13:28:29 UTC
Description of problem:

I have two repos, one with ModuleX:myStream and the other repo with ModuleX:MyStreamB. First repo has modules.yaml with myStream being a default while the other repo has modules.yaml with myStreamB being a default.

When installing ModuleX I would expect dnf to report a conflict in defaults, however it wants to install ModuleX:myStream.

# yum module install ModuleX
Updating Subscription Management repositories.
Unable to read consumer identity
This system is not registered to Red Hat Subscription Management. You can use subscription-manager to register.
Updating Subscription Management repositories.
Unable to read consumer identity
This system is not registered to Red Hat Subscription Management. You can use subscription-manager to register.
Last metadata expiration check: 0:04:32 ago on Tue 04 Dec 2018 08:13:26 AM EST.
Dependencies resolved.
======================================================================================================
 Package              Arch                  Version              Repository                      Size
======================================================================================================
Installing group/module packages:
 TestX                noarch                1-1                  modularity-test                6.4 k
Installing module profiles:
 ModuleX/default                                                                                     
Enabling module streams:
 ModuleX                                    myStream                                                 

Transaction Summary
======================================================================================================
Install  1 Package

Total size: 6.4 k
Installed size: 0  
Is this ok [y/N]: 
Operation aborted.


Version-Release number of selected component (if applicable):
dnf-4.0.9-1.el8.noarch
libdnf-0.22.3-1.el8.x86_64
libmodulemd-1.6.2-2.el8.x86_64

How reproducible:
always

Steps to Reproduce:
1. use the attached repositories
2. yum module install moduleX


Actual results:
no conflict reported

Expected results:
dnf should fail reporting an error


Additional info:
Regarding the possibility of happening it real-life. While Red Hat should avoid having such thing as we have one AppStream repo, it is quite common that our customers are doing copies of our repositories and therefore it is not that unlikely that they would have two distinct repos enabled on the system (one AppStream copy older than the other).
Additionally, module-defaults specification
  https://github.com/fedora-modularity/libmodulemd/blob/master/mod-defaults/spec.v1.yaml
 doesn't provide a solution for a situation with two AppStream repos, one being an older copy of the other. While modules itself have versions, modules,yaml doesn't have the version, only the version of the specification. Having also a version of the modules.yaml file could help libmodulemd or dnf to select the proper (never) defaults.

Comment 1 Karel Srot 2018-12-04 13:29:40 UTC
Created attachment 1511313 [details]
reproducer

attaching my reproducer - two repos with conflicting default stream

Comment 2 Stephen Gallagher 2018-12-04 14:22:55 UTC
(In reply to Karel Srot from comment #1)
> Created attachment 1511313 [details]
> reproducer
> 
> attaching my reproducer - two repos with conflicting default stream

Can you please attach the contents of /etc/yum.repos.d as well?

Comment 3 Karel Srot 2018-12-04 14:57:31 UTC
# cat > /etc/yum.repos.d/test.repo <<EOF
[orig]
name=orig
baseurl=file:///tmp/orig
gpgcheck=0

[copy]
name=copy
baseurl=file:///tmp/copy
gpgcheck=0
EOF

Comment 4 Stephen Gallagher 2018-12-04 14:59:46 UTC
I can reproduce this issue and I confirm it's a bug. I assume at the moment that it is a libmodulemd bug in the merge logic.

Comment 5 Stephen Gallagher 2018-12-04 15:24:59 UTC
OK, the problem is *not* in libmodulemd after all. We're properly reporting an error here, which DNF turns into a C++ exception... which they later catch, log as a warning and then ignore.

The catch-and-ignore happens in ModulePackageContainer::add() and ModulePackageContainer::addDefaultsFromDisk().

This needs to be fixed at the libdnf layer, so reassigning.

Comment 7 Jaroslav Mracek 2018-12-04 16:40:50 UTC
How to test?
Run reproducer provided by reporter.

Comment 8 Jaroslav Mracek 2018-12-04 16:43:02 UTC
How to test? Part 2.
A logger.warning should be present.

Comment 9 Stephen Gallagher 2018-12-04 16:44:05 UTC
(In reply to Jaroslav Mracek from comment #8)
> How to test? Part 2.
> A logger.warning should be present.

Is a warning a fatal error? The effect here should be that the repodata is considered corrupt and therefore not usable.

Comment 14 Jaroslav Mracek 2018-12-06 14:12:10 UTC
Patches:
https://github.com/rpm-software-management/libdnf/pull/654
https://github.com/rpm-software-management/dnf/pull/1284


Now it will fail with all information provided by libmodulemd. The solution has to be confirmed by Modularity team that the behavior is intended (see patches above).

Risks:
1. Change in API (libdnf, hawkey, dnf):
The change produce new errors that could be not handled on side of API users
(anaconda, lorax, ...).

2. Different behavior that according to reporter will effect wide range of users

3. If conflict in defaults appears it will be impossible to upgrade, downgrade, install packages, or even consume security updates till conflict is resolved.

Comment 15 Karel Srot 2018-12-06 14:36:50 UTC
(In reply to Jaroslav Mracek from comment #14)
> 2. Different behavior that according to reporter will effect wide range of
> users

Just to add. I was pointing out that the issue may not be that uncommon. However, as  Stephen pointed out RH policy is not to change default, therefore the issue should not appear just by enabling and outdated copy or the repo. 
Moreover, even if there would be a conflict I believe it is better to error out rather than following _randomly_ (confirmed by testing that the default profile selected may differ per system) selected default profile.

Comment 17 Daniel Mach 2018-12-10 12:37:33 UTC
Setting FutureFeature as this requires an API change.

Comment 18 Petr Šabata 2018-12-11 18:28:02 UTC
We discussed this at the interlock meeting today and, considering the impact when using third party or misconfigured repos, decided to implement this fix in 8.0.0.

Comment 20 Karel Srot 2018-12-20 14:11:02 UTC
I did tested the behaviour with dnf-4.0.9.1-1.el8.noarch libdnf-0.22.4-1.el8.x86_64 and the output is quite confusing.

# dnf --disablerepo=\* --enablerepo=bz1577196\* module list
Last metadata expiration check: 0:06:18 ago on Thu 20 Dec 2018 08:59:37 AM EST.
Conflicting default streams when merging defaults for module ModuleX
bz1577196copy
Name                                Stream                                   Profiles                               Summary                                          
ModuleX                             myStreamB [d]                            default                                Module ModuleX summary                           

bz1577196orig
Name                                Stream                                   Profiles                               Summary                                          
ModuleX                             myStream                                 default [d]                            Module ModuleX summary                           

Hint: [d]efault, [e]nabled, [x]disabled, [i]nstalled

So, there is a conflict but still the default stream/profile is indicated. This is a bit strange.
Let's try to install ModuleX then.

# dnf --disablerepo=\* --enablerepo=bz1577196\* install ModuleX
Last metadata expiration check: 0:00:50 ago on Thu 20 Dec 2018 09:06:06 AM EST.
Conflicting default streams when merging defaults for module ModuleX
No match for argument: ModuleX
Error: Unable to find a match

OK, failed to install due to conflicting streams? Still, it was listed above as available, let's try again

# dnf --disablerepo=\* --enablerepo=bz1577196\* install ModuleX:myStream/default
Last metadata expiration check: 0:01:43 ago on Thu 20 Dec 2018 09:06:06 AM EST.
Conflicting default streams when merging defaults for module ModuleX
No match for argument: ModuleX:myStream/default
Error: Unable to find a match

This is also weird. There is a conflict in defaults but I did specify module:stream/profile so no defaults should apply.

The behavior above is confusing. Either the module should not be even listed... or even better, given the conflict is in defaults only defaults should be completely ignored, however the module should be still available and installable.

Stephen, Petr, what is your opinion?

Comment 21 Stephen Gallagher 2019-01-02 16:15:45 UTC
(In reply to Karel Srot from comment #20)
> The behavior above is confusing. Either the module should not be even
> listed... or even better, given the conflict is in defaults only defaults
> should be completely ignored, however the module should be still available
> and installable.
> 
> Stephen, Petr, what is your opinion?

I'm of two minds here. On the one hand, if we've gotten into a situation where conflicting defaults are present, it does indicate a serious problem with the repodata. But maybe it's not absolutely necessary to completely fail processing. I suppose we *could* treat conflicting defaults as "no defaults exist for this module". Then at least users would be able to set their choices manually.

This change would have to occur in libmodulemd, as right now the library treats it as a fatal error. It will mean new API, since currently we can only return a complete, merged object OR an error message. Is this the route we want to take?

If so, I'll ask that the DNF folks please tell me what they want the API to look like and I'll get it written immediately.

Comment 22 Karel Srot 2019-02-05 09:58:53 UTC
Hello,
can we get a devel feedback on the problem above? I am fine with accepting current behaviour for 8.0, just want to double check whether we should file a new bug for the proposed change (do not use conflicting defaults).

Comment 24 Stephen Gallagher 2019-02-05 13:07:54 UTC
(In reply to Karel Srot from comment #22)
> Hello,
> can we get a devel feedback on the problem above? I am fine with accepting
> current behaviour for 8.0, just want to double check whether we should file
> a new bug for the proposed change (do not use conflicting defaults).

Sorry, we forgot to report the decision back to this Bugzilla.

After careful consideration, we determined that there were no scenarios where it would be unsafe to treat a conflict of default streams as having "no default stream". So we have implemented a change in libmodulemd (see BZ#1666871) to do so. DNF did not require any code changes to address this.

The case of conflicts between the default profiles for a stream is, however, impossible to resolve automatically and should still result in DNF having a hard failure and reporting the conflict. We should probably use this ticket to track testing that this behavior is honored.

Comment 25 Karel Srot 2019-02-05 14:00:45 UTC
I would rather propose to file a new bug for the default profile conflicts.

Comment 26 Stephen Gallagher 2019-02-05 14:43:11 UTC
(In reply to Karel Srot from comment #25)
> I would rather propose to file a new bug for the default profile conflicts.

Fine by me.


Note You need to log in before you can comment on or make changes to this bug.