Merge lp:~vila/uci-engine/makedirs-races into lp:uci-engine

[Vincent Ladeuil]

Fix other call sites where creating needed dirs may be racy

Since I had a brief look at where other call sites (including the one I
copied the if path.exists(): makesdirs() ;) I thought it was faster to fix
them to address the second part of the review remark: "I worry it getting
copied elsewhere" ;)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:829
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1537/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1537/rebuild

review: Approve (continuous-integration)

[Paul Larson (pwlars)]

Looks fine, the only thing I would say is that since this appears to be such a common problem, we should probably just make this a helper function in ci_utils and call it from there. safe_make_dirs() or something.

review: Approve

[Evan (ev)]

Way to go the extra mile on that previous MP. Agree with Paul that this could probably be a utility function.

review: Approve

[Ubuntu CI Bot (uci-bot)]

The attempt to merge lp:~vila/uci-engine/makedirs-races into lp:uci-engine failed. Below is the output from the failed tests.

ERROR:root:/var/lib/lxc not mounted.

=== modified file 'cli/ci_cli/image.py'
--- cli/ci_cli/image.py	2014-08-29 18:28:56 +0000
+++ cli/ci_cli/image.py	2014-10-06 13:12:24 +0000
@@ -12,6 +12,8 @@
 #
 # You should have received a copy of the GNU General Public License along
 # with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import errno
 import logging
 import os
 import requests
@@ -51,8 +53,14 @@
     if response.status_code != 200:
         raise ImageObjectNotFound('Failed to download {}'.format(temp_url))
     # Create destination path parts if needed.
-    if not os.path.exists(os.path.dirname(dest_path)):
-        os.makedirs(os.path.dirname(dest_path))
+    base_dir = os.path.dirname(dest_path)
+    try:
+        # Try to create needed dirs
+        os.makedirs(base_dir)
+    except OSError as e:
+        # They are already there, no worries
+        if e.errno != errno.EEXIST:
+            raise
     with open(dest_path, 'wb') as fp:
         for chunk in response.iter_content(chunk_size=4096):
             # Ignore keep-alives.

=== modified file 'lander/lander/__init__.py'
--- lander/lander/__init__.py	2014-08-08 21:31:03 +0000
+++ lander/lander/__init__.py	2014-10-06 13:12:24 +0000
@@ -13,6 +13,7 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+import errno
 import json
 import logging
 import os
@@ -31,8 +32,13 @@
     if directory is not None:
         log_dir = os.path.join(log_dir, directory)
 
-    if not os.path.exists(log_dir):
+    try:
+        # Try to create needed dirs
         os.makedirs(log_dir)
+    except OSError as e:
+        # They are already there, no worries
+        if e.errno != errno.EEXIST:
+            raise
 
     path = os.path.join(log_dir, filename)
 

=== modified file 'test_runner/tstrun/run_worker.py'
--- test_runner/tstrun/run_worker.py	2014-10-01 09:27:59 +0000
+++ test_runner/tstrun/run_worker.py	2014-10-06 13:12:24 +0000
@@ -16,6 +16,7 @@
 from __future__ import unicode_literals
 
 
+import errno
 import os
 
 
@@ -127,8 +128,13 @@
             conf.set('vm.release', series)
             conf.set('vm.ppas', ','.join(ppa_list))
             base_dir = conf.get('vm.vms_dir')
-            if not os.path.exists(base_dir):
+            try:
+                # Try to create needed dirs
                 os.makedirs(base_dir)
+            except OSError as e:
+                # They are already there, no worries
+                if e.errno != errno.EEXIST:
+                    raise
             conf.store.save_changes()
             self.test_bed = testbed.TestBed(conf)
         except:

=== modified file 'test_runner/tstrun/testbed.py'
--- test_runner/tstrun/testbed.py	2014-10-03 13:35:36 +0000
+++ test_runner/tstrun/testbed.py	2014-10-06 13:12:24 +0000
@@ -15,6 +15,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 from __future__ import unicode_literals
 
+import errno
 import logging
 import os
 import subprocess
@@ -248,8 +249,13 @@
         # provide this key.
         if not os.path.exists(self.test_bed_key_path):
             base_dir = os.path.dirname(self.test_bed_key_path)
-            if not os.path.exists(base_dir):
+            try:
+                # Try to create needed dirs
                 os.makedirs(base_dir)
+            except OSError as e:
+                # They are already there, no worries
+                if e.errno != errno.EEXIST:
+                    raise
             # First time the test runner instance needs to create a test bed
             # instance, we need to create the ssh key pair.
             subprocess.call(