Check for online_data_migrations when syncing db

Due to bug #1908037, the database sync can fail if the volume types
have not been migrated to the __DEFAULT__ volume type. When this fails,
a message will be sent telling the user to run online_data_migrations.
Check for this error, and when it occurs, run the online_data_migrations
and then retry the db sync.

Closes-Bug: #1908037

Change-Id: I0cd9a9db256b0adffbb244222a66c85d055840c4

hooks/cinder_utils.py

@@ -37,6 +37,7 @@ from charmhelpers.core.hookenv import (
     related_units,
     log,
     DEBUG,
+    INFO,
     hook_name,
 )
 
@@ -785,7 +786,27 @@ def migrate_database(upgrade=False):
         return
 
     cmd = ['cinder-manage', 'db', 'sync']
-    subprocess.check_call(cmd)
+    try:
+        subprocess.check_output(cmd, universal_newlines=True)
+    except subprocess.CalledProcessError as e:
+        # LP #1908037 - Due to a bug in cinder, the sync can fail if the
+        # volume types have not been migrated to __DEFAULT__ volume type.
+        # Check the output message from the logs to see if online data
+        # migrations are necessary and run that first.
+        if not upgrade:
+            raise
+
+        need_online_migration_msg = (
+            "Please run `cinder-manage db online_data_migrations`")
+        if need_online_migration_msg not in e.output:
+            raise
+
+        log("Running online_data_migrations", level=INFO)
+        subprocess.check_call(['cinder-manage', 'db',
+                               'online_data_migrations'])
+        log("Re-running database migration", level=INFO)
+        subprocess.check_call(cmd)
+
     # Notify peers so that services get restarted
     log("Notifying peer(s) that db is initialised and restarting services",
         level=DEBUG)

unit_tests/test_cinder_utils.py

@@ -739,9 +739,11 @@ class TestCinderUtils(CharmTestCase):
         rid = 'cluster:0'
         self.relation_ids.return_value = [rid]
         args = {'cinder-db-initialised': "unit/0-%s" % uuid}
-        with patch('subprocess.check_call') as check_call:
+        with patch('subprocess.check_output') as check_output:
             cinder_utils.migrate_database()
-            check_call.assert_called_with(['cinder-manage', 'db', 'sync'])
+            check_output.assert_called_once_with(
+                ['cinder-manage', 'db', 'sync'], universal_newlines=True
+            )
             self.relation_set.assert_called_with(relation_id=rid, **args)
             self.service_restart.assert_called_with('svc1')
 
@@ -749,11 +751,38 @@ class TestCinderUtils(CharmTestCase):
     def test_migrate_database_already_initisalised(self,
                                                    mock_is_db_initialised):
         mock_is_db_initialised.return_value = True
-        with patch('subprocess.check_call') as check_call:
+        with patch('subprocess.check_output') as check_call:
             cinder_utils.migrate_database()
             self.assertFalse(check_call.called)
             self.assertFalse(self.service_restart.called)
 
+    @patch.object(cinder_utils, 'is_db_initialised')
+    @patch.object(cinder_utils, 'enabled_services')
+    @patch.object(cinder_utils, 'local_unit', lambda *args: 'unit/0')
+    @patch.object(cinder_utils, 'uuid')
+    def test_migrate_database_need_online_migration(
+            self, mock_uuid, mock_enabled_services, mock_is_db_initialised):
+        'It runs online migrations when required'
+        mock_is_db_initialised.return_value = True
+        uuid = 'a-great-uuid'
+        mock_uuid.uuid4.return_value = uuid
+        mock_enabled_services.return_value = ['svc1']
+        rid = 'cluster:0'
+        self.relation_ids.return_value = [rid]
+        with patch('subprocess.check_output') as check_output, \
+             patch('subprocess.check_call') as check_call:
+            check_output.side_effect = subprocess.CalledProcessError(
+                1, 'cinder db-manage sync',
+                ("Please run `cinder-manage db online_data_migrations`. "
+                 "There are still untyped volumes unmigrated."))
+            cinder_utils.migrate_database(upgrade=True)
+            check_output.assert_called_with(['cinder-manage', 'db', 'sync'],
+                                            universal_newlines=True)
+            check_call.assert_has_calls([
+                call(['cinder-manage', 'db', 'online_data_migrations']),
+                call(['cinder-manage', 'db', 'sync'])
+            ])
+
     @patch.object(cinder_utils, 'resource_map')
     def test_register_configs(self, resource_map):
         resource_map.return_value = OrderedDict([