From 600db4841e4e76f160ebfc367bf0ef42beee47c4 Mon Sep 17 00:00:00 2001 From: Kota Tsuyuzaki Date: Tue, 14 Feb 2017 17:54:48 -0800 Subject: [PATCH] Follow up on reconstructor handoffs_only This is a follow-up for https://review.openstack.org/#/c/425493 This patch includes: - Add more tests on the configuration with handoffs_first and handoffs_only - Remove unnecessary space in a warning log line. (2 places) - Change test conf from True/False to "True"/"False" (string) because in the conf dict, those value should be string. Co-Authored-By: Janie Richling Change-Id: Ida90c32d16481a15fa68c9fdb380932526c366f6 --- swift/obj/reconstructor.py | 6 +-- test/unit/obj/test_reconstructor.py | 79 ++++++++++++++++++++++++++--- 2 files changed, 75 insertions(+), 10 deletions(-) diff --git a/swift/obj/reconstructor.py b/swift/obj/reconstructor.py index fe2b5fc07b..4d9459e2ca 100644 --- a/swift/obj/reconstructor.py +++ b/swift/obj/reconstructor.py @@ -152,7 +152,7 @@ class ObjectReconstructor(Daemon): if 'handoffs_first' in conf: self.logger.warning( 'The handoffs_first option is deprecated in favor ' - 'of handoffs_only. This option may be ignored in a ' + 'of handoffs_only. This option may be ignored in a ' 'future release.') # honor handoffs_first for backwards compatibility default_handoffs_only = config_true_value(conf['handoffs_first']) @@ -985,11 +985,11 @@ class ObjectReconstructor(Daemon): if self.handoffs_only: if self.handoffs_remaining > 0: self.logger.info(_( - "Handoffs only mode still has handoffs remaining. " + "Handoffs only mode still has handoffs remaining. " "Next pass will continue to revert handoffs.")) else: self.logger.warning(_( - "Handoffs only mode found no handoffs remaining. " + "Handoffs only mode found no handoffs remaining. " "You should disable handoffs_only once all nodes " "are reporting no handoffs remaining.")) diff --git a/test/unit/obj/test_reconstructor.py b/test/unit/obj/test_reconstructor.py index 0e870a8650..a67ce0815a 100644 --- a/test/unit/obj/test_reconstructor.py +++ b/test/unit/obj/test_reconstructor.py @@ -1184,22 +1184,22 @@ class TestObjectReconstructor(unittest.TestCase): def test_handoffs_only_default(self): # sanity neither option added to default conf - self.conf.pop('handoffs_only', None) self.conf.pop('handoffs_first', None) + self.conf.pop('handoffs_only', None) self.reconstructor = object_reconstructor.ObjectReconstructor( self.conf, logger=self.logger) self.assertFalse(self.reconstructor.handoffs_only) def test_handoffs_first_enables_handoffs_only(self): + self.conf['handoffs_first'] = "True" self.conf.pop('handoffs_only', None) # sanity - self.conf['handoffs_first'] = True self.reconstructor = object_reconstructor.ObjectReconstructor( self.conf, logger=self.logger) self.assertTrue(self.reconstructor.handoffs_only) warnings = self.logger.get_lines_for_level('warning') expected = [ 'The handoffs_first option is deprecated in favor ' - 'of handoffs_only. This option may be ignored in a ' + 'of handoffs_only. This option may be ignored in a ' 'future release.', 'Handoff only mode is not intended for normal operation, ' 'use handoffs_only with care.', @@ -1207,22 +1207,22 @@ class TestObjectReconstructor(unittest.TestCase): self.assertEqual(expected, warnings) def test_handoffs_only_ignores_handoffs_first(self): - self.conf['handoffs_first'] = True - self.conf['handoffs_only'] = False + self.conf['handoffs_first'] = "True" + self.conf['handoffs_only'] = "False" self.reconstructor = object_reconstructor.ObjectReconstructor( self.conf, logger=self.logger) self.assertFalse(self.reconstructor.handoffs_only) warnings = self.logger.get_lines_for_level('warning') expected = [ 'The handoffs_first option is deprecated in favor of ' - 'handoffs_only. This option may be ignored in a future release.', + 'handoffs_only. This option may be ignored in a future release.', 'Ignored handoffs_first option in favor of handoffs_only.', ] self.assertEqual(expected, warnings) def test_handoffs_only_enabled(self): self.conf.pop('handoffs_first', None) # sanity - self.conf['handoffs_only'] = True + self.conf['handoffs_only'] = "True" self.reconstructor = object_reconstructor.ObjectReconstructor( self.conf, logger=self.logger) self.assertTrue(self.reconstructor.handoffs_only) @@ -1233,6 +1233,71 @@ class TestObjectReconstructor(unittest.TestCase): ] self.assertEqual(expected, warnings) + def test_handoffs_only_true_and_first_true(self): + self.conf['handoffs_first'] = "True" + self.conf['handoffs_only'] = "True" + self.reconstructor = object_reconstructor.ObjectReconstructor( + self.conf, logger=self.logger) + self.assertTrue(self.reconstructor.handoffs_only) + warnings = self.logger.get_lines_for_level('warning') + expected = [ + 'The handoffs_first option is deprecated in favor of ' + 'handoffs_only. This option may be ignored in a future release.', + 'Handoff only mode is not intended for normal operation, ' + 'use handoffs_only with care.', + ] + self.assertEqual(expected, warnings) + + def test_handoffs_only_false_and_first_false(self): + self.conf['handoffs_only'] = "False" + self.conf['handoffs_first'] = "False" + self.reconstructor = object_reconstructor.ObjectReconstructor( + self.conf, logger=self.logger) + self.assertFalse(self.reconstructor.handoffs_only) + warnings = self.logger.get_lines_for_level('warning') + expected = [ + 'The handoffs_first option is deprecated in favor of ' + 'handoffs_only. This option may be ignored in a future release.', + ] + self.assertEqual(expected, warnings) + + def test_handoffs_only_none_and_first_false(self): + self.conf['handoffs_first'] = "False" + self.conf.pop('handoffs_only', None) # sanity + self.reconstructor = object_reconstructor.ObjectReconstructor( + self.conf, logger=self.logger) + self.assertFalse(self.reconstructor.handoffs_only) + warnings = self.logger.get_lines_for_level('warning') + expected = [ + 'The handoffs_first option is deprecated in favor of ' + 'handoffs_only. This option may be ignored in a future release.', + ] + self.assertEqual(expected, warnings) + + def test_handoffs_only_false_and_first_none(self): + self.conf.pop('handoffs_first', None) # sanity + self.conf['handoffs_only'] = "False" + self.reconstructor = object_reconstructor.ObjectReconstructor( + self.conf, logger=self.logger) + self.assertFalse(self.reconstructor.handoffs_only) + warnings = self.logger.get_lines_for_level('warning') + self.assertEqual(len(warnings), 0) + + def test_handoffs_only_true_and_first_false(self): + self.conf['handoffs_first'] = "False" + self.conf['handoffs_only'] = "True" + self.reconstructor = object_reconstructor.ObjectReconstructor( + self.conf, logger=self.logger) + self.assertTrue(self.reconstructor.handoffs_only) + warnings = self.logger.get_lines_for_level('warning') + expected = [ + 'The handoffs_first option is deprecated in favor of ' + 'handoffs_only. This option may be ignored in a future release.', + 'Handoff only mode is not intended for normal operation, ' + 'use handoffs_only with care.', + ] + self.assertEqual(expected, warnings) + def test_two_ec_policies(self): with patch_policies([ StoragePolicy(0, name='zero', is_deprecated=True),