Backup and restore depend on DS tasks succeeding, but they only waited for the task to end and didn't check the result.

This patch adds a new helper, run_task, that runs a task, waits for it, and checks the result. Backup and restore are changed to use this helper.


--
PetrĀ³
From a557d1e816280719404f7e2dbabad4f425b21e5e Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Tue, 18 Nov 2014 14:05:59 +0100
Subject: [PATCH] Raise an error when a critical DS task fails

The backup/restore ignored results of backup/restore task,
because the helper used does not check for errors.

Add an error-checking helper to installutils, and have
backup/restore use it.

The wait_for_task helper is moved to installutils as well. Its
return value is changed. (The return value was always ignored
in current code.)
---
 ipaserver/install/installutils.py | 55 +++++++++++++++++++++++++++++++++++++++
 ipaserver/install/ipa_backup.py   | 19 ++------------
 ipaserver/install/ipa_restore.py  | 20 +++-----------
 ipaserver/install/replication.py  | 21 +--------------
 4 files changed, 61 insertions(+), 54 deletions(-)

diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index d3f09eccb161cc9371fa7c1109816f47194d9cb6..955fa348c6f519b5145288dd58cf8b8ffe997903 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -29,6 +29,7 @@
 import traceback
 import textwrap
 from contextlib import contextmanager
+import time
 
 from dns import resolver, rdatatype
 from dns.exception import DNSException
@@ -46,6 +47,8 @@
 from ipaplatform import services
 from ipaplatform.paths import paths
 
+log = log_mgr.get_logger(__name__)
+
 # Used to determine install status
 IPA_MODULES = [
     'httpd', 'kadmin', 'dirsrv', 'pki-cad', 'pki-tomcatd', 'install',
@@ -1035,3 +1038,55 @@ def load_external_cert(files, subject_base):
     ca_file.flush()
 
     return cert_file, ca_file
+
+
+def wait_for_task(conn, dn):
+    """Check task status
+
+    Task is complete when the nsTaskExitCode attr is set.
+
+    :return: the task's entry
+
+    Usually you will want to call run_task, which checks the task's exit code
+    """
+    assert isinstance(dn, DN)
+    attrlist = [
+        'nsTaskLog', 'nsTaskStatus', 'nsTaskExitCode', 'nsTaskCurrentItem',
+        'nsTaskTotalItems']
+    while True:
+        entry = conn.get_entry(dn, attrlist)
+        if entry.single_value.get('nsTaskExitCode'):
+            break
+        time.sleep(1)
+    for log_item in entry.get('nsTaskLog', []):
+        log.debug('DS task log: %s', log_item)
+    return entry
+
+
+def run_task(conn, entry, task_name, raiseonerr=True):
+    """Run a DS task, wait for it to finish, check it completed successfully
+
+    :param conn: connected LDAPClient
+    :param entry: task to run, as an LDAP entry
+    :param task_name: name to use in messages
+    :param raiseonerr: if true (default), raise RuntimeError if the task failed
+    """
+    dn = entry.dn
+    try:
+        conn.add_entry(entry)
+    except Exception as e:
+        raise admintool.ScriptError(
+            'Unable to add %s task: %s' % (task_name, e))
+
+    log.info("Waiting for %s task to finish", task_name)
+    entry = wait_for_task(conn, dn)
+    exit_code = int(entry.single_value['nsTaskExitCode'])
+    if exit_code:
+        result = entry.single_value.get('nsTaskStatus', 'no status')
+        if raiseonerr:
+            raise RuntimeError('%s task failed: %s: %s' % (
+                task_name, exit_code, result))
+        else:
+            log.warn('%s task failed: %s: %s',
+                     task_name, exit_code, result)
+    log.info("%s task finished", task_name)
diff --git a/ipaserver/install/ipa_backup.py b/ipaserver/install/ipa_backup.py
index 682b626e55b9ead5424369e1bab85109ff6b8207..4dc715cf880ee97bc0c27760e5ce3f082b0cef4f 100644
--- a/ipaserver/install/ipa_backup.py
+++ b/ipaserver/install/ipa_backup.py
@@ -35,7 +35,6 @@
 from ipapython.config import IPAOptionParser
 from ipapython.dn import DN
 from ipaserver.install.dsinstance import realm_to_serverid, DS_USER
-from ipaserver.install.replication import wait_for_task
 from ipaserver.install import installutils
 from ipapython import ipaldap
 from ipalib.session import ISO8601_DATETIME_FMT
@@ -403,14 +402,7 @@ def db2ldif(self, instance, backend, online=True):
                 }
             )
 
