HNAS: Fix managed snapshots not being mounted

When managing snapshots, the driver is not checking if the export
exists on backend.

This patch fixes this issue by checking the export on backend. A new
export is created if required.

Change-Id: I2da32016ef99bb307a9e075de0472d56a4817e3c
Depends-On: Ie0ce3c4f2e7e29bf82f5c09a80a3fb132b0481a8
Closes-bug: #1660415

manila/share/drivers/hitachi/hnas/driver.py

@@ -1117,6 +1117,87 @@ class HitachiHNASDriver(driver.ShareDriver):
             export = r'\\%s\%s' % (ip, hnas_id)
         return export
 
+    def _ensure_snapshot(self, snapshot, hnas_snapshot_id):
+        """Ensure that snapshot is exported.
+
+        :param snapshot: Snapshot that will be checked.
+        :param hnas_snapshot_id: HNAS ID of snapshot that will be checked.
+
+        :returns: Returns a list of dicts containing the snapshot's export
+            locations or None if mount_snapshot_support is False.
+        """
+        self._check_protocol(snapshot['share_id'],
+                             snapshot['share']['share_proto'])
+        self._check_fs_mounted()
+
+        export_list = None
+        if snapshot['share'].get('mount_snapshot_support'):
+            if snapshot['share']['share_proto'].lower() == 'nfs':
+                self.hnas.check_export(hnas_snapshot_id, is_snapshot=True)
+            else:
+                self.hnas.check_cifs(hnas_snapshot_id)
+
+            export_list = self._get_export_locations(
+                snapshot['share']['share_proto'],
+                hnas_snapshot_id,
+                is_snapshot=True)
+
+        return export_list
+
+    def ensure_snapshot(self, context, snapshot, share_server=None):
+        """Ensure that snapshot is exported.
+
+        :param context: The `context.RequestContext` object for the request.
+        :param snapshot: Snapshot that will be checked.
+        :param share_server: Data structure with share server information.
+            Not used by this driver.
+
+        :returns: Returns a list of dicts containing the EVS IP concatenated
+            with the path of snapshot in the filesystem or None if
+            mount_snapshot_support is False.
+
+            Example for NFS::
+
+            [
+              {
+                'path': '172.24.44.10:/snapshots/id',
+                'metadata': {},
+                'is_admin_only': False
+              },
+              {
+                'path': '192.168.0.10:/snapshots/id',
+                'metadata': {},
+                'is_admin_only': True
+              }
+            ]
+
+            Example for CIFS::
+
+            [
+              {
+                'path': '\\172.24.44.10\id',
+                'metadata': {},
+                'is_admin_only': False
+              },
+              {
+                'path': '\\192.168.0.10\id',
+                'metadata': {},
+                'is_admin_only': True
+              }
+            ]
+        """
+        LOG.debug("Ensuring snapshot in HNAS: %(snap)s.",
+                  {'snap': snapshot['id']})
+
+        hnas_snapshot_id = self._get_hnas_snapshot_id(snapshot)
+
+        export_list = self._ensure_snapshot(snapshot, hnas_snapshot_id)
+
+        LOG.debug("Snapshot ensured in HNAS: %(snap)s, protocol %(proto)s.",
+                  {'snap': snapshot['id'],
+                   'proto': snapshot['share']['share_proto']})
+        return export_list
+
     def manage_existing_snapshot(self, snapshot, driver_options):
         """Manages a snapshot that exists only in HNAS.
 
@@ -1174,16 +1255,30 @@ class HitachiHNASDriver(driver.ShareDriver):
                     "HNAS.") % {'snap_id': hnas_snapshot_id}
             raise exception.ManageInvalidShareSnapshot(reason=msg)
 
+        try:
+            self._ensure_snapshot(snapshot, hnas_snapshot_id)
+        except exception.HNASItemNotFoundException:
+            LOG.warning(_LW("Export does not exist for snapshot %s, "
+                            "creating a new one."), snapshot['id'])
+            self._create_export(hnas_share_id,
+                                snapshot['share']['share_proto'],
+                                snapshot_id=hnas_snapshot_id)
+
+        output = {'size': snapshot_size}
+        if snapshot['share'].get('mount_snapshot_support'):
+            export_locations = self._get_export_locations(
+                snapshot['share']['share_proto'],
+                hnas_snapshot_id,
+                is_snapshot=True)
+            output['export_locations'] = export_locations
+
         LOG.info(_LI("Snapshot %(snap_path)s for share %(shr_id)s was "
                      "successfully managed with ID %(snap_id)s."),
                  {'snap_path': snapshot['provider_location'],
-                  'shr_id': snapshot['share_id'], 'snap_id': snapshot['id']})
+                  'shr_id': snapshot['share_id'],
+                  'snap_id': snapshot['id']})
 
-        export_locations = self._get_export_locations(
+        return output
-            snapshot['share']['share_proto'], hnas_snapshot_id,
-            is_snapshot=True)
-
-        return {'size': snapshot_size, 'export_locations': export_locations}
 
     def unmanage_snapshot(self, snapshot):
         """Unmanage a share snapshot

