Server Upgrade: handle errors better

* Prevent to continue with upgrade if a fatal error happened
* Use exceptions to handle failures

https://fedorahosted.org/freeipa/ticket/4904

Reviewed-By: David Kupka <dkupka@redhat.com>

4 files changed, 29 insertions, 24 deletions

diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index e216edbfa..f1d24e49d 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -506,7 +506,11 @@ class DsInstance(service.Service):
 
     def apply_updates(self):
         data_upgrade = upgradeinstance.IPAUpgrade(self.realm)
-        data_upgrade.create_instance()
+        try:
+            data_upgrade.create_instance()
+        except Exception as e:
+            # very fatal errors only will raise exception
+            raise RuntimeError("Update failed: %s" % e)
         installutils.store_version()
 
 
diff --git a/ipaserver/install/ipa_ldap_updater.py b/ipaserver/install/ipa_ldap_updater.py
index 5a0d61219..40e8e7f72 100644
--- a/ipaserver/install/ipa_ldap_updater.py
+++ b/ipaserver/install/ipa_ldap_updater.py
@@ -32,7 +32,7 @@ from ipalib import api
 from ipapython import ipautil, admintool
 from ipaplatform.paths import paths
 from ipaserver.install import installutils, dsinstance, schemaupdate
-from ipaserver.install.ldapupdate import LDAPUpdate, UPDATES_DIR
+from ipaserver.install.ldapupdate import LDAPUpdate, UPDATES_DIR, BadSyntax
 from ipaserver.install.upgradeinstance import IPAUpgrade
 
 
@@ -108,17 +108,19 @@ class LDAPUpdater_Upgrade(LDAPUpdater):
         realm = krbV.default_context().default_realm
         upgrade = IPAUpgrade(realm, self.files,
                              schema_files=options.schema_files)
-        upgrade.create_instance()
 
-        if upgrade.badsyntax:
+        try:
+            upgrade.create_instance()
+        except BadSyntax:
             raise admintool.ScriptError(
                 'Bad syntax detected in upgrade file(s).', 1)
-        elif upgrade.upgradefailed:
+        except RuntimeError:
             raise admintool.ScriptError('IPA upgrade failed.', 1)
-        elif upgrade.modified:
-            self.log.info('Update complete')
         else:
-            self.log.info('Update complete, no data were modified')
+            if upgrade.modified:
+                self.log.info('Update complete')
+            else:
+                self.log.info('Update complete, no data were modified')
 
 
 class LDAPUpdater_NonUpgrade(LDAPUpdater):
diff --git a/ipaserver/install/ipa_server_upgrade.py b/ipaserver/install/ipa_server_upgrade.py
index 148d1fe7e..7e85c0dca 100644
--- a/ipaserver/install/ipa_server_upgrade.py
+++ b/ipaserver/install/ipa_server_upgrade.py
@@ -11,6 +11,7 @@ from ipaplatform.paths import paths
 from ipapython import admintool, ipautil
 from ipaserver.install import installutils
 from ipaserver.install.upgradeinstance import IPAUpgrade
+from ipaserver.install.ldapupdate import BadSyntax
 
 
 class ServerUpgrade(admintool.AdminTool):
@@ -73,17 +74,19 @@ class ServerUpgrade(admintool.AdminTool):
 
         realm = krbV.default_context().default_realm
         data_upgrade = IPAUpgrade(realm)
-        data_upgrade.create_instance()
 
-        if data_upgrade.badsyntax:
+        try:
+            data_upgrade.create_instance()
+        except BadSyntax:
             raise admintool.ScriptError(
                 'Bad syntax detected in upgrade file(s).', 1)
-        elif data_upgrade.upgradefailed:
+        except RuntimeError:
             raise admintool.ScriptError('IPA upgrade failed.', 1)
-        elif data_upgrade.modified:
-            self.log.info('Data update complete')
         else:
-            self.log.info('Data update complete, no data were modified')
+            if data_upgrade.modified:
+                self.log.info('Update complete')
+            else:
+                self.log.info('Update complete, no data were modified')
 
         # store new data version after upgrade
         installutils.store_version()
diff --git a/ipaserver/install/upgradeinstance.py b/ipaserver/install/upgradeinstance.py
index 862606fdc..2540df60f 100644
--- a/ipaserver/install/upgradeinstance.py
+++ b/ipaserver/install/upgradeinstance.py
@@ -167,8 +167,6 @@ class IPAUpgrade(service.Service):
         self.savefilename = '%s/%s.ipa.%s' % (paths.ETC_DIRSRV_SLAPD_INSTANCE_TEMPLATE % serverid, DSE, ext)
         self.files = files
         self.modified = False
-        self.badsyntax = False
-        self.upgradefailed = False
         self.serverid = serverid
         self.schema_files = schema_files
         self.realm = realm_name
@@ -307,13 +305,11 @@ class IPAUpgrade(service.Service):
             if len(self.files) == 0:
                 self.files = ld.get_all_files(ldapupdate.UPDATES_DIR)
             self.modified = (ld.update(self.files) or self.modified)
-        except ldapupdate.BadSyntax, e:
-            root_logger.error('Bad syntax in upgrade %s' % str(e))
-            self.modified = False
-            self.badsyntax = True
-        except Exception, e:
+        except ldapupdate.BadSyntax as e:
+            root_logger.error('Bad syntax in upgrade %s', e)
+            raise
+        except Exception as e:
             # Bad things happened, return gracefully
-            self.modified = False
-            self.upgradefailed = True
-            root_logger.error('Upgrade failed with %s' % str(e))
+            root_logger.error('Upgrade failed with %s', e)
             root_logger.debug('%s', traceback.format_exc())
+            raise RuntimeError(e)