Fix replication_status on InvalidReplicationTarget

When we cannot do a replication failover because the driver raises an
InvalidReplicationTarget exception we revert the replication status from
"failing-over" to its previous status.

Since we are not saving the previous status we were deciding what it was
based on the requested secondary_backend_id, which is wrong, as it
assumes we cannot failover from one secondary to another secondary.

This patch fixes this by changing the deciding factor from the
secondary_backend_id to the current backend_id in the service.

We had no failover_host manager tests, and this patch doesn't try to add
all the tests that we should have, it just adds the tests pertinent for
this patch to avoid regressions and test this specific functionality.
Missing tests should be added in another patch.

Closes-Bug: #1641715
Change-Id: I5a911278aef8060e14577099b0e03daf2039a783

cinder/tests/unit/utils.py

@@ -375,6 +375,16 @@ def create_qos(ctxt, testcase_instance=None, **kwargs):
     return qos
 
 
+def create_service(ctxt, binary='cinder-volume', host='host@backend',
+                   topic='topic', disabled=False, availability_zone='cinder',
+                   **kwargs):
+    kwargs.update(binary=binary, host=host, topic=topic, disabled=disabled,
+                  availability_zone=availability_zone)
+    svc = objects.Service(ctxt, **kwargs)
+    svc.create()
+    return svc
+
+
 class ZeroIntervalLoopingCall(loopingcall.FixedIntervalLoopingCall):
     def start(self, interval, **kwargs):
         kwargs['initial_delay'] = 0

cinder/tests/unit/volume/test_replication_manager.py

@@ -0,0 +1,64 @@
+# Copyright (c) 2016 Red Hat, Inc.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import ddt
+import mock
+
+from cinder import exception
+from cinder import objects
+from cinder.objects import fields
+from cinder.tests.unit import utils
+from cinder.tests.unit import volume as base
+from cinder.volume import manager
+
+
+@ddt.ddt
+class ReplicationTestCase(base.BaseVolumeTestCase):
+    def setUp(self):
+        super(ReplicationTestCase, self).setUp()
+        self.host = 'host@backend#pool'
+        self.manager = manager.VolumeManager(host=self.host)
+
+    @mock.patch('cinder.objects.VolumeList.get_all_by_host')
+    @mock.patch('cinder.volume.driver.BaseVD.failover_host',
+                side_effect=exception.InvalidReplicationTarget(''))
+    @ddt.data(('backend2', 'default', fields.ReplicationStatus.FAILED_OVER),
+              ('backend2', 'backend3', fields.ReplicationStatus.FAILED_OVER),
+              (None, 'backend2', fields.ReplicationStatus.ENABLED),
+              ('', 'backend2', fields.ReplicationStatus.ENABLED))
+    @ddt.unpack
+    def test_failover_host_invalid_target(self, svc_backend, new_backend,
+                                          expected, mock_failover,
+                                          mock_getall):
+        """Test replication failover_host with invalid_target.
+
+        When failingover fails due to an invalid target exception we return
+        replication_status to its previous status, and we decide what that is
+        depending on the currect active backend.
+        """
+        svc = utils.create_service(
+            self.context,
+            host=self.host,
+            active_backend_id=svc_backend,
+            replication_status=fields.ReplicationStatus.FAILING_OVER)
+
+        self.manager.failover_host(self.context, new_backend)
+        mock_getall.assert_called_once_with(self.context, self.host)
+        mock_failover.assert_called_once_with(self.context,
+                                              mock_getall.return_value,
+                                              secondary_id=new_backend)
+
+        db_svc = objects.Service.get_by_id(self.context, svc.id)
+        self.assertEqual(expected, db_svc.replication_status)

cinder/volume/manager.py

@@ -4034,8 +4034,11 @@ class VolumeManager(manager.CleanableManager,
         except exception.InvalidReplicationTarget:
             LOG.exception(_LE("Invalid replication target specified "
                               "for failover"))
-            # Preserve the replication_status
-            if secondary_backend_id == "default":
+            # Preserve the replication_status: Status should be failed over if
+            # we were failing back or if we were failing over from one
+            # secondary to another secondary. In both cases active_backend_id
+            # will be set.
+            if service.active_backend_id:
                 service.replication_status = (
                     fields.ReplicationStatus.FAILED_OVER)
             else: