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
(cherry picked from commit 36c12676ff185f524a7f4d2f2f4c70b5d60462b4)

docker/base/set_configs.py

@@ -29,6 +29,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):
@@ -383,10 +386,157 @@ def handle_permissions(config):
                         set_perms(os.path.join(root, file_), uid, gid, perm)
 
 
+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":
@@ -395,6 +545,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')

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 <https://launchpad.net/bugs/2060855>`__

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)