Storwize: Replication status still active when primary copy is offline

If primary backend is in abnormal state, even secondary backend is in good
state, we should report error status.

Closes-Bug: #1384040

Change-Id: I777ea6e7ba8c0a4aab77021a561b94285b36b402

diff --git a/cinder/tests/test_storwize_svc.py b/cinder/tests/test_storwize_svc.py
index 51425740fc4a83a836f03e89732479238b72b154..0ae601602769b40d8837314e5417ee20cc8c6c3c 100644 (file)
--- a/cinder/tests/test_storwize_svc.py
+++ b/cinder/tests/test_storwize_svc.py
@@ -1657,6 +1657,15 @@ port_speed!N/A
     def error_injection(self, cmd, error):
         self._next_cmd_error[cmd] = error
 
+    def change_vdiskcopy_attr(self, vol_name, key, value, copy="primary"):
+        if copy == 'primary':
+            self._volumes_list[vol_name]['copies']['0'][key] = value
+        elif copy == 'secondary':
+            self._volumes_list[vol_name]['copies']['1'][key] = value
+        else:
+            msg = _("The copy should be primary or secondary")
+            raise exception.InvalidInput(reason=msg)
+
 
 class StorwizeSVCFakeDriver(storwize_svc.StorwizeSVCDriver):
     def __init__(self, *args, **kwargs):
@@ -3218,13 +3227,69 @@ class StorwizeSVCDriverTestCase(test.TestCase):
         model_update = self.driver.get_replication_status(self.ctxt, volume)
         self.assertEqual('copying', model_update['replication_status'])
 
+        # Primary copy offline, secondary copy online, data consistent
+        self.sim.change_vdiskcopy_attr(volume['name'], 'status', 'offline')
+        model_update = self.driver.get_replication_status(self.ctxt, volume)
+        self.assertEqual('active-stop', model_update['replication_status'])
+
+        # Primary copy offline, secondary copy online, data inconsistent
+        self.sim.change_vdiskcopy_attr(volume['name'], 'sync', 'No',
+                                       copy="secondary")
+        model_update = self.driver.get_replication_status(self.ctxt, volume)
+        self.assertEqual('error', model_update['replication_status'])
+
+        # Primary copy online, secondary copy offline, data consistent
+        self.sim.change_vdiskcopy_attr(volume['name'], 'sync', 'yes',
+                                       copy="secondary")
+        self.sim.change_vdiskcopy_attr(volume['name'], 'status', 'offline',
+                                       copy="secondary")
+        self.sim.change_vdiskcopy_attr(volume['name'], 'status', 'online')
+        model_update = self.driver.get_replication_status(self.ctxt, volume)
+        self.assertEqual('error', model_update['replication_status'])
+
+        # Primary copy online, secondary copy offline, data inconsistent
+        self.sim.change_vdiskcopy_attr(volume['name'], 'sync', 'no',
+                                       copy="secondary")
+        model_update = self.driver.get_replication_status(self.ctxt, volume)
+        self.assertEqual('error', model_update['replication_status'])
+
+        # Primary copy offline, secondary copy offline, data consistent
+        self.sim.change_vdiskcopy_attr(volume['name'], 'sync', 'yes',
+                                       copy="secondary")
+        self.sim.change_vdiskcopy_attr(volume['name'], 'status', 'offline',
+                                       copy="primary")
+        model_update = self.driver.get_replication_status(self.ctxt, volume)
+        self.assertEqual('error', model_update['replication_status'])
+
+        # Primary copy offline, secondary copy offline, data inconsistent
+        self.sim.change_vdiskcopy_attr(volume['name'], 'sync', 'no',
+                                       copy="secondary")
+        model_update = self.driver.get_replication_status(self.ctxt, volume)
+        self.assertEqual('error', model_update['replication_status'])
+
+        # Primary copy online, secondary copy online, data inconsistent
+        self.sim.change_vdiskcopy_attr(volume['name'], 'status', 'online',
+                                       copy="secondary")
+        self.sim.change_vdiskcopy_attr(volume['name'], 'status', 'online',
+                                       copy="primary")
+        self.sim.change_vdiskcopy_attr(volume['name'], 'sync', 'no',
+                                       copy="secondary")
+        model_update = self.driver.get_replication_status(self.ctxt, volume)
+        self.assertEqual('copying', model_update['replication_status'])
+
+        # Primary copy online, secondary copy online, data consistent
+        self.sim.change_vdiskcopy_attr(volume['name'], 'sync', 'yes',
+                                       copy="secondary")
+        model_update = self.driver.get_replication_status(self.ctxt, volume)
+        self.assertEqual('active', model_update['replication_status'])
+
         # Check the volume copy created on pool opentack2.
         attrs = self.driver._helpers.get_vdisk_attributes(volume['name'])
         self.assertIn('openstack2', attrs['mdisk_grp_name'])
 
         primary_status = attrs['primary']
-
         self.driver.promote_replica(self.ctxt, volume)
+
         # After promote_replica, primary copy should be swiched.
         attrs = self.driver._helpers.get_vdisk_attributes(volume['name'])
         self.assertEqual(primary_status[0], attrs['primary'][1])

diff --git a/cinder/volume/drivers/ibm/storwize_svc/replication.py b/cinder/volume/drivers/ibm/storwize_svc/replication.py
index a05b44dedc83c3369abddc161a198a0dd0f3f4dc..d89e773a0bbbe2d2ac5afc3ba41ea5a7c6fce08d 100644 (file)
--- a/cinder/volume/drivers/ibm/storwize_svc/replication.py
+++ b/cinder/volume/drivers/ibm/storwize_svc/replication.py
@@ -134,35 +134,44 @@ class StorwizeSVCReplicationStretchedCluster(StorwizeSVCReplication):
 
         primary = copies.get('primary', None)
         secondary = copies.get('secondary', None)
-        status = None
 
-        # Check status of primary copy
+        # Check status of primary copy, set status 'error' as default
+        status = 'error'
         if not primary:
             primary = {'status': 'not found',
                        'sync': 'no'}
-        if primary['status'] != 'online':
-            status = 'error'
         else:
-            status = 'active'
+            if primary['status'] == 'online':
+                status = 'active'
 
         extended1 = (_('Primary copy status: %(status)s'
                        ' and synchronized: %(sync)s') %
                      {'status': primary['status'],
                       'sync': primary['sync']})
+
         # Check status of secondary copy
         if not secondary:
             secondary = {'status': 'not found',
                          'sync': 'no',
                          'sync_progress': '0'}
-
-        if secondary['status'] != 'online':
             status = 'error'
         else:
-            if secondary['sync'] == 'yes':
-                status = 'active'
-                secondary['sync_progress'] = '100'
+            if secondary['status'] != 'online':
+                status = 'error'
             else:
-                status = 'copying'
+                if secondary['sync'] == 'yes':
+                    secondary['sync_progress'] = '100'
+                    # Only change the status if not in error state
+                    if status != 'error':
+                        status = 'active'
+                    else:
+                        # Primary offline, secondary online, data consistent,
+                        # stop copying
+                        status = 'active-stop'
+                else:
+                    # Primary and secondary both online, the status is copying
+                    if status != 'error':
+                        status = 'copying'
 
         extended2 = (_('Secondary copy status: %(status)s'
                        ' and synchronized: %(sync)s,'