Fix kolla_set_configs --check with a directory

There are several issues with kolla_set_configs --check:

1. We calculate the destination path incorrectly when comparing a file
   in a directory, due to passing arguments to os.path.relpath in the
   wrong order
2. For directories that have not changed, we also attempt to compare
   them as files, which fails when they are open()ed.
3. If the config JSON does not have a config_files key, it fails with a
   KeyError.

The first two issues affect the fluentd container, which specifies
directories as the source, without using a glob. The third affects OVN
containers.

This patch fixes these issues.

Closes-Bug: #1890567

Change-Id: I8921befe51da4282121443849177a7ca5ebe8822
This commit is contained in:
Mark Goddard 2020-08-06 10:50:25 +01:00 committed by Radosław Piliszek
parent b4dde581c5
commit c5320eb223
3 changed files with 53 additions and 5 deletions

View File

@ -204,8 +204,8 @@ class ConfigFile(object):
return False return False
for file_ in files: for file_ in files:
full_path = os.path.join(root, file_) full_path = os.path.join(root, file_)
dest_full_path = os.path.join(dest, os.path.relpath(source, dest_full_path = os.path.join(dest, os.path.relpath(full_path,
full_path)) source))
if not self._cmp_file(full_path, dest_full_path): if not self._cmp_file(full_path, dest_full_path):
return False return False
return True return True
@ -225,8 +225,9 @@ class ConfigFile(object):
# otherwise means copy the source to dest # otherwise means copy the source to dest
if dest.endswith(os.sep): if dest.endswith(os.sep):
dest = os.path.join(dest, os.path.basename(source)) dest = os.path.join(dest, os.path.basename(source))
if os.path.isdir(source) and not self._cmp_dir(source, dest): if os.path.isdir(source):
bad_state_files.append(source) if not self._cmp_dir(source, dest):
bad_state_files.append(source)
elif not self._cmp_file(source, dest): elif not self._cmp_file(source, dest):
bad_state_files.append(source) bad_state_files.append(source)
if len(bad_state_files) != 0: if len(bad_state_files) != 0:
@ -404,7 +405,7 @@ def execute_config_strategy(config):
def execute_config_check(config): def execute_config_check(config):
for data in config['config_files']: for data in config.get('config_files', []):
config_file = ConfigFile(**data) config_file = ConfigFile(**data)
config_file.check() config_file.check()

View File

@ -0,0 +1,6 @@
---
fixes:
- |
Fixes an issue with the ``kolla_set_configs --check`` command when the
source is a directory. `LP#1890567
<https://bugs.launchpad.net/kolla/+bug/1890567>`__

View File

@ -288,3 +288,44 @@ class ConfigFileTest(base.BaseTestCase):
'/foo/bar.conf'), '/foo/bar.conf'),
mock.call('/var/lib/kolla/config_files/bar.yml', mock.call('/var/lib/kolla/config_files/bar.yml',
'/foo/bar.yml')]) '/foo/bar.yml')])
@mock.patch.object(set_configs.ConfigFile, '_cmp_file')
@mock.patch.object(set_configs.ConfigFile, '_cmp_dir')
@mock.patch('os.path.isdir', return_value=False)
@mock.patch('glob.glob')
def test_check_source_dir(self, mock_glob, mock_isdir, mock_cmp_dir,
mock_cmp_file):
config_file = set_configs.ConfigFile(
'/var/lib/kolla/config_files/bar', '/foo', 'user1', '0644')
mock_glob.return_value = ['/var/lib/kolla/config_files/bar']
mock_isdir.return_value = True
mock_cmp_dir.return_value = True
config_file.check()
mock_isdir.assert_called_once_with('/var/lib/kolla/config_files/bar')
mock_cmp_dir.assert_called_once_with(
'/var/lib/kolla/config_files/bar', '/foo')
mock_cmp_file.assert_not_called()
@mock.patch.object(set_configs.ConfigFile, '_cmp_file')
@mock.patch.object(set_configs.ConfigFile, '_cmp_dir')
@mock.patch('os.path.isdir', return_value=False)
@mock.patch('glob.glob')
def test_check_source_dir_no_equal(self, mock_glob, mock_isdir,
mock_cmp_dir, mock_cmp_file):
config_file = set_configs.ConfigFile(
'/var/lib/kolla/config_files/bar', '/foo', 'user1', '0644')
mock_glob.return_value = ['/var/lib/kolla/config_files/bar']
mock_isdir.return_value = True
mock_cmp_dir.return_value = False
self.assertRaises(set_configs.ConfigFileBadState,
config_file.check)
mock_isdir.assert_called_once_with('/var/lib/kolla/config_files/bar')
mock_cmp_dir.assert_called_once_with(
'/var/lib/kolla/config_files/bar', '/foo')
mock_cmp_file.assert_not_called()