From d86fb3834022cb2c5b0ef665a59ee9c886a8774e Mon Sep 17 00:00:00 2001
From: "Li, Chen" <chen.li@intel.com>
Date: Fri, 11 Sep 2015 14:09:51 +0800
Subject: [PATCH] Check the snapshot directory before copy it

1. HDFS do not support "-r" option when copy data
2. only un-empty directories can be copied

Change-Id: I525556842191be6ca273293457327d3d3480832d
Closes-Bug: 1494568
---
 manila/share/drivers/hdfs/hdfs_native.py      | 15 +++++--
 .../share/drivers/hdfs/test_hdfs_native.py    | 40 ++++++++++++++++---
 2 files changed, 46 insertions(+), 9 deletions(-)

diff --git a/manila/share/drivers/hdfs/hdfs_native.py b/manila/share/drivers/hdfs/hdfs_native.py
index d57717fe07..b075ac6f72 100644
--- a/manila/share/drivers/hdfs/hdfs_native.py
+++ b/manila/share/drivers/hdfs/hdfs_native.py
@@ -227,10 +227,19 @@ class HDFSNativeShareDriver(driver.ExecuteMixin, driver.ShareDriver):
         share_path = '/' + share['name']
         snapshot_path = self._get_snapshot_path(snapshot)
 
-        cmd = [self._hdfs_bin, 'dfs', '-cp', '-r',
-               snapshot_path, share_path]
         try:
-            self._hdfs_execute(*cmd)
+            # check if the directory is empty
+            (out, __) = self._hdfs_execute(
+                self._hdfs_bin, 'dfs', '-ls', snapshot_path)
+            # only copy files when the snapshot directory is not empty
+            if out:
+                copy_path = snapshot_path + "/*"
+
+                cmd = [self._hdfs_bin, 'dfs', '-cp',
+                       copy_path, share_path]
+
+                self._hdfs_execute(*cmd)
+
         except exception.ProcessExecutionError as e:
             msg = (_('Failed to create share %(sharename)s from '
                      'snapshot %(snapshotname)s. Error: %(excmsg)s.') %
diff --git a/manila/tests/share/drivers/hdfs/test_hdfs_native.py b/manila/tests/share/drivers/hdfs/test_hdfs_native.py
index 26cb3bd25d..d9dd188c04 100644
--- a/manila/tests/share/drivers/hdfs/test_hdfs_native.py
+++ b/manila/tests/share/drivers/hdfs/test_hdfs_native.py
@@ -111,8 +111,10 @@ class HDFSNativeShareDriverTestCase(test.TestCase):
         self._driver._hdfs_execute.assert_called_once_with(
             'fake_hdfs_bin', 'dfs', '-mkdir', share_dir)
 
-    def test_create_share_from_snapshot(self):
-        self._driver._hdfs_execute = mock.Mock(return_value=True)
+    def test_create_share_from_empty_snapshot(self):
+        return_hdfs_execute = (None, None)
+        self._driver._hdfs_execute = mock.Mock(
+            return_value=return_hdfs_execute)
         self._driver._create_share = mock.Mock(return_value=True)
         self._driver._get_share_path = mock.Mock(return_value=self.
                                                  fakesharepath)
@@ -126,8 +128,34 @@ class HDFSNativeShareDriverTestCase(test.TestCase):
         self._driver._get_snapshot_path.assert_called_once_with(
             self.snapshot)
         self._driver._hdfs_execute.assert_called_once_with(
-            'fake_hdfs_bin', 'dfs', '-cp', '-r',
-            self.fakesnapshotpath, '/' + self.share['name'])
+            'fake_hdfs_bin', 'dfs', '-ls', self.fakesnapshotpath)
+        self._driver._get_share_path.assert_called_once_with(self.share)
+        self.assertEqual(self.fakesharepath, result)
+
+    def test_create_share_from_snapshot(self):
+        return_hdfs_execute = ("fake_content", None)
+        self._driver._hdfs_execute = mock.Mock(
+            return_value=return_hdfs_execute)
+        self._driver._create_share = mock.Mock(return_value=True)
+        self._driver._get_share_path = mock.Mock(return_value=self.
+                                                 fakesharepath)
+        self._driver._get_snapshot_path = mock.Mock(return_value=self.
+                                                    fakesnapshotpath)
+        result = self._driver.create_share_from_snapshot(self._context,
+                                                         self.share,
+                                                         self.snapshot,
+                                                         share_server=None)
+        self._driver._create_share.assert_called_once_with(self.share)
+        self._driver._get_snapshot_path.assert_called_once_with(
+            self.snapshot)
+
+        calls = [mock.call('fake_hdfs_bin', 'dfs',
+                           '-ls', self.fakesnapshotpath),
+                 mock.call('fake_hdfs_bin', 'dfs', '-cp',
+                           self.fakesnapshotpath + '/*',
+                           '/' + self.share['name'])]
+
+        self._driver._hdfs_execute.assert_has_calls(calls)
         self._driver._get_share_path.assert_called_once_with(self.share)
         self.assertEqual(self.fakesharepath, result)
 
@@ -145,9 +173,9 @@ class HDFSNativeShareDriverTestCase(test.TestCase):
                           self.snapshot, share_server=None)
         self._driver._create_share.assert_called_once_with(self.share)
         self._driver._get_snapshot_path.assert_called_once_with(self.snapshot)
+
         self._driver._hdfs_execute.assert_called_once_with(
-            'fake_hdfs_bin', 'dfs', '-cp', '-r',
-            self.fakesnapshotpath, '/' + self.share['name'])
+            'fake_hdfs_bin', 'dfs', '-ls', self.fakesnapshotpath)
         self.assertFalse(self._driver._get_share_path.called)
 
     def test_create_snapshot(self):