From a967d472957598374937650da222c731d9e30704 Mon Sep 17 00:00:00 2001 From: Tim Burke Date: Fri, 19 Mar 2021 10:24:59 -0700 Subject: [PATCH] relinker: Accept policy names, too Change-Id: Icf1517bd930c74e9552b88250a7b4019e0ab413e --- swift/cli/relinker.py | 5 ++-- swift/common/storage_policy.py | 9 ++++++ test/unit/cli/test_relinker.py | 37 ++++++++++++++++++++----- test/unit/common/test_storage_policy.py | 19 +++++++++++++ 4 files changed, 60 insertions(+), 10 deletions(-) diff --git a/swift/cli/relinker.py b/swift/cli/relinker.py index ea301b8663..c564e88827 100644 --- a/swift/cli/relinker.py +++ b/swift/cli/relinker.py @@ -37,9 +37,8 @@ EXIT_NO_APPLICABLE_POLICY = 2 EXIT_ERROR = 1 -def policy(policy_index): - value = non_negative_int(policy_index) - value = POLICIES.get_by_index(value) +def policy(policy_name_or_index): + value = POLICIES.get_by_name_or_index(policy_name_or_index) if value is None: raise ValueError return value diff --git a/swift/common/storage_policy.py b/swift/common/storage_policy.py index d156f882fc..41685ce5f6 100644 --- a/swift/common/storage_policy.py +++ b/swift/common/storage_policy.py @@ -819,6 +819,15 @@ class StoragePolicyCollection(object): return None return self.by_index.get(index) + def get_by_name_or_index(self, name_or_index): + by_name = self.get_by_name(name_or_index) + by_index = self.get_by_index(name_or_index) + if by_name and by_index and by_name != by_index: + raise PolicyError( + "Found different polices when searching by " + "name (%s) and by index (%s)" % (by_name, by_index)) + return by_name or by_index + @property def legacy(self): return self.get_by_index(None) diff --git a/test/unit/cli/test_relinker.py b/test/unit/cli/test_relinker.py index 425d8444e1..b7ce68c64b 100644 --- a/test/unit/cli/test_relinker.py +++ b/test/unit/cli/test_relinker.py @@ -1007,27 +1007,26 @@ class TestRelinker(unittest.TestCase): # invalid policy with mock.patch('sys.stdout'), mock.patch('sys.stderr'): with self.assertRaises(SystemExit) as cm: - self.assertEqual(0, relinker.main([ + relinker.main([ 'relink', '--swift-dir', self.testdir, '--policy', '9', '--skip-mount', '--devices', self.devices, '--device', self.existing_device, - ])) + ]) self.assertEqual(2, cm.exception.code) - # policy must be index not name with mock.patch('sys.stdout'), mock.patch('sys.stderr'): with self.assertRaises(SystemExit) as cm: - self.assertEqual(0, relinker.main([ + relinker.main([ 'relink', '--swift-dir', self.testdir, - '--policy', 'gold', + '--policy', 'pewter', '--skip-mount', '--devices', self.devices, '--device', self.existing_device, - ])) + ]) self.assertEqual(2, cm.exception.code) # policy with no object @@ -1074,6 +1073,30 @@ class TestRelinker(unittest.TestCase): '0 removed, 0 errors)'], self.logger.get_lines_for_level('info')) + # policy name works, too + self.logger.clear() + with mock.patch.object(relinker.logging, 'getLogger', + return_value=self.logger): + self.assertEqual(0, relinker.main([ + 'relink', + '--swift-dir', self.testdir, + '--policy', 'gold', + '--skip-mount', + '--devices', self.devices, + '--device', self.existing_device, + ])) + self.assertTrue(os.path.isdir(self.expected_dir)) + self.assertTrue(os.path.isfile(self.expected_file)) + stat_old = os.stat(os.path.join(self.objdir, self.object_fname)) + stat_new = os.stat(self.expected_file) + self.assertEqual(stat_old.st_ino, stat_new.st_ino) + self.assertEqual( + ['Processing files for policy gold under %s/%s (cleanup=False)' + % (self.devices, self.existing_device), + '0 hash dirs processed (cleanup=False) ' + '(0 files, 0 linked, 0 removed, 0 errors)'], + self.logger.get_lines_for_level('info')) + @patch_policies( [StoragePolicy(0, name='gold', is_default=True), ECStoragePolicy(1, name='platinum', ec_type=DEFAULT_TEST_EC_TYPE, @@ -1105,7 +1128,7 @@ class TestRelinker(unittest.TestCase): self.assertEqual([mock.call(POLICIES[1])], mocked.call_args_list) self.assertEqual(0, res) - res, mocked = do_relink(['--policy', 0]) + res, mocked = do_relink(['--policy', '0']) self.assertEqual([], mocked.call_args_list) self.assertEqual(2, res) diff --git a/test/unit/common/test_storage_policy.py b/test/unit/common/test_storage_policy.py index ea172e368b..3ee7bfddbc 100644 --- a/test/unit/common/test_storage_policy.py +++ b/test/unit/common/test_storage_policy.py @@ -367,6 +367,25 @@ class TestStoragePolicies(unittest.TestCase): self.assertEqual(pol1, policies.get_by_name(name)) self.assertEqual(policies.get_by_name(name).name, 'One') + def test_wacky_int_names(self): + # checking duplicate on insert + test_policies = [StoragePolicy(0, '1', True, aliases='-1'), + StoragePolicy(1, '0', False)] + policies = StoragePolicyCollection(test_policies) + + with self.assertRaises(PolicyError): + policies.get_by_name_or_index('0') + self.assertEqual(policies.get_by_name('1'), test_policies[0]) + self.assertEqual(policies.get_by_index(0), test_policies[0]) + + with self.assertRaises(PolicyError): + policies.get_by_name_or_index('1') + self.assertEqual(policies.get_by_name('0'), test_policies[1]) + self.assertEqual(policies.get_by_index(1), test_policies[1]) + + self.assertIsNone(policies.get_by_index(-1)) + self.assertEqual(policies.get_by_name_or_index('-1'), test_policies[0]) + def test_multiple_names(self): # checking duplicate on insert test_policies = [StoragePolicy(0, 'zero', True),