manila/share/drivers/hitachi/hnas/ssh.py

@@ -130,7 +130,7 @@ class HNASSSHBackend(object):
                 raise exception.HNASBackendException(msg=msg)
 
     def get_nfs_host_list(self, share_id):
-        export = self._get_share_export(share_id)
+        export = self._get_export(share_id)
         return export[0].export_configuration
 
     def update_nfs_access_rule(self, host_list, share_id=None,
@@ -400,8 +400,8 @@ class HNASSSHBackend(object):
             msg = (_("Virtual volume %s does not have any quota.") % vvol_name)
             raise exception.HNASItemNotFoundException(msg=msg)
 
-    def check_export(self, vvol_name):
+    def check_export(self, vvol_name, is_snapshot=False):
-        export = self._get_share_export(vvol_name)
+        export = self._get_export(vvol_name, is_snapshot=is_snapshot)
         if (vvol_name in export[0].export_name and
                 self.fs_name in export[0].file_system_label):
             return
@@ -475,17 +475,20 @@ class HNASSSHBackend(object):
                                                    quota.usage_unit)
             return bytes_usage / units.Gi
 
-    def _get_share_export(self, share_id):
+    def _get_export(self, name, is_snapshot=False):
-        share_id = '/shares/' + share_id
+        if is_snapshot:
+            name = '/snapshots/' + name
+        else:
+            name = '/shares/' + name
 
-        command = ['nfs-export', 'list ', share_id]
+        command = ['nfs-export', 'list ', name]
         export_list = []
         try:
             output, err = self._execute(command)
         except processutils.ProcessExecutionError as e:
             if 'does not exist' in e.stderr:
-                msg = _("Export %(share)s was not found in EVS "
+                msg = _("Export %(name)s was not found in EVS "
-                        "%(evs_id)s.") % {'share': share_id,
+                        "%(evs_id)s.") % {'name': name,
                                           'evs_id': self.evs_id}
                 raise exception.HNASItemNotFoundException(msg=msg)
             else:

manila/tests/share/drivers/hitachi/hnas/test_driver.py

@@ -65,6 +65,34 @@ share_invalid_host = {
                                   'aa4a7710-f326-41fb-ad18-b4ad587fc87a'}],
 }
 
