Implement Share Migration Ocata improvements

- Added preserve-snapshots parameter
- Updated driver-assisted options descriptions

Change-Id: Id86b025798efa9956facd82dbb3fa24c55be16db
Implements: blueprint ocata-migration-improvements

manila_ui/api/manila.py

@@ -33,7 +33,7 @@ from openstack_dashboard.api import base
 LOG = logging.getLogger(__name__)
 
 MANILA_UI_USER_AGENT_REPR = "manila_ui_plugin_for_horizon"
-MANILA_VERSION = "2.26"  # requires manilaclient 1.12.0 or newer
+MANILA_VERSION = "2.29"  # requires manilaclient 1.12.0 or newer
 MANILA_SERVICE_TYPE = "sharev2"
 
 # API static values
@@ -139,14 +139,15 @@ def share_manage(request, service_host, protocol, export_path,
 
 
 def migration_start(request, share, dest_host, force_host_assisted_migration,
-                    writable, preserve_metadata, nondisruptive,
+                    writable, preserve_metadata, preserve_snapshots,
-                    new_share_network_id, new_share_type_id):
+                    nondisruptive, new_share_network_id, new_share_type_id):
     return manilaclient(request).shares.migration_start(
         share,
         host=dest_host,
         force_host_assisted_migration=force_host_assisted_migration,
         writable=writable,
         preserve_metadata=preserve_metadata,
+        preserve_snapshots=preserve_snapshots,
         nondisruptive=nondisruptive,
         new_share_network_id=new_share_network_id,
         new_share_type_id=new_share_type_id
@@ -428,6 +429,10 @@ def availability_zone_list(request):
     return manilaclient(request).availability_zones.list()
 
 
+def pool_list(request, detailed=False):
+    return manilaclient(request).pools.list(detailed=detailed)
+
+
 @memoized
 def is_replication_enabled():
     manila_config = getattr(settings, 'OPENSTACK_MANILA_FEATURES', {})

manila_ui/dashboards/admin/shares/forms.py

@@ -40,56 +40,63 @@ ST_EXTRA_SPECS_FORM_ATTRS = {
 
 class MigrationStart(forms.SelfHandlingForm):
     name = forms.CharField(
-        label=_("Share Name"), required=False,
+        label=_("Share Name"),
         widget=forms.TextInput(attrs={'readonly': 'readonly'}))
     share_id = forms.CharField(
-        label=_("ID"), required=False,
+        label=_("ID"),
         widget=forms.TextInput(attrs={'readonly': 'readonly'}))
-    host = forms.CharField(
+    host = forms.ChoiceField(
-        max_length=255, label=_("Host to migrate share"),
+        label=_("Host to migrate share"),
-        help_text=_("Destination host where share will be migrated to. Use the"
+        help_text=_("Destination host and pool where share will be migrated "
-                    " format 'host@backend#pool'."))
+                    "to."))
     force_host_assisted_migration = forms.BooleanField(
         label=_("Force Host Assisted Migration"),
         required=False, initial=False,
-        help_text=("Defines whether the migration of this share should skip "
+        help_text=_("Enforces the use of the host-assisted migration approach,"
-                   "the attempt to be assisted by a storage vendor backend. "
+                    " which bypasses driver optimizations."))
-                   "This will force the use of the Data Service to perform "
-                   "migration."))
     nondisruptive = forms.BooleanField(
-        label=_("Non-disruptive"),
+        label=_("Nondisruptive"),
-        required=False, initial=False,
+        required=False, initial=True,
-        help_text=("Defines whether the migration of this share should be "
+        help_text=_("Enforces migration to be nondisruptive. If set to True, "
-                   "performed only if it is non-disruptive If set so, this "
+                    "host-assisted migration will not be attempted."))
-                   "will prevent the use of the Data Service for migration."))
     writable = forms.BooleanField(
         label=_("Writable"), required=False, initial=True,
-        help_text=("Defines whether this share should remain writable during "
+        help_text=_("Enforces migration to keep the share writable while "
-                   "migration. If set so, this will prevent the use of the "
+                    "contents are being moved. If set to True, host-assisted "
-                   "Data Service for migration."))
+                    "migration will not be attempted."))
     preserve_metadata = forms.BooleanField(
         label=_("Preserve Metadata"), required=False, initial=True,
-        help_text=("Defines whether this share should have all its file "
+        help_text=_("Enforces migration to preserve all file metadata when "
-                   "metadata preserved during migration. If set so, this will "
+                    "moving its contents. If set to True, host-assisted "
-                   "prevent the use of the Data Service for migration."))
+                    "migration will not be attempted."))
+    preserve_snapshots = forms.BooleanField(
+        label=_("Preserve Snapshots"), required=False, initial=True,
+        help_text=_("Enforces migration of the share snapshots to the "
+                    "destination. If set to True, host-assisted migration will"
+                    " not be attempted."))
     new_share_network = forms.ChoiceField(
         label=_("New share network to be set in migrated share"),
         required=False,
-        help_text=_("Input the new share network where the share should"
+        help_text=_('Specify the new share network for the share. Do not '
-                    " be migrated to if you would like to change it."))
+                    'specify this parameter if the migrating share has to be '
+                    'retained within its current share network.'))
     new_share_type = forms.ChoiceField(
         label=_("New share type to be set in migrating share"), required=False,
-        help_text=_("Input the new share type which the migrating share will "
+        help_text=_('Specify the new share type for the share. Do not specify '
-                    "be set to if you would like to change the type."))
+                    'this parameter if the migrating share has to be retained '
+                    'with its current share type.'))
 
     def __init__(self, request, *args, **kwargs):
         super(MigrationStart, self).__init__(request, *args, **kwargs)
         share_networks = manila.share_network_list(request)
         share_types = manila.share_type_list(request)
