From ce39a50782f15980d7d1afdf66d930153c24a9ac Mon Sep 17 00:00:00 2001
From: Rodrigo Barbieri <rodrigo.barbieri@fit-tecnologia.org.br>
Date: Thu, 3 Nov 2016 17:08:54 -0200
Subject: [PATCH] Add Admin network support to HNAS driver

Adding Admin Network support to more easily integrate HNAS
EVS networks into the admin network to perform share migrations.

DocImpact

Implements: blueprint hnas-admin-network

Change-Id: I797595aec580e9ec2dcfdf88e965ae29988b1dee
---
 manila/share/drivers/hitachi/hnas/driver.py   | 255 ++++++++++++++----
 .../share/drivers/hitachi/hnas/test_driver.py |  60 ++++-
 ...p-admin-network-hnas-9b714736e521101e.yaml |   7 +
 3 files changed, 262 insertions(+), 60 deletions(-)
 create mode 100644 releasenotes/notes/bp-admin-network-hnas-9b714736e521101e.yaml

diff --git a/manila/share/drivers/hitachi/hnas/driver.py b/manila/share/drivers/hitachi/hnas/driver.py
index d899ecc29d..a024b56a5b 100644
--- a/manila/share/drivers/hitachi/hnas/driver.py
+++ b/manila/share/drivers/hitachi/hnas/driver.py
@@ -48,6 +48,8 @@ hitachi_hnas_opts = [
     cfg.StrOpt('hitachi_hnas_evs_ip',
                deprecated_name='hds_hnas_evs_ip',
                help="Specify IP for mounting shares."),
+    cfg.StrOpt('hitachi_hnas_admin_network_ip',
+               help="Specify IP for mounting shares in the Admin network."),
     cfg.StrOpt('hitachi_hnas_file_system_name',
                deprecated_name='hds_hnas_file_system_name',
                help="Specify file-system name for creating shares."),
@@ -89,6 +91,7 @@ class HitachiHNASDriver(driver.ShareDriver):
     1.0.0 - Initial Version.
     2.0.0 - Refactoring, bugfixes, implemented Share Shrink and Update Access.
     3.0.0 - New driver location, implemented support for CIFS protocol.
+    3.1.0 - Added admin network export location support.
     """
 
     def __init__(self, *args, **kwargs):
@@ -108,6 +111,8 @@ class HitachiHNASDriver(driver.ShareDriver):
         hnas_password = self.configuration.safe_get('hitachi_hnas_password')
         hnas_evs_id = self.configuration.safe_get('hitachi_hnas_evs_id')
         self.hnas_evs_ip = self.configuration.safe_get('hitachi_hnas_evs_ip')
+        self.hnas_admin_network_ip = self.configuration.safe_get(
+            'hitachi_hnas_admin_network_ip')
         self.fs_name = self.configuration.safe_get(
             'hitachi_hnas_file_system_name')
         self.cifs_snapshot = self.configuration.safe_get(
@@ -277,21 +282,53 @@ class HitachiHNASDriver(driver.ShareDriver):
         :param share: Share that will be created.
         :param share_server: Data structure with share server information.
             Not used by this driver.
-        :returns: Returns a path of EVS IP concatenate with the path
-            of share in the filesystem (e.g. ['172.24.44.10:/shares/id'] for
-            NFS and ['\\172.24.44.10\id'] for CIFS).
+        :returns: Returns a list of dicts containing the EVS IP concatenated
+            with the path of share in the filesystem.
+
+            Example for NFS::
+
+            [
+              {
+                'path': '172.24.44.10:/shares/id',
+                'metadata': {},
+                'is_admin_only': False
+              },
+              {
+                'path': '192.168.0.10:/shares/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("Creating share in HNAS: %(shr)s.",
-                  {'shr': share['id']})
+        LOG.debug("Creating share in HNAS: %(shr)s.", {'shr': share['id']})
 
         self._check_protocol(share['id'], share['share_proto'])
 
-        uri = self._create_share(share['id'], share['size'],
-                                 share['share_proto'])
+        export_list = self._create_share(share['id'], share['size'],
+                                         share['share_proto'])
 
-        LOG.debug("Share created successfully on path: %(uri)s.",
-                  {'uri': uri})
-        return uri
+        LOG.debug("Share %(share)s created successfully on path(s): "
+                  "%(paths)s.",
+                  {'paths': ', '.join([x['path'] for x in export_list]),
+                   'share': share['id']})
+
+        return export_list
 
     def delete_share(self, context, share, share_server=None):
         """Deletes share.
@@ -357,21 +394,53 @@ class HitachiHNASDriver(driver.ShareDriver):
             to new share.
         :param share_server: Data structure with share server information.
             Not used by this driver.
-        :returns: Returns a path of EVS IP concatenate with the path
-            of new share in the filesystem (e.g. ['172.24.44.10:/shares/id']).
+        :returns: Returns a list of dicts containing the EVS IP concatenated
+            with the path of share in the filesystem.
+
+            Example for NFS::
+
+            [
+              {
+                'path': '172.24.44.10:/shares/id',
+                'metadata': {},
+                'is_admin_only': False
+              },
+              {
+                'path': '192.168.0.10:/shares/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("Creating a new share from snapshot: %(ss_id)s.",
                   {'ss_id': snapshot['id']})
 
         hnas_src_share_id = self._get_hnas_share_id(snapshot['share_id'])
 
-        uri = self._create_share_from_snapshot(share, hnas_src_share_id,
-                                               snapshot)
+        export_list = self._create_share_from_snapshot(
+            share, hnas_src_share_id, snapshot)
 
-        LOG.debug("Share %(share)s created successfully on path: %(uri)s.",
-                  {'uri': uri,
+        LOG.debug("Share %(share)s created successfully on path(s): "
+                  "%(paths)s.",
+                  {'paths': ', '.join([x['path'] for x in export_list]),
                    'share': share['id']})
-        return uri
+        return export_list
 
     def ensure_share(self, context, share, share_server=None):
         """Ensure that share is exported.
@@ -380,17 +449,45 @@ class HitachiHNASDriver(driver.ShareDriver):
         :param share: Share that will be checked.
         :param share_server: Data structure with share server information.
             Not used by this driver.
-        :returns: Returns a list of EVS IP concatenated with the path
-            of share in the filesystem (e.g. ['172.24.44.10:/shares/id'] for
-            NFS and ['\\172.24.44.10\id'] for CIFS).
+        :returns: Returns a list of dicts containing the EVS IP concatenated
+            with the path of share in the filesystem.
+
+            Example for NFS::
+
+            [
+              {
+                'path': '172.24.44.10:/shares/id',
+                'metadata': {},
+                'is_admin_only': False
+              },
+              {
+                'path': '192.168.0.10:/shares/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 share in HNAS: %(shr)s.", {'shr': share['id']})
 
         hnas_share_id = self._get_hnas_share_id(share['id'])
 
-        export = self._ensure_share(share, hnas_share_id)
-
-        export_list = [export]
+        export_list = self._ensure_share(share, hnas_share_id)
 
         LOG.debug("Share ensured in HNAS: %(shr)s, protocol %(proto)s.",
                   {'shr': share['id'], 'proto': share['share_proto']})
@@ -459,8 +556,45 @@ class HitachiHNASDriver(driver.ShareDriver):
 
         :param share: Share that will be managed.
         :param driver_options: Empty dict or dict with 'volume_id' option.
-        :returns: Returns a dict with size of share managed
-            and its export location.
+        :returns: Returns a dict with size of the share managed and a list of
+            dicts containing its export locations.
+
+            Example for NFS::
+
+            {
+              'size': 10,
+              'export_locations': [
+                {
+                  'path': '172.24.44.10:/shares/id',
+                  'metadata': {},
+                  'is_admin_only': False
+                },
+                {
+                  'path': '192.168.0.10:/shares/id',
+                  'metadata': {},
+                  'is_admin_only': True
+                }
+              ]
+            }
+
+            Example for CIFS::
+
+            {
+              'size': 10,
+              'export_locations': [
+                {
+                  'path': '\\172.24.44.10\id',
+                  'metadata': {},
+                  'is_admin_only': False
+                },
+                {
+                  'path': '\\192.168.0.10\id',
+                  'metadata': {},
+                  'is_admin_only': True
+                }
+              ]
+            }
+
         """
         hnas_share_id = self._get_hnas_share_id(share['id'])
 
@@ -581,8 +715,9 @@ class HitachiHNASDriver(driver.ShareDriver):
         :param share_size: Size limit of share.
         :param share_proto: Protocol of share that will be created
             (NFS or CIFS)
-        :returns: Returns a path IP:/shares/share_id for NFS or \\IP\share_id
-            for CIFS if the export was created successfully.
+        :returns: Returns a list of dicts containing the new share's export
+            locations.
+
         """
         self._check_fs_mounted()
 
@@ -599,20 +734,21 @@ class HitachiHNASDriver(driver.ShareDriver):
                 self.hnas.nfs_export_add(share_id)
                 LOG.debug("NFS Export created to %(shr)s.",
                           {'shr': share_id})
-                uri = self.hnas_evs_ip + ":/shares/" + share_id
             else:
                 # Create CIFS share with vvol path
                 self.hnas.cifs_share_add(share_id)
                 LOG.debug("CIFS share created to %(shr)s.",
                           {'shr': share_id})
-                uri = r'\\%s\%s' % (self.hnas_evs_ip, share_id)
-            return uri
+
         except exception.HNASBackendException as e:
             with excutils.save_and_reraise_exception():
                 self.hnas.vvol_delete(share_id)
                 msg = six.text_type(e)
                 LOG.exception(msg)
 
+        export_list = self._get_export_locations(share_proto, share_id)
+        return export_list
+
     def _check_fs_mounted(self):
         mounted = self.hnas.check_fs_mounted()
         if not mounted:
@@ -624,24 +760,25 @@ class HitachiHNASDriver(driver.ShareDriver):
 
         :param share: Share that will be checked.
         :param hnas_share_id: HNAS ID of share that will be checked.
-        :returns: Returns a path IP:/shares/share_id for NFS or \\IP\share_id
-            for CIFS if the export is ok.
+        :returns: Returns a list of dicts containing the share's export
+            locations.
+
         """
         self._check_protocol(share['id'], share['share_proto'])
-
-        path = os.path.join('/shares', hnas_share_id)
-
         self._check_fs_mounted()
 
         self.hnas.check_vvol(hnas_share_id)
         self.hnas.check_quota(hnas_share_id)
+
         if share['share_proto'].lower() == 'nfs':
             self.hnas.check_export(hnas_share_id)
-            export = self.hnas_evs_ip + ":" + path
         else:
             self.hnas.check_cifs(hnas_share_id)
-            export = r'\\%s\%s' % (self.hnas_evs_ip, hnas_share_id)
-        return export
+
+        export_list = self._get_export_locations(
+            share['share_proto'], hnas_share_id)
+
+        return export_list
 
     def _shrink_share(self, hnas_share_id, share, new_size):
         """Shrinks a share to new size.
@@ -706,8 +843,8 @@ class HitachiHNASDriver(driver.ShareDriver):
 
         :param share: share that will be managed.
         :param hnas_share_id: HNAS ID of share that will be managed.
-        :returns: Returns a dict with size of share managed
-            and its export location.
+        :returns: Returns a dict with size of the share managed and a list of
+            dicts containing its export locations.
         """
         self._ensure_share(share, hnas_share_id)
 
@@ -718,12 +855,10 @@ class HitachiHNASDriver(driver.ShareDriver):
                    % share['id'])
             raise exception.ManageInvalidShare(reason=msg)
 
-        if share['share_proto'].lower() == 'nfs':
-            path = self.hnas_evs_ip + os.path.join(':/shares', hnas_share_id)
-        else:
-            path = r'\\%s\%s' % (self.hnas_evs_ip, hnas_share_id)
+        export_list = self._get_export_locations(
+            share['share_proto'], hnas_share_id)
 
-        return {'size': share_size, 'export_locations': [path]}
+        return {'size': share_size, 'export_locations': export_list}
 
     def _create_snapshot(self, hnas_share_id, snapshot):
         """Creates a snapshot of share.
@@ -789,7 +924,8 @@ class HitachiHNASDriver(driver.ShareDriver):
             taken.
         :param snapshot: a dict from snapshot that will be copied to
             new share.
-        :returns: Returns the path for new share.
+        :returns: Returns a list of dicts containing the new share's export
+            locations.
         """
         dest_path = os.path.join('/shares', share['id'])
         src_path = os.path.join('/snapshots', src_hnas_share_id,
@@ -815,11 +951,9 @@ class HitachiHNASDriver(driver.ShareDriver):
         try:
             if share['share_proto'].lower() == 'nfs':
                 self.hnas.nfs_export_add(share['id'])
-                uri = self.hnas_evs_ip + ":" + dest_path
+
             else:
                 self.hnas.cifs_share_add(share['id'])
-                uri = r'\\%s\%s' % (self.hnas_evs_ip, share['id'])
-            return uri
         except exception.HNASBackendException:
             with excutils.save_and_reraise_exception():
                 msg = _LE('Failed to create share %(share_id)s from snapshot '
@@ -828,6 +962,9 @@ class HitachiHNASDriver(driver.ShareDriver):
                                     'snap': snapshot['id']})
                 self.hnas.vvol_delete(share['id'])
 
+        return self._get_export_locations(
+            share['share_proto'], share['id'])
+
     def _check_protocol(self, share_id, protocol):
         if protocol.lower() not in ('nfs', 'cifs'):
             msg = _("Only NFS or CIFS protocol are currently supported. "
@@ -835,3 +972,23 @@ class HitachiHNASDriver(driver.ShareDriver):
                     "%(proto)s.") % {'share': share_id,
                                      'proto': protocol}
             raise exception.ShareBackendException(msg=msg)
+
+    def _get_export_locations(self, share_proto, hnas_share_id):
+        export_list = []
+        for ip in (self.hnas_evs_ip, self.hnas_admin_network_ip):
+            if ip:
+                path = self._get_export_path(ip, share_proto, hnas_share_id)
+                export_list.append({
+                    "path": path,
+                    "is_admin_only": ip == self.hnas_admin_network_ip,
+                    "metadata": {},
+                })
+        return export_list
+
+    def _get_export_path(self, ip, share_proto, hnas_share_id):
+        if share_proto.lower() == 'nfs':
+            path = os.path.join('/shares', hnas_share_id)
+            export = ':'.join((ip, path))
+        else:
+            export = r'\\%s\%s' % (ip, hnas_share_id)
+        return export
diff --git a/manila/tests/share/drivers/hitachi/hnas/test_driver.py b/manila/tests/share/drivers/hitachi/hnas/test_driver.py
index e06776901b..1f39188fcc 100644
--- a/manila/tests/share/drivers/hitachi/hnas/test_driver.py
+++ b/manila/tests/share/drivers/hitachi/hnas/test_driver.py
@@ -145,6 +145,7 @@ class HitachiHNASTestCase(test.TestCase):
         CONF.set_default('driver_handles_share_servers', False)
         CONF.hitachi_hnas_evs_id = '2'
         CONF.hitachi_hnas_evs_ip = '172.24.44.10'
+        CONF.hitachi_hnas_admin_network_ip = '10.20.30.40'
         CONF.hitachi_hnas_ip = '172.24.44.1'
         CONF.hitachi_hnas_ip_port = 'hitachi_hnas_ip_port'
         CONF.hitachi_hnas_user = 'hitachi_hnas_user'
@@ -334,6 +335,18 @@ class HitachiHNASTestCase(test.TestCase):
             share_cifs['id'])
         self.assertTrue(self.mock_log.debug.called)
 
+    def _get_export(self, share, ip, is_admin_only):
+        if share['share_proto'].lower() == 'nfs':
+            export = ':'.join((ip, '/shares/' + share['id']))
+        else:
+            export = r'\\%s\%s' % (ip, share['id'])
+
+        return {
+            "path": export,
+            "is_admin_only": is_admin_only,
+            "metadata": {},
+        }
+
     @ddt.data(share_nfs, share_cifs)
     def test_create_share(self, share):
         self.mock_object(driver.HitachiHNASDriver, "_check_fs_mounted",
@@ -350,18 +363,21 @@ class HitachiHNASTestCase(test.TestCase):
         ssh.HNASSSHBackend.vvol_create.assert_called_once_with(share['id'])
         ssh.HNASSSHBackend.quota_add.assert_called_once_with(share['id'],
                                                              share['size'])
+        expected = [
+            self._get_export(
+                share, self._driver.hnas_evs_ip, False),
+            self._get_export(
+                share, self._driver.hnas_admin_network_ip, True)]
+
         if share['share_proto'].lower() == 'nfs':
-            self.assertEqual(self._driver.hnas_evs_ip + ":/shares/" +
-                             share_nfs['id'], result)
             ssh.HNASSSHBackend.nfs_export_add.assert_called_once_with(
                 share_nfs['id'])
             self.assertFalse(ssh.HNASSSHBackend.cifs_share_add.called)
         else:
-            self.assertEqual("\\\\" + self._driver.hnas_evs_ip + "\\" +
-                             share_cifs['id'], result)
             ssh.HNASSSHBackend.cifs_share_add.assert_called_once_with(
                 share_cifs['id'])
             self.assertFalse(ssh.HNASSSHBackend.nfs_export_add.called)
+        self.assertEqual(expected, result)
 
     def test_create_share_export_error(self):
         self.mock_object(driver.HitachiHNASDriver, "_check_fs_mounted",
@@ -507,15 +523,20 @@ class HitachiHNASTestCase(test.TestCase):
         ssh.HNASSSHBackend.check_vvol.assert_called_once_with(share['id'])
         ssh.HNASSSHBackend.check_quota.assert_called_once_with(share['id'])
 
+        expected = [
+            self._get_export(
+                share, self._driver.hnas_evs_ip, False),
+            self._get_export(
+                share, self._driver.hnas_admin_network_ip, True)]
+
         if share['share_proto'].lower() == 'nfs':
-            self.assertEqual(['172.24.44.10:/shares/' + share['id']], result)
             ssh.HNASSSHBackend.check_export.assert_called_once_with(
                 share['id'])
             self.assertFalse(ssh.HNASSSHBackend.check_cifs.called)
         else:
-            self.assertEqual(['\\\\172.24.44.10\\' + share['id']], result)
             ssh.HNASSSHBackend.check_cifs.assert_called_once_with(share['id'])
             self.assertFalse(ssh.HNASSSHBackend.check_export.called)
+        self.assertEqual(expected, result)
 
     def test_ensure_share_invalid_protocol(self):
         ex = self.assertRaises(exception.ShareBackendException,
@@ -567,9 +588,15 @@ class HitachiHNASTestCase(test.TestCase):
 
     @ddt.data(share_nfs, share_cifs)
     def test_manage_existing(self, share):
+
+        expected_exports = [
+            self._get_export(
+                share, self._driver.hnas_evs_ip, False),
+            self._get_export(
+                share, self._driver.hnas_admin_network_ip, True)]
+
         expected_out = {'size': share['size'],
-                        'export_locations':
-                            [share['export_locations'][0]['path']]}
+                        'export_locations': expected_exports}
 
         self.mock_object(ssh.HNASSSHBackend, "get_share_quota", mock.Mock(
             return_value=share['size']))
@@ -661,17 +688,23 @@ class HitachiHNASTestCase(test.TestCase):
             '/snapshots/' + share['id'] + '/' + snapshot['id'],
             '/shares/' + share['id'])
 
+        expected = [
+            self._get_export(
+                share, self._driver.hnas_evs_ip, False),
+            self._get_export(
+                share, self._driver.hnas_admin_network_ip, True)]
+
         if share['share_proto'].lower() == 'nfs':
             ssh.HNASSSHBackend.nfs_export_add.assert_called_once_with(
                 share['id'])
-            self.assertEqual('172.24.44.10:/shares/' + share_nfs['id'], result)
             self.assertFalse(ssh.HNASSSHBackend.cifs_share_add.called)
         else:
             ssh.HNASSSHBackend.cifs_share_add.assert_called_once_with(
                 share['id'])
-            self.assertEqual('\\\\172.24.44.10\\' + share['id'], result)
             self.assertFalse(ssh.HNASSSHBackend.nfs_export_add.called)
 
+        self.assertEqual(expected, result)
+
     def test_create_share_from_snapshot_empty_snapshot(self):
         self.mock_object(driver.HitachiHNASDriver, "_check_fs_mounted",
                          mock.Mock())
@@ -683,8 +716,13 @@ class HitachiHNASTestCase(test.TestCase):
 
         result = self._driver.create_share_from_snapshot('context', share_nfs,
                                                          snapshot_nfs)
+        expected = [
+            self._get_export(
+                share_nfs, self._driver.hnas_evs_ip, False),
+            self._get_export(
+                share_nfs, self._driver.hnas_admin_network_ip, True)]
 
-        self.assertEqual('172.24.44.10:/shares/' + share_nfs['id'], result)
+        self.assertEqual(expected, result)
         self.assertTrue(self.mock_log.warning.called)
         ssh.HNASSSHBackend.vvol_create.assert_called_once_with(share_nfs['id'])
         ssh.HNASSSHBackend.quota_add.assert_called_once_with(share_nfs['id'],
diff --git a/releasenotes/notes/bp-admin-network-hnas-9b714736e521101e.yaml b/releasenotes/notes/bp-admin-network-hnas-9b714736e521101e.yaml
new file mode 100644
index 0000000000..94400283ea
--- /dev/null
+++ b/releasenotes/notes/bp-admin-network-hnas-9b714736e521101e.yaml
@@ -0,0 +1,7 @@
+---
+features:
+  - Added admin network support to Hitachi HNAS Driver
+upgrade:
+  - Added a new config option to Hitachi HNAS Driver to
+    allow configuration of Admin Network support.
+