+share_mount_support_nfs = {
+    'id': '62125744-fcdd-4f55-a8c1-d1498102f634',
+    'name': '62125744-fcdd-4f55-a8c1-d1498102f634',
+    'size': 50,
+    'host': 'hnas',
+    'share_proto': 'NFS',
+    'share_type_id': 1,
+    'share_network_id': 'bb329e24-3bdb-491d-acfd-dfe70c09b98d',
+    'share_server_id': 'cc345a53-491d-acfd-3bdb-dfe70c09b98d',
+    'export_locations': [{'path': '172.24.44.10:/shares/'
+                                  '62125744-fcdd-4f55-a8c1-d1498102f634'}],
+    'mount_snapshot_support': True,
+}
+
+share_mount_support_cifs = {
+    'id': 'd6e7dc6b-f65f-49d9-968d-936f75474f29',
+    'name': 'd6e7dc6b-f65f-49d9-968d-936f75474f29',
+    'size': 50,
+    'host': 'hnas',
+    'share_proto': 'CIFS',
+    'share_type_id': 1,
+    'share_network_id': 'bb329e24-3bdb-491d-acfd-dfe70c09b98d',
+    'share_server_id': 'cc345a53-491d-acfd-3bdb-dfe70c09b98d',
+    'export_locations': [{'path': '172.24.44.10:/shares/'
+                                  'd6e7dc6b-f65f-49d9-968d-936f75474f29'}],
+    'mount_snapshot_support': True,
+}
+
 access_nfs_rw = {
     'id': 'acdc7172b-fe07-46c4-b78f-df3e0324ccd0',
     'access_type': 'ip',
@@ -115,6 +143,22 @@ manage_snapshot = {
                          '/snapshot18-05-2106',
 }
 
+snapshot_mount_support_nfs = {
+    'id': '3377b015-a695-4a5a-8aa5-9b931b023380',
+    'share_id': '62125744-fcdd-4f55-a8c1-d1498102f634',
+    'share': share_mount_support_nfs,
+    'provider_location': '/snapshots/62125744-fcdd-4f55-a8c1-d1498102f634'
+                         '/3377b015-a695-4a5a-8aa5-9b931b023380',
+}
+
+snapshot_mount_support_cifs = {
+    'id': 'f9916515-5cb8-4612-afa6-7f2baa74223a',
+    'share_id': 'd6e7dc6b-f65f-49d9-968d-936f75474f29',
+    'share': share_mount_support_cifs,
+    'provider_location': '/snapshots/d6e7dc6b-f65f-49d9-968d-936f75474f29'
+                         '/f9916515-5cb8-4612-afa6-7f2baa74223a',
+}
+
 invalid_share = {
     'id': 'aa4a7710-f326-41fb-ad18-b4ad587fc87a',
     'name': 'aa4a7710-f326-41fb-ad18-b4ad587fc87a',
@@ -895,9 +939,42 @@ class HitachiHNASTestCase(test.TestCase):
          assert_called_once_with(fake_data))
         self.assertTrue(self.mock_log.info.called)
 
+    @ddt.data(snapshot_nfs, snapshot_cifs,
+              snapshot_mount_support_nfs, snapshot_mount_support_cifs)
+    def test_ensure_snapshot(self, snapshot):
+        result = self._driver.ensure_snapshot('context', snapshot)
+
+        if snapshot['share'].get('mount_snapshot_support'):
+            expected = [
+                self._get_export(
+                    snapshot['id'], snapshot['share']['share_proto'],
+                    self._driver.hnas_evs_ip, False, is_snapshot=True),
+                self._get_export(
+                    snapshot['id'], snapshot['share']['share_proto'],
+                    self._driver.hnas_admin_network_ip, True,
+                    is_snapshot=True)]
+
+            if snapshot['share']['share_proto'].lower() == 'nfs':
+                ssh.HNASSSHBackend.check_export.assert_called_once_with(
+                    snapshot['id'], is_snapshot=True)
+                self.assertFalse(ssh.HNASSSHBackend.check_cifs.called)
+            else:
+                ssh.HNASSSHBackend.check_cifs.assert_called_once_with(
+                    snapshot['id'])
+                self.assertFalse(ssh.HNASSSHBackend.check_export.called)
+        else:
+            expected = None
+
+        self.assertEqual(expected, result)
+
     def test_manage_existing_snapshot(self):
         self.mock_object(ssh.HNASSSHBackend, 'check_snapshot',
                          mock.Mock(return_value=True))
+        self.mock_object(self._driver, '_ensure_snapshot',
+                         mock.Mock(return_value=[]))
+
+        path_info = manage_snapshot['provider_location'].split('/')
+        hnas_snapshot_id = path_info[3]
 
         out = self._driver.manage_existing_snapshot(manage_snapshot,
                                                     {'size': 20})
@@ -905,10 +982,55 @@ class HitachiHNASTestCase(test.TestCase):
         ssh.HNASSSHBackend.check_snapshot.assert_called_with(
             '/snapshots/aa4a7710-f326-41fb-ad18-b4ad587fc87a'
             '/snapshot18-05-2106')
+        self._driver._ensure_snapshot.assert_called_with(
+            manage_snapshot,
+            hnas_snapshot_id)
         self.assertEqual(20, out['size'])
         self.assertTrue(self.mock_log.debug.called)
         self.assertTrue(self.mock_log.info.called)
 
+    @ddt.data(None, exception.HNASItemNotFoundException('Fake error.'))
+    def test_manage_existing_snapshot_with_mount_support(self, exc):
+        export_locations = [{
+            'path': '172.24.44.10:/snapshots/'
+                    '3377b015-a695-4a5a-8aa5-9b931b023380'}]
+
+        self.mock_object(ssh.HNASSSHBackend, 'check_snapshot',
+                         mock.Mock(return_value=True))
+        self.mock_object(self._driver, '_ensure_snapshot',
+                         mock.Mock(return_value=[], side_effect=exc))
+        self.mock_object(self._driver, '_get_export_locations',
+                         mock.Mock(return_value=export_locations))
+        if exc:
+            self.mock_object(self._driver, '_create_export')
+
+        path_info = snapshot_mount_support_nfs['provider_location'].split('/')
+        hnas_snapshot_id = path_info[3]
+
+        out = self._driver.manage_existing_snapshot(
+            snapshot_mount_support_nfs,
+            {'size': 20, 'export_locations': export_locations})
+
+        ssh.HNASSSHBackend.check_snapshot.assert_called_with(
+            '/snapshots/62125744-fcdd-4f55-a8c1-d1498102f634'
+            '/3377b015-a695-4a5a-8aa5-9b931b023380')
+        self._driver._ensure_snapshot.assert_called_with(
+            snapshot_mount_support_nfs,
+            hnas_snapshot_id)
+        self._driver._get_export_locations.assert_called_with(
+            snapshot_mount_support_nfs['share']['share_proto'],
+            hnas_snapshot_id,
+            is_snapshot=True)
+        if exc:
+            self._driver._create_export.assert_called_with(
+                snapshot_mount_support_nfs['share_id'],
+                snapshot_mount_support_nfs['share']['share_proto'],
+                snapshot_id=hnas_snapshot_id)
+        self.assertEqual(20, out['size'])
+        self.assertEqual(export_locations, out['export_locations'])
+        self.assertTrue(self.mock_log.debug.called)
+        self.assertTrue(self.mock_log.info.called)
+
     @ddt.data('fake_size', '128GB', '512 GB', {'size': 128})
     def test_manage_snapshot_invalid_size_exception(self, size):
         self.assertRaises(exception.ManageInvalidShareSnapshot,

manila/tests/share/drivers/hitachi/hnas/test_ssh.py

@@ -668,7 +668,7 @@ class HNASSSHTestCase(test.TestCase):
         self._driver_ssh._execute.assert_called_with(fake_cifs_del_command)
 
     def test_get_nfs_host_list(self):
-        self.mock_object(ssh.HNASSSHBackend, "_get_share_export", mock.Mock(
+        self.mock_object(ssh.HNASSSHBackend, "_get_export", mock.Mock(
             return_value=[ssh.Export(HNAS_RESULT_export)]))
 
         host_list = self._driver_ssh.get_nfs_host_list('fake_id')
@@ -1067,14 +1067,15 @@ class HNASSSHTestCase(test.TestCase):
                           self._driver_ssh.check_quota, 'vvol')
         self._driver_ssh._execute.assert_called_with(fake_check_quota_command)
 
-    def test_check_export(self):
+    @ddt.data(True, False)
-        self.mock_object(ssh.HNASSSHBackend, "_get_share_export", mock.Mock(
+    def test_check_export(self, is_snapshot):
+        self.mock_object(ssh.HNASSSHBackend, "_get_export", mock.Mock(
             return_value=[ssh.Export(HNAS_RESULT_export)]))
 
-        self._driver_ssh.check_export("vvol_test")
+        self._driver_ssh.check_export("vvol_test", is_snapshot)
 
     def test_check_export_error(self):
-        self.mock_object(ssh.HNASSSHBackend, "_get_share_export", mock.Mock(
+        self.mock_object(ssh.HNASSSHBackend, "_get_export", mock.Mock(
             return_value=[ssh.Export(HNAS_RESULT_wrong_export)]))
 
         self.assertRaises(exception.HNASItemNotFoundException,
@@ -1202,12 +1203,16 @@ class HNASSSHTestCase(test.TestCase):
 
         self.assertEqual(1024, self._driver_ssh.get_share_usage("vvol_test"))
 
-    def test__get_share_export(self):
+    @ddt.data(True, False)
+    def test__get_share_export(self, is_snapshot):
         self.mock_object(ssh.HNASSSHBackend, '_execute', mock.Mock(
             return_value=[HNAS_RESULT_export_ip, '']))
 
-        export_list = self._driver_ssh._get_share_export(share_id='share_id')
+        export_list = self._driver_ssh._get_export(
-        path = '/shares/share_id'
+            name='fake_name', is_snapshot=is_snapshot)
+        path = '/shares/fake_name'
+        if is_snapshot:
+            path = '/snapshots/fake_name'
 
         command = ['nfs-export', 'list ', path]
 
@@ -1225,7 +1230,7 @@ class HNASSSHTestCase(test.TestCase):
                 stderr="NFS Export List: Export 'id' does not exist.")))
 
         self.assertRaises(exception.HNASItemNotFoundException,
-                          self._driver_ssh._get_share_export, 'fake_id')
+                          self._driver_ssh._get_export, 'fake_id')
 
     def test__get_share_export_exception_error(self):
 
@@ -1234,7 +1239,7 @@ class HNASSSHTestCase(test.TestCase):
         ))
 
         self.assertRaises(putils.ProcessExecutionError,
-                          self._driver_ssh._get_share_export, 'fake_id')
+                          self._driver_ssh._get_export, 'fake_id')
 
     def test__execute(self):
         key = self.ssh_private_key

releasenotes/notes/fix-hnas-mount-on-manage-snapshot-91e094c579ddf1a3.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - Fixed Hitachi HNAS driver not checking export on backend
+    when managing a snapshot.