Merge "bug fix remove volume migration type 'cold'"

watcher/applier/actions/volume_migration.py

@@ -36,13 +36,16 @@ class VolumeMigrate(base.BaseAction):
 
     By using this action, you will be able to migrate cinder volume.
     Migration type 'swap' can only be used for migrating attached volume.
-    Migration type 'cold' can only be used for migrating detached volume.
+    Migration type 'migrate' can be used for migrating detached volume to
+    the pool of same volume type.
+    Migration type 'retype' can be used for changing volume type of
+    detached volume.
 
     The action schema is::
 
         schema = Schema({
             'resource_id': str,  # should be a UUID
-            'migration_type': str,  # choices -> "swap", "cold"
+            'migration_type': str,  # choices -> "swap", "migrate","retype"
             'destination_node': str,
             'destination_type': str,
         })
@@ -60,7 +63,8 @@ class VolumeMigrate(base.BaseAction):
 
     MIGRATION_TYPE = 'migration_type'
     SWAP = 'swap'
-    COLD = 'cold'
+    RETYPE = 'retype'
+    MIGRATE = 'migrate'
     DESTINATION_NODE = "destination_node"
     DESTINATION_TYPE = "destination_type"
 
@@ -85,7 +89,7 @@ class VolumeMigrate(base.BaseAction):
                 },
                 'migration_type': {
                     'type': 'string',
-                    "enum": ["swap", "cold"]
+                    "enum": ["swap", "retype", "migrate"]
                 },
                 'destination_node': {
                     "anyof": [
@@ -127,20 +131,6 @@ class VolumeMigrate(base.BaseAction):
     def destination_type(self):
         return self.input_parameters.get(self.DESTINATION_TYPE)
 
-    def _cold_migrate(self, volume, dest_node, dest_type):
-        if not self.cinder_util.can_cold(volume, dest_node):
-            raise exception.Invalid(
-                message=(_("Invalid state for cold migration")))
-
-        if dest_node:
-            return self.cinder_util.migrate(volume, dest_node)
-        elif dest_type:
-            return self.cinder_util.retype(volume, dest_type)
-        else:
-            raise exception.Invalid(
-                message=(_("destination host or destination type is "
-                           "required when migration type is cold")))
-
     def _can_swap(self, volume):
         """Judge volume can be swapped"""
 
@@ -212,12 +202,14 @@ class VolumeMigrate(base.BaseAction):
 
         try:
             volume = self.cinder_util.get_volume(volume_id)
-            if self.migration_type == self.COLD:
+            if self.migration_type == self.SWAP:
-                return self._cold_migrate(volume, dest_node, dest_type)
-            elif self.migration_type == self.SWAP:
                 if dest_node:
                     LOG.warning("dest_node is ignored")
                 return self._swap_volume(volume, dest_type)
+            elif self.migration_type == self.RETYPE:
+                return self.cinder_util.retype(volume, dest_type)
+            elif self.migration_type == self.MIGRATE:
+                return self.cinder_util.migrate(volume, dest_node)
             else:
                 raise exception.Invalid(
                     message=(_("Migration of type '%(migration_type)s' is not "

watcher/common/cinder_helper.py

@@ -109,23 +109,6 @@ class CinderHelper(object):
             return True
         return False
 
-    def can_cold(self, volume, host=None):
-        """Judge volume can be migrated"""
-        can_cold = False
-        status = self.get_volume(volume).status
-        snapshot = self._has_snapshot(volume)
-
-        same_host = False
-        if host and getattr(volume, 'os-vol-host-attr:host') == host:
-            same_host = True
-
-        if (status == 'available' and
-                snapshot is False and
-                same_host is False):
-            can_cold = True
-
-        return can_cold
-
     def get_deleting_volume(self, volume):
         volume = self.get_volume(volume)
         all_volume = self.get_volume_list()

watcher/tests/applier/actions/test_volume_migration.py

@@ -84,7 +84,7 @@ class TestMigration(base.TestCase):
         self.action_swap.input_parameters = self.input_parameters_swap
 
         self.input_parameters_migrate = {
-            "migration_type": "cold",
+            "migration_type": "migrate",
             "destination_node": "storage1-poolname",
             "destination_type": "",
             baction.BaseAction.RESOURCE_ID: self.VOLUME_UUID,
@@ -93,7 +93,7 @@ class TestMigration(base.TestCase):
         self.action_migrate.input_parameters = self.input_parameters_migrate
 
         self.input_parameters_retype = {
-            "migration_type": "cold",
+            "migration_type": "retype",
             "destination_node": "",
             "destination_type": "storage1-typename",
             baction.BaseAction.RESOURCE_ID: self.VOLUME_UUID,
@@ -130,7 +130,7 @@ class TestMigration(base.TestCase):
     def test_parameters_migrate(self):
         params = {baction.BaseAction.RESOURCE_ID:
                   self.VOLUME_UUID,
-                  self.action.MIGRATION_TYPE: 'cold',
+                  self.action.MIGRATION_TYPE: 'migrate',
                   self.action.DESTINATION_NODE: 'node-1',
                   self.action.DESTINATION_TYPE: None}
         self.action_migrate.input_parameters = params
@@ -139,7 +139,7 @@ class TestMigration(base.TestCase):
     def test_parameters_retype(self):
         params = {baction.BaseAction.RESOURCE_ID:
                   self.VOLUME_UUID,
-                  self.action.MIGRATION_TYPE: 'cold',
+                  self.action.MIGRATION_TYPE: 'retype',
                   self.action.DESTINATION_NODE: None,
                   self.action.DESTINATION_TYPE: 'type-1'}
         self.action_retype.input_parameters = params
@@ -157,7 +157,6 @@ class TestMigration(base.TestCase):
     def test_migrate_success(self):
         volume = self.fake_volume()
 
-        self.m_c_helper.can_cold.return_value = True
         self.m_c_helper.get_volume.return_value = volume
         result = self.action_migrate.execute()
         self.assertTrue(result)
@@ -166,16 +165,9 @@ class TestMigration(base.TestCase):
             "storage1-poolname"
         )
 
-    def test_migrate_fail(self):
-        self.m_c_helper.can_cold.return_value = False
-        result = self.action_migrate.execute()
-        self.assertFalse(result)
-        self.m_c_helper.migrate.assert_not_called()
-
     def test_retype_success(self):
         volume = self.fake_volume()
 
-        self.m_c_helper.can_cold.return_value = True
         self.m_c_helper.get_volume.return_value = volume
         result = self.action_retype.execute()
         self.assertTrue(result)
@@ -184,12 +176,6 @@ class TestMigration(base.TestCase):
             "storage1-typename",
         )
 
-    def test_retype_fail(self):
-        self.m_c_helper.can_cold.return_value = False
-        result = self.action_migrate.execute()
-        self.assertFalse(result)
-        self.m_c_helper.migrate.assert_not_called()
-
     def test_swap_success(self):
         volume = self.fake_volume(
             status='in-use', attachments=[{'server_id': 'server_id'}])

watcher/tests/common/test_cinder_helper.py

@@ -136,33 +136,6 @@ class TestCinderHelper(base.TestCase):
         volume.volume_type = kwargs.get('volume_type', 'fake_type')
         return volume
 
-    def test_can_cold_success(self, mock_cinder):
-        cinder_util = cinder_helper.CinderHelper()
-
-        volume = self.fake_volume()
-        cinder_util.cinder.volumes.get.return_value = volume
-        result = cinder_util.can_cold(volume)
-        self.assertTrue(result)
-
-    def test_can_cold_fail(self, mock_cinder):
-        cinder_util = cinder_helper.CinderHelper()
-
-        volume = self.fake_volume(status='in-use')
-        cinder_util.cinder.volumes.get.return_value = volume
-        result = cinder_util.can_cold(volume)
-        self.assertFalse(result)
-
-        volume = self.fake_volume(snapshot_id='snapshot_id')
-        cinder_util.cinder.volumes.get.return_value = volume
-        result = cinder_util.can_cold(volume)
-        self.assertFalse(result)
-
-        volume = self.fake_volume()
-        setattr(volume, 'os-vol-host-attr:host', 'host@backend#pool')
-        cinder_util.cinder.volumes.get.return_value = volume
-        result = cinder_util.can_cold(volume, 'host@backend#pool')
-        self.assertFalse(result)
-
     @mock.patch.object(time, 'sleep', mock.Mock())
     def test_migrate_success(self, mock_cinder):