diff --git a/tripleoclient/constants.py b/tripleoclient/constants.py index 4e45e8e76..1503cbfb2 100644 --- a/tripleoclient/constants.py +++ b/tripleoclient/constants.py @@ -205,6 +205,16 @@ try: except (configparser.NoOptionError, FileNotFoundError): UNDERCLOUD_OUTPUT_DIR = CLOUD_HOME_DIR +# regex patterns to exclude when looking for unused params +# - exclude *Image params as they may be unused because the service is not +# enabled +# - exclude SwiftFetchDir*Tempurl because it's used by ceph and generated by us +# - exclude PythonInterpreter because it's generated by us and only used +# in some custom scripts +UNUSED_PARAMETER_EXCLUDES_RE = ['^(Docker|Container).*Image$', + '^SwiftFetchDir(Get|Put)Tempurl$', + '^PythonInterpreter$'] + EXPORT_PASSWORD_EXCLUDE_PATTERNS = [ 'ceph.*' ] diff --git a/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py b/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py index 15cdc29d4..07ed19130 100644 --- a/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py +++ b/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py @@ -118,6 +118,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): shutil.rmtree = self.real_shutil self.mock_tar.stop() + @mock.patch('tripleoclient.utils.build_stack_data', autospec=True) @mock.patch('tripleo_common.utils.plan.default_image_params', autospec=True) @mock.patch('tripleoclient.utils.get_rc_params', @@ -159,7 +160,8 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): mock_get_ctlplane_attrs, mock_nic_ansiblei, mock_process_env, mock_roles_data, mock_container_prepare, mock_generate_password, - mock_rc_params, mock_default_image_params): + mock_rc_params, mock_default_image_params, + mock_stack_data): fixture = deployment.DeploymentWorkflowFixture() self.useFixture(fixture) clients = self.app.client_manager @@ -186,7 +188,8 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): "id": "network id" } mock_get_template_contents.return_value = [{}, "template"] - + mock_stack_data.return_value = {'environment_parameters': {}, + 'heat_resource_tree': {}} parsed_args = self.check_parser(self.cmd, arglist, verifylist) baremetal = clients.baremetal @@ -233,6 +236,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): mock_create_tempest_deployer_input.assert_called_with() mock_copy.assert_called_once() + @mock.patch('tripleoclient.utils.build_stack_data', autospec=True) @mock.patch('tripleo_common.utils.plan.default_image_params', autospec=True) @mock.patch('tripleoclient.utils.get_rc_params', autospec=True) @@ -275,7 +279,8 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): mock_get_ctlplane_attrs, mock_process_env, mock_roles_data, mock_container_prepare, mock_generate_password, - mock_rc_params, mock_default_image_params): + mock_rc_params, mock_default_image_params, + mock_stack_data): fixture = deployment.DeploymentWorkflowFixture() self.useFixture(fixture) plane_management_fixture = deployment.PlanManagementFixture() @@ -288,7 +293,8 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): verifylist = [ ('templates', '/usr/share/openstack-tripleo-heat-templates/'), ] - + mock_stack_data.return_value = {'environment_parameters': {}, + 'heat_resource_tree': {}} mock_tmpdir.return_value = self.tmp_dir.path clients = self.app.client_manager @@ -336,6 +342,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): mock_validate_args.assert_called_once_with(parsed_args) self.assertFalse(mock_invoke_plan_env_wf.called) + @mock.patch('tripleoclient.utils.build_stack_data', autospec=True) @mock.patch('tripleoclient.utils.get_rc_params', autospec=True) @mock.patch('tripleo_common.utils.plan.generate_passwords', return_value={}) @@ -377,7 +384,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): mock_chdir, mock_overcloudrc, mock_process_env, mock_roles_data, mock_image_prepare, mock_generate_password, - mock_rc_params): + mock_rc_params, mock_stack_data): fixture = deployment.DeploymentWorkflowFixture() self.useFixture(fixture) plane_management_fixture = deployment.PlanManagementFixture() @@ -392,7 +399,8 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): ('templates', '/usr/share/openstack-tripleo-heat-templates/'), ('skip_deploy_identifier', True) ] - + mock_stack_data.return_value = {'environment_parameters': {}, + 'heat_resource_tree': {}} mock_tmpdir.return_value = "/tmp/tht" clients = self.app.client_manager @@ -421,6 +429,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): self.cmd.take_action(parsed_args) mock_copy.assert_called_once() + @mock.patch('tripleoclient.utils.build_stack_data', autospec=True) @mock.patch('tripleoclient.utils.get_rc_params', autospec=True) @mock.patch('tripleo_common.utils.plan.generate_passwords', return_value={}) @@ -459,7 +468,8 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): mock_roles_data, mock_image_prepare, mock_generate_password, - mock_rc_params): + mock_rc_params, + mock_stack_data): fixture = deployment.DeploymentWorkflowFixture() self.useFixture(fixture) plane_management_fixture = deployment.PlanManagementFixture() @@ -481,7 +491,8 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): "id": "network id" } mock_get_template_contents.return_value = [{}, "template"] - + mock_stack_data.return_value = {'environment_parameters': {}, + 'heat_resource_tree': {}} parsed_args = self.check_parser(self.cmd, arglist, verifylist) baremetal = clients.baremetal @@ -582,6 +593,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): def _fake_heat_deploy(self, stack, stack_name, template_path, parameters, environments, timeout, tht_root, env, run_validations, + roles_file, env_files_tracker=None, deployment_options=None): assertEqual( @@ -670,7 +682,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): self.cmd, {}, 'overcloud', '/fake/path/' + constants.OVERCLOUD_YAML_NAME, {}, ['~/overcloud-env.json'], 1, '/fake/path', {}, False, - deployment_options=None, env_files_tracker=None) + None, deployment_options=None, env_files_tracker=None) @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.' '_heat_deploy', autospec=True) @@ -883,6 +895,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): self.cmd.take_action, parsed_args) + @mock.patch('tripleoclient.utils.build_stack_data', autospec=True) @mock.patch('tripleo_common.utils.plan.default_image_params', autospec=True) @mock.patch('tripleoclient.utils.get_rc_params', autospec=True) @@ -929,7 +942,8 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): mock_image_prepare, mock_generate_password, mock_rc_params, - mock_default_image_params): + mock_default_image_params, + mock_stack_data): fixture = deployment.DeploymentWorkflowFixture() self.useFixture(fixture) plane_management_fixture = deployment.PlanManagementFixture() @@ -937,6 +951,8 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): utils_fixture = deployment.UtilsFixture() self.useFixture(utils_fixture) + mock_stack_data.return_value = {'environment_parameters': {}, + 'heat_resource_tree': {}} arglist = ['--templates', '--ntp-server', 'ntp'] verifylist = [ ('templates', '/usr/share/openstack-tripleo-heat-templates/'), @@ -1263,7 +1279,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): 'UndercloudHostsEntries': ['192.168.0.1 uc.ctlplane.localhost uc.ctlplane'], 'CtlplaneNetworkAttributes': {}}, mock.ANY, - 451, mock.ANY, mock.ANY, False, + 451, mock.ANY, mock.ANY, False, None, deployment_options={}, env_files_tracker=mock.ANY)], mock_hd.mock_calls) self.assertIn( diff --git a/tripleoclient/v1/overcloud_deploy.py b/tripleoclient/v1/overcloud_deploy.py index e50330f38..5a162a135 100644 --- a/tripleoclient/v1/overcloud_deploy.py +++ b/tripleoclient/v1/overcloud_deploy.py @@ -260,6 +260,7 @@ class DeployOvercloud(command.Command): def _heat_deploy(self, stack, stack_name, template_path, parameters, env_files, timeout, tht_root, env, run_validations, + roles_file, env_files_tracker=None, deployment_options=None): """Verify the Baremetal nodes are available and do a stack update""" @@ -279,6 +280,10 @@ class DeployOvercloud(command.Command): template_file=template_path) files = dict(list(template_files.items()) + list(env_files.items())) + workflow_params.check_deprecated_parameters( + self.clients, stack_name, tht_root, template, + roles_file, files, env_files_tracker) + self.log.info("Deploying templates in the directory {0}".format( os.path.abspath(tht_root))) deployment.deploy_without_plan( @@ -417,6 +422,7 @@ class DeployOvercloud(command.Command): parsed_args.stack, parameters, env_files, parsed_args.timeout, env, parsed_args.run_validations, + parsed_args.roles_file, env_files_tracker=env_files_tracker, deployment_options=deployment_options) @@ -444,6 +450,7 @@ class DeployOvercloud(command.Command): stack_name, parameters, env_files, timeout, env, run_validations, + roles_file, env_files_tracker=None, deployment_options=None): overcloud_yaml = os.path.join(tht_root, constants.OVERCLOUD_YAML_NAME) @@ -452,6 +459,7 @@ class DeployOvercloud(command.Command): parameters, env_files, timeout, tht_root, env, run_validations, + roles_file, env_files_tracker=env_files_tracker, deployment_options=deployment_options) except Exception as e: diff --git a/tripleoclient/workflows/parameters.py b/tripleoclient/workflows/parameters.py index 83cc02533..d9ecd7905 100644 --- a/tripleoclient/workflows/parameters.py +++ b/tripleoclient/workflows/parameters.py @@ -12,6 +12,7 @@ import logging import os +import re import yaml from heatclient.common import template_utils @@ -19,6 +20,7 @@ from tripleo_common.utils import stack_parameters as stk_parameters from tripleoclient.constants import ANSIBLE_TRIPLEO_PLAYBOOKS from tripleoclient.constants import OVERCLOUD_YAML_NAME +from tripleoclient.constants import UNUSED_PARAMETER_EXCLUDES_RE from tripleoclient import exceptions from tripleoclient import utils from tripleoclient.workflows import roles @@ -95,8 +97,9 @@ def build_derived_params_environment(clients, stack_name, # Get role list role_list = roles.get_roles( - clients, roles_file, template, files, - env_files_tracker, detail=False, valid=True) + clients, roles_file, tht_root, stack_name, + template, files, env_files_tracker, + detail=False, valid=True) invoke_plan_env_workflows( clients, @@ -109,6 +112,123 @@ def build_derived_params_environment(clients, stack_name, ) +def check_deprecated_parameters(clients, stack_name, tht_root, template, + roles_file, files, env_files_tracker): + """Checks for deprecated parameters and adds warning if present. + + :param clients: application client object. + :type clients: Object + + :param container: Name of the stack container. + :type container: String + """ + + # Get role list + role_list = roles.get_roles( + clients, roles_file, tht_root, stack_name, + template, files, env_files_tracker, + detail=False, valid=True) + + # Build stack_data + stack_data = utils.build_stack_data( + clients, stack_name, template, + files, env_files_tracker) + user_params = stack_data.get('environment_parameters', {}) + heat_resource_tree = stack_data.get('heat_resource_tree', {}) + heat_resource_tree_params = heat_resource_tree.get('parameters', {}) + heat_resource_tree_resources = heat_resource_tree.get('resources', {}) + all_params = heat_resource_tree_params.keys() + parameter_groups = [ + i.get('parameter_groups') + for i in heat_resource_tree_resources.values() + if i.get('parameter_groups') + ] + params_role_specific_tag = [ + i.get('name') + for i in heat_resource_tree_params.values() + if 'tags' in i and 'role_specific' in i['tags'] + ] + + r = re.compile(".*Count") + filtered_names = list(filter(r.match, all_params)) + valid_role_name_list = list() + for name in filtered_names: + default = heat_resource_tree_params[name].get('default', 0) + if default and int(default) > 0: + role_name = name.rstrip('Count') + if [i for i in role_list if i == role_name]: + valid_role_name_list.append(role_name) + + deprecated_params = [ + i[0] for i in parameter_groups + if i[0].get('label') == 'deprecated' + ] + # We are setting a frozenset here because python 3 complains that dict is + # a unhashable type. + # On user_defined, we check if the size is higher than 0 because an empty + # frozenset still is a subset of a frozenset, so we can't use issubset + # here. + user_params_keys = frozenset(user_params.keys()) + deprecated_result = [ + { + 'parameter': i, + 'deprecated': True, + 'user_defined': len( + [x for x in frozenset(i) if x in user_params_keys]) > 0 + } + for i in deprecated_params + ] + unused_params = [i for i in user_params.keys() if i not in all_params] + user_provided_role_specific = [ + v for i in role_list + for k, v in user_params.items() + if k in i + ] + invalid_role_specific_params = [ + i for i in user_provided_role_specific + if i in params_role_specific_tag + ] + deprecated_parameters = [ + param['parameter'] for param in deprecated_result + if param.get('user_defined') + ] + + if deprecated_parameters: + deprecated_join = ', '.join(deprecated_parameters) + LOG.warning( + 'WARNING: Following parameter(s) are deprecated and still ' + 'defined. Deprecated parameters will be removed soon!' + ' {deprecated_join}'.format( + deprecated_join=deprecated_join + ) + ) + + # exclude our known params that may not be used + ignore_re = re.compile('|'.join(UNUSED_PARAMETER_EXCLUDES_RE)) + unused_params = [p for p in unused_params if not ignore_re.search(p)] + + if unused_params: + unused_join = ', '.join(unused_params) + LOG.warning( + 'WARNING: Following parameter(s) are defined but not ' + 'currently used. These parameters ' + 'may be valid but not in use due to the service or ' + 'deployment configuration.' + ' {unused_join}'.format( + unused_join=unused_join + ) + ) + + if invalid_role_specific_params: + invalid_join = ', '.join(invalid_role_specific_params) + LOG.warning( + 'WARNING: Following parameter(s) are not supported as ' + 'role-specific inputs. {invalid_join}'.format( + invalid_join=invalid_join + ) + ) + + def generate_fencing_parameters(clients, nodes_json, delay, ipmi_level, ipmi_cipher, ipmi_lanplus): """Generate and return fencing parameters. diff --git a/tripleoclient/workflows/roles.py b/tripleoclient/workflows/roles.py index fb8d11b3a..6f69dd58e 100644 --- a/tripleoclient/workflows/roles.py +++ b/tripleoclient/workflows/roles.py @@ -50,8 +50,9 @@ def get_roles(clients, roles_file, tht_root, valid_roles = [] for name in role_names: - role_count = stack_data['parameters'].get( - name + 'Count', {}).get('default', 0) + role_count = stack_data['heat_resource_tree'][ + 'parameters'].get(name + 'Count', {}).get( + 'default', 0) if role_count > 0: valid_roles.append(name)