Fix kolla-set-configs --check to detect state mismatch
If a configuration file is tracked in the state file but no longer appears in config.json, it should either be restored or removed. This patch introduces a new exception StateMismatch and updates execute_config_check() to detect such cases. If any destination path is present in the defaults state but missing from config.json, we now raise StateMismatch. A dedicated unit test has been added to verify this behavior. Closes-Bug: #2114173 Signed-off-by: Michal Arbet <michal.arbet@ultimum.io> Change-Id: I6e0b4aaa5722990e3ac647578023f474db3d4381
This commit is contained in:
@@ -68,6 +68,10 @@ class ConfigFileCommandDiffers(ExitingException):
|
||||
pass
|
||||
|
||||
|
||||
class StateMismatch(ExitingException):
|
||||
pass
|
||||
|
||||
|
||||
class ConfigFile(object):
|
||||
|
||||
def __init__(self, source, dest, owner=None, perm=None, optional=False,
|
||||
@@ -583,6 +587,56 @@ def execute_command_check(config):
|
||||
|
||||
|
||||
def execute_config_check(config):
|
||||
"""Check configuration state consistency and validate config file entries.
|
||||
|
||||
This function compares the current config file destinations from the
|
||||
provided config dictionary with those stored in the defaults state file.
|
||||
If any destinations are found in the state file but not in the config,
|
||||
a StateMismatch exception is raised. These missing files would otherwise
|
||||
be restored or removed depending on their backup state.
|
||||
|
||||
After validating consistency, the function performs standard checks on
|
||||
each declared configuration file, including content, permissions, and
|
||||
ownership validation.
|
||||
|
||||
Args:
|
||||
config (dict): The configuration dictionary containing 'config_files'
|
||||
entries as expected by Kolla.
|
||||
|
||||
Raises:
|
||||
StateMismatch: If there are entries in the defaults state not present
|
||||
in the provided config.
|
||||
"""
|
||||
state = get_defaults_state()
|
||||
|
||||
# Build a set of all current destination paths from config.json
|
||||
# If the destination is a directory, we append the
|
||||
# basename of the source
|
||||
current_dests = {
|
||||
entry['dest'] if not entry['dest'].endswith('/') else
|
||||
os.path.join(entry['dest'], os.path.basename(entry['source']))
|
||||
for entry in config.get('config_files', [])
|
||||
if entry.get('dest')
|
||||
}
|
||||
|
||||
# Detect any paths that are present in the state file but
|
||||
# missing from config.json.
|
||||
# These would be either restored (if state[dest] has a backup)
|
||||
# or removed (if dest is null)
|
||||
removed_dests = [
|
||||
path for path in state.keys()
|
||||
if path not in current_dests
|
||||
]
|
||||
|
||||
if removed_dests:
|
||||
raise StateMismatch(
|
||||
f"The following config files are tracked in state but missing "
|
||||
f"from config.json. "
|
||||
f"They would be restored or removed: {sorted(removed_dests)}"
|
||||
)
|
||||
|
||||
# Perform the regular content, permissions, and ownership
|
||||
# checks on the declared files
|
||||
for data in config.get('config_files', []):
|
||||
config_file = ConfigFile(**data)
|
||||
config_file.check()
|
||||
|
||||
6
releasenotes/notes/bug-2114173-70d0b815a42ae239.yaml
Normal file
6
releasenotes/notes/bug-2114173-70d0b815a42ae239.yaml
Normal file
@@ -0,0 +1,6 @@
|
||||
---
|
||||
fixes:
|
||||
- |
|
||||
Fixes set_configs.py not detecting removed config files
|
||||
during --check, which prevented container restart when
|
||||
needed. `LP#2114173 <https://launchpad.net/bugs/2114173>`__
|
||||
@@ -892,3 +892,47 @@ class ConfigFileTest(base.BaseTestCase):
|
||||
|
||||
# Verify that the updated state was saved
|
||||
mock_set_defaults_state.assert_called_once_with(expected_state)
|
||||
|
||||
|
||||
class ExecuteConfigCheckStateMismatchTest(base.BaseTestCase):
|
||||
|
||||
@mock.patch.object(set_configs, 'get_defaults_state')
|
||||
def test_execute_config_check_raises_state_mismatch(
|
||||
self, mock_get_defaults_state
|
||||
):
|
||||
"""Test execute_config_check() when state has extra config file.
|
||||
|
||||
This test simulates the scenario where the state file contains
|
||||
a destination that no longer exists in config.json. It verifies:
|
||||
- get_defaults_state() returns a state with an extra entry.
|
||||
- execute_config_check() raises StateMismatch when config.json
|
||||
omits a tracked destination.
|
||||
"""
|
||||
config = {
|
||||
"command": "/bin/true",
|
||||
"config_files": [
|
||||
{
|
||||
"source": "/etc/foo/foo.conf",
|
||||
"dest": "/etc/foo/foo.conf",
|
||||
"owner": "user1",
|
||||
"perm": "0644"
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
mock_get_defaults_state.return_value = {
|
||||
"/etc/foo/foo.conf": {
|
||||
"source": "/etc/foo/foo.conf",
|
||||
"preserve_properties": True,
|
||||
"dest": "/etc/kolla/defaults/etc/foo/foo.conf"
|
||||
},
|
||||
"/etc/old/obsolete.conf": {
|
||||
"source": "/etc/old/obsolete.conf",
|
||||
"preserve_properties": True,
|
||||
"dest": "/etc/kolla/defaults/etc/old/obsolete.conf"
|
||||
}
|
||||
}
|
||||
|
||||
self.assertRaises(set_configs.StateMismatch,
|
||||
set_configs.execute_config_check,
|
||||
config)
|
||||
|
||||
Reference in New Issue
Block a user