Check volume_backend in retype

The retype command will always attempt a call
to the driver.retype method.  In *most* cases this
will hit the default impl which returns False because
most drivers don't implement any retype (most, a few do).

The problem is that the drivers that do implement this in
most cases will iterate through the settings and just make
the changes that are valid and ignore the rest, and then
return True. I think this is "ok" for the drivers to do,
drivers should be allowed to be somewhat dumb WRT Cinder
state management and placement info.  If we give them an
invalid command (which we're doing here) then it's on us
higher up the chain IMO.

The result is that for example if you're trying to retype
from backend-a to backend-b and backend-a implements retype
it can return True telling the manager that the volume was
succesfully retyped, even when it wasn't.

There's a lot of confusion around this bug, YES the
filter scheduler is used to determine if the retype is
valid and to what host.  That's not the issue, the issue
is that regardless of the source and destination host settings
that are provided from the filter-scheduler, we always make the
call to the driver, introducing the opportunity for a false
success status being reported back.

This patch adds a very simple check between the source and
destination host settings as provided by the scheduler and in
the case that the two are "different"(not including pool designations)
we skip calling the driver.retype method altogether and fall through
to the migrate process.

This introduces a new hosts_are_equivalent method in
cinder.volume.utils

Conflicts:
	cinder/volume/utils.py

Closes-Bug: #1452823
(cherry picked from commit bbb83aff023c90cfc186459603320ad3c622a40b)
Change-Id: Idfd7dfa2284fcea66cf23c4273efda61ee8f7eba

diff --git a/cinder/tests/test_volume_utils.py b/cinder/tests/test_volume_utils.py
index cd49bf6b93954e9b740de4679fd9a303e3e24968..ee1a550df48ecba1c5a9dbecd04b3e2393393cbb 100644 (file)
--- a/cinder/tests/test_volume_utils.py
+++ b/cinder/tests/test_volume_utils.py
@@ -655,3 +655,14 @@ class VolumeUtilsTestCase(test.TestCase):
         expected = None
         self.assertEqual(expected,
                          volume_utils.append_host(host, pool))
+
+    def test_compare_hosts(self):
+        host_1 = 'fake_host@backend1'
+        host_2 = 'fake_host@backend1#pool1'
+        self.assertTrue(volume_utils.hosts_are_equivalent(host_1, host_2))
+
+        host_2 = 'fake_host@backend1'
+        self.assertTrue(volume_utils.hosts_are_equivalent(host_1, host_2))
+
+        host_2 = 'fake_host2@backend1'
+        self.assertFalse(volume_utils.hosts_are_equivalent(host_1, host_2))

diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py
index 150816f26ba87f221ea1968ef628a87b25d4c3f5..d26e8ef6851ef35dafc7838f45cfbdc942d5424d 100644 (file)
--- a/cinder/volume/manager.py
+++ b/cinder/volume/manager.py
@@ -1698,7 +1698,17 @@ class VolumeManager(manager.SchedulerDependentManager):
 
         # Call driver to try and change the type
         retype_model_update = None
-        if not retyped:
+
+        # NOTE(jdg): Check to see if the destination host is the same
+        # as the current.  If it's not don't call the driver.retype
+        # method, otherwise drivers that implement retype may report
+        # success, but it's invalid in the case of a migrate.
+
+        # We assume that those that support pools do this internally
+        # so we strip off the pools designation
+        if (not retyped and
+                vol_utils.hosts_are_equivalent(self.driver.host,
+                                               host['host'])):
             try:
                 new_type = volume_types.get_volume_type(context, new_type_id)
                 ret = self.driver.retype(context,

diff --git a/cinder/volume/utils.py b/cinder/volume/utils.py
index d242118581818721489c8b4e8dfa4ec151f83f95..2ce3613699b86ad958653a4d41205d06611290b1 100644 (file)
--- a/cinder/volume/utils.py
+++ b/cinder/volume/utils.py
@@ -498,3 +498,7 @@ def append_host(host, pool):
 
     new_host = "#".join([host, pool])
     return new_host
+
+
+def hosts_are_equivalent(host_1, host_2):
+    return extract_host(host_1) == extract_host(host_2)