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 6e0f514c8f)

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)

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")