From 7a7c182e35fd2f1142903a22c0c599c9425ae807 Mon Sep 17 00:00:00 2001
From: wlhc <1216083447@qq.com>
Date: Wed, 14 Dec 2016 21:05:49 -0500
Subject: [PATCH] Fix ``exportfs -u`` usage in generic driver.

When the generic driver extends or shrinks a share it disables access
to the share by removing its export.  Currently it uses the command
``exportfs -u local_path`` to do so, but this fails with current nfs
packages.

Fix the error by calling ``exportfs -u host:local_path`` instead.

Change-Id: Ic489a1607bf82964bf2859e89b3da1f572436d17
Closes-Bug: #1649782
---
 manila/share/drivers/helpers.py               |  6 ++-
 manila/tests/share/drivers/test_helpers.py    | 45 +++++++++++++++----
 ...782-fixed-incorrect-exportfs-exportfs.yaml |  4 ++
 3 files changed, 46 insertions(+), 9 deletions(-)
 create mode 100644 releasenotes/notes/bug-1649782-fixed-incorrect-exportfs-exportfs.yaml

diff --git a/manila/share/drivers/helpers.py b/manila/share/drivers/helpers.py
index c57955dfcd..769ffe804f 100644
--- a/manila/share/drivers/helpers.py
+++ b/manila/share/drivers/helpers.py
@@ -327,7 +327,11 @@ class NFSHelper(NASHelperBase):
 
         local_path = os.path.join(self.configuration.share_mount_path,
                                   share_name)
-        self._ssh_exec(server, ['sudo', 'exportfs', '-u', local_path])
+        out, err = self._ssh_exec(server, ['sudo', 'exportfs'])
+        hosts = self.get_host_list(out, local_path)
+        for host in hosts:
+            self._ssh_exec(server, ['sudo', 'exportfs', '-u',
+                                    ':'.join((host, local_path))])
         self._sync_nfs_temp_and_perm_files(server)
 
     @nfs_synchronized
diff --git a/manila/tests/share/drivers/test_helpers.py b/manila/tests/share/drivers/test_helpers.py
index 0abeed7e6c..74f1dae314 100644
--- a/manila/tests/share/drivers/test_helpers.py
+++ b/manila/tests/share/drivers/test_helpers.py
@@ -247,11 +247,32 @@ class NFSHelperTestCase(test.TestCase):
 
         self.assertEqual('/foo/bar', result)
 
-    def test_disable_access_for_maintenance(self):
+    @ddt.data(
+        ('/shares/fake_share1\n\t\t1.1.1.10\n'
+         '/shares/fake_share2\n\t\t1.1.1.16\n'
+         '/mnt/fake_share1 1.1.1.11', False),
+        ('/shares/fake_share_name\n\t\t1.1.1.10\n'
+         '/shares/fake_share_name\n\t\t1.1.1.16\n'
+         '/mnt/fake_share1\n\t\t1.1.1.11', True),
+        ('/mnt/fake_share_name\n\t\t1.1.1.11\n'
+         '/shares/fake_share_name\n\t\t1.1.1.10\n'
+         '/shares/fake_share_name\n\t\t1.1.1.16\n', True))
+    @ddt.unpack
+    def test_disable_access_for_maintenance(self, output, hosts_match):
         fake_maintenance_path = "fake.path"
-        share_mount_path = os.path.join(
-            self._helper.configuration.share_mount_path, self.share_name)
-        self.mock_object(self._helper, '_ssh_exec')
+        self._helper.configuration.share_mount_path = '/shares'
+        local_path = os.path.join(self._helper.configuration.share_mount_path,
+                                  self.share_name)
+
+        def fake_ssh_exec(*args, **kwargs):
+            if 'exportfs' in args[1] and '-u' not in args[1]:
+                return output, ''
+            else:
+                return '', ''
+
+        self.mock_object(self._helper, '_ssh_exec',
+                         mock.Mock(side_effect=fake_ssh_exec))
+
         self.mock_object(self._helper, '_sync_nfs_temp_and_perm_files')
         self.mock_object(self._helper, '_get_maintenance_file_path',
                          mock.Mock(return_value=fake_maintenance_path))
@@ -265,10 +286,18 @@ class NFSHelperTestCase(test.TestCase):
              '|', 'grep', self.share_name,
              '|', 'sudo', 'tee', fake_maintenance_path]
         )
-        self._helper._ssh_exec.assert_any_call(
-            self.server,
-            ['sudo', 'exportfs', '-u', share_mount_path]
-        )
+        self._helper._ssh_exec.assert_has_calls([
+            mock.call(self.server, ['sudo', 'exportfs']),
+        ])
+
+        if hosts_match:
+            self._helper._ssh_exec.assert_has_calls([
+                mock.call(self.server, ['sudo', 'exportfs', '-u',
+                                        ':'.join(['1.1.1.10', local_path])]),
+                mock.call(self.server, ['sudo', 'exportfs', '-u',
+                                        ':'.join(['1.1.1.16', local_path])]),
+            ])
+
         self._helper._sync_nfs_temp_and_perm_files.assert_called_once_with(
             self.server
         )
diff --git a/releasenotes/notes/bug-1649782-fixed-incorrect-exportfs-exportfs.yaml b/releasenotes/notes/bug-1649782-fixed-incorrect-exportfs-exportfs.yaml
new file mode 100644
index 0000000000..e418117723
--- /dev/null
+++ b/releasenotes/notes/bug-1649782-fixed-incorrect-exportfs-exportfs.yaml
@@ -0,0 +1,4 @@
+---
+fixes:
+  - Fixed incorrect exportfs command used while extending and shrinking
+    shares on Generic driver.