RHEL Engineering is moving the tracking of its product development work on RHEL 6 through RHEL 9 to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "RHEL project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs in the statuses "NEW", "ASSIGNED", and "POST" are being migrated throughout September 2023. Bugs of Red Hat partners with an assigned Engineering Partner Manager (EPM) are migrated in late September as per pre-agreed dates. Bugs against components "kernel", "kernel-rt", and "kpatch" are only migrated if still in "NEW" or "ASSIGNED". If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "RHEL project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/RHEL-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.
Bug 1947958 - DNF no longer raises an error when an unknown group is installed
Summary: DNF no longer raises an error when an unknown group is installed
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 8
Classification: Red Hat
Component: dnf
Version: 8.4
Hardware: Unspecified
OS: Unspecified
medium
unspecified
Target Milestone: beta
: ---
Assignee: Lukáš Hrázký
QA Contact: Tomáš Bajer
URL:
Whiteboard:
Depends On: 2006661
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-04-09 15:36 UTC by Brian Lane
Modified: 2022-05-10 16:41 UTC (History)
6 users (show)

Fixed In Version: dnf-4.7.0-5.el8
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2022-05-10 15:23:43 UTC
Type: Bug
Target Upstream Version:
Embargoed:

Attachments (Terms of Use)
Simple python reproducer (289 bytes, text/x-python)
2021-04-09 15:36 UTC, Brian Lane 		no flags Details
View All


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2022:2051 0 None None None 2022-05-10 15:23:48 UTC

Description Brian Lane 2021-04-09 15:36:22 UTC 
Created attachment 1770671 [details]
Simple python reproducer

Description of problem:
dnf-4.4.2-2 no longer raises an error when using group_install() with an unknown group.

