From 36c12676ff185f524a7f4d2f2f4c70b5d60462b4 Mon Sep 17 00:00:00 2001 From: Michal Arbet Date: Wed, 10 Apr 2024 20:11:23 +0200 Subject: [PATCH] Fix handling configs in base image This commit restructures the handling of configuration files in set_configs.py, introducing functions for managing default configuration files first. Closes-Bug: #2060855 Change-Id: If91e0330dc149143c82d2183b8ddf6fa9f68d80e --- docker/base/set_configs.py | 151 ++++++ .../notes/bug-2060855-77516da722d04761.yaml | 6 + tests/test_set_config.py | 460 ++++++++++++++++++ 3 files changed, 617 insertions(+) create mode 100644 releasenotes/notes/bug-2060855-77516da722d04761.yaml diff --git a/docker/base/set_configs.py b/docker/base/set_configs.py index bd2747536c..ab5b8c68bd 100644 --- a/docker/base/set_configs.py +++ b/docker/base/set_configs.py @@ -34,6 +34,9 @@ logging.basicConfig( LOG = logging.getLogger(__name__) LOG.setLevel(logging.INFO) +KOLLA_DEFAULTS = "/etc/kolla/defaults" +KOLLA_DEFAULTS_STATE = KOLLA_DEFAULTS + '/' + 'state' + class ExitingException(Exception): def __init__(self, message, exit_code=1): @@ -403,10 +406,157 @@ def handle_permissions(config): handle_exclusion(root, file_) +def get_defaults_state(): + """Retrieve the saved default configuration state from default state file. + + This function creates the directory for Kolla defaults if it does not + exist, and then attempts to read the current configuration state from + a JSON file. If the file exists, it reads and returns the content. + If not, it returns an empty dictionary. + + Simply said, when the container starts for the first time, the state file + doesn't exist, and it returns an empty dictionary. + However, if it has already been started before, it will contain the state + as it was when it first ran. + + Returns: + dict: The configuration state stored in the Kolla defaults state file. + + Example: + { + "/etc/cinder/cinder.conf": { + "source": "/etc/cinder/cinder.conf", + "preserve_properties": true, + "dest": null + }, + "/etc/apache2/conf-enabled/cinder-wsgi.conf": { + "source": "/etc/apache2/conf-enabled/cinder-wsgi.conf", + "preserve_properties": true, + "dest": null + }, + "/etc/cinder/cinder_audit_map.conf": { + "source": "/etc/cinder/cinder_audit_map.conf", + "preserve_properties": true, + "dest": "/etc/kolla/defaults/etc/cinder/cinder_audit_map.conf" + } + } + + From above example: + /etc/cinder/cinder.conf didn't exist + /etc/apache2/conf-enabled/cinder-wsgi.conf didn't exist + /etc/cinder/cinder_audit_map.conf exists and saved + """ + os.makedirs(KOLLA_DEFAULTS, exist_ok=True) + if os.path.exists(KOLLA_DEFAULTS_STATE): + with open(KOLLA_DEFAULTS_STATE, 'r') as f: + return json.load(f) + else: + return {} + + +def set_defaults_state(state): + """Save the provided configuration state to the defaults state file. + + This function writes the provided state (a dictionary) to a JSON file at + the specified Kolla defaults state location, ensuring that it is properly + formatted with indentation for readability. + + Args: + state (dict): The configuration state to save to the Kolla defaults + state file. + """ + with open(KOLLA_DEFAULTS_STATE, 'w') as f: + json.dump(state, f, indent=4) + + +def remove_or_restore_configs(state): + """Remove or restore configuration files based on their current state. + + This function iterates over the configuration files in the provided state. + If the destination is `None`, it removes the file or directory. Otherwise, + it swaps the source and destination, restoring the configuration file + by copying it back to its original location. + + Args: + state (dict): The current default state of configuration files, mapping + file paths to their source and destination information. + """ + for k, v in state.items(): + if v['dest'] is None: + if os.path.exists(k): + if os.path.isfile(k): + os.remove(k) + else: + shutil.rmtree(k) + else: + v['source'], v['dest'] = v['dest'], v['source'] + config_file = ConfigFile(**v) + config_file.copy() + + +def backup_configs(config, state): + """Back up new configuration files and update the default state. + + This function processes new configuration files provided in the + input `config`. For each file, it checks if the destination exists in the + current state. If not, it backs up the file by copying it to the default + directory. It then updates the state with the new configuration file's + information. + + Args: + config (dict): The input configuration containing a list of config + files. + state (dict): The current default state to be updated with the new + config files. + """ + if 'config_files' in config: + for data in config['config_files']: + if data['dest'] in state.keys(): + continue + src = data['source'] + if data['dest'].endswith('/'): + dst = data['dest'] + data['source'].split('/')[-1] + else: + dst = data['dest'] + default = KOLLA_DEFAULTS + dst + if os.path.exists(src): + copy = {'source': dst, 'preserve_properties': True} + if os.path.exists(dst): + copy['dest'] = default + if dst not in state: + config_file = ConfigFile(**copy) + config_file.copy() + state[dst] = copy + else: + copy['dest'] = None + if dst not in state: + state[dst] = copy + + +def handle_defaults(config): + """Handle the default config files by copying/removing them as needed. + + This function loads the current default state and manages the configuration + files. It first processes existing configuration files in the default + state, either removing or restoring them based on their destination status. + It then backs up any new configuration files from the input config, + updating the default state accordingly. + + Args: + config (dict): A dictionary containing the list of configuration files + to be handled. + """ + state = get_defaults_state() + remove_or_restore_configs(state) + backup_configs(config, state) + set_defaults_state(state) + + def execute_config_strategy(config): config_strategy = os.environ.get("KOLLA_CONFIG_STRATEGY") LOG.info("Kolla config strategy set to: %s", config_strategy) if config_strategy == "COPY_ALWAYS": + handle_defaults(config) copy_config(config) handle_permissions(config) elif config_strategy == "COPY_ONCE": @@ -415,6 +565,7 @@ def execute_config_strategy(config): "The config strategy prevents copying new configs", exit_code=0) else: + handle_defaults(config) copy_config(config) handle_permissions(config) os.mknod('/configured') diff --git a/releasenotes/notes/bug-2060855-77516da722d04761.yaml b/releasenotes/notes/bug-2060855-77516da722d04761.yaml new file mode 100644 index 0000000000..ac20a32e58 --- /dev/null +++ b/releasenotes/notes/bug-2060855-77516da722d04761.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixes the inconsistency issue between config.json and + real state in the container. + `LP#2060855 `__ diff --git a/tests/test_set_config.py b/tests/test_set_config.py index a5e09084ea..3b528e4459 100644 --- a/tests/test_set_config.py +++ b/tests/test_set_config.py @@ -415,3 +415,463 @@ class ConfigFileTest(base.BaseTestCase): self.assertIs(False, config_file._cmp_file('/var/lib/kolla/config_files/bar', '/foo')) + + @mock.patch('os.makedirs') + @mock.patch('os.path.exists') + @mock.patch('builtins.open', new_callable=mock.mock_open, + read_data='{"foo": "bar"}') + def test_get_defaults_state_exist( + self, mock_open, mock_exists, mock_makedirs): + """Test get_defaults_state() when the default state file exists. + + This test mocks the behavior of the function when the default state + file exists. It ensures that: + - The directory for Kolla defaults is created if needed. + - The state file is opened and read successfully. + - The correct state data is returned as a dictionary. + + Mocks: + - os.makedirs: Ensures the directory is created. + - os.path.exists: Simulates that the state file exists. + - open: Mocks the file opening and reading, returning a sample JSON + content. + """ + + # Simulate that the state file exists + mock_exists.side_effect = lambda \ + path: path == set_configs.KOLLA_DEFAULTS_STATE + + result = set_configs.get_defaults_state() + + # Check that the directory creation was called + mock_makedirs.assert_called_once_with(set_configs.KOLLA_DEFAULTS, + exist_ok=True) + + # Verify that the function checked if the state file exists + mock_exists.assert_called_once_with(set_configs.KOLLA_DEFAULTS_STATE) + + # Verify that the state file was opened for reading + mock_open.assert_called_once_with( + set_configs.KOLLA_DEFAULTS_STATE, 'r') + + # Validate the result is as expected + self.assertEqual(result, {"foo": "bar"}) + + @mock.patch('os.makedirs') + @mock.patch('os.path.exists', return_value=False) + def test_get_defaults_state_not_exist(self, mock_exists, mock_makedirs): + """Test get_defaults_state() when the default state file doesn't exist. + + This test simulates the scenario where the default state file is + missing. + It verifies that: + - The directory for Kolla defaults is created if needed. + - The state file is checked but not found. + - An empty dictionary is returned since the state file is missing. + + Mocks: + - os.makedirs: Ensures the directory is created. + - os.path.exists: Simulates that the state file does not exist. + """ + + # Simulate that the file does not exist + mock_exists.side_effect = lambda path: False + + result = set_configs.get_defaults_state() + + # Check that the directory creation was called + mock_makedirs.assert_called_once_with(set_configs.KOLLA_DEFAULTS, + exist_ok=True) + # Verify that the function checked if the state file exists + mock_exists.assert_called_once_with(set_configs.KOLLA_DEFAULTS_STATE) + + # Result should be an empty dictionary since the state file is missing + self.assertEqual(result, {}) + + @mock.patch('builtins.open', new_callable=mock.mock_open) + @mock.patch('json.dump') + def test_set_defaults_state(self, mock_json_dump, mock_open): + """Test set_defaults_state() to ensure proper saving of the state. + + This test verifies that the provided state is correctly saved as a JSON + file with proper indentation. It checks: + - The state file is opened for writing. + - The provided state dictionary is dumped into the file in JSON format + with indentation for readability. + + Mocks: + - open: Mocks the file opening for writing. + - json.dump: Mocks the JSON dumping process. + """ + + state = {"foo": "bar"} + + set_configs.set_defaults_state(state) + + # Ensure the state file is opened for writing + mock_open.assert_called_once_with( + set_configs.KOLLA_DEFAULTS_STATE, 'w') + + # Check that the JSON state is dumped with proper indentation + mock_json_dump.assert_called_once_with(state, mock_open(), indent=4) + + @mock.patch.object(set_configs, 'set_defaults_state') + @mock.patch.object(set_configs, 'ConfigFile') + @mock.patch('os.path.exists') + @mock.patch.object(set_configs, 'get_defaults_state', return_value={}) + def test_handle_defaults_state_not_exist_config_exist( + self, mock_get_defaults_state, mock_exists, mock_config_file, + mock_set_defaults_state): + """Test handle_defaults() when no existing default config is present. + + This test simulates the case where no prior default configuration file + exists and a new configuration file needs to be backed up. It verifies: + - The current default state is retrieved (empty in this case). + - The configuration file exists, and a backup is created for it. + - The new state is saved after processing the configuration. + + Mocks: + - get_defaults_state: Returns an empty state. + - os.path.exists: Simulates that the source file exists. + - ConfigFile: Mocks the behavior of the ConfigFile class for file + copying. + - set_defaults_state: Ensures the new state is saved. + """ + + config = { + 'config_files': [ + { + 'source': '/source/file', 'dest': '/dest/file' + } + ] + } + + # Simulate the file exists + mock_exists.return_value = True + + copy = { + 'source': '/dest/file', + 'dest': set_configs.KOLLA_DEFAULTS + '/dest/file', + 'preserve_properties': True + } + + expected_state = { + '/dest/file': copy + } + + # Create a mock instance of ConfigFile + mock_config_file_instance = mock_config_file.return_value + mock_config_file_instance.copy = mock.MagicMock() + + # Call the function being tested + set_configs.handle_defaults(config) + + # Check that the directory creation was called + mock_get_defaults_state.assert_called_once() + + # Verify that the ConfigFile was instantiated correctly + mock_config_file.assert_called_once_with(**copy) + + # Ensure the copy method was called for the ConfigFile instance + mock_config_file_instance.copy.assert_called_once() + + # Check that the updated state was saved + mock_set_defaults_state.assert_called_once_with(expected_state) + + @mock.patch.object(set_configs, 'set_defaults_state') + @mock.patch.object(set_configs, 'ConfigFile') + @mock.patch('os.path.exists') + @mock.patch.object(set_configs, 'get_defaults_state', return_value={}) + def test_handle_defaults_state_not_exist_config_not_exist( + self, mock_get_defaults_state, mock_exists, mock_config_file, + mock_set_defaults_state): + """Test handle_defaults() with no config file and no default state. + + This test simulates the scenario where the configuration file does not + exist, and no existing default state is present. It verifies: + - The current default state is retrieved (empty). + - Since the configuration file doesn't exist, no backup is made. + - The state is updated accordingly. + + Mocks: + - get_defaults_state: Returns an empty state. + - os.path.exists: Simulates that the source + exists (in /var/lib/kolla/config/) but the + destination does not + (real destination where file should be copied to). + - ConfigFile: Ensures no ConfigFile instance is created since no backup + is needed. + - set_defaults_state: Ensures the updated state is saved. + """ + + config = { + 'config_files': [ + { + 'source': '/source/file', 'dest': '/dest/file' + } + ] + } + + # Simulate source exists but dest does not + def mock_exists_side_effect(path): + if path == '/source/file': + return True # Source exists + return False # Destination does not exist + + mock_exists.side_effect = mock_exists_side_effect + + copy = { + 'source': '/dest/file', + 'dest': None, + 'preserve_properties': True + } + + expected_state = { + '/dest/file': copy + } + + # Create a mock instance of ConfigFile + mock_config_file_instance = mock_config_file.return_value + mock_config_file_instance.copy = mock.MagicMock() + + # Call the function being tested + set_configs.handle_defaults(config) + + # Ensure the current default state was retrieved + mock_get_defaults_state.assert_called_once() + + # Verify that ConfigFile was not instantiated (because + # dest doesn't exist - nothing to backup) + mock_config_file.assert_not_called() + + # Check that the updated state was saved + mock_set_defaults_state.assert_called_once_with(expected_state) + + @mock.patch.object(set_configs, 'set_defaults_state') + @mock.patch.object(set_configs, 'ConfigFile') + @mock.patch('os.remove') + @mock.patch('shutil.rmtree') + @mock.patch('os.path.isfile') + @mock.patch('os.path.exists') + @mock.patch.object(set_configs, 'get_defaults_state', return_value={ + "/dest/file": { + "source": "/dest/file", + "preserve_properties": True, + "dest": None + } + }) + def test_handle_defaults_state_exist_config_exist_is_file( + self, mock_get_defaults_state, mock_exists, mock_isfile, + mock_rmtree, mock_remove, mock_config_file, + mock_set_defaults_state): + """Test handle_defaults() when the configuration exists and is a file. + + This test simulates the scenario where a configuration file already + exists. + It verifies: + - The current default state is retrieved. + - The destination is identified as a file. + - The file is removed. + - The updated state is saved correctly after the file is handled. + + Mocks: + - get_defaults_state: Returns an existing state. + - os.path.exists: Simulates the destination file exists. + - os.path.isfile: Ensures the destination is identified as a file. + - os.remove: Ensures the file is removed. + - set_defaults_state: Ensures the state is saved after processing. + """ + + config = { + 'config_files': [ + { + 'source': '/source/file', 'dest': '/dest/file' + } + ] + } + + # Simulate that destination exists and is a file + mock_exists.side_effect = lambda path: path == "/dest/file" + mock_isfile.side_effect = lambda path: path == "/dest/file" + + # Expected state after handling defaults + expected_state = { + "/dest/file": { + "source": "/dest/file", + "preserve_properties": True, + "dest": None + } + } + + # Call the function being tested + set_configs.handle_defaults(config) + + # Ensure the current default state was retrieved + mock_get_defaults_state.assert_called_once() + + # Verify that the file check was performed + mock_isfile.assert_called_once_with("/dest/file") + + # Ensure the file is removed since it exists + mock_remove.assert_called_once_with("/dest/file") + + # Ensure rmtree was not called since it's a file, not a directory + mock_rmtree.assert_not_called() + + # Verify that the updated state was saved + mock_set_defaults_state.assert_called_once_with(expected_state) + + @mock.patch.object(set_configs, 'set_defaults_state') + @mock.patch.object(set_configs, 'ConfigFile') + @mock.patch('os.remove') + @mock.patch('shutil.rmtree') + @mock.patch('os.path.isfile') + @mock.patch('os.path.exists') + @mock.patch.object(set_configs, 'get_defaults_state', return_value={ + "/dest/file": { + "source": "/dest/file", + "preserve_properties": True, + "dest": None + } + }) + def test_handle_defaults_state_exist_config_exist_is_dir( + self, mock_get_defaults_state, mock_exists, mock_isfile, + mock_rmtree, mock_remove, mock_config_file, + mock_set_defaults_state): + """Test handle_defaults() when the conf file exists and is a directory. + + This test simulates the scenario where the configuration exists as + a directory. + It verifies: + - The current default state is retrieved. + - The configuration is identified as a directory. + - The configuration directory is removed using shutil.rmtree(). + - The updated state is saved correctly after handling the directory. + + Mocks: + - get_defaults_state: Returns an existing state. + - os.path.exists: Simulates the destination directory exists. + - os.path.isfile: Ensures the destination is not a file + (it’s a directory). + - shutil.rmtree: Ensures the directory is removed. + - ConfigFile: Mocks the ConfigFile handling. + - set_defaults_state: Ensures the state is saved after processing. + """ + + config = { + 'config_files': [ + { + 'source': '/source/file', 'dest': '/dest/file' + } + ] + } + + # Simulate that destination exists and is a file + mock_exists.side_effect = lambda path: path == "/dest/file" + # Simulate that destination exists, but it's a directory, not a file + mock_isfile.side_effect = lambda path: False + + # Expected state after handling defaults + expected_state = { + "/dest/file": { + "source": "/dest/file", + "preserve_properties": True, + "dest": None + } + } + + # Create a mock instance of ConfigFile + mock_config_file_instance = mock_config_file.return_value + mock_config_file_instance.copy = mock.MagicMock() + + # Call the function being tested + set_configs.handle_defaults(config) + + # Ensure the current default state was retrieved + mock_get_defaults_state.assert_called_once() + + # Verify that a file check was performed + mock_isfile.assert_called_once_with("/dest/file") + + # Check that os.remove was called for the existing file + mock_remove.assert_not_called() + + # Since it's a directory, ensure rmtree was called to remove it + mock_rmtree.assert_called_once_with("/dest/file") + + # Verify that the updated state was saved + mock_set_defaults_state.assert_called_once_with(expected_state) + + @mock.patch.object(set_configs, 'set_defaults_state') + @mock.patch.object(set_configs, 'ConfigFile') + @mock.patch('os.path.exists') + @mock.patch.object(set_configs, 'get_defaults_state', return_value={ + "/dest/file": { + "source": "/source/file", + "dest": "/dest/file", + "preserve_properties": True + } + }) + def test_handle_defaults_state_exist_config_restored( + self, mock_get_defaults_state, mock_exists, mock_config_file, + mock_set_defaults_state): + """Test handle_defaults() when dest is not None in the default state. + + This test simulates the case where the destination in the default state + is not None, meaning a swap of source and destination is required. + It verifies: + - The current default state is retrieved. + - The source and destination are swapped in the ConfigFile + and restored. + - The state is updated correctly after processing. + + Mocks: + - get_defaults_state: Returns the current state. + - ConfigFile: Mocks the behavior of ConfigFile with swapped + source/dest. + - set_defaults_state: Ensures the state is saved after the swap and + processing. + """ + + # Configuration input (irrelevant in this case, as we're + # only focusing on state) + # Everything else is covered by other tests + config = {} + + copy = { + "source": "/dest/file", + "dest": "/source/file", # Swapped source and dest + "preserve_properties": True + } + + # Expected state after swapping source and dest + expected_state = { + "/dest/file": { + "source": "/dest/file", + "dest": "/source/file", # Swapped source and dest + "preserve_properties": True + } + } + + # Create a mock instance of ConfigFile + mock_config_file_instance = mock_config_file.return_value + mock_config_file_instance.copy = mock.MagicMock() + + # Call the function being tested + set_configs.handle_defaults(config) + + # Ensure the current default state was retrieved + mock_get_defaults_state.assert_called_once() + + # Verify that ConfigFile was instantiated with the swapped + # source and dest + mock_config_file.assert_called_once_with(**copy) + + # Ensure the copy method was called for the ConfigFile instance + mock_config_file_instance.copy.assert_called_once() + + # Ensure the copy method was called on the ConfigFile instance + mock_config_file_instance.copy.assert_called_once() + + # Verify that the updated state was saved + mock_set_defaults_state.assert_called_once_with(expected_state)