+        dests = manila.pool_list(request)
+        dest_choices = [('', '')] + [(d.name, d.name) for d in dests]
         st_choices = [('', '')] + [(st.id, st.name) for st in share_types]
         sn_choices = (
             [('', '')] +
             [(sn.id, sn.name or sn.id) for sn in share_networks])
+        self.fields['host'].choices = dest_choices
         self.fields['new_share_type'].choices = st_choices
         self.fields['new_share_network'].choices = sn_choices
 
@@ -102,6 +109,7 @@ class MigrationStart(forms.SelfHandlingForm):
                     data['force_host_assisted_migration']),
                 writable=data['writable'],
                 preserve_metadata=data['preserve_metadata'],
+                preserve_snapshots=data['preserve_snapshots'],
                 nondisruptive=data['nondisruptive'],
                 dest_host=data['host'],
                 new_share_network_id=data['new_share_network'],

manila_ui/tests/api/test_manila.py

@@ -201,15 +201,17 @@ class ManilaApiTests(base.APITestCase):
 
     def test_migration_start(self):
         api.migration_start(self.request, 'fake_share', 'fake_host', False,
-                            True, True, False, 'fake_net_id', 'fake_type_id')
+                            True, True, True, True, 'fake_net_id',
+                            'fake_type_id')
 
         self.manilaclient.shares.migration_start.assert_called_once_with(
             'fake_share',
             host='fake_host',
             force_host_assisted_migration=False,
-            nondisruptive=False,
+            nondisruptive=True,
             writable=True,
             preserve_metadata=True,
+            preserve_snapshots=True,
             new_share_network_id='fake_net_id',
             new_share_type_id='fake_type_id'
         )

manila_ui/tests/dashboards/admin/shares/test_forms.py

@@ -283,13 +283,21 @@ class ManilaDashboardsAdminMigrationFormTests(base.APITestCase):
             mock.Mock(return_value=[base.FakeEntity('st1_id', 'st1_name'),
                                     base.FakeEntity('st2_id', 'st2_name')]))
 