Older versions used to raise dnf.exceptions.MarkingError, and in RHEL 8.3 with dnf-4.2.23-4 it started raising a plain ValueError (see bug #1943206)


Version-Release number of selected component (if applicable):
dnf-4.4.2-2 

How reproducible:
always

Steps to Reproduce:
1. Run attached reproducer and observe output


[root@localhost ~]# rpm -q dnf
dnf-4.4.2-11.el8.noarch
[root@localhost ~]# ./1943206.py 
Group id 'base' does not exist, skipping.
OK: Installed 'base' group
Group id 'unknown' does not exist, skipping.
ERROR: Should have been traceback for 'unknown' group
Comment 1 Lukáš Hrázký 2021-04-13 13:54:56 UTC 
Well, apologies for the mess. This was changed here: https://github.com/rpm-software-management/dnf/commit/f54a1001eb4eefda952de522902c2dd18527023f

In an attempt to bring some sanity to the code, but unfortunately installation of both groups and environments goes through this wrapper which just eats the exception (and prints the warning): https://github.com/rpm-software-management/dnf/blob/395541fbf8f87f81cdca7567f22be1182e55bea7/dnf/comps.py#L94

It's a bad interface which we maintained for compatibility. To fix this we either need to break the interface or add a new one (or revert to the previous inconsistent behavior, which seems like the worst option).

(FTR I haven't yet managed to find where the MarkingError -> ValueError change happened)

Jaroslav, can you propose a solution? IIRC I wanted to not catch the exception, if we did that, at least now lorax would have something to catch, this way it's still not working correctly.
Comment 2 Lukáš Hrázký 2021-07-20 13:44:16 UTC 
PR: https://github.com/rpm-software-management/dnf/pull/1784

With the above, dnf will start throwing CompsError when a group or environment is not found for the install methods.
Comment 3 Lukáš Hrázký 2021-07-22 15:58:17 UTC 
Brian, we've discussed this in the team and concluded raising a CompsError (admittedly yet another exception class than what was thrown before) is the best way to move forward here. Not raising any exception is clearly a bug and ValueError is also not the right exception to raise.

Is this fine with you, assuming you'll have to add another except block for catching this one?

Right now this bug is slated for 8.5, but that now depends mainly on the API users being able to adapt to this API change for 8.5, so it may well need to be pushed to 8.6.

Again, apologies for the inconvenience...
Comment 4 Brian Lane 2021-07-22 17:56:05 UTC 
(In reply to Lukáš Hrázký from comment #3)
> Brian, we've discussed this in the team and concluded raising a CompsError
> (admittedly yet another exception class than what was thrown before) is the
> best way to move forward here. Not raising any exception is clearly a bug
> and ValueError is also not the right exception to raise.
> 
> Is this fine with you, assuming you'll have to add another except block for
> catching this one?
> 
> Right now this bug is slated for 8.5, but that now depends mainly on the API
> users being able to adapt to this API change for 8.5, so it may well need to
> be pushed to 8.6.
> 
> Again, apologies for the inconvenience...

That sounds fine, I can catch multiple exceptions to cover all the cases if I need to :)
Comment 5 Lukáš Hrázký 2021-09-06 13:57:27 UTC 
Ok, thanks Brian, this bug has been moved to 8.6. Can you add the catch for CompsError into lorax for 8.6, so that we can merge this upstream and schedule it for 8.6 release? Thanks!
Comment 6 Brian Lane 2021-09-07 17:28:52 UTC 
(In reply to Lukáš Hrázký from comment #5)
> Ok, thanks Brian, this bug has been moved to 8.6. Can you add the catch for
> CompsError into lorax for 8.6, so that we can merge this upstream and
> schedule it for 8.6 release? Thanks!

Sure, once this has acks I can push it as Related to this bug.
Comment 7 Lukáš Hrázký 2021-10-11 13:20:29 UTC 
Brian, the acks are in, can you make the change (unless it's been done already)? We should move forward with this in a timely manner. Thanks!
Comment 8 Brian Lane 2021-10-11 17:31:56 UTC 
I have a PR here - https://github.com/weldr/lorax/pull/1179

Note that this bug needs to have ITR set before it will turn on release+ and allow commits.
Comment 9 Lukáš Hrázký 2021-10-12 09:28:08 UTC 
Acceptation Criteria:
The Base.group_install() and Base.environment_install() methods raise an exception of type dnf.exceptions.CompsError in case the group or environment doesn't exist.
Comment 11 Lukáš Hrázký 2021-10-15 11:31:58 UTC 
Ok, the release+ is there, Brian!

We are also planning to add a conflict on lorax versions lower than the one with this fix to dnf so that we force the upgrade and don't allow a broken combination of versions. Can you let me know the version in which the fix will land? Thank you.
Comment 12 Brian Lane 2021-10-15 16:02:17 UTC 
(In reply to Lukáš Hrázký from comment #11)
> Ok, the release+ is there, Brian!
> 
> We are also planning to add a conflict on lorax versions lower than the one
> with this fix to dnf so that we force the upgrade and don't allow a broken
> combination of versions. Can you let me know the version in which the fix
> will land? Thank you.

That doesn't seem like the right thing to do, dnf should not be obsoleting any version of a package that uses it.

This new version of lorax will handle all the variations of this error, so I don't think there is anything for dnf to do.
Also, this change is in a disused corner of lorax, the chances of someone hitting it are very low at this point since they should all be using osbuild-composer anyway.
Comment 13 Lukáš Hrázký 2021-10-18 08:18:42 UTC 
(In reply to Brian Lane from comment #12)
> That doesn't seem like the right thing to do, dnf should not be obsoleting
> any version of a package that uses it.
> 
> This new version of lorax will handle all the variations of this error, so I
> don't think there is anything for dnf to do.

No, not obsoleting, conflicting:

Assume dnf-1 didn't raise CompsError and dnf-2 does.
lorax-1 doesn't catch CompsError (it tracebacks) and lorax-2 does handle CompsError gracefully.

We would add in dnf.spec:
Conflicts: lorax < 2

So that you can't upgrade dnf to version 2 and keep lorax at 1, which would mean lorax now can crash on a non-existent group.


Tbf. I'm not a personal fan of this just because it adds a relation between dnf and lorax in the opposite direction (lorax depends on dnf, now dnf references lorax in its .spec and from dnf's standpoint it's a bit arbitrary), but it is the proper way to prevent the described breakage in the rpm packaging system it would seem.

> Also, this change is in a disused corner of lorax, the chances of someone
> hitting it are very low at this point since they should all be using
> osbuild-composer anyway.

That might be an argument to not add the conflict just because the case is so niche now.
Comment 14 Brian Lane 2021-10-18 16:07:37 UTC 
(In reply to Lukáš Hrázký from comment #13)
> (In reply to Brian Lane from comment #12)
> > That doesn't seem like the right thing to do, dnf should not be obsoleting
> > any version of a package that uses it.
> > 
> > This new version of lorax will handle all the variations of this error, so I
> > don't think there is anything for dnf to do.
> 
> No, not obsoleting, conflicting:
> 
> Assume dnf-1 didn't raise CompsError and dnf-2 does.
> lorax-1 doesn't catch CompsError (it tracebacks) and lorax-2 does handle
> CompsError gracefully.
> 
> We would add in dnf.spec:
> Conflicts: lorax < 2
> 
> So that you can't upgrade dnf to version 2 and keep lorax at 1, which would
> mean lorax now can crash on a non-existent group.
> 
> 
> Tbf. I'm not a personal fan of this just because it adds a relation between
> dnf and lorax in the opposite direction (lorax depends on dnf, now dnf
> references lorax in its .spec and from dnf's standpoint it's a bit
> arbitrary), but it is the proper way to prevent the described breakage in
> the rpm packaging system it would seem.

Yeah, that just seems wrong to me. If anything lorax could add a requirement on the newer version of dnf after it is built.

> 
> > Also, this change is in a disused corner of lorax, the chances of someone
> > hitting it are very low at this point since they should all be using
> > osbuild-composer anyway.
> 
> That might be an argument to not add the conflict just because the case is
> so niche now.

I can agree to that :) It really is code that is only hit during testing, so I really don't want to see anything referring to lorax added to dnf.
Comment 15 Jaroslav Mracek 2021-10-20 11:34:33 UTC 
Ok, we will not create a conflict.
Comment 23 errata-xmlrpc 2022-05-10 15:23:43 UTC 
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (dnf bug fix and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2022:2051
Note You need to log in before you can comment on or make changes to this bug.