From e313c896219b3942d66a00d5388ba2cda6bcce87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Andr=C3=A9?= Date: Wed, 21 Jun 2017 15:52:16 +0200 Subject: [PATCH] Introduce merge and preserve_properties for config_files The `merge` option will basically copy all the files recursively inside the container without removing the existing files or directory, while the `preserve_properties` option keeps the file attributes (permissions, ownership, timestamps) in the container. This is useful if you have extracted some config files from a container and want to use them in another container based on the same image. This makes TripleO usage of Kolla much more robust. Change-Id: I78dcec741a941dc21adba33ba33a6dc6ff1d217c --- docker/base/set_configs.py | 66 +++++++++++++------ ...ig_files_new_options-0267e1ab804335ba.yaml | 7 ++ tests/test_set_config.py | 41 ++++++++---- 3 files changed, 82 insertions(+), 32 deletions(-) create mode 100644 releasenotes/notes/config_files_new_options-0267e1ab804335ba.yaml diff --git a/docker/base/set_configs.py b/docker/base/set_configs.py index 59dc2f38bf..2290908172 100644 --- a/docker/base/set_configs.py +++ b/docker/base/set_configs.py @@ -57,25 +57,42 @@ class ConfigFileBadState(ExitingException): class ConfigFile(object): - def __init__(self, source, dest, owner, perm, optional=False): + def __init__(self, source, dest, owner=None, perm=None, optional=False, + preserve_properties=False, merge=False): self.source = source self.dest = dest self.owner = owner self.perm = perm self.optional = optional + self.merge = merge + self.preserve_properties = preserve_properties def __str__(self): return ''.format(self.source, self.dest) - def _copy_dir(self, source, dest): - LOG.info('Copying dir from %s to %s', source, dest) - shutil.copytree(source, dest) - for root, dirs, files in os.walk(dest): - for dir_ in dirs: - self._set_permission(os.path.join(root, dir_)) - for file_ in files: - self._set_permission(os.path.join(root, file_)) + def _copy_file(self, source, dest): + self._delete_path(dest) + # dest endswith / means copy the to folder + LOG.info('Copying file from %s to %s', source, dest) + shutil.copy(source, dest) + self._set_properties(source, dest) + + def _merge_directories(self, source, dest): + LOG.info('Copying %s to %s', source, dest) + if os.path.isdir(source): + if os.path.exists(dest) and not os.path.isdir(dest): + self._delete_path(dest) + if not os.path.isdir(dest): + os.makedirs(dest) + self._set_properties(source, dest) + + dir_content = os.listdir(source) + for to_copy in dir_content: + self._merge_directories(os.path.join(source, to_copy), + os.path.join(dest, to_copy)) + else: + self._copy_file(source, dest) def _delete_path(self, path): if not os.path.exists(path): @@ -92,13 +109,18 @@ class ConfigFile(object): if not os.path.exists(parent_path): os.makedirs(parent_path) - def _copy_file(self, source, dest): - # dest endswith / means copy the to folder - LOG.info('Coping file from %s to %s', source, dest) - shutil.copy(source, dest) - self._set_permission(dest) + def _set_properties(self, source, dest): + if self.preserve_properties: + self._set_properties_from_file(source, dest) + else: + self._set_properties_from_conf(dest) - def _set_permission(self, path): + def _set_properties_from_file(self, source, dest): + shutil.copystat(source, dest) + stat = os.stat(source) + os.chown(dest, stat.st_uid, stat.st_gid) + + def _set_properties_from_conf(self, path): handle_permissions({'owner': self.owner, 'path': path, 'perm': self.perm}) @@ -118,12 +140,10 @@ class ConfigFile(object): # otherwise means copy the source to dest if dest.endswith(os.sep): dest = os.path.join(dest, os.path.basename(source)) - self._delete_path(dest) + if not self.merge: + self._delete_path(dest) self._create_parent_dirs(dest) - if os.path.isdir(source): - self._copy_dir(source, dest) - else: - self._copy_file(source, dest) + self._merge_directories(source, dest) def _cmp_file(self, source, dest): # check exsit @@ -203,7 +223,7 @@ class ConfigFile(object): def validate_config(config): - required_keys = {'source', 'dest', 'owner', 'perm'} + required_keys = {'source', 'dest'} if 'command' not in config: raise InvalidConfig('Config is missing required "command" key') @@ -214,6 +234,10 @@ def validate_config(config): if not data.viewkeys() >= required_keys: message = 'Config is missing required keys: %s' % required_keys raise InvalidConfig(message) + if ('owner' not in data or 'perm' not in data) \ + and not data.get('preserve_properties', False): + raise InvalidConfig( + 'Config needs preserve_properties or owner and perm') def validate_source(data): diff --git a/releasenotes/notes/config_files_new_options-0267e1ab804335ba.yaml b/releasenotes/notes/config_files_new_options-0267e1ab804335ba.yaml new file mode 100644 index 0000000000..a5d307f23f --- /dev/null +++ b/releasenotes/notes/config_files_new_options-0267e1ab804335ba.yaml @@ -0,0 +1,7 @@ +--- +features: + - New `merge` option to config_files, that copies all the files recursively + inside the container without removing the existing files. + - New `preserve_properties` option to config_files, that keeps all of the + files and directories attributes (permissions, ownership, timestamps) when + copying them in the container. diff --git a/tests/test_set_config.py b/tests/test_set_config.py index fa8e45301d..aff97b3bb1 100644 --- a/tests/test_set_config.py +++ b/tests/test_set_config.py @@ -115,14 +115,34 @@ class ConfigFileTest(base.BaseTestCase): mock_isdir.assert_called_with(config_file.dest) mock_remove.assert_called_with(config_file.dest) - @mock.patch('os.chmod') - @mock.patch.object(set_configs, 'handle_permissions') - def test_set_permission(self, - mock_handle_permissions, - mock_chmod): + @mock.patch('shutil.copystat') + @mock.patch('os.stat') + @mock.patch('os.chown') + def test_set_properties_from_file(self, + mock_chown, + mock_stat, + mock_copystat): + + stat_result = mock.MagicMock() + mock_stat.return_value = stat_result config_file = copy.deepcopy(FAKE_CONFIG_FILE) - config_file._set_permission(config_file.dest) + config_file._set_properties_from_file(config_file.source, + config_file.dest) + + mock_copystat.assert_called_with(config_file.source, config_file.dest) + mock_stat.assert_called_with(config_file.source) + mock_chown.assert_called_with(config_file.dest, stat_result.st_uid, + stat_result.st_gid) + + @mock.patch('os.chmod') + @mock.patch.object(set_configs, 'handle_permissions') + def test_set_properties_from_conf(self, + mock_handle_permissions, + mock_chmod): + + config_file = copy.deepcopy(FAKE_CONFIG_FILE) + config_file._set_properties_from_conf(config_file.dest) mock_handle_permissions.assert_called_with({'owner': 'user1', 'path': config_file.dest, 'perm': '0644'}) @@ -167,14 +187,14 @@ class ConfigFileTest(base.BaseTestCase): mock_copy_file.assert_called_with(config_file.source, config_file.dest) - @mock.patch.object(set_configs.ConfigFile, '_copy_dir') + @mock.patch.object(set_configs.ConfigFile, '_merge_directories') @mock.patch('os.path.isdir', return_value=True) @mock.patch.object(set_configs.ConfigFile, '_create_parent_dirs') @mock.patch.object(set_configs.ConfigFile, '_delete_path') @mock.patch('glob.glob') def test_copy_one_source_dir(self, mock_glob, mock_delete_path, mock_create_parent_dirs, mock_isdir, - mock_copy_dir): + mock_merge_directories): config_file = copy.deepcopy(FAKE_CONFIG_FILE) mock_glob.return_value = [config_file.source] @@ -184,9 +204,8 @@ class ConfigFileTest(base.BaseTestCase): mock_glob.assert_called_with(config_file.source) mock_delete_path.assert_called_with(config_file.dest) mock_create_parent_dirs.assert_called_with(config_file.dest) - mock_isdir.assert_called_with(config_file.source) - mock_copy_dir.assert_called_with(config_file.source, - config_file.dest) + mock_merge_directories.assert_called_with(config_file.source, + config_file.dest) @mock.patch.object(set_configs.ConfigFile, '_copy_file') @mock.patch('os.path.isdir', return_value=False)