From 75c3a8dee4a7f643d7654e2ea4641fd1bd18af62 Mon Sep 17 00:00:00 2001 From: Carl Baldwin Date: Wed, 3 Jun 2015 16:56:23 +0000 Subject: [PATCH] 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 --- neutron/agent/linux/utils.py | 9 ++++++++- neutron/tests/fullstack/fullstack_fixtures.py | 10 +--------- neutron/tests/unit/agent/linux/test_utils.py | 9 +++++++++ 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/neutron/agent/linux/utils.py b/neutron/agent/linux/utils.py index c38ed138489..dc22a0e069b 100644 --- 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 f714273aeea..e1959f86771 100644 --- 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 512f1bd7788..aa510f96de7 100644 --- 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):