Enhance utils.ensure_dir to be resilient to concurrent workers

In rare cases, concurrent workers may attempt to ensure a directory
exists.  One may successfully create the directory while the other
gets an oserror that it already exists.  This patch detects the
problem and returns successfully in both cases.

Change-Id: I224be69168ede8a496a5f7d59b04b722f4de7192

diff --git a/neutron/agent/linux/utils.py b/neutron/agent/linux/utils.py
index c38ed138489cb116007b8fdbc398a6ffb5302536..dc22a0e069bc9c777442e0b67945eb1dc67a20b2 100644 (file)
--- a/neutron/agent/linux/utils.py
+++ b/neutron/agent/linux/utils.py
@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import errno
 import fcntl
 import glob
 import grp
@@ -191,7 +192,13 @@ def find_child_pids(pid):
 def ensure_dir(dir_path):
     """Ensure a directory with 755 permissions mode."""
     if not os.path.isdir(dir_path):
-        os.makedirs(dir_path, 0o755)
+        try:
+            os.makedirs(dir_path, 0o755)
+        except OSError as e:
+            # Make sure that the error was that the directory was created
+            # by a different (concurrent) worker. If not, raise the error.
+            if e.errno != errno.EEXIST:
+                raise
 
 
 def _get_conf_base(cfg_root, uuid, ensure_conf_dir):

diff --git a/neutron/tests/fullstack/fullstack_fixtures.py b/neutron/tests/fullstack/fullstack_fixtures.py
index f714273aeea3ecccd168357361da802d74017ea2..e1959f8677188603037ab066d26344f8c1030d42 100644 (file)
--- a/neutron/tests/fullstack/fullstack_fixtures.py
+++ b/neutron/tests/fullstack/fullstack_fixtures.py
@@ -13,7 +13,6 @@
 #    under the License.
 
 from distutils import spawn
-import errno
 import functools
 import os
 
@@ -51,14 +50,7 @@ class ProcessFixture(fixtures.Fixture):
     def start(self):
         fmt = self.process_name + "--%Y-%m-%d--%H%M%S.log"
         log_dir = os.path.join(DEFAULT_LOG_DIR, self.test_name)
-        if not os.path.exists(log_dir):
-            try:
-                os.makedirs(log_dir)
-            except OSError as e:
-                # Make sure that the error was that the directory was created
-                # by a different (concurrent) worker. If not, raise the error.
-                if e.errno != errno.EEXIST:
-                    raise
+        utils.ensure_dir(log_dir)
 
         cmd = [spawn.find_executable(self.exec_name),
                '--log-dir', log_dir,

diff --git a/neutron/tests/unit/agent/linux/test_utils.py b/neutron/tests/unit/agent/linux/test_utils.py
index 512f1bd7788011b71eed90348a3499913545e16b..aa510f96de7b3f49529d39cc61ca2c9f9f78edd1 100644 (file)
--- a/neutron/tests/unit/agent/linux/test_utils.py
+++ b/neutron/tests/unit/agent/linux/test_utils.py
@@ -12,6 +12,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import errno
 import mock
 import socket
 import testtools
@@ -281,6 +282,14 @@ class TestBaseOSUtils(base.BaseTestCase):
         getegid.assert_called_once_with()
         getgrgid.assert_called_once_with(self.EGID)
 
+    @mock.patch('os.makedirs')
+    @mock.patch('os.path.exists', return_value=False)
+    def test_ensure_dir_no_fail_if_exists(self, path_exists, makedirs):
+        error = OSError()
+        error.errno = errno.EEXIST
+        makedirs.side_effect = error
+        utils.ensure_dir("/etc/create/concurrently")
+
 
 class TestUnixDomainHttpConnection(base.BaseTestCase):
     def test_connect(self):