Don't warn on lock removal if file doesn't exist

There may be cases were library users want to request the removal of an
external lock that may not have been created and don't want to pollute
the logs when the file is not present.

We raise the log level from info to warning, but we don't log it if the
file didn't exist, since the end result the caller wanted is there.

Change-Id: I5ce8be34c9f2c4c59ea99dabc6760c3300f743a3

2 files changed, 22 insertions, 5 deletions

diff --git a/oslo_concurrency/lockutils.py b/oslo_concurrency/lockutils.py
index 7fe049b..3236289 100644
--- a/oslo_concurrency/lockutils.py
+++ b/oslo_concurrency/lockutils.py
@@ -14,6 +14,7 @@
 #    under the License.
 
 import contextlib
+import errno
 import functools
 import logging
 import os
@@ -200,9 +201,10 @@ def remove_external_lock_file(name, lock_file_prefix=None, lock_path=None,
         lock_file_path = _get_lock_path(name, lock_file_prefix, lock_path)
         try:
             os.remove(lock_file_path)
-        except OSError:
-            LOG.info('Failed to remove file %(file)s',
-                     {'file': lock_file_path})
+        except OSError as exc:
+            if exc.errno != errno.ENOENT:
+                LOG.warning('Failed to remove file %(file)s',
+                            {'file': lock_file_path})
 
 
 def internal_lock(name, semaphores=None):
diff --git a/oslo_concurrency/tests/unit/test_lockutils.py b/oslo_concurrency/tests/unit/test_lockutils.py
index 1bd74a2..2aea215 100644
--- a/oslo_concurrency/tests/unit/test_lockutils.py
+++ b/oslo_concurrency/tests/unit/test_lockutils.py
@@ -13,6 +13,7 @@
 #    under the License.
 
 import collections
+import errno
 import multiprocessing
 import os
 import signal
@@ -327,8 +328,8 @@ class LockTestCase(test_base.BaseTestCase):
         remove_mock.assert_called_once_with(path_mock.return_value)
         log_mock.assert_not_called()
 
-    @mock.patch('logging.Logger.info')
-    @mock.patch('os.remove', side_effect=OSError)
+    @mock.patch('logging.Logger.warning')
+    @mock.patch('os.remove', side_effect=OSError(errno.ENOENT, None))
     @mock.patch('oslo_concurrency.lockutils._get_lock_path')
     def test_remove_lock_external_file_doesnt_exists(self, path_mock,
                                                      remove_mock, log_mock):
@@ -339,6 +340,20 @@ class LockTestCase(test_base.BaseTestCase):
                                           mock.sentinel.prefix,
                                           mock.sentinel.lock_path)
         remove_mock.assert_called_once_with(path_mock.return_value)
+        log_mock.assert_not_called()
+
+    @mock.patch('logging.Logger.warning')
+    @mock.patch('os.remove', side_effect=OSError(errno.EPERM, None))
+    @mock.patch('oslo_concurrency.lockutils._get_lock_path')
+    def test_remove_lock_external_file_permission_error(
+            self, path_mock, remove_mock, log_mock):
+        lockutils.remove_external_lock_file(mock.sentinel.name,
+                                            mock.sentinel.prefix,
+                                            mock.sentinel.lock_path)
+        path_mock.assert_called_once_with(mock.sentinel.name,
+                                          mock.sentinel.prefix,
+                                          mock.sentinel.lock_path)
+        remove_mock.assert_called_once_with(path_mock.return_value)
         log_mock.assert_called()
 
     def test_no_slash_in_b64(self):