Merge "Fix deploy templates arg validation" into stable/wallaby

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