Bug 1917461

Summary: etckeeper dnf plugin doesn't halt dnf on unclean /etc
Product: [Fedora] Fedora EPEL Reporter: David Allsopp <David.Allsopp>
Component: etckeeperAssignee: Thomas Moschny <thomas.moschny>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: high Docs Contact:
Priority: unspecified    
Version: epel8CC: gulikoza, thomas.moschny, uckelman
Target Milestone: ---   
Target Release: ---   
Hardware: x86_64   
OS: Linux   
Whiteboard:
Fixed In Version: etckeeper-1.18.20-1.el8 etckeeper-1.18.20-1.el9 etckeeper-1.18.20-1.fc36 etckeeper-1.18.20-1.fc37 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2023-02-06 00:41:18 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description David Allsopp 2021-01-18 14:28:19 UTC
Description of problem:

When configured to avoid committing changes to /etc before installation, the etckeeper dnf plugin warns that /etc is not clean but allows dnf to proceed anyway. It should abort the dnf install, as the EPEL 7 (yum) version does.

Version-Release number of selected component (if applicable): 1.18.14-1

How reproducible: Every time.

Steps to Reproduce:
1. Ensure AVOID_COMMIT_BEFORE_INSTALL=1 uncommented in /etc/etckeeper/etckeeper.conf
2. Touch a new file or edit something in /etc - etckeeper unclean should exit with code 0
3. Run dnf install for a package

Actual results:

Last metadata expiration check: 0:27:06 ago on Mon 18 Jan 2021 13:59:04 GMT.
"etckeeper pre-install" failed (exit code 1)

and dnf proceeds as normal with the installation.

Expected results:

Equivalent behaviour of the yum plugin in EPEL 7:

etckeeper: pre transaction commit

** etckeeper detected uncommitted changes in /etc prior to yum run
** Aborting yum run. Manually commit and restart.

Additional info:

Looks like something's wrong with the python3 translation of the plugin?

Comment 1 Joel Uckelman 2021-09-10 21:05:42 UTC
This happens on Fedora 34 as well. I'm not 100% certain, but I believe it did still work in Fedora 32. (I can check in a few days.)

It's _extremely_ unpleasant to have an autocommit happen when you were expecting etckeeper to prevent autocommits.

Comment 2 gulikoza 2022-11-21 19:22:09 UTC
It seems that etckeeper-1.18.12-fix-output-for-ansible.patch removes raising the exception when commit should not happen and only prints logger warning.

This patch restores upstream exception (and fixes changes being committed together with the updates):

diff --git a/etckeeper-dnf/etckeeper.py b/etckeeper-dnf/etckeeper.py
--- a/etckeeper-dnf/etckeeper.py
+++ b/etckeeper-dnf/etckeeper.py
@@ -25,10 +25,8 @@ class Etckeeper(dnf.Plugin):
                 ret = subprocess.call(("etckeeper", command),
                                       stdout=devnull, stderr=devnull,
                                       close_fds=True)
-                if ret > 0:
-                    logger.warning('"etckeeper %s" failed (exit code %d)' % (command, ret))
-                if ret < 0:
-                    logger.warning('"etckeeper %s" died (signal %d)' % (command, -ret))
+                if ret != 0:
+                    raise dnf.exceptions.Error('"etckeeper %s" returned: %d' % (command, ret))
         except OSError as err:
             logger.warning('Failed to run "etckeeper %s": %s' % (command, err))


Results:

# dnf update
Last metadata expiration check: 2:16:56 ago on Mon 21 Nov 2022 06:01:49 PM CET.
Error: "etckeeper pre-install" returned: 1

The error message however could be improved further to indicate why it failed (as in EPEL 7).

Comment 3 Thomas Moschny 2022-12-02 20:44:47 UTC
See http://etckeeper.branchable.com/todo/DNF:_fix_logging__44___so_it_will_work_from_Ansible/#comment-001e7df84c078099b13e9329de359c2b .

Before applying the patch, output caused problems when DNF was used from Ansible.

On the other hand, not all DNF versions seem to cancel the transaction when an exception is generated in the plugin's hook.

It is not clear to me what the correct solution is.

On which OS did you test your patch?