+        self.mock_object(
+            api, "pool_list",
+            mock.Mock(return_value=[base.FakeEntity('ubuntu@beta#BETA',
+                                                    'ubuntu@beta#BETA'),
+                                    base.FakeEntity('ubuntu@alpha#ALPHA',
+                                                    'ubuntu@alpha#ALPHA')]))
+
         form = forms.MigrationStart(self.request, **self._get_initial())
 
         data = {
             'force_host_assisted_migration': False,
             'writable': True,
             'preserve_metadata': True,
-            'nondisruptive': False,
+            'preserve_snapshots': True,
+            'nondisruptive': True,
             'new_share_network': 'fake_net_id',
             'new_share_type': 'fake_type_id',
             'host': 'fake_host',
@@ -302,6 +310,7 @@ class ManilaDashboardsAdminMigrationFormTests(base.APITestCase):
 
         api.share_network_list.assert_called_once_with(mock.ANY)
         api.share_type_list.assert_called_once_with(mock.ANY)
+        api.pool_list.assert_called_once_with(mock.ANY)
 
     @mock.patch('horizon.messages.success')
     def test_migration_complete(self, mock_horizon_messages_success):

manila_ui/tests/dashboards/admin/shares/tests.py

@@ -142,12 +142,19 @@ class SharesTests(test.BaseAdminViewTests):
 
         st_choices = [test.FakeEntity('st1_id', 'st1_name'),
                       test.FakeEntity('st2_id', 'st2_name')]
+
+        dest_choices = [
+            test.FakeEntity('ubuntu@beta#BETA', 'ubuntu@beta#BETA'),
+            test.FakeEntity('ubuntu@alpha#ALPHA', 'ubuntu@alpha#ALPHA')
+        ]
+
         formData = {
             'share_id': share.id,
             'name': share.name,
-            'host': 'fake_host',
+            'host': 'ubuntu@alpha#ALPHA',
             'writable': True,
             'preserve_metadata': True,
+            'preserve_snapshots': True,
             'force_host_assisted_migration': True,
             'nondisruptive': True,
             'new_share_network': 'sn2_id',
@@ -163,6 +170,9 @@ class SharesTests(test.BaseAdminViewTests):
         self.mock_object(
             api_manila, "share_type_list",
             mock.Mock(return_value=st_choices))
+        self.mock_object(
+            api_manila, "pool_list",
+            mock.Mock(return_value=dest_choices))
 
         res = self.client.post(url, formData)
 
@@ -174,11 +184,13 @@ class SharesTests(test.BaseAdminViewTests):
                 formData['force_host_assisted_migration']),
             writable=formData['writable'],
             preserve_metadata=formData['preserve_metadata'],
+            preserve_snapshots=formData['preserve_snapshots'],
             nondisruptive=formData['nondisruptive'],
             new_share_network_id=formData['new_share_network'],
             new_share_type_id=formData['new_share_type'])
         api_manila.share_network_list.assert_called_once_with(mock.ANY)
         api_manila.share_type_list.assert_called_once_with(mock.ANY)
+        api_manila.pool_list.assert_called_once_with(mock.ANY)
         self.assertEqual(302, res.status_code)
         self.assertTemplateNotUsed(res, 'admin/shares/migration_start.html')
         self.assertRedirectsNoFollow(res, INDEX_URL)
@@ -206,6 +218,14 @@ class SharesTests(test.BaseAdminViewTests):
                     return_value=[test.FakeEntity('st1_id', 'st1_name'),
                                   test.FakeEntity('st2_id', 'st2_name')]))
 
+            self.mock_object(
+                api_manila, "pool_list",
+                mock.Mock(return_value=[test.FakeEntity('ubuntu@beta#BETA',
+                                                        'ubuntu@beta#BETA'),
+                                        test.FakeEntity('ubuntu@alpha#ALPHA',
+                                                        'ubuntu@alpha#ALPHA')
+                                        ]))
+
         res = self.client.get(url)
 
         api_manila.share_get.assert_called_once_with(mock.ANY, share.id)
@@ -218,6 +238,7 @@ class SharesTests(test.BaseAdminViewTests):
         if method == 'migration_start':
             api_manila.share_network_list.assert_called_once_with(mock.ANY)
             api_manila.share_type_list.assert_called_once_with(mock.ANY)
+            api_manila.pool_list.assert_called_once_with(mock.ANY)
 
     @ddt.data('migration_start', 'migration_cancel', 'migration_complete',
               'migration_get_progress')

releasenotes/notes/bp-ocata-migration-improvements-e0ecccbe95e3c265.yaml

@@ -0,0 +1,6 @@
+---
+features:
+  - Added preserve-snapshots option to share migration.
+fixes:
+  - Updated descriptions of share migration parameters.
+