openstack
/
watcher
Code Issues Proposed changes

Remove true/false return from action.execute() 

In watcher/applier/workflow_engine/default.py, we are checking the
return value of action.execute(). As the "TODO" above indicates it
(line 118), we should get rid of this and only flag an action as
failed if an exception was raised during its execute(). We will
need to update the related unit tests.

Change-Id: Ia8ff7abd9994c3504e733ccd1d629cafe9d4b839
Closes-Bug: #1548383
This commit is contained in:
Tin Lam 2016-02-24 23:23:30 -06:00
parent 59427eb0d9
commit d7f4f42772
5 changed files with 105 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
watcher/applier/action_plan/default.py
View File

@ -53,16 +53,15 @@ class DefaultActionPlanHandler(base.BaseActionPlanHandler):
event_types.EventTypes.LAUNCH_ACTION_PLAN,
ap_objects.State.ONGOING)
applier = default.DefaultApplier(self.ctx, self.applier_manager)
result = applier.execute(self.action_plan_uuid)
applier.execute(self.action_plan_uuid)
state = ap_objects.State.SUCCEEDED
except Exception as e:
LOG.exception(e)
result = False
state = ap_objects.State.FAILED
finally:
if result is True:
status = ap_objects.State.SUCCEEDED
else:
status = ap_objects.State.FAILED
# update state
self.notify(self.action_plan_uuid,
event_types.EventTypes.LAUNCH_ACTION_PLAN,
status)
state)

17
watcher/applier/workflow_engine/default.py
View File

@ -22,6 +22,7 @@ from taskflow import task
from watcher._i18n import _LE, _LW, _LC
from watcher.applier.workflow_engine import base
from watcher.common import exception
from watcher.objects import action as obj_action
LOG = log.getLogger(__name__)
@ -77,10 +78,9 @@ class DefaultWorkFlowEngine(base.BaseWorkFlowEngine):
e = engines.load(flow)
e.run()
return True
except Exception as e:
LOG.exception(e)
return False
raise exception.WorkflowExecutionException(error=e)
class TaskFlowActionContainer(task.Task):
@ -121,14 +121,9 @@ class TaskFlowActionContainer(task.Task):
try:
LOG.debug("Running action %s", self.name)
# todo(jed) remove return (true or false) raise an Exception
result = self.action.execute()
if result is not True:
self.engine.notify(self._db_action,
obj_action.State.FAILED)
else:
self.engine.notify(self._db_action,
obj_action.State.SUCCEEDED)
self.action.execute()
self.engine.notify(self._db_action,
obj_action.State.SUCCEEDED)
except Exception as e:
LOG.exception(e)
LOG.error(_LE('The WorkFlow Engine has failed '

3
watcher/common/exception.py
View File

@ -234,6 +234,9 @@ class PatchError(Invalid):
# decision engine
class WorkflowExecutionException(WatcherException):
msg_fmt = _('Workflow execution error: %(error)s')
class IllegalArgumentException(WatcherException):
msg_fmt = _('Illegal argument')

43
watcher/locale/watcher.pot
View File

@ -7,9 +7,9 @@
#, fuzzy
msgid ""
msgstr ""
"Project-Id-Version: python-watcher 0.24.1.dev4\n"
"Project-Id-Version: python-watcher 0.24.1.dev12\n"
"Report-Msgid-Bugs-To: EMAIL@ADDRESS\n"
"POT-Creation-Date: 2016-03-14 15:29+0100\n"
"POT-Creation-Date: 2016-03-16 18:18-0500\n"
"PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
"Language-Team: LANGUAGE <LL@li.org>\n"
@ -109,17 +109,17 @@ msgstr ""
msgid "Migration of type %(migration_type)s is not supported."
msgstr ""
#: watcher/applier/workflow_engine/default.py:134
#: watcher/applier/workflow_engine/default.py:129
#, python-format
msgid "The WorkFlow Engine has failed to execute the action %s"
msgstr ""
#: watcher/applier/workflow_engine/default.py:152
#: watcher/applier/workflow_engine/default.py:147
#, python-format
msgid "Revert action %s"
msgstr ""
#: watcher/applier/workflow_engine/default.py:158
#: watcher/applier/workflow_engine/default.py:153
msgid "Oops! We need disaster recover plan"
msgstr ""
@ -292,64 +292,69 @@ msgstr ""
msgid "Couldn't apply patch '%(patch)s'. Reason: %(reason)s"
msgstr ""
#: watcher/common/exception.py:239
#: watcher/common/exception.py:238
#, python-format
msgid "Workflow execution error: %(error)s"
msgstr ""
#: watcher/common/exception.py:242
msgid "Illegal argument"
msgstr ""
#: watcher/common/exception.py:243
#: watcher/common/exception.py:246
msgid "No such metric"
msgstr ""
#: watcher/common/exception.py:247
#: watcher/common/exception.py:250
msgid "No rows were returned"
msgstr ""
#: watcher/common/exception.py:251
#: watcher/common/exception.py:254
#, python-format
msgid "%(client)s connection failed. Reason: %(reason)s"
msgstr ""
#: watcher/common/exception.py:255
#: watcher/common/exception.py:258
msgid "'Keystone API endpoint is missing''"
msgstr ""
#: watcher/common/exception.py:259
#: watcher/common/exception.py:262
msgid "The list of hypervisor(s) in the cluster is empty"
msgstr ""
#: watcher/common/exception.py:263
#: watcher/common/exception.py:266
msgid "The metrics resource collector is not defined"
msgstr ""
#: watcher/common/exception.py:267
#: watcher/common/exception.py:270
msgid "the cluster state is not defined"
msgstr ""
#: watcher/common/exception.py:273
#: watcher/common/exception.py:276
#, python-format
msgid "The instance '%(name)s' is not found"
msgstr ""
#: watcher/common/exception.py:277
#: watcher/common/exception.py:280
msgid "The hypervisor is not found"
msgstr ""
#: watcher/common/exception.py:281
#: watcher/common/exception.py:284
#, python-format
msgid "Error loading plugin '%(name)s'"
msgstr ""
#: watcher/common/exception.py:285
#: watcher/common/exception.py:288
#, python-format
msgid "The identifier '%(name)s' is a reserved word"
msgstr ""
#: watcher/common/exception.py:289
#: watcher/common/exception.py:292
#, python-format
msgid "The %(name)s resource %(id)s is not soft deleted"
msgstr ""
#: watcher/common/exception.py:293
#: watcher/common/exception.py:296
msgid "Limit should be positive"
msgstr ""

105
watcher/tests/applier/workflow_engine/test_default_workflow_engine.py
View File

@ -25,6 +25,7 @@ from stevedore import extension
from watcher.applier.actions import base as abase
from watcher.applier.workflow_engine import default as tflow
from watcher.common import exception
from watcher.common import utils
from watcher import objects
from watcher.tests.db import base
@ -63,10 +64,15 @@ class TestDefaultWorkFlowEngine(base.DbTestCase):
context=self.context,
applier_manager=mock.MagicMock())
def test_execute(self):
@mock.patch('taskflow.engines.load')
@mock.patch('taskflow.patterns.graph_flow.Flow.link')
def test_execute(self, graph_flow, engines):
actions = mock.MagicMock()
result = self.engine.execute(actions)
self.assertEqual(True, result)
try:
self.engine.execute(actions)
self.assertTrue(engines.called)
except Exception as exc:
self.fail(exc)
def create_action(self, action_type, parameters, next):
action = {
@ -91,64 +97,84 @@ class TestDefaultWorkFlowEngine(base.DbTestCase):
for a in actions:
self.check_action_state(a, expected_state)
def test_execute_with_no_actions(self):
@mock.patch('taskflow.engines.load')
@mock.patch('taskflow.patterns.graph_flow.Flow.link')
def test_execute_with_no_actions(self, graph_flow, engines):
actions = []
result = self.engine.execute(actions)
self.assertEqual(True, result)
try:
self.engine.execute(actions)
self.assertFalse(graph_flow.called)
self.assertTrue(engines.called)
except Exception as exc:
self.fail(exc)
def test_execute_with_one_action(self):
actions = [self.create_action("nop", {'message': 'test'}, None)]
result = self.engine.execute(actions)
self.assertEqual(True, result)
self.check_actions_state(actions, objects.action.State.SUCCEEDED)
try:
self.engine.execute(actions)
self.check_actions_state(actions, objects.action.State.SUCCEEDED)
except Exception as exc:
self.fail(exc)
def test_execute_with_two_actions(self):
actions = []
next = self.create_action("sleep", {'duration': 0.0}, None)
first = self.create_action("nop", {'message': 'test'}, next.id)
second = self.create_action("sleep", {'duration': 0.0}, None)
first = self.create_action("nop", {'message': 'test'}, second.id)
actions.append(first)
actions.append(next)
actions.append(second)
result = self.engine.execute(actions)
self.assertEqual(True, result)
self.check_actions_state(actions, objects.action.State.SUCCEEDED)
try:
self.engine.execute(actions)
self.check_actions_state(actions, objects.action.State.SUCCEEDED)
except Exception as exc:
self.fail(exc)
def test_execute_with_three_actions(self):
actions = []
next2 = self.create_action("nop", {'message': 'next'}, None)
next = self.create_action("sleep", {'duration': 0.0}, next2.id)
first = self.create_action("nop", {'message': 'hello'}, next.id)
third = self.create_action("nop", {'message': 'next'}, None)
second = self.create_action("sleep", {'duration': 0.0}, third.id)
first = self.create_action("nop", {'message': 'hello'}, second.id)
self.check_action_state(first, objects.action.State.PENDING)
self.check_action_state(next, objects.action.State.PENDING)
self.check_action_state(next2, objects.action.State.PENDING)
self.check_action_state(second, objects.action.State.PENDING)
self.check_action_state(third, objects.action.State.PENDING)
actions.append(first)
actions.append(next)
actions.append(next2)
actions.append(second)
actions.append(third)
result = self.engine.execute(actions)
self.assertEqual(True, result)
self.check_actions_state(actions, objects.action.State.SUCCEEDED)
try:
self.engine.execute(actions)
self.check_actions_state(actions, objects.action.State.SUCCEEDED)
except Exception as exc:
self.fail(exc)
def test_execute_with_exception(self):
actions = []
next2 = self.create_action("no_exist", {'message': 'next'}, None)
next = self.create_action("sleep", {'duration': 0.0}, next2.id)
first = self.create_action("nop", {'message': 'hello'}, next.id)
third = self.create_action("no_exist", {'message': 'next'}, None)
second = self.create_action("sleep", {'duration': 0.0}, third.id)
first = self.create_action("nop", {'message': 'hello'}, second.id)
self.check_action_state(first, objects.action.State.PENDING)
self.check_action_state(next, objects.action.State.PENDING)
self.check_action_state(next2, objects.action.State.PENDING)
actions.append(first)
actions.append(next)
actions.append(next2)
self.check_action_state(second, objects.action.State.PENDING)
self.check_action_state(third, objects.action.State.PENDING)
actions.append(first)
actions.append(second)
actions.append(third)
self.assertRaises(exception.WorkflowExecutionException,
self.engine.execute, actions)
result = self.engine.execute(actions)
self.assertEqual(False, result)
self.check_action_state(first, objects.action.State.SUCCEEDED)
self.check_action_state(next, objects.action.State.SUCCEEDED)
self.check_action_state(next2, objects.action.State.FAILED)
self.check_action_state(second, objects.action.State.SUCCEEDED)
self.check_action_state(third, objects.action.State.FAILED)
@mock.patch("watcher.common.loader.default.DriverManager")
def test_execute_with_action_exception(self, m_driver):
@ -161,6 +187,7 @@ class TestDefaultWorkFlowEngine(base.DbTestCase):
obj=None),
namespace=FakeAction.namespace())
actions = [self.create_action("dontcare", {}, None)]
result = self.engine.execute(actions)
self.assertEqual(False, result)
self.assertRaises(exception.WorkflowExecutionException,
self.engine.execute, actions)
self.check_action_state(actions[0], objects.action.State.FAILED)