openstack
/
python-tripleoclient
Code Issues Proposed changes

Merge "Simplify error handling in tripleo_deploy" into stable/ussuri

This commit is contained in:
Zuul 2020-08-07 16:09:42 +00:00 committed by Gerrit Code Review
parent 54b47e0f4c e352506699
commit ebc0aa15ae
4 changed files with 203 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
tripleoclient/tests/test_utils.py
View File

@ -114,12 +114,11 @@ class TestRunAnsiblePlaybook(TestCase):
return_value="/foo/inventory.yaml") return_value="/foo/inventory.yaml")
def test_run_success_default(self, mock_dump_artifact, mock_run, def test_run_success_default(self, mock_dump_artifact, mock_run,
mock_mkdirs, mock_exists, mock_mkstemp): mock_mkdirs, mock_exists, mock_mkstemp):
retcode, output = utils.run_ansible_playbook( utils.run_ansible_playbook(
playbook='existing.yaml', playbook='existing.yaml',
inventory='localhost,', inventory='localhost,',
workdir='/tmp' workdir='/tmp'
) )
self.assertEqual(retcode, 0)
@mock.patch('os.path.exists', return_value=True) @mock.patch('os.path.exists', return_value=True)
@mock.patch('os.makedirs') @mock.patch('os.makedirs')
@ -132,12 +131,11 @@ class TestRunAnsiblePlaybook(TestCase):
return_value="/foo/inventory.yaml") return_value="/foo/inventory.yaml")
def test_run_success_ansible_cfg(self, mock_dump_artifact, mock_run, def test_run_success_ansible_cfg(self, mock_dump_artifact, mock_run,
mock_mkdirs, mock_exists): mock_mkdirs, mock_exists):
retcode, output = utils.run_ansible_playbook( utils.run_ansible_playbook(
playbook='existing.yaml', playbook='existing.yaml',
inventory='localhost,', inventory='localhost,',
workdir='/tmp' workdir='/tmp'
) )
self.assertEqual(retcode, 0)
@mock.patch('tempfile.mkstemp', return_value=('foo', '/tmp/fooBar.cfg')) @mock.patch('tempfile.mkstemp', return_value=('foo', '/tmp/fooBar.cfg'))
@mock.patch('os.path.exists', return_value=True) @mock.patch('os.path.exists', return_value=True)
@ -152,13 +150,12 @@ class TestRunAnsiblePlaybook(TestCase):
def test_run_success_connection_local(self, mock_dump_artifact, mock_run, def test_run_success_connection_local(self, mock_dump_artifact, mock_run,
mock_mkdirs, mock_exists, mock_mkdirs, mock_exists,
mock_mkstemp): mock_mkstemp):
retcode, output = utils.run_ansible_playbook( utils.run_ansible_playbook(
playbook='existing.yaml', playbook='existing.yaml',
inventory='localhost,', inventory='localhost,',
workdir='/tmp', workdir='/tmp',
connection='local' connection='local'
) )
self.assertEqual(retcode, 0)
@mock.patch('os.makedirs', return_value=None) @mock.patch('os.makedirs', return_value=None)
@mock.patch('tempfile.mkstemp', return_value=('foo', '/tmp/fooBar.cfg')) @mock.patch('tempfile.mkstemp', return_value=('foo', '/tmp/fooBar.cfg'))
@ -173,14 +170,13 @@ class TestRunAnsiblePlaybook(TestCase):
def test_run_success_gathering_policy(self, mock_dump_artifact, mock_run, def test_run_success_gathering_policy(self, mock_dump_artifact, mock_run,
mock_exists, mock_mkstemp, mock_exists, mock_mkstemp,
mock_makedirs): mock_makedirs):
retcode, output = utils.run_ansible_playbook( utils.run_ansible_playbook(
playbook='existing.yaml', playbook='existing.yaml',
inventory='localhost,', inventory='localhost,',
workdir='/tmp', workdir='/tmp',
connection='local', connection='local',
gathering_policy='smart' gathering_policy='smart'
) )
self.assertEqual(retcode, 0)
@mock.patch('os.makedirs', return_value=None) @mock.patch('os.makedirs', return_value=None)
@mock.patch('tempfile.mkstemp', return_value=('foo', '/tmp/fooBar.cfg')) @mock.patch('tempfile.mkstemp', return_value=('foo', '/tmp/fooBar.cfg'))
@ -197,7 +193,7 @@ class TestRunAnsiblePlaybook(TestCase):
arglist = { arglist = {
'var_one': 'val_one', 'var_one': 'val_one',
} }
retcode, output = utils.run_ansible_playbook( utils.run_ansible_playbook(
playbook='existing.yaml', playbook='existing.yaml',
inventory='localhost,', inventory='localhost,',
workdir='/tmp', workdir='/tmp',
@ -205,7 +201,6 @@ class TestRunAnsiblePlaybook(TestCase):
gathering_policy='smart', gathering_policy='smart',
extra_vars=arglist extra_vars=arglist
) )
self.assertEqual(retcode, 0)
@mock.patch('six.moves.builtins.open') @mock.patch('six.moves.builtins.open')
@mock.patch('tripleoclient.utils.makedirs') @mock.patch('tripleoclient.utils.makedirs')
@ -214,7 +209,7 @@ class TestRunAnsiblePlaybook(TestCase):
ansible_runner.ArtifactLoader = mock.MagicMock() ansible_runner.ArtifactLoader = mock.MagicMock()
ansible_runner.Runner.run = mock.MagicMock(return_value=('', 0)) ansible_runner.Runner.run = mock.MagicMock(return_value=('', 0))
ansible_runner.runner_config = mock.MagicMock() ansible_runner.runner_config = mock.MagicMock()
retcode, output = utils.run_ansible_playbook( utils.run_ansible_playbook(
playbook='existing.yaml', playbook='existing.yaml',
inventory='localhost,', inventory='localhost,',
workdir='/tmp', workdir='/tmp',

186
tripleoclient/tests/v1/tripleo/test_tripleo_deploy.py
View File

@ -956,6 +956,187 @@ class TestDeployUndercloud(TestPluginV1):
mock_cleanupdirs.assert_called_once() mock_cleanupdirs.assert_called_once()
self.assertEqual(mock_killheat.call_count, 2) self.assertEqual(mock_killheat.call_count, 2)
@mock.patch.object(
ansible_runner.runner_config,
'RunnerConfig',
return_value=fakes.FakeRunnerConfig()
)
@mock.patch.object(
ansible_runner.Runner,
'run',
return_value=fakes.fake_ansible_runner_run_return(1)
)
@mock.patch('os.path.exists')
@mock.patch('os.chdir')
@mock.patch('tripleoclient.utils.reset_cmdline')
@mock.patch('tripleoclient.utils.copy_clouds_yaml')
@mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.'
'_download_stack_outputs')
@mock.patch('tripleo_common.actions.ansible.'
'write_default_ansible_cfg')
# TODO(cjeanner) drop once we have proper oslo.privsep
@mock.patch('os.chmod')
# TODO(cjeanner) drop once we have proper oslo.privsep
@mock.patch('subprocess.check_call', autospec=True)
# TODO(cjeanner) drop once we have proper oslo.privsep
@mock.patch('getpass.getuser', return_value='stack')
@mock.patch('os.mkdir')
@mock.patch('six.moves.builtins.open')
@mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.'
'_populate_templates_dir')
@mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.'
'_create_install_artifact', return_value='/tmp/foo.tar.bzip2')
@mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.'
'_cleanup_working_dirs')
@mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.'
'_create_working_dirs')
@mock.patch('tripleoclient.utils.wait_api_port_ready',
autospec=True)
@mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.'
'_deploy_tripleo_heat_templates', autospec=True,
return_value='undercloud, 0')
@mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.'
'_download_ansible_playbooks', autospec=True,
return_value='/foo')
@mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.'
'_launch_heat')
@mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.'
'_kill_heat')
@mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.'
'_configure_puppet')
@mock.patch('os.geteuid', return_value=0)
@mock.patch('os.environ', return_value='CREATE_COMPLETE')
@mock.patch('tripleoclient.utils.wait_for_stack_ready', return_value=True)
@mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.'
'_set_default_plan')
@mock.patch('ansible_runner.utils.dump_artifact', autospec=True,
return_value="/foo/inventory.yaml")
def test_take_action_ansible_err(self, mock_dump_artifact,
mock_def_plan, mock_poll,
mock_environ, mock_geteuid, mock_puppet,
mock_killheat, mock_launchheat,
mock_download, mock_tht,
mock_wait_for_port, mock_createdirs,
mock_cleanupdirs, mock_tarball,
mock_templates_dir, mock_open, mock_os,
mock_user, mock_cc, mock_chmod, mock_ac,
mock_outputs, mock_copy, mock_cmdline,
mock_chdir, mock_file_exists, mock_run,
mock_run_prepare):
parsed_args = self.check_parser(self.cmd,
['--local-ip', '127.0.0.1',
'--templates', '/tmp/thtroot',
'--stack', 'undercloud',
'--output-dir', '/my',
'--standalone',
'--standalone-role', 'Undercloud',
# TODO(cjeanner) drop once we have
# proper oslo.privsep
'--deployment-user', 'stack',
'-e', '/tmp/thtroot/puppet/foo.yaml',
'-e', '/tmp/thtroot//docker/bar.yaml',
'-e', '/tmp/thtroot42/notouch.yaml',
'-e', '~/custom.yaml',
'-e', 'something.yaml',
'-e', '../../../outside.yaml'], [])
mock_file_exists.return_value = True
fake_orchestration = mock_launchheat(parsed_args)
self.assertRaises(exceptions.DeploymentError,
self.cmd.take_action, parsed_args)
mock_createdirs.assert_called_once()
mock_puppet.assert_called_once()
mock_launchheat.assert_called_with(parsed_args)
mock_tht.assert_called_once_with(self.cmd, fake_orchestration,
parsed_args)
mock_download.assert_called_with(self.cmd, fake_orchestration,
'undercloud', 'Undercloud',
sys.executable)
mock_tarball.assert_called_once()
mock_cleanupdirs.assert_called_once()
self.assertEqual(mock_killheat.call_count, 2)
@mock.patch('os.chdir')
@mock.patch('tripleoclient.utils.reset_cmdline')
@mock.patch('tripleoclient.utils.copy_clouds_yaml')
@mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.'
'_download_stack_outputs')
@mock.patch('tripleo_common.actions.ansible.'
'write_default_ansible_cfg')
# TODO(cjeanner) drop once we have proper oslo.privsep
@mock.patch('os.chmod')
# TODO(cjeanner) drop once we have proper oslo.privsep
@mock.patch('subprocess.check_call', autospec=True)
# TODO(cjeanner) drop once we have proper oslo.privsep
@mock.patch('getpass.getuser', return_value='stack')
@mock.patch('os.mkdir')
@mock.patch('six.moves.builtins.open')
@mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.'
'_populate_templates_dir')
@mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.'
'_create_install_artifact', return_value='/tmp/foo.tar.bzip2')
@mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.'
'_cleanup_working_dirs')
@mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.'
'_create_working_dirs')
@mock.patch('tripleoclient.utils.wait_api_port_ready')
@mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.'
'_deploy_tripleo_heat_templates', autospec=True,
return_value='undercloud, 0')
@mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.'
'_download_ansible_playbooks', autospec=True,
return_value='/foo')
@mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.'
'_launch_heat')
@mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.'
'_kill_heat')
@mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.'
'_configure_puppet')
@mock.patch('os.geteuid', return_value=0)
@mock.patch('os.environ', return_value='CREATE_COMPLETE')
@mock.patch('tripleoclient.utils.wait_for_stack_ready', return_value=True)
@mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.'
'_set_default_plan')
def test_take_action_other_err(self,
mock_def_plan, mock_poll,
mock_environ, mock_geteuid, mock_puppet,
mock_killheat, mock_launchheat,
mock_download, mock_tht,
mock_wait_for_port, mock_createdirs,
mock_cleanupdirs, mock_tarball,
mock_templates_dir, mock_open, mock_os,
mock_user, mock_cc, mock_chmod, mock_ac,
mock_outputs, mock_copy, mock_cmdline,
mock_chdir):
parsed_args = self.check_parser(self.cmd,
['--local-ip', '127.0.0.1',
'--templates', '/tmp/thtroot',
'--stack', 'undercloud',
'--output-dir', '/my',
'--standalone',
'--standalone-role', 'Undercloud',
# TODO(cjeanner) drop once we have
# proper oslo.privsep
'--deployment-user', 'stack',
'-e', '/tmp/thtroot/puppet/foo.yaml',
'-e', '/tmp/thtroot//docker/bar.yaml',
'-e', '/tmp/thtroot42/notouch.yaml',
'-e', '~/custom.yaml',
'-e', 'something.yaml',
'-e', '../../../outside.yaml'], [])
mock_wait_for_port.side_effect = exceptions.DeploymentError
self.assertRaises(exceptions.DeploymentError,
self.cmd.take_action, parsed_args)
mock_createdirs.assert_called_once()
mock_puppet.assert_called_once()
mock_launchheat.assert_called_with(parsed_args)
mock_tht.assert_not_called()
mock_download.assert_not_called()
mock_tarball.assert_called_once()
mock_cleanupdirs.assert_called_once()
self.assertEqual(mock_killheat.call_count, 1)
@mock.patch('tripleoclient.utils.reset_cmdline') @mock.patch('tripleoclient.utils.reset_cmdline')
@mock.patch('tripleoclient.utils.copy_clouds_yaml') @mock.patch('tripleoclient.utils.copy_clouds_yaml')
def test_take_action(self, mock_copy, mock_cmdline): def test_take_action(self, mock_copy, mock_cmdline):
@ -963,6 +1144,7 @@ class TestDeployUndercloud(TestPluginV1):
['--local-ip', '127.0.0.1', ['--local-ip', '127.0.0.1',
'--templates', '/tmp/thtroot', '--templates', '/tmp/thtroot',
'--stack', 'undercloud', '--stack', 'undercloud',
'--standalone',
'--output-dir', '/my'], []) '--output-dir', '/my'], [])
self.assertRaises(exceptions.DeploymentError, self.assertRaises(exceptions.DeploymentError,
self.cmd.take_action, parsed_args) self.cmd.take_action, parsed_args)
@ -970,8 +1152,7 @@ class TestDeployUndercloud(TestPluginV1):
@mock.patch('tripleoclient.utils.reset_cmdline') @mock.patch('tripleoclient.utils.reset_cmdline')
@mock.patch('tripleoclient.utils.copy_clouds_yaml') @mock.patch('tripleoclient.utils.copy_clouds_yaml')
@mock.patch('tripleoclient.v1.tripleo_deploy.Deploy._standalone_deploy', @mock.patch('tripleoclient.v1.tripleo_deploy.Deploy._standalone_deploy')
return_value=1)
def test_take_action_failure(self, mock_deploy, mock_copy, mock_cmdline): def test_take_action_failure(self, mock_deploy, mock_copy, mock_cmdline):
parsed_args = self.check_parser(self.cmd, parsed_args = self.check_parser(self.cmd,
['--local-ip', '127.0.0.1', ['--local-ip', '127.0.0.1',
@ -979,6 +1160,7 @@ class TestDeployUndercloud(TestPluginV1):
'--stack', 'undercloud', '--stack', 'undercloud',
'--output-dir', '/my', '--output-dir', '/my',
'--standalone'], []) '--standalone'], [])
mock_deploy.side_effect = exceptions.DeploymentError
self.assertRaises(exceptions.DeploymentError, self.assertRaises(exceptions.DeploymentError,
self.cmd.take_action, parsed_args) self.cmd.take_action, parsed_args)
mock_copy.assert_called_once() mock_copy.assert_called_once()