Comment 4 Thomas Moschny 2022-12-02 21:43:36 UTC
Looking at bug 1701807, the corresponding patch https://github.com/rpm-software-management/dnf/pull/1383 seems to have been included in dnf-4.2.5, so is included in el 8.1 and later. So we could try your patch (i.e., partly reverting the etckeeper-1.18.12-fix-output-for-ansible.patch).

Comment 5 Fedora Update System 2022-12-03 14:03:34 UTC
FEDORA-EPEL-2022-b416abaa5f has been submitted as an update to Fedora EPEL 8. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2022-b416abaa5f

Comment 6 Fedora Update System 2022-12-03 14:03:35 UTC
FEDORA-EPEL-2022-f6e51740fc has been submitted as an update to Fedora EPEL 9. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2022-f6e51740fc

Comment 7 Fedora Update System 2022-12-03 14:03:36 UTC
FEDORA-2022-7d3ab5fc50 has been submitted as an update to Fedora 37. https://bodhi.fedoraproject.org/updates/FEDORA-2022-7d3ab5fc50

Comment 8 Fedora Update System 2022-12-03 14:03:37 UTC
FEDORA-2022-0d644cc063 has been submitted as an update to Fedora 36. https://bodhi.fedoraproject.org/updates/FEDORA-2022-0d644cc063

Comment 9 Fedora Update System 2022-12-04 01:44:08 UTC
FEDORA-2022-7d3ab5fc50 has been pushed to the Fedora 37 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf upgrade --enablerepo=updates-testing --refresh --advisory=FEDORA-2022-7d3ab5fc50`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2022-7d3ab5fc50

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 10 Fedora Update System 2022-12-04 02:27:18 UTC
FEDORA-EPEL-2022-b416abaa5f has been pushed to the Fedora EPEL 8 testing repository.

You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2022-b416abaa5f

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 11 Fedora Update System 2022-12-04 02:36:26 UTC
FEDORA-EPEL-2022-f6e51740fc has been pushed to the Fedora EPEL 9 testing repository.

You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2022-f6e51740fc

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 12 Fedora Update System 2022-12-04 02:39:58 UTC
FEDORA-2022-0d644cc063 has been pushed to the Fedora 36 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf upgrade --enablerepo=updates-testing --refresh --advisory=FEDORA-2022-0d644cc063`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2022-0d644cc063

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 13 gulikoza 2022-12-11 09:51:57 UTC
> On which OS did you test your patch?

I'm using el 8.6 or el 8.7.
I didn't see that initially dnf did not terminate after an exception, but I suppose this is now included after el 8.1

I haven't tested Ansible so I can't say if exception causes additional problems there.

Comment 14 Fedora Update System 2023-01-29 00:22:02 UTC
FEDORA-EPEL-2023-892e5376ac has been pushed to the Fedora EPEL 9 testing repository.

You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2023-892e5376ac

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 15 Fedora Update System 2023-01-29 01:48:47 UTC
FEDORA-2023-a427abed1e has been pushed to the Fedora 37 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf upgrade --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-a427abed1e`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-a427abed1e

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 16 Fedora Update System 2023-01-29 01:52:09 UTC
FEDORA-2023-11e40b5991 has been pushed to the Fedora 36 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf upgrade --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-11e40b5991`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-11e40b5991

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 17 Fedora Update System 2023-01-29 02:38:48 UTC
FEDORA-EPEL-2023-8fe90c6cd1 has been pushed to the Fedora EPEL 8 testing repository.

You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2023-8fe90c6cd1

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 18 Fedora Update System 2023-02-06 00:41:18 UTC
FEDORA-EPEL-2023-8fe90c6cd1 has been pushed to the Fedora EPEL 8 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 19 Fedora Update System 2023-02-06 01:00:22 UTC
FEDORA-EPEL-2023-892e5376ac has been pushed to the Fedora EPEL 9 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 20 Fedora Update System 2023-02-06 01:32:37 UTC
FEDORA-2023-11e40b5991 has been pushed to the Fedora 36 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 21 Fedora Update System 2023-02-06 01:37:34 UTC
FEDORA-2023-a427abed1e has been pushed to the Fedora 37 stable repository.
If problem still persists, please make note of it in this bug report.