Merge pull request #45517 from kstreee/fix-mkdir

Fixes base dir making logic to ensure not raising the exception when base directory already exists.

salt/cache/localfs.py

@@ -14,6 +14,7 @@ from __future__ import absolute_import
 import logging
 import os
 import os.path
+import errno
 import shutil
 import tempfile
 
@@ -45,13 +46,14 @@ def store(bank, key, data, cachedir):
     Store information in a file.
     '''
     base = os.path.join(cachedir, os.path.normpath(bank))
-    if not os.path.isdir(base):
-        try:
-            os.makedirs(base)
-        except OSError as exc:
+    try:
+        os.makedirs(base)
+    except OSError as exc:
+        if exc.errno != errno.EEXIST:
             raise SaltCacheError(
-                'The cache directory, {0}, does not exist and could not be '
-                'created: {1}'.format(base, exc)
+                'The cache directory, {0}, could not be created: {1}'.format(
+                    base, exc
+                )
             )
 
     outfile = os.path.join(base, '{0}.p'.format(key))

salt/utils/gitfs.py

@@ -2531,13 +2531,17 @@ class GitFS(GitBase):
                 return fnd
 
             salt.fileserver.wait_lock(lk_fn, dest)
-            if os.path.isfile(blobshadest) and os.path.isfile(dest):
+            try:
                 with salt.utils.fopen(blobshadest, 'r') as fp_:
                     sha = fp_.read()
                     if sha == blob_hexsha:
                         fnd['rel'] = path
                         fnd['path'] = dest
                         return _add_file_stat(fnd, blob_mode)
+            except IOError as exc:
+                if exc.errno != errno.ENOENT:
+                    raise exc
+
             with salt.utils.fopen(lk_fn, 'w+') as fp_:
                 fp_.write('')
             for filename in glob.glob(hashes_glob):
@@ -2623,17 +2627,25 @@ class GitFS(GitBase):
                                         load['saltenv'],
                                         '{0}.hash.{1}'.format(relpath,
                                                               self.opts['hash_type']))
-        if not os.path.isfile(hashdest):
-            if not os.path.exists(os.path.dirname(hashdest)):
-                os.makedirs(os.path.dirname(hashdest))
-            ret['hsum'] = salt.utils.get_hash(path, self.opts['hash_type'])
-            with salt.utils.fopen(hashdest, 'w+') as fp_:
-                fp_.write(ret['hsum'])
-            return ret
-        else:
+
+        try:
             with salt.utils.fopen(hashdest, 'rb') as fp_:
                 ret['hsum'] = fp_.read()
             return ret
+        except IOError as exc:
+            if exc.errno != errno.ENOENT:
+                raise exc
+
+        try:
+            os.makedirs(os.path.dirname(hashdest))
+        except OSError as exc:
+            if exc.errno != errno.EEXIST:
+                raise exc
+
+        ret['hsum'] = salt.utils.get_hash(path, self.opts['hash_type'])
+        with salt.utils.fopen(hashdest, 'w+') as fp_:
+            fp_.write(ret['hsum'])
+        return ret
 
     def _file_lists(self, load, form):
         '''

tests/unit/cache/test_localfs.py

@@ -5,6 +5,7 @@ unit tests for the localfs cache
 
 # Import Python libs
 from __future__ import absolute_import
+import os
 import shutil
 import tempfile
 
@@ -48,16 +49,24 @@ class LocalFSTest(TestCase, LoaderModuleMockMixin):
             with patch.dict(localfs.__context__, {'serial': serializer}):
                 localfs.store(bank='bank', key='key', data='payload data', cachedir=tmp_dir)
 
-    # 'store' function tests: 4
+    # 'store' function tests: 5
 
-    def test_store_no_base_cache_dir(self):
+    def test_handled_exception_cache_dir(self):
         '''
         Tests that a SaltCacheError is raised when the base directory doesn't exist and
         cannot be created.
         '''
-        with patch('os.path.isdir', MagicMock(return_value=None)):
-            with patch('os.makedirs', MagicMock(side_effect=OSError)):
-                self.assertRaises(SaltCacheError, localfs.store, bank='', key='', data='', cachedir='')
+        with patch('os.makedirs', MagicMock(side_effect=OSError(os.errno.EEXIST, ''))):
+            with patch('tempfile.mkstemp', MagicMock(side_effect=Exception)):
+                self.assertRaises(Exception, localfs.store, bank='', key='', data='', cachedir='')
+
+    def test_unhandled_exception_cache_dir(self):
+        '''
+        Tests that a SaltCacheError is raised when the base directory doesn't exist and
+        cannot be created.
+        '''
+        with patch('os.makedirs', MagicMock(side_effect=OSError(1, ''))):
+            self.assertRaises(SaltCacheError, localfs.store, bank='', key='', data='', cachedir='')
 
     def test_store_close_mkstemp_file_handle(self):
         '''
@@ -67,7 +76,7 @@ class LocalFSTest(TestCase, LoaderModuleMockMixin):
         This test mocks the call to mkstemp, but forces an OSError to be raised when the
         close() function is called on a file descriptor that doesn't exist.
         '''
-        with patch('os.path.isdir', MagicMock(return_value=True)):
+        with patch('os.makedirs', MagicMock(side_effect=OSError(os.errno.EEXIST, ''))):
             with patch('tempfile.mkstemp', MagicMock(return_value=(12345, 'foo'))):
                 self.assertRaises(OSError, localfs.store, bank='', key='', data='', cachedir='')
 
@@ -76,7 +85,7 @@ class LocalFSTest(TestCase, LoaderModuleMockMixin):
         Tests that a SaltCacheError is raised when there is a problem writing to the
         cache file.
         '''
-        with patch('os.path.isdir', MagicMock(return_value=True)):
+        with patch('os.makedirs', MagicMock(side_effect=OSError(os.errno.EEXIST, ''))):
             with patch('tempfile.mkstemp', MagicMock(return_value=('one', 'two'))):
                 with patch('os.close', MagicMock(return_value=None)):
                     with patch('salt.utils.fopen', MagicMock(side_effect=IOError)):