From c870147325bed36da01f60694235a519e22ba707 Mon Sep 17 00:00:00 2001 From: Alex Schultz Date: Thu, 13 Jan 2022 13:01:05 -0700 Subject: [PATCH] Fix deploy templates arg validation With the addition of the vip file validation, we broke the check for the templates arg resulting in a stack trace if templates is not provided rather then a user friendly error message. This change moves the vip file validation to it's own check later in the process once the working directory has been created. The working directory is not created until after the validation of params. NOTE: This has been reworked for wallaby as there was some refactoring in Xena that made this conflict a bunch. Change-Id: I67b7dc71542533607e3280b4fac3acfb95203b41 Closes-Bug: #1955573 (cherry picked from commit 6e0f514c8f656f9b934252d39a929fac90ba24b8) --- .../overcloud_deploy/test_overcloud_deploy.py | 24 ++++++++++------- tripleoclient/v1/overcloud_deploy.py | 27 ++++++++++--------- 2 files changed, 30 insertions(+), 21 deletions(-) diff --git a/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py b/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py index 632be9cfd..c466b14f5 100644 --- a/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py +++ b/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py @@ -281,6 +281,8 @@ 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_vip_file') @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.' '_validate_args') @mock.patch('heatclient.common.template_utils.get_template_contents', @@ -291,6 +293,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): @mock.patch('tripleoclient.utils.makedirs') def test_tht_deploy(self, mock_md, mock_tmpdir, mock_cd, mock_chmod, mock_get_template_contents, mock_validate_args, + mock_validate_vip_file, mock_breakpoints_cleanup, mock_postconfig, mock_invoke_plan_env_wf, mock_get_undercloud_host_entry, @@ -361,8 +364,8 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): utils_overcloud_fixture.mock_deploy_tht.assert_called_with( output_dir=self.cmd.working_dir) - mock_validate_args.assert_called_once_with(parsed_args, - self.cmd.working_dir) + mock_validate_args.assert_called_once_with(parsed_args) + mock_validate_vip_file.assert_not_called() self.assertFalse(mock_invoke_plan_env_wf.called) @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.' @@ -392,6 +395,8 @@ 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_vip_file') @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.' '_validate_args') @mock.patch('tripleoclient.utils.create_parameters_env', autospec=True) @@ -403,6 +408,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): self, mock_tmpdir, mock_rm, mock_get_template_contents, mock_create_parameters_env, mock_validate_args, + mock_validate_vip_file, mock_breakpoints_cleanup, mock_postconfig, mock_stack_network_check, mock_ceph_fsid, mock_swift_rgw, @@ -701,10 +707,9 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - working_dir = self.tmp_dir.join('working_dir') self.assertRaises(oscexc.CommandError, self.cmd._validate_args, - parsed_args, working_dir) + parsed_args) @mock.patch('os.path.isfile', autospec=True) def test_validate_args_missing_rendered_files(self, mock_isfile): @@ -719,9 +724,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): mock_isfile.side_effect = [False, True] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - working_dir = self.tmp_dir.join('working_dir') - - self.cmd._validate_args(parsed_args, working_dir) + self.cmd._validate_args(parsed_args) calls = [mock.call(env_path), mock.call(env_path.replace(".yaml", ".j2.yaml"))] @@ -934,6 +937,8 @@ 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_vip_file') @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.' '_validate_args') @mock.patch('tripleoclient.utils.create_parameters_env', autospec=True) @@ -948,6 +953,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): mock_create_tempest_deployer_input, mock_create_parameters_env, mock_validate_args, + mock_validate_vip_file, mock_breakpoints_cleanup, mock_deploy_post_config, mock_stack_network_check, @@ -1045,8 +1051,8 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): mock_create_tempest_deployer_input.assert_called_with( output_dir=self.cmd.working_dir) - mock_validate_args.assert_called_once_with(parsed_args, - self.cmd.working_dir) + mock_validate_args.assert_called_once_with(parsed_args) + mock_validate_vip_file.assert_not_called() mock_copy.assert_called_once() @mock.patch('tripleoclient.utils.get_rc_params', autospec=True) diff --git a/tripleoclient/v1/overcloud_deploy.py b/tripleoclient/v1/overcloud_deploy.py index 0443108ed..68770a173 100644 --- a/tripleoclient/v1/overcloud_deploy.py +++ b/tripleoclient/v1/overcloud_deploy.py @@ -490,7 +490,7 @@ class DeployOvercloud(command.Command): utils.remove_known_hosts(overcloud_ip_or_fqdn) - def _validate_args(self, parsed_args, working_dir): + 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") @@ -565,16 +565,16 @@ class DeployOvercloud(command.Command): self._validate_args_environment_directory( parsed_args.environment_directories) - if parsed_args.vip_file: - # Check vip_file only used with network data v2 - networks_file_path = utils.get_networks_file_path( - working_dir, parsed_args.stack) - if not utils.is_network_data_v2(networks_file_path): - raise oscexc.CommandError( - 'The --vip-file option can only be used in combination ' - 'with a network data v2 format networks file. The ' - 'provided file {} is network data v1 format'.format( - networks_file_path)) + def _validate_vip_file(self, stack, working_dir): + # Check vip_file only used with network data v2 + networks_file_path = utils.get_networks_file_path( + working_dir, stack) + if not utils.is_network_data_v2(networks_file_path): + raise oscexc.CommandError( + 'The --vip-file option can only be used in combination ' + 'with a network data v2 format networks file. The ' + 'provided file {} is network data v1 format'.format( + networks_file_path)) def _validate_args_environment_directory(self, directories): default = os.path.expanduser(constants.DEFAULT_ENV_DIRECTORY) @@ -1214,6 +1214,8 @@ class DeployOvercloud(command.Command): self._update_args_from_answers_file(parsed_args) + self._validate_args(parsed_args) + # Make a copy of the files provided on command line in the working dir # If the command is re-run without providing the argument the "backup" # from the previous run in the working dir is used. @@ -1225,7 +1227,8 @@ class DeployOvercloud(command.Command): utils.check_deprecated_service_is_enabled( parsed_args.environment_files) - self._validate_args(parsed_args, self.working_dir) + if parsed_args.vip_file: + self._validate_vip_file(parsed_args.stack, self.working_dir) if parsed_args.dry_run: self.log.info("Validation Finished")