From ee80f99aec8719fc3ad1d86bf8cd6bd48fc1bd33 Mon Sep 17 00:00:00 2001 From: Chinemerem Date: Fri, 20 Sep 2024 15:28:04 -0700 Subject: [PATCH] Make object-expirer respect internal_client_conf_path Previously the object-expirer would not respect an internal_client_conf_path option if its config was loaded from a "legacy" config filepath (e.g. **/object-expirer.conf). Legacy object-expirer config expected config sections for an internal-client to be included in the same file. However, "modern" object-expirer config files (e.g. **/object-server.conf) expect internal-client config to be loaded from a separate file specified by internal_client_conf_path and defaulting to 'internal_client_conf_path'. "Legacy" expirer config is still used in production clusters but it is useful to use a shared internal-client config. This patch therefore makes the internal_client_conf_path option always be respected regardless of the path of the object-expirer conf file. If 'internal_client_conf_path' is not specified, "modern" config will continue to use the default '/etc/swift/internal-client.conf', but "legacy" config will default to the path of the expirer conf file (i.e. the same as the previous behavior). Related-Change: Ib21568f9b9d8547da87a99d65ae73a550e9c3230 Change-Id: I24ec702cd2ed074ca9df084cefc896418cece394 --- swift/obj/expirer.py | 9 ++++----- test/unit/obj/test_expirer.py | 27 ++++++++++++++++++++++----- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/swift/obj/expirer.py b/swift/obj/expirer.py index 58870a9431..d6c3d5b6c4 100644 --- a/swift/obj/expirer.py +++ b/swift/obj/expirer.py @@ -186,12 +186,11 @@ class ObjectExpirer(Daemon): self.delay_reaping_times = read_conf_for_delay_reaping_times(conf) def _make_internal_client(self, is_legacy_conf): + default_ic_conf_path = '/etc/swift/internal-client.conf' if is_legacy_conf: - ic_conf_path = self.conf_path - else: - ic_conf_path = \ - self.conf.get('internal_client_conf_path') or \ - '/etc/swift/internal-client.conf' + default_ic_conf_path = self.conf_path + ic_conf_path = self.conf.get( + 'internal_client_conf_path', default_ic_conf_path) request_tries = int(self.conf.get('request_tries') or 3) return InternalClient( ic_conf_path, 'Swift Object Expirer', request_tries, diff --git a/test/unit/obj/test_expirer.py b/test/unit/obj/test_expirer.py index bba695ff2f..501c5522d9 100644 --- a/test/unit/obj/test_expirer.py +++ b/test/unit/obj/test_expirer.py @@ -306,9 +306,9 @@ class TestObjectExpirer(TestCase): self.assertEqual(x.expiring_objects_account, '.expiring_objects') self.assertIs(x.swift, self.fake_swift) - def test_init_internal_client_path(self): - # default -> /etc/swift/object-expirer.conf - conf = {'internal_client_conf_path': 'ignored'} + def test_init_internal_client_path_from_expirer_conf(self): + # conf read from object-expirer.conf, no internal_client_conf_path + conf = {'__file__': '/etc/swift/object-expirer.conf'} with mock.patch.object(expirer, 'InternalClient', return_value=self.fake_swift) as mock_ic: x = expirer.ObjectExpirer(conf, logger=self.logger) @@ -319,20 +319,23 @@ class TestObjectExpirer(TestCase): self.assertEqual(self.logger.get_lines_for_level('warning'), []) self.assertIs(x.swift, self.fake_swift) + def test_init_internal_client_path_from_internal_and_other_conf(self): # conf read from /etc/swift/object-expirer.conf # -> /etc/swift/object-expirer.conf conf = {'__file__': '/etc/swift/object-expirer.conf', - 'internal_client_conf_path': 'ignored'} + 'internal_client_conf_path': + '/etc/swift/other-internal-client.conf'} with mock.patch.object(expirer, 'InternalClient', return_value=self.fake_swift) as mock_ic: x = expirer.ObjectExpirer(conf, logger=self.logger) self.assertEqual(mock_ic.mock_calls, [mock.call( - '/etc/swift/object-expirer.conf', 'Swift Object Expirer', 3, + '/etc/swift/other-internal-client.conf', 'Swift Object Expirer', 3, use_replication_network=True, global_conf={'log_name': 'object-expirer-ic'})]) self.assertEqual(self.logger.get_lines_for_level('warning'), []) self.assertIs(x.swift, self.fake_swift) + def test_init_internal_client_path_from_server_conf(self): # conf read from object-server.conf, no internal_client_conf_path # specified -> /etc/swift/internal-client.conf conf = {'__file__': '/etc/swift/object-server.conf'} @@ -346,6 +349,7 @@ class TestObjectExpirer(TestCase): self.assertEqual(self.logger.get_lines_for_level('warning'), []) self.assertIs(x.swift, self.fake_swift) + def test_init_internal_client_path_from_server_and_other_conf(self): # conf read from object-server.conf, internal_client_conf_path is # specified -> internal_client_conf_path value conf = {'__file__': '/etc/swift/object-server.conf', @@ -361,6 +365,7 @@ class TestObjectExpirer(TestCase): self.assertEqual(self.logger.get_lines_for_level('warning'), []) self.assertIs(x.swift, self.fake_swift) + def test_init_internal_client_path_from_other_and_other_conf(self): # conf read from other file, internal_client_conf_path is # specified -> internal_client_conf_path value conf = {'__file__': '/etc/swift/other-object-server.conf', @@ -376,6 +381,18 @@ class TestObjectExpirer(TestCase): self.assertEqual(self.logger.get_lines_for_level('warning'), []) self.assertIs(x.swift, self.fake_swift) + def test_init_internal_client_path_from_empty_conf(self): + conf = {} + with mock.patch.object(expirer, 'InternalClient', + return_value=self.fake_swift) as mock_ic: + x = expirer.ObjectExpirer(conf, logger=self.logger) + self.assertEqual(mock_ic.mock_calls, [mock.call( + '/etc/swift/object-expirer.conf', 'Swift Object Expirer', 3, + use_replication_network=True, + global_conf={'log_name': 'object-expirer-ic'})]) + self.assertEqual(self.logger.get_lines_for_level('warning'), []) + self.assertIs(x.swift, self.fake_swift) + def test_init_internal_client_log_name(self): def _do_test_init_ic_log_name(conf, exp_internal_client_log_name): with mock.patch(