On 04/14/2015 04:24 PM, Martin Basti wrote:
On 14/04/15 16:12, Martin Basti wrote:
On 14/04/15 14:25, Martin Babinsky wrote:
This patch addresses https://fedorahosted.org/freeipa/ticket/4966
The noise during rollback/uninstall is caused mainly by unsuccessful
attempts to remove files that do not exist anymore. These errors are
now logged at debug level and do not pop-up to stdout/stderr.
Hello, thank you for the patch.
1)
The option add_warning is quite unclear to me. It does not show
warning but error. I suggest something like, show_hint,
show_user_action, or something show_additional_..., or
promt_manual_removal
Martin^2
Continue...
2)
if file_exists(preferences_fname):
try:
os.remove(preferences_fname)
except OSError as e:
log_file_removal_error(e, preferences_fname, True)
In this case file not found error should never happen.
Could you remove the 'if file_exists' part and handle just exception?
I just reverted this bit to original form in order to not fix something
that isn't broken. Is that ok?
3)
this is inconsistent with change above, choose one style please:
if os.path.exists(ca_file):
try:
os.unlink(ca_file)
except OSError, e:
root_logger.error(
"Failed to remove '%s': %s", ca_file, e)
--
Martin Basti
Attaching updated patch.
--
Martin^3 Babinsky
From b672b8a3f11918c699d4f4391d2a0c1c0cc1503d Mon Sep 17 00:00:00 2001
From: Martin Babinsky <mbabi...@redhat.com>
Date: Tue, 14 Apr 2015 13:55:33 +0200
Subject: [PATCH] suppress errors arising from deleting non-existent files
during client uninstall
When rolling back partially configured IPA client a number of OSErrors pop up
due to uninstaller trying to remove files that do not exist anymore. This
patch supresses these errors while keeping them in log as debug messages.
https://fedorahosted.org/freeipa/ticket/4966
---
ipa-client/ipa-install/ipa-client-install | 35 +++++++++++++++++++++++--------
1 file changed, 26 insertions(+), 9 deletions(-)
diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 3f9a7419a10ddcb4618e80789a06a05058d1e8a4..d39672f6be7695b2440b9732a389ede2ba8cceb6 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -224,6 +224,29 @@ def logging_setup(options):
console_format='%(message)s')
+def log_file_removal_error(e, filename, prompt_manual_removal=False):
+ """
+ Logs OSErrors raised during file removal according to their nature.
+ OSError 2 (no such file/directory) will be logged only at debug
+ level, other OSError exceptions will be logged as errors.
+
+ :param e: OSError instance
+ :param filename: filename which triggered the error
+ :param add_warning: If set to `True`, prints another message which
+ instructs the user to manually remove the offending file (if it exists)
+ """
+ assert isinstance(e, OSError)
+ logger_func = root_logger.error
+ if e.errno == 2:
+ logger_func = root_logger.debug
+ prompt_manual_removal = False
+
+ logger_func("Failed to remove file %s: %s", filename, e)
+ if prompt_manual_removal:
+ logger_func('Please remove %s manually, as it can cause subsequent '
+ 'installation to fail.', filename)
+
+
def log_service_error(name, action, error):
root_logger.error("%s failed to %s: %s", name, action, str(error))
@@ -532,7 +555,7 @@ def uninstall(options, env):
try:
os.remove(filename)
except OSError, e:
- root_logger.error("Failed to remove %s: %s", filename, e)
+ log_file_removal_error(e, filename)
for nickname, trust_flags in ipa_certs:
while sys_db.has_nickname(nickname):
@@ -763,10 +786,7 @@ def uninstall(options, env):
try:
os.remove(paths.IPA_DEFAULT_CONF)
except OSError, e:
- root_logger.warning('/etc/ipa/default.conf could not be removed: %s',
- str(e))
- root_logger.warning('Please remove /etc/ipa/default.conf manually, '
- 'as it can cause subsequent installation to fail.')
+ log_file_removal_error(e, paths.IPA_DEFAULT_CONF, True)
# Remove the CA cert from the systemwide certificate store
tasks.remove_ca_certs_from_systemwide_ca_store()
@@ -775,10 +795,7 @@ def uninstall(options, env):
try:
os.remove(CACERT)
except OSError, e:
- root_logger.warning('%s could not be removed: %s', CACERT, str(e))
- root_logger.warning('Please remove %s manually, '
- 'as it can cause subsequent '
- 'installation to fail.', CACERT)
+ log_file_removal_error(e, CACERT, True)
root_logger.info("Client uninstall complete.")
--
2.1.0
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
