Skip to content

Commit a1ad96c

Browse files
committed
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 <[email protected]> Change-Id: I6e0b4aaa5722990e3ac647578023f474db3d4381 (cherry picked from commit cf3c65f)
1 parent 6661f59 commit a1ad96c

File tree

3 files changed

+104
-0
lines changed

3 files changed

+104
-0
lines changed

docker/base/set_configs.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@ class ConfigFileCommandDiffers(ExitingException):
6868
pass
6969

7070

71+
class StateMismatch(ExitingException):
72+
pass
73+
74+
7175
class ConfigFile(object):
7276

7377
def __init__(self, source, dest, owner=None, perm=None, optional=False,
@@ -583,6 +587,56 @@ def execute_command_check(config):
583587

584588

585589
def execute_config_check(config):
590+
"""Check configuration state consistency and validate config file entries.
591+
592+
This function compares the current config file destinations from the
593+
provided config dictionary with those stored in the defaults state file.
594+
If any destinations are found in the state file but not in the config,
595+
a StateMismatch exception is raised. These missing files would otherwise
596+
be restored or removed depending on their backup state.
597+
598+
After validating consistency, the function performs standard checks on
599+
each declared configuration file, including content, permissions, and
600+
ownership validation.
601+
602+
Args:
603+
config (dict): The configuration dictionary containing 'config_files'
604+
entries as expected by Kolla.
605+
606+
Raises:
607+
StateMismatch: If there are entries in the defaults state not present
608+
in the provided config.
609+
"""
610+
state = get_defaults_state()
611+
612+
# Build a set of all current destination paths from config.json
613+
# If the destination is a directory, we append the
614+
# basename of the source
615+
current_dests = {
616+
entry['dest'] if not entry['dest'].endswith('/') else
617+
os.path.join(entry['dest'], os.path.basename(entry['source']))
618+
for entry in config.get('config_files', [])
619+
if entry.get('dest')
620+
}
621+
622+
# Detect any paths that are present in the state file but
623+
# missing from config.json.
624+
# These would be either restored (if state[dest] has a backup)
625+
# or removed (if dest is null)
626+
removed_dests = [
627+
path for path in state.keys()
628+
if path not in current_dests
629+
]
630+
631+
if removed_dests:
632+
raise StateMismatch(
633+
f"The following config files are tracked in state but missing "
634+
f"from config.json. "
635+
f"They would be restored or removed: {sorted(removed_dests)}"
636+
)
637+
638+
# Perform the regular content, permissions, and ownership
639+
# checks on the declared files
586640
for data in config.get('config_files', []):
587641
config_file = ConfigFile(**data)
588642
config_file.check()
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
fixes:
3+
- |
4+
Fixes set_configs.py not detecting removed config files
5+
during --check, which prevented container restart when
6+
needed. `LP#2114173 <https://launchpad.net/bugs/2114173>`__

tests/test_set_config.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -875,3 +875,47 @@ def test_handle_defaults_state_exist_config_restored(
875875

876876
# Verify that the updated state was saved
877877
mock_set_defaults_state.assert_called_once_with(expected_state)
878+
879+
880+
class ExecuteConfigCheckStateMismatchTest(base.BaseTestCase):
881+
882+
@mock.patch.object(set_configs, 'get_defaults_state')
883+
def test_execute_config_check_raises_state_mismatch(
884+
self, mock_get_defaults_state
885+
):
886+
"""Test execute_config_check() when state has extra config file.
887+
888+
This test simulates the scenario where the state file contains
889+
a destination that no longer exists in config.json. It verifies:
890+
- get_defaults_state() returns a state with an extra entry.
891+
- execute_config_check() raises StateMismatch when config.json
892+
omits a tracked destination.
893+
"""
894+
config = {
895+
"command": "/bin/true",
896+
"config_files": [
897+
{
898+
"source": "/etc/foo/foo.conf",
899+
"dest": "/etc/foo/foo.conf",
900+
"owner": "user1",
901+
"perm": "0644"
902+
}
903+
]
904+
}
905+
906+
mock_get_defaults_state.return_value = {
907+
"/etc/foo/foo.conf": {
908+
"source": "/etc/foo/foo.conf",
909+
"preserve_properties": True,
910+
"dest": "/etc/kolla/defaults/etc/foo/foo.conf"
911+
},
912+
"/etc/old/obsolete.conf": {
913+
"source": "/etc/old/obsolete.conf",
914+
"preserve_properties": True,
915+
"dest": "/etc/kolla/defaults/etc/old/obsolete.conf"
916+
}
917+
}
918+
919+
self.assertRaises(set_configs.StateMismatch,
920+
set_configs.execute_config_check,
921+
config)

0 commit comments

Comments
 (0)