-            try:
-                conn.add_entry(ent)
-            except Exception, e:
-                raise admintool.ScriptError('Unable to add LDIF task: %s'
-                    % e)
-
-            self.log.info("Waiting for LDIF to finish")
-            wait_for_task(conn, dn)
+            installutils.run_task(conn, ent, 'LDIF export')
         else:
             args = ['%s/db2ldif' % self.__find_scripts_dir(instance),
                     '-r',
@@ -450,14 +442,7 @@ def db2bak(self, instance, online=True):
                 }
             )
 
-            try:
-                conn.add_entry(ent)
-            except Exception, e:
-                raise admintool.ScriptError('Unable to to add backup task: %s'
-                    % e)
-
-            self.log.info("Waiting for BAK to finish")
-            wait_for_task(conn, dn)
+            installutils.run_task(conn, ent, 'BAK backup')
         else:
             args = ['%s/db2bak' % self.__find_scripts_dir(instance), bakdir]
             (stdout, stderr, rc) = run(args, raiseonerr=False)
diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py
index 7276ed305e11e9ce37407ffa5f4c5f2206f34802..edad79ff5c71849f41697dff7f2e9791e365a856 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -33,7 +33,7 @@
 from ipaserver.install.dsinstance import (realm_to_serverid,
                                           create_ds_user, DS_USER)
 from ipaserver.install.cainstance import PKI_USER, create_ca_user
-from ipaserver.install.replication import (wait_for_task, ReplicationManager,
+from ipaserver.install.replication import (ReplicationManager,
                                            get_cs_replication_manager)
 from ipaserver.install import installutils
 from ipaserver.install import httpinstance
@@ -435,14 +435,7 @@ def ldif2db(self, instance, backend, online=True):
             )
             ent['nsInstance'] = [backend]
 
-            try:
-                conn.add_entry(ent)
-            except Exception, e:
-                raise admintool.ScriptError(
-                    'Unable to bind to LDAP server: %s' % e)
-
-            self.log.info("Waiting for LDIF to finish")
-            wait_for_task(conn, dn)
+            installutils.run_task(conn, ent, 'LDIF import')
         else:
             args = ['%s/ldif2db' % self.__find_scripts_dir(instance),
                     '-i', ldiffile]
@@ -492,14 +485,7 @@ def bak2db(self, instance, backend, online=True):
             if backend is not None:
                 ent['nsInstance'] = [backend]
 
-            try:
-                conn.add_entry(ent)
-            except Exception, e:
-                raise admintool.ScriptError('Unable to bind to LDAP server: %s'
-                    % e)
-
-            self.log.info("Waiting for restore to finish")
-            wait_for_task(conn, dn)
+            installutils.run_task(conn, ent, 'BAK restore')
         else:
             args = ['%s/bak2db' % self.__find_scripts_dir(instance),
                     os.path.join(self.dir, instance)]
diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py
index 5778cab036ad87ccb5b69254aa307a6bc8dec871..e86aefe52b4432b6f6d165d66cb8c38a5526cd80 100644
--- a/ipaserver/install/replication.py
+++ b/ipaserver/install/replication.py
@@ -31,6 +31,7 @@
 from ipapython.dn import DN
 from ipaplatform import services
 from ipaplatform.paths import paths
+from ipaserver.install.installutils import wait_for_task
 
 # the default container used by AD for user entries
 WIN_USER_CONTAINER = DN(('cn', 'Users'))
@@ -114,26 +115,6 @@ def enable_replication_version_checking(hostname, realm, dirman_passwd):
         conn.unbind()
 
 
-def wait_for_task(conn, dn):
-    """Check task status
-
-    Task is complete when the nsTaskExitCode attr is set.
-
-    :return: the task's return code
-    """
-    assert isinstance(dn, DN)
-    attrlist = [
-        'nsTaskLog', 'nsTaskStatus', 'nsTaskExitCode', 'nsTaskCurrentItem',
-        'nsTaskTotalItems']
-    while True:
-        entry = conn.get_entry(dn, attrlist)
-        if entry.single_value.get('nsTaskExitCode'):
-            exit_code = int(entry.single_value['nsTaskExitCode'])
-            break
-        time.sleep(1)
-    return exit_code
-
-
 def wait_for_entry(connection, entry, timeout=7200, attr='', quiet=True):
     """Wait for entry and/or attr to show up"""
 
-- 
2.1.0

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to