14
tripleoclient/utils.py
View File

@ -251,7 +251,7 @@ def run_ansible_playbook(playbook, inventory, workdir, playbook_dir=None,
parallel_run=False, parallel_run=False,
callback_whitelist=constants.ANSIBLE_CWL, callback_whitelist=constants.ANSIBLE_CWL,
ansible_cfg=None, ansible_timeout=30, ansible_cfg=None, ansible_timeout=30,
reproduce_command=False, fail_on_rc=True, reproduce_command=False,
timeout=None): timeout=None):
"""Simple wrapper for ansible-playbook. """Simple wrapper for ansible-playbook.
@ -342,11 +342,6 @@ def run_ansible_playbook(playbook, inventory, workdir, playbook_dir=None,
and retry purposes. and retry purposes.
:type reproduce_command: Boolean :type reproduce_command: Boolean
:param fail_on_rc: Enable or disable raising an exception whenever the
return code from the playbook execution results in a
non 0 exit code. The default is True.
:type fail_on_rc: Boolean
:param timeout: Timeout for ansible to finish playbook execution (minutes). :param timeout: Timeout for ansible to finish playbook execution (minutes).
:type timeout: int :type timeout: int
""" """
@ -703,16 +698,11 @@ def run_ansible_playbook(playbook, inventory, workdir, playbook_dir=None,
if not quiet: if not quiet:
LOG.error(err_msg) LOG.error(err_msg)
if fail_on_rc:
raise RuntimeError(err_msg) raise RuntimeError(err_msg)
LOG.info( LOG.info(
'Ansible execution success. playbook: {}'.format( 'Ansible execution success. playbook: {}'.format(
playbook playbook))
)
)
return rc, status
def convert(data): def convert(data):

31
tripleoclient/v1/tripleo_deploy.py
View File

@ -1221,7 +1221,7 @@ class Deploy(command.Command):
# Set default plan if not specified by user # Set default plan if not specified by user
self._set_default_plan() self._set_default_plan()
rc = 1 is_complete = False
try: try:
# NOTE(bogdando): Look for the unique virtual update mark matching # NOTE(bogdando): Look for the unique virtual update mark matching
# the heat stack name we are going to create below. If found the # the heat stack name we are going to create below. If found the
@ -1313,7 +1313,7 @@ class Deploy(command.Command):
operation[k] = ','.join([operation[k], v]) operation[k] = ','.join([operation[k], v])
else: else:
operation[k] = v operation[k] = v
rc = utils.run_ansible_playbook( utils.run_ansible_playbook(
inventory=os.path.join( inventory=os.path.join(
self.ansible_dir, self.ansible_dir,
'inventory.yaml' 'inventory.yaml'
@ -1321,15 +1321,8 @@ class Deploy(command.Command):
workdir=self.ansible_dir, workdir=self.ansible_dir,
verbosity=utils.playbook_verbosity(self=self), verbosity=utils.playbook_verbosity(self=self),
extra_env_variables=extra_env_var, extra_env_variables=extra_env_var,
fail_on_rc=False, **operation)
**operation is_complete = True
)[0]
if rc != 0:
break
except Exception as e:
self.log.error("Exception: %s" % six.text_type(e))
self.log.error(traceback.print_exc())
raise exceptions.DeploymentError(six.text_type(e))
finally: finally:
if not parsed_args.keep_running: if not parsed_args.keep_running:
self._kill_heat(parsed_args) self._kill_heat(parsed_args)
@ -1348,7 +1341,7 @@ class Deploy(command.Command):
if tar_filename: if tar_filename:
self.log.warning('Install artifact is located at %s' % self.log.warning('Install artifact is located at %s' %
tar_filename) tar_filename)
if not parsed_args.output_only and rc != 0: if not is_complete:
# We only get here on error. # We only get here on error.
# Alter the stack virtual state for failed deployments # Alter the stack virtual state for failed deployments
if (self.stack_update_mark and if (self.stack_update_mark and
@ -1366,7 +1359,6 @@ class Deploy(command.Command):
self.log.error(DEPLOY_FAILURE_MESSAGE.format( self.log.error(DEPLOY_FAILURE_MESSAGE.format(
self.heat_launch.install_tmp self.heat_launch.install_tmp
)) ))
raise exceptions.DeploymentError('Deployment failed.')
else: else:
# We only get here if no errors # We only get here if no errors
if parsed_args.output_only: if parsed_args.output_only:
@ -1402,21 +1394,18 @@ class Deploy(command.Command):
_('Not creating the stack %s virtual update mark ' _('Not creating the stack %s virtual update mark '
'file') % parsed_args.stack) 'file') % parsed_args.stack)
return rc
def take_action(self, parsed_args): def take_action(self, parsed_args):
self.log.debug("take_action(%s)" % parsed_args) self.log.debug("take_action(%s)" % parsed_args)
try: try:
if parsed_args.standalone: if parsed_args.standalone:
if self._standalone_deploy(parsed_args) != 0: self._standalone_deploy(parsed_args)
msg = _('Deployment failed.')
self.log.error(msg)
raise exceptions.DeploymentError(msg)
else: else:
msg = _('Non-standalone is currently not supported') msg = _('Non-standalone is currently not supported')
self.log.error(msg) self.log.error(msg)
raise exceptions.DeploymentError(msg) except Exception as ex:
self.log.error("Exception: %s" % six.text_type(ex))
self.log.error(traceback.print_exc())
raise exceptions.DeploymentError(six.text_type(ex))
finally: finally:
# Copy clouds.yaml from /etc/openstack so credentials can be # Copy clouds.yaml from /etc/openstack so credentials can be
# read by the deployment user and not only root. # read by the deployment user and not only root.