openstack/swift
Code Issues Proposed changes

Make object-expirer respect internal_client_conf_path

Browse Source 
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
This commit is contained in:
Chinemerem 2024-09-20 15:28:04 -07:00
parent 94dc4cad09
commit ee80f99aec
2 changed files with 26 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
swift/obj/expirer.py
View File

@ -186,12 +186,11 @@ class ObjectExpirer(Daemon):
self.delay_reaping_times = read_conf_for_delay_reaping_times(conf) self.delay_reaping_times = read_conf_for_delay_reaping_times(conf)
def _make_internal_client(self, is_legacy_conf): def _make_internal_client(self, is_legacy_conf):
default_ic_conf_path = '/etc/swift/internal-client.conf'
if is_legacy_conf: if is_legacy_conf:
ic_conf_path = self.conf_path default_ic_conf_path = self.conf_path
else: ic_conf_path = self.conf.get(
ic_conf_path = \ 'internal_client_conf_path', default_ic_conf_path)
self.conf.get('internal_client_conf_path') or \
'/etc/swift/internal-client.conf'
request_tries = int(self.conf.get('request_tries') or 3) request_tries = int(self.conf.get('request_tries') or 3)
return InternalClient( return InternalClient(
ic_conf_path, 'Swift Object Expirer', request_tries, ic_conf_path, 'Swift Object Expirer', request_tries,

27
test/unit/obj/test_expirer.py
View File

@ -306,9 +306,9 @@ class TestObjectExpirer(TestCase):
self.assertEqual(x.expiring_objects_account, '.expiring_objects') self.assertEqual(x.expiring_objects_account, '.expiring_objects')
self.assertIs(x.swift, self.fake_swift) self.assertIs(x.swift, self.fake_swift)
def test_init_internal_client_path(self): def test_init_internal_client_path_from_expirer_conf(self):
# default -> /etc/swift/object-expirer.conf # conf read from object-expirer.conf, no internal_client_conf_path
conf = {'internal_client_conf_path': 'ignored'} conf = {'__file__': '/etc/swift/object-expirer.conf'}
with mock.patch.object(expirer, 'InternalClient', with mock.patch.object(expirer, 'InternalClient',
return_value=self.fake_swift) as mock_ic: return_value=self.fake_swift) as mock_ic:
x = expirer.ObjectExpirer(conf, logger=self.logger) x = expirer.ObjectExpirer(conf, logger=self.logger)
@ -319,20 +319,23 @@ class TestObjectExpirer(TestCase):
self.assertEqual(self.logger.get_lines_for_level('warning'), []) self.assertEqual(self.logger.get_lines_for_level('warning'), [])
self.assertIs(x.swift, self.fake_swift) 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 # conf read from /etc/swift/object-expirer.conf
# -> /etc/swift/object-expirer.conf # -> /etc/swift/object-expirer.conf
conf = {'__file__': '/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', with mock.patch.object(expirer, 'InternalClient',
return_value=self.fake_swift) as mock_ic: return_value=self.fake_swift) as mock_ic:
x = expirer.ObjectExpirer(conf, logger=self.logger) x = expirer.ObjectExpirer(conf, logger=self.logger)
self.assertEqual(mock_ic.mock_calls, [mock.call( 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, use_replication_network=True,
global_conf={'log_name': 'object-expirer-ic'})]) global_conf={'log_name': 'object-expirer-ic'})])
self.assertEqual(self.logger.get_lines_for_level('warning'), []) self.assertEqual(self.logger.get_lines_for_level('warning'), [])
self.assertIs(x.swift, self.fake_swift) 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 # conf read from object-server.conf, no internal_client_conf_path
# specified -> /etc/swift/internal-client.conf # specified -> /etc/swift/internal-client.conf
conf = {'__file__': '/etc/swift/object-server.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.assertEqual(self.logger.get_lines_for_level('warning'), [])
self.assertIs(x.swift, self.fake_swift) 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 # conf read from object-server.conf, internal_client_conf_path is
# specified -> internal_client_conf_path value # specified -> internal_client_conf_path value
conf = {'__file__': '/etc/swift/object-server.conf', conf = {'__file__': '/etc/swift/object-server.conf',
@ -361,6 +365,7 @@ class TestObjectExpirer(TestCase):
self.assertEqual(self.logger.get_lines_for_level('warning'), []) self.assertEqual(self.logger.get_lines_for_level('warning'), [])
self.assertIs(x.swift, self.fake_swift) 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 # conf read from other file, internal_client_conf_path is
# specified -> internal_client_conf_path value # specified -> internal_client_conf_path value
conf = {'__file__': '/etc/swift/other-object-server.conf', 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.assertEqual(self.logger.get_lines_for_level('warning'), [])
self.assertIs(x.swift, self.fake_swift) 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 test_init_internal_client_log_name(self):
def _do_test_init_ic_log_name(conf, exp_internal_client_log_name): def _do_test_init_ic_log_name(conf, exp_internal_client_log_name):
with mock.patch( with mock.patch(