From ad3e655130311836c18a98639f190db197e74f44 Mon Sep 17 00:00:00 2001
From: Patrick Uiterwijk <puiterwijk@redhat.com>
Date: Aug 30 2016 15:00:11 +0000
Subject: [PATCH 1/2] Fix upgrade-database for configfiles


We are not storing schema version information for configfiles, since we won't
be able to use them and the user is responsible for updating them.
With this patch, we tell upgrade-db to continue with the rest of the databases
if any of them is a configfile, rather than abandoning the entire process.

Signed-off-by: Patrick Uiterwijk <puiterwijk@redhat.com>
Reviewed-by: Pierre-Yves Chibon <pingou@pingoured.fr>

---

diff --git a/ipsilon/install/ipsilon-upgrade-database b/ipsilon/install/ipsilon-upgrade-database
index 795e190..1b70146 100755
--- a/ipsilon/install/ipsilon-upgrade-database
+++ b/ipsilon/install/ipsilon-upgrade-database
@@ -6,21 +6,18 @@ __requires__ = ['sqlalchemy >= 0.8']
 import pkg_resources  # pylint: disable=unused-import
 
 import sys
+import logging
+logging.basicConfig(level=logging.INFO)
+
 from ipsilon import find_config
 from ipsilon.tools import dbupgrade
-import logging
 
 logger = logging.getLogger(__name__)
 
 
 if __name__ == '__main__':
-    def_logger = logging.getLogger()
-    ch = logging.StreamHandler(sys.stdout)
-    ch.setLevel(logging.ERROR)
-    def_logger.addHandler(ch)
-
     try:
         dbupgrade.execute_upgrade(find_config())
     except Exception as ex:
-        logger.error(ex)
+        logger.error('Error upgrading database', exc_info=True)
         sys.exit(1)
diff --git a/ipsilon/tools/dbupgrade.py b/ipsilon/tools/dbupgrade.py
index b684bce..4ff079a 100644
--- a/ipsilon/tools/dbupgrade.py
+++ b/ipsilon/tools/dbupgrade.py
@@ -16,7 +16,15 @@ logger = logging.getLogger(__name__)
 
 
 def _upgrade_database(datastore):
+    """
+    This function actually triggers the update on the datastore class.
+    Returns True for success, False for failure, and None if it didn't do
+    anything because the datastore is readonly, as happens with configfiles.
+    """
     logger.debug('Considering datastore %s', datastore.__class__.__name__)
+    if datastore.is_readonly:
+        logger.warning('Datastore is readonly. Please fix manually!')
+        return None
     # pylint: disable=protected-access
     current_version = datastore._get_schema_version()
     # pylint: disable=protected-access
@@ -34,9 +42,6 @@ def _upgrade_database(datastore):
         logger.debug('Schema for %s is up-to-date',
                      datastore.__class__.__name__)
     if upgrade_required:
-        if datastore.is_readonly:
-            logger.warning('Datastore is readonly. Please fix manually!')
-            return False
         try:
             datastore.upgrade_database()
         except Exception as ex:  # pylint: disable=broad-except
@@ -64,7 +69,7 @@ def execute_upgrade(cfgfile):
 
     adminstore = AdminStore()
     # First try to upgrade the config store before continuing
-    if not _upgrade_database(adminstore):
+    if _upgrade_database(adminstore) not in [True, None]:
         return upgrade_failed()
 
     admin_config = adminstore.load_config()
@@ -84,7 +89,7 @@ def execute_upgrade(cfgfile):
     else:
         dburi = cherrypy.config['tools.sessions.storage_dburi']
         SqlSession.setup(storage_dburi=dburi)
-        if not _upgrade_database(SqlSession._store):
+        if _upgrade_database(SqlSession._store) not in [True, None]:
             return upgrade_failed()
 
     # Now handle the rest of the default datastores
@@ -92,7 +97,7 @@ def execute_upgrade(cfgfile):
         store = store()
         logger.debug('Handling default datastore %s',
                      store.__class__.__name__)
-        if not _upgrade_database(store):
+        if _upgrade_database(store) not in [True, None]:
             return upgrade_failed()
 
     # And now datastores for any of the plugins
@@ -111,7 +116,7 @@ def execute_upgrade(cfgfile):
             for store in plugin.used_datastores():
                 logger.debug('Handling plugin datastore %s',
                              store.__class__.__name__)
-                if not _upgrade_database(store):
+                if _upgrade_database(store) not in [True, None]:
                     return upgrade_failed()
 
     # We are done with the init/upgrade

From 080083960bb3ec48d189aaf61c0d3f87a3920744 Mon Sep 17 00:00:00 2001
From: Patrick Uiterwijk <puiterwijk@redhat.com>
Date: Aug 30 2016 15:00:14 +0000
Subject: [PATCH 2/2] Test upgrade-database with file configuration


Signed-off-by: Patrick Uiterwijk <puiterwijk@redhat.com>
Reviewed-by: Pierre-Yves Chibon <pingou@pingoured.fr>

---

diff --git a/tests/fconf.py b/tests/fconf.py
index a5747b1..86c0809 100755
--- a/tests/fconf.py
+++ b/tests/fconf.py
@@ -9,6 +9,7 @@ import os
 import pwd
 import sys
 from string import Template
+import subprocess
 import uuid
 
 
@@ -149,6 +150,15 @@ class IpsilonTest(IpsilonTestBase):
 
         fixup_idp_conf(self.testdir)
 
+        print "Testing database upgrade"
+        cfgfile = os.path.join(self.testdir, 'etc', idpname, 'ipsilon.conf')
+        cmd = [os.path.join(self.rootdir,
+                            'ipsilon/install/ipsilon-upgrade-database'),
+               cfgfile]
+        subprocess.check_call(cmd,
+                              cwd=os.path.join(self.testdir, 'lib', idpname),
+                              env=env)
+
         print "Starting IDP's httpd server"
         self.start_http_server(idpconf, env)