Refactor _validate_args and _update_args_from_answers_file
Make the methods functions, as they do not need the class properties. _validate_args: Move the checks for non environment files outside the 'parsed_args.environment_files' condition. We want to check for plan_environment_file, networks_file and answers_file even if no environment files are provided. _update_args_from_answers_file: Rename the variable to s/args/parsed_args/ to use the same name as elsewhere. Change-Id: I9b541cebf45a155653b39d94994016b44e29f9ac
This commit is contained in:
Harald Jensås
parent
bf03fe4a9c
commit
dc8b39adf0
|
|
@ -258,8 +258,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
|
|||
'_deploy_postconfig', autospec=True)
|
||||
@mock.patch('tripleo_common.update.add_breakpoints_cleanup_into_env',
|
||||
autospec=True)
|
||||
@mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.'
|
||||
'_validate_args')
|
||||
@mock.patch('tripleoclient.v1.overcloud_deploy._validate_args')
|
||||
@mock.patch('heatclient.common.template_utils.get_template_contents',
|
||||
autospec=True)
|
||||
@mock.patch('os.chmod', autospec=True)
|
||||
|
|
@ -361,8 +360,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
|
|||
'_deploy_postconfig', autospec=True)
|
||||
@mock.patch('tripleo_common.update.add_breakpoints_cleanup_into_env',
|
||||
autospec=True)
|
||||
@mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.'
|
||||
'_validate_args')
|
||||
@mock.patch('tripleoclient.v1.overcloud_deploy._validate_args')
|
||||
@mock.patch('tripleoclient.utils.create_parameters_env', autospec=True)
|
||||
@mock.patch('heatclient.common.template_utils.get_template_contents',
|
||||
autospec=True)
|
||||
|
|
@ -651,7 +649,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
|
|||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||
|
||||
self.assertRaises(oscexc.CommandError,
|
||||
self.cmd._validate_args,
|
||||
overcloud_deploy._validate_args,
|
||||
parsed_args)
|
||||
|
||||
@mock.patch('os.path.isfile', autospec=True)
|
||||
|
|
@ -668,7 +666,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
|
|||
mock_isfile.side_effect = [False, True]
|
||||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||
|
||||
self.cmd._validate_args(parsed_args)
|
||||
overcloud_deploy._validate_args(parsed_args)
|
||||
calls = [mock.call(env_path),
|
||||
mock.call(env_path.replace(".yaml", ".j2.yaml"))]
|
||||
mock_isfile.assert_has_calls(calls)
|
||||
|
|
@ -868,8 +866,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
|
|||
@mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.'
|
||||
'_deploy_postconfig', autospec=True)
|
||||
@mock.patch('tripleo_common.update.add_breakpoints_cleanup_into_env')
|
||||
@mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.'
|
||||
'_validate_args')
|
||||
@mock.patch('tripleoclient.v1.overcloud_deploy._validate_args')
|
||||
@mock.patch('tripleoclient.utils.create_parameters_env', autospec=True)
|
||||
@mock.patch('tripleoclient.utils.create_tempest_deployer_input',
|
||||
autospec=True)
|
||||
|
|
|
|||
|
|
@ -53,6 +53,72 @@ def _validate_args_environment_dir(dirs):
|
|||
": {0}".format(", ".join(not_found)))
|
||||
|
||||
|
||||
def _update_args_from_answers_file(parsed_args):
|
||||
if parsed_args.answers_file is None:
|
||||
return
|
||||
|
||||
with open(parsed_args.answers_file, 'r') as answers_file:
|
||||
answers = yaml.safe_load(answers_file)
|
||||
|
||||
if parsed_args.templates is None:
|
||||
parsed_args.templates = answers['templates']
|
||||
if 'environments' in answers:
|
||||
if parsed_args.environment_files is not None:
|
||||
answers['environments'].extend(parsed_args.environment_files)
|
||||
parsed_args.environment_files = answers['environments']
|
||||
|
||||
|
||||
def _validate_args(parsed_args):
|
||||
if parsed_args.templates is None and parsed_args.answers_file is None:
|
||||
raise oscexc.CommandError(
|
||||
"You must specify either --templates or --answers-file")
|
||||
|
||||
if parsed_args.baremetal_deployment and not parsed_args.deployed_server:
|
||||
raise oscexc.CommandError(
|
||||
"Error: --deployed-server must be used when using "
|
||||
"--baremetal-deployment")
|
||||
|
||||
if (parsed_args.baremetal_deployment
|
||||
and (parsed_args.config_download_only or parsed_args.setup_only)):
|
||||
raise oscexc.CommandError(
|
||||
"Error: --config-download-only/--setup-only must not be used when "
|
||||
"using --baremetal-deployment")
|
||||
|
||||
if parsed_args.environment_directories:
|
||||
_validate_args_environment_dir(parsed_args.environment_directories)
|
||||
|
||||
not_found = [x for x in [parsed_args.networks_file,
|
||||
parsed_args.plan_environment_file,
|
||||
parsed_args.plan_environment_file,
|
||||
parsed_args.answers_file]
|
||||
if x and not os.path.isfile(x)]
|
||||
|
||||
jinja2_envs = []
|
||||
if parsed_args.environment_files:
|
||||
for env in parsed_args.environment_files:
|
||||
if env.endswith(".j2.yaml"):
|
||||
jinja2_envs.append(env)
|
||||
continue
|
||||
|
||||
# Tolerate missing file if there's a j2.yaml file that will
|
||||
# be rendered in the plan but not available locally (yet)
|
||||
if (not os.path.isfile(env)
|
||||
and not os.path.isfile(env.replace(".yaml", ".j2.yaml"))):
|
||||
not_found.append(env)
|
||||
|
||||
if not_found:
|
||||
raise oscexc.CommandError(
|
||||
"Error: The following files were not found: {}".format(
|
||||
", ".join(not_found)))
|
||||
|
||||
if jinja2_envs:
|
||||
rewritten_paths = [e.replace(".j2.yaml", ".yaml") for e in jinja2_envs]
|
||||
raise oscexc.CommandError(
|
||||
"Error: The following jinja2 files were provided: {}. Did you "
|
||||
"mean {}?".format(' -e '.join(jinja2_envs),
|
||||
' -e '.join(rewritten_paths)))
|
||||
|
||||
|
||||
class DeployOvercloud(command.Command):
|
||||
"""Deploy Overcloud"""
|
||||
|
||||
|
|
@ -70,18 +136,6 @@ class DeployOvercloud(command.Command):
|
|||
'Assuming --deployed-server')
|
||||
parsed_args.deployed_server = True
|
||||
|
||||
def _update_args_from_answers_file(self, args):
|
||||
if args.answers_file is not None:
|
||||
with open(args.answers_file, 'r') as answers_file:
|
||||
answers = yaml.safe_load(answers_file)
|
||||
|
||||
if args.templates is None:
|
||||
args.templates = answers['templates']
|
||||
if 'environments' in answers:
|
||||
if args.environment_files is not None:
|
||||
answers['environments'].extend(args.environment_files)
|
||||
args.environment_files = answers['environments']
|
||||
|
||||
def _update_parameters(self, args, parameters,
|
||||
tht_root, user_tht_root):
|
||||
parameters['RootStackName'] = args.stack
|
||||
|
|
@ -355,69 +409,6 @@ class DeployOvercloud(command.Command):
|
|||
|
||||
utils.remove_known_hosts(overcloud_ip_or_fqdn)
|
||||
|
||||
def _validate_args(self, parsed_args):
|
||||
if parsed_args.templates is None and parsed_args.answers_file is None:
|
||||
raise oscexc.CommandError(
|
||||
"You must specify either --templates or --answers-file")
|
||||
|
||||
if parsed_args.environment_files:
|
||||
nonexisting_envs = []
|
||||
jinja2_envs = []
|
||||
for env_file in parsed_args.environment_files:
|
||||
|
||||
if env_file.endswith(".j2.yaml"):
|
||||
jinja2_envs.append(env_file)
|
||||
elif not os.path.isfile(env_file):
|
||||
# Tolerate missing file if there's a j2.yaml file that will
|
||||
# be rendered in the plan but not available locally (yet)
|
||||
if not os.path.isfile(env_file.replace(".yaml",
|
||||
".j2.yaml")):
|
||||
nonexisting_envs.append(env_file)
|
||||
|
||||
# Check networks_file existence
|
||||
if parsed_args.networks_file:
|
||||
if not os.path.isfile(parsed_args.networks_file):
|
||||
nonexisting_envs.append(parsed_args.networks_file)
|
||||
|
||||
# check plan_environment_file existence
|
||||
if parsed_args.plan_environment_file:
|
||||
if not os.path.isfile(parsed_args.plan_environment_file):
|
||||
nonexisting_envs.append(parsed_args.plan_environment_file)
|
||||
|
||||
# check answers_file existence
|
||||
if parsed_args.answers_file:
|
||||
if not os.path.isfile(parsed_args.answers_file):
|
||||
nonexisting_envs.append(parsed_args.answers_file)
|
||||
|
||||
if jinja2_envs:
|
||||
rewritten_paths = [e.replace(".j2.yaml", ".yaml")
|
||||
for e in jinja2_envs]
|
||||
raise oscexc.CommandError(
|
||||
"Error: The following jinja2 files were provided: -e "
|
||||
"{}. Did you mean -e {}?".format(
|
||||
' -e '.join(jinja2_envs),
|
||||
' -e '.join(rewritten_paths)))
|
||||
if nonexisting_envs:
|
||||
raise oscexc.CommandError(
|
||||
"Error: The following files were not found: {0}".format(
|
||||
", ".join(nonexisting_envs)))
|
||||
|
||||
if (parsed_args.baremetal_deployment
|
||||
and not parsed_args.deployed_server):
|
||||
raise oscexc.CommandError(
|
||||
"Error: --deployed-server must be used when using "
|
||||
"--baremetal-deployment")
|
||||
|
||||
if (parsed_args.baremetal_deployment
|
||||
and (parsed_args.config_download_only
|
||||
or parsed_args.setup_only)):
|
||||
raise oscexc.CommandError(
|
||||
"Error: --config-download-only/--setup-only must not be "
|
||||
"used when using --baremetal-deployment")
|
||||
|
||||
if parsed_args.environment_directories:
|
||||
_validate_args_environment_dir(parsed_args.environment_directories)
|
||||
|
||||
def _provision_baremetal(self, parsed_args, tht_root):
|
||||
|
||||
if not parsed_args.baremetal_deployment:
|
||||
|
|
@ -938,7 +929,7 @@ class DeployOvercloud(command.Command):
|
|||
sc_logger = logging.getLogger("swiftclient")
|
||||
sc_logger.setLevel(logging.CRITICAL)
|
||||
|
||||
self._validate_args(parsed_args)
|
||||
_validate_args(parsed_args)
|
||||
|
||||
# Throw warning if deprecated service is enabled and
|
||||
# ask user if deployment should still be continued.
|
||||
|
|
@ -946,7 +937,7 @@ class DeployOvercloud(command.Command):
|
|||
utils.check_deprecated_service_is_enabled(
|
||||
parsed_args.environment_files)
|
||||
|
||||
self._update_args_from_answers_file(parsed_args)
|
||||
_update_args_from_answers_file(parsed_args)
|
||||
|
||||
if parsed_args.dry_run:
|
||||
self.log.info("Validation Finished")
|
||||
|
|
|
|||
Loading…
Reference in New Issue