From 4ef7b10e0f6e9753a7d65de15ca156f7ea6f036d Mon Sep 17 00:00:00 2001
From: Tim Burke <tim.burke@gmail.com>
Date: Thu, 18 Mar 2021 12:26:08 -0700
Subject: [PATCH] relinker: Tolerate missing files when cleaning up partitions

Co-Authored-By: Clay Gerrard <clay.gerrard@gmail.com>
Change-Id: I1ec6d8c62ca47c2a039672331551a62c7fdab5cc
---
 swift/cli/relinker.py          |  6 ++++-
 test/unit/cli/test_relinker.py | 48 ++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/swift/cli/relinker.py b/swift/cli/relinker.py
index dab2b21ecd..f582d4352c 100644
--- a/swift/cli/relinker.py
+++ b/swift/cli/relinker.py
@@ -216,7 +216,11 @@ class Relinker(object):
                     # get_hashes
                     try:
                         for f in ('hashes.pkl', 'hashes.invalid', '.lock'):
-                            os.unlink(os.path.join(partition_path, f))
+                            try:
+                                os.unlink(os.path.join(partition_path, f))
+                            except OSError as e:
+                                if e.errno != errno.ENOENT:
+                                    raise
                     except OSError:
                         pass
                 try:
diff --git a/test/unit/cli/test_relinker.py b/test/unit/cli/test_relinker.py
index 3bd68aa13a..4a948112e8 100644
--- a/test/unit/cli/test_relinker.py
+++ b/test/unit/cli/test_relinker.py
@@ -1594,6 +1594,54 @@ class TestRelinker(unittest.TestCase):
         self.assertIn('1 hash dirs processed (cleanup=True) '
                       '(0 files, 0 linked, 1 removed, 0 errors)', info_lines)
 
+    def test_cleanup_old_part_careful(self):
+        self._common_test_cleanup()
+        # make some extra junk file in the part
+        extra_file = os.path.join(self.part_dir, 'extra')
+        with open(extra_file, 'w'):
+            pass
+        self.assertEqual(0, relinker.main([
+            'cleanup',
+            '--swift-dir', self.testdir,
+            '--devices', self.devices,
+            '--skip-mount',
+        ]))
+        # old partition can't be cleaned up
+        self.assertTrue(os.path.exists(self.part_dir))
+
+    def test_cleanup_old_part_robust(self):
+        self._common_test_cleanup()
+
+        orig_get_hashes = DiskFileManager.get_hashes
+        calls = []
+
+        def mock_get_hashes(mgr, device, part, suffixes, policy):
+            orig_resp = orig_get_hashes(mgr, device, part, suffixes, policy)
+            if part == self.part:
+                expected_files = ['.lock', 'hashes.pkl', 'hashes.invalid']
+                self.assertEqual(set(expected_files),
+                                 set(os.listdir(self.part_dir)))
+                # unlink a random file, should be empty
+                os.unlink(os.path.join(self.part_dir, 'hashes.pkl'))
+                calls.append(True)
+            elif part == self.next_part:
+                # sometimes our random obj needs to rehash the next part too
+                pass
+            else:
+                self.fail('Unexpected call to get_hashes for %r' % part)
+            return orig_resp
+
+        with mock.patch.object(DiskFileManager, 'get_hashes', mock_get_hashes):
+            self.assertEqual(0, relinker.main([
+                'cleanup',
+                '--swift-dir', self.testdir,
+                '--devices', self.devices,
+                '--skip-mount',
+            ]))
+        self.assertEqual([True], calls)
+        # old partition can still be cleaned up
+        self.assertFalse(os.path.exists(self.part_dir))
+
     def test_cleanup_reapable(self):
         # relink a tombstone
         fname_ts = self.objname[:-4] + "ts"