From f8aa548ca692e330bdc47bf32b3f1c309e50d45c Mon Sep 17 00:00:00 2001 From: Jill Rouleau Date: Wed, 7 Mar 2018 11:31:42 -0800 Subject: [PATCH] config download support for a fixed directory Makes config download write directly to the directory specified by --config-dir, instead of a tmpdir underneath that location. This change also breaks Config.download_config tasks into unique funtions to support accessing directory creation functionality independently from config retrieval and playbook creation. Change-Id: Ie00f71b12f56262985c47810be0e80402e9e558e Depends-On: Ibb59b044dc0c14bf48dc3d500536195230414c81 Depends-On: Icbd6dd65893f785b7ca8edf2eab46dbc7492e050 Closes-Bug: 1752118 --- ...oad-dont-use-tmpdirs-3641db9fd687f85e.yaml | 9 + tripleo_common/actions/config.py | 5 +- tripleo_common/tests/utils/test_config.py | 176 ++++++++++++++++-- tripleo_common/utils/config.py | 56 ++++-- 4 files changed, 213 insertions(+), 33 deletions(-) create mode 100644 releasenotes/notes/config-download-dont-use-tmpdirs-3641db9fd687f85e.yaml diff --git a/releasenotes/notes/config-download-dont-use-tmpdirs-3641db9fd687f85e.yaml b/releasenotes/notes/config-download-dont-use-tmpdirs-3641db9fd687f85e.yaml new file mode 100644 index 000000000..276a6685e --- /dev/null +++ b/releasenotes/notes/config-download-dont-use-tmpdirs-3641db9fd687f85e.yaml @@ -0,0 +1,9 @@ +--- +upgrade: + - | + ``openstack overcloud config download`` now writes directly to the + directory specified by ``--config-dir``. The directory contents will be + overwritten, preserving any contents not originating from the stack. A + ``--no-preserve-config`` option is provided which will cause the + ``--config-dir`` to be deleted and recreated if the``--config-dir`` + location exists. Tmpdirs are no longer used. diff --git a/tripleo_common/actions/config.py b/tripleo_common/actions/config.py index 85050f333..cc560c0e6 100644 --- a/tripleo_common/actions/config.py +++ b/tripleo_common/actions/config.py @@ -37,11 +37,14 @@ class GetOvercloudConfig(templates.ProcessTemplatesAction): """ def __init__(self, container=constants.DEFAULT_CONTAINER_NAME, - config_dir=tempfile.gettempdir(), + config_dir=None, container_config=constants.CONFIG_CONTAINER_NAME): super(GetOvercloudConfig, self).__init__(container) self.container = container self.config_dir = config_dir + if not self.config_dir: + self.config_dir = tempfile.mkdtemp(prefix='tripleo-', + suffix='-config') self.container_config = container_config def run(self, context): diff --git a/tripleo_common/tests/utils/test_config.py b/tripleo_common/tests/utils/test_config.py index ec4e848b8..e8c7d2f4c 100644 --- a/tripleo_common/tests/utils/test_config.py +++ b/tripleo_common/tests/utils/test_config.py @@ -35,9 +35,9 @@ class TestConfig(base.TestCase): @patch.object(ooo_config.shutil, 'copyfile') @patch.object(ooo_config.Config, '_mkdir') @patch.object(ooo_config.Config, '_open_file') - @mock.patch('tempfile.mkdtemp', autospec=True) + @patch.object(ooo_config.shutil, 'rmtree') def test_overcloud_config_generate_config(self, - mock_tmpdir, + mock_rmtree, mock_open, mock_mkdir, mock_copyfile): @@ -54,8 +54,7 @@ class TestConfig(base.TestCase): heat = mock.MagicMock() heat.stacks.get.return_value = fakes.create_tht_stack() self.config = ooo_config.Config(heat) - mock_tmpdir.return_value = "/tmp/tht" - self.config.download_config('overcloud', '/tmp', config_type_list) + self.config.download_config('overcloud', '/tmp/tht', config_type_list) expected_mkdir_calls = [call('/tmp/tht/%s' % r) for r in fake_role] mock_mkdir.assert_has_calls(expected_mkdir_calls, any_order=True) @@ -75,9 +74,9 @@ class TestConfig(base.TestCase): @patch.object(ooo_config.shutil, 'copyfile') @patch.object(ooo_config.Config, '_mkdir') @patch.object(ooo_config.Config, '_open_file') - @mock.patch('tempfile.mkdtemp', autospec=True) + @patch.object(ooo_config.shutil, 'rmtree') def test_overcloud_config_one_config_type(self, - mock_tmpdir, + mock_rmtree, mock_open, mock_mkdir, mock_copyfile): @@ -89,8 +88,8 @@ class TestConfig(base.TestCase): heat = mock.MagicMock() heat.stacks.get.return_value = fakes.create_tht_stack() self.config = ooo_config.Config(heat) - mock_tmpdir.return_value = "/tmp/tht" - self.config.download_config('overcloud', '/tmp', ['config_settings']) + self.config.download_config('overcloud', '/tmp/tht', + ['config_settings']) expected_mkdir_calls = [call('/tmp/tht/%s' % r) for r in fake_role] expected_calls = [call('/tmp/tht/%s/%s.yaml' % (r, expected_config_type)) @@ -100,15 +99,14 @@ class TestConfig(base.TestCase): @mock.patch('os.mkdir') @mock.patch('six.moves.builtins.open') - @mock.patch('tempfile.mkdtemp', autospec=True) - def test_overcloud_config_wrong_config_type(self, mock_tmpdir, + @patch.object(ooo_config.shutil, 'rmtree') + def test_overcloud_config_wrong_config_type(self, mock_rmtree, mock_open, mock_mkdir): - args = {'name': 'overcloud', 'config_dir': '/tmp', + args = {'name': 'overcloud', 'config_dir': '/tmp/tht', 'config_type': ['bad_config']} heat = mock.MagicMock() heat.stacks.get.return_value = fakes.create_tht_stack() self.config = ooo_config.Config(heat) - mock_tmpdir.return_value = "/tmp/tht" self.assertRaises( KeyError, self.config.download_config, *args) @@ -415,3 +413,157 @@ class TestConfig(base.TestCase): self.tmp_dir = self.useFixture(fixtures.TempDir()).path self.assertRaises(ValueError, self.config.download_config, stack, self.tmp_dir) + + @patch.object(ooo_config.shutil, 'copyfile') + @patch.object(ooo_config.Config, '_mkdir') + @patch.object(ooo_config.Config, '_open_file') + @patch.object(ooo_config.shutil, 'rmtree') + @patch.object(ooo_config.os.path, 'exists') + def test_overcloud_config_dont_preserve_config(self, + mock_os_path_exists, + mock_rmtree, + mock_open, + mock_mkdir, + mock_copyfile): + config_type_list = ['config_settings', 'global_config_settings', + 'logging_sources', 'monitoring_subscriptions', + 'service_config_settings', + 'service_metadata_settings', + 'service_names', + 'upgrade_batch_tasks', 'upgrade_tasks', + 'external_deploy_tasks'] + fake_role = [role for role in + fakes.FAKE_STACK['outputs'][1]['output_value']] + + mock_os_path_exists.get.return_value = True + heat = mock.MagicMock() + heat.stacks.get.return_value = fakes.create_tht_stack() + self.config = ooo_config.Config(heat) + self.config.download_config('overcloud', '/tmp/tht', config_type_list, + False) + + expected_rmtree_calls = [call('/tmp/tht')] + mock_rmtree.assert_has_calls(expected_rmtree_calls) + + expected_mkdir_calls = [call('/tmp/tht/%s' % r) for r in fake_role] + mock_mkdir.assert_has_calls(expected_mkdir_calls, any_order=True) + expected_calls = [] + for config in config_type_list: + for role in fake_role: + if 'external' in config: + continue + elif config == 'step_config': + expected_calls += [call('/tmp/tht/%s/%s.pp' % + (role, config))] + else: + expected_calls += [call('/tmp/tht/%s/%s.yaml' % + (role, config))] + mock_open.assert_has_calls(expected_calls, any_order=True) + + @patch.object(ooo_config.shutil, 'rmtree') + @patch.object(ooo_config.os.path, 'exists') + def test_create_config_dir(self, mock_os_path_exists, mock_rmtree): + mock_os_path_exists.get.return_value = True + heat = mock.MagicMock() + heat.stacks.get.return_value = fakes.create_tht_stack() + self.config = ooo_config.Config(heat) + self.config.create_config_dir('/tmp/tht', False) + expected_rmtree_calls = [call('/tmp/tht')] + mock_rmtree.assert_has_calls(expected_rmtree_calls) + + @patch('tripleo_common.utils.config.Config.get_config_dict') + @patch('tripleo_common.utils.config.Config.get_deployment_data') + def test_write_config(self, mock_deployment_data, mock_config_dict): + heat = mock.MagicMock() + self.config = ooo_config.Config(heat) + stack = mock.MagicMock() + heat.stacks.get.return_value = stack + + stack.outputs = [ + {'output_key': 'RoleNetHostnameMap', + 'output_value': { + 'Controller': { + 'ctlplane': [ + 'overcloud-controller-0.ctlplane.localdomain']}, + 'Compute': { + 'ctlplane': [ + 'overcloud-novacompute-0.ctlplane.localdomain', + 'overcloud-novacompute-1.ctlplane.localdomain', + 'overcloud-novacompute-2.ctlplane.localdomain']}}}, + {'output_key': 'ServerIdData', + 'output_value': { + 'server_ids': { + 'Controller': [ + '00b3a5e1-5e8e-4b55-878b-2fa2271f15ad'], + 'Compute': [ + 'a7db3010-a51f-4ae0-a791-2364d629d20d', + '8b07cd31-3083-4b88-a433-955f72039e2c', + '169b46f8-1965-4d90-a7de-f36fb4a830fe']}}}] + deployment_data, configs = \ + self._get_config_data('config_data.yaml') + self.configs = configs + + stack_data = self.config.fetch_config('overcloud') + mock_deployment_data.return_value = deployment_data + mock_config_dict.side_effect = self._get_config_dict + + config_dir = self.useFixture(fixtures.TempDir()).path + + self.config.write_config(stack_data, 'overcloud', config_dir) + + for f in ['Controller', + 'Compute', ]: + self.assertEqual( + yaml.safe_load( + open(os.path.join(config_dir, 'group_vars', f)).read()), + self._get_yaml_file(f)) + + for d in ['ControllerHostEntryDeployment', + 'NetworkDeployment', + 'MyExtraConfigPost', + 'MyPostConfig']: + self.assertEqual( + yaml.safe_load( + open(os.path.join(config_dir, 'Controller', + 'overcloud-controller-0', + d)).read()), + self._get_yaml_file(os.path.join( + 'overcloud-controller-0', + d))) + + for d in ['ComputeHostEntryDeployment', + 'NetworkDeployment', + 'MyExtraConfigPost']: + self.assertEqual( + yaml.safe_load( + open(os.path.join(config_dir, 'Compute', + 'overcloud-novacompute-0', + d)).read()), + self._get_yaml_file(os.path.join( + 'overcloud-novacompute-0', + d))) + + for d in ['ComputeHostEntryDeployment', + 'NetworkDeployment', + 'MyExtraConfigPost']: + self.assertEqual( + yaml.safe_load( + open(os.path.join(config_dir, 'Compute', + 'overcloud-novacompute-1', + d)).read()), + self._get_yaml_file(os.path.join( + 'overcloud-novacompute-1', + d))) + + for d in ['ComputeHostEntryDeployment', + 'NetworkDeployment', + 'MyExtraConfigPost', + 'AnsibleDeployment']: + self.assertEqual( + yaml.safe_load( + open(os.path.join(config_dir, 'Compute', + 'overcloud-novacompute-2', + d)).read()), + self._get_yaml_file(os.path.join( + 'overcloud-novacompute-2', + d))) diff --git a/tripleo_common/utils/config.py b/tripleo_common/utils/config.py index 83ad01813..992e6292b 100644 --- a/tripleo_common/utils/config.py +++ b/tripleo_common/utils/config.py @@ -18,7 +18,6 @@ import os import re import shutil import six -import tempfile import warnings import yaml @@ -143,24 +142,30 @@ class Config(object): str(e)) raise OSError(message) - def download_config(self, name, config_dir, config_type=None): + def create_config_dir(self, config_dir, preserve_config_dir=True): + # Create config directory + if os.path.exists(config_dir) and preserve_config_dir is False: + try: + self.log.info("Directory %s already exists, removing" + % config_dir) + shutil.rmtree(config_dir) + except OSError as e: + message = 'Failed to remove: %s, error: %s' % (config_dir, + str(e)) + raise OSError(message) + + def fetch_config(self, name): # Get the stack object stack = self.client.stacks.get(name) self.stack_outputs = {i['output_key']: i['output_value'] for i in stack.outputs} + return stack - # Create config directory - self._mkdir(config_dir) - tmp_path = tempfile.mkdtemp(prefix='tripleo-', - suffix='-config', - dir=config_dir) - self.log.info("Generating configuration under the directory: " - "%s" % tmp_path) - + def write_config(self, stack, name, config_dir, config_type=None): # Get role data: role_data = self.stack_outputs.get('RoleData', {}) for role_name, role in six.iteritems(role_data): - role_path = os.path.join(tmp_path, role_name) + role_path = os.path.join(config_dir, role_name) self._mkdir(role_path) for config in config_type or role.keys(): if config in constants.EXTERNAL_TASKS: @@ -190,7 +195,7 @@ class Config(object): default_flow_style=False) role_config = self.get_role_config() for config_name, config in six.iteritems(role_config): - conf_path = os.path.join(tmp_path, config_name + ".yaml") + conf_path = os.path.join(config_dir, config_name + ".yaml") with self._open_file(conf_path) as conf_file: if isinstance(config, list) or isinstance(config, dict): yaml.safe_dump(config, conf_file, default_flow_style=False) @@ -263,16 +268,16 @@ class Config(object): if deployment_name not in role_pre_deployments: role_pre_deployments.append(deployment_name) - env, templates_path = self.get_jinja_env(tmp_path) + env, templates_path = self.get_jinja_env(config_dir) - templates_dest = os.path.join(tmp_path, 'templates') + templates_dest = os.path.join(config_dir, 'templates') self._mkdir(templates_dest) shutil.copyfile(os.path.join(templates_path, 'heat-config.j2'), os.path.join(templates_dest, 'heat-config.j2')) - group_vars_dir = os.path.join(tmp_path, 'group_vars') + group_vars_dir = os.path.join(config_dir, 'group_vars') self._mkdir(group_vars_dir) - host_vars_dir = os.path.join(tmp_path, 'host_vars') + host_vars_dir = os.path.join(config_dir, 'host_vars') self._mkdir(host_vars_dir) for server, deployments in server_deployments.items(): @@ -281,7 +286,7 @@ class Config(object): for d in deployments: server_deployment_dir = os.path.join( - tmp_path, server_roles[server], server) + config_dir, server_roles[server], server) self._mkdir(server_deployment_dir) deployment_path = os.path.join( server_deployment_dir, d['deployment_name']) @@ -332,12 +337,23 @@ class Config(object): post_deployments=deployments['post_deployments'])) for role_name, role in six.iteritems(role_data): - role_path = os.path.join(tmp_path, role_name) + role_path = os.path.join(config_dir, role_name) shutil.copyfile( os.path.join(templates_path, 'deployments.yaml'), os.path.join(role_path, 'deployments.yaml')) self.log.info("The TripleO configuration has been successfully " - "generated into: %s" % tmp_path) - return tmp_path + "generated into: %s" % config_dir) + return config_dir + + def download_config(self, name, config_dir, config_type=None, + preserve_config_dir=True): + # One step does it all + stack = self.fetch_config(name) + self.create_config_dir(config_dir, preserve_config_dir) + self._mkdir(config_dir) + self.log.info("Generating configuration under the directory: " + "%s" % config_dir) + self.write_config(stack, name, config_dir, config_type) + return config_dir