openstack/rally
Code Issues Proposed changes

Save task's title and description

Browse Source 
Task format v2 allows to specify task title and description. It should
be saved in the database, but was not.

This patch moves initialization of TaskConfig before creating task
object. If config is totally invalid, it is not a task and there is no
reason to store trace in the db.

Change-Id: I3be9c21ca634a92fc85685f4f7b333cb79636f6d
This commit is contained in:
Andrey Kurilin 2017-09-14 18:16:17 +03:00
parent 7165f31726
commit e4e105821e
6 changed files with 94 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
rally/api.py
View File

@ -392,6 +392,13 @@ class _Task(APIGroup):
task = objects.Task(deployment_uuid=deployment, temporary=True)
deployment = objects.Deployment.get(deployment)
try:
config = engine.TaskConfig(config)
except Exception as e:
if logging.is_debug():
LOG.exception("Invalid Task")
raise exceptions.InvalidTaskException(str(e))
engine.TaskEngine(config, task, deployment).validate()
def start(self, deployment, config, task=None, abort_on_sla_failure=False):
@ -429,8 +436,17 @@ class _Task(APIGroup):
uuid=deployment["uuid"],
status=deployment["status"])
try:
config = engine.TaskConfig(config)
except Exception as e:
if logging.is_debug():
LOG.exception("Invalid Task")
raise exceptions.InvalidTaskException(str(e))
if task is None:
task = objects.Task(deployment_uuid=deployment["uuid"])
task = objects.Task(deployment_uuid=deployment["uuid"],
title=config.title,
description=config.description)
task_engine = engine.TaskEngine(
config, task, deployment,

13
rally/task/engine.py
View File

@ -237,23 +237,14 @@ class TaskEngine(object):
abort_on_sla_failure=False):
"""TaskEngine constructor.
:param config: Dict with configuration of specified benchmark scenarios
:param config: An instance of a TaskConfig
:param task: Instance of Task,
the current task which is being performed
:param deployment: Instance of Deployment,
:param abort_on_sla_failure: True if the execution should be stopped
when some SLA check fails
"""
try:
self.config = TaskConfig(config)
except Exception as e:
task.set_failed(type(e).__name__,
str(e),
json.dumps(traceback.format_exc()))
if logging.is_debug():
LOG.exception("Invalid Task")
raise exceptions.InvalidTaskException(str(e))
self.config = config
self.task = task
self.deployment = deployment
self.abort_on_sla_failure = abort_on_sla_failure

2
tests/unit/doc/test_task_samples.py
View File

@ -85,7 +85,7 @@ class TaskSampleTestCase(test.TestCase):
except Exception:
print(traceback.format_exc())
self.fail("Invalid JSON file: %s" % path)
eng = engine.TaskEngine(task_config,
eng = engine.TaskEngine(engine.TaskConfig(task_config),
mock.MagicMock(), mock.Mock())
eng.validate(only_syntax=True)
except Exception:

2
tests/unit/rally_jobs/test_jobs.py
View File

@ -78,7 +78,7 @@ class RallyJobsTestCase(test.TestCase):
task_inst = api._Task(api.API(skip_db_check=True))
task = task_inst.render_template(
task_template=task_file.read(), **args)
task = yaml.safe_load(task)
task = engine.TaskConfig(yaml.safe_load(task))
task_obj = fakes.FakeTask({"uuid": full_path})
eng = engine.TaskEngine(task, task_obj, mock.Mock())

62
tests/unit/task/test_engine.py
View File

@ -50,30 +50,17 @@ class TaskEngineTestCase(test.TestCase):
"sla": sla or {},
"hooks": hooks or []}
@mock.patch("rally.task.engine.TaskConfig")
def test_init(self, mock_task_config):
def test_init(self):
config = mock.MagicMock()
task = mock.MagicMock()
mock_task_config.return_value = fake_task_instance = mock.MagicMock()
eng = engine.TaskEngine(config, task, mock.Mock())
mock_task_config.assert_has_calls([mock.call(config)])
self.assertEqual(eng.config, fake_task_instance)
self.assertEqual(eng.config, config)
self.assertEqual(eng.task, task)
def test_init_empty_config(self):
config = None
task = mock.Mock()
exception = self.assertRaises(exceptions.InvalidTaskException,
engine.TaskEngine, config, task,
mock.Mock())
self.assertIn("Input task is empty", str(exception))
self.assertTrue(task.set_failed.called)
@mock.patch("rally.task.engine.TaskConfig")
@mock.patch("jsonschema.validate")
def test_validate(self, mock_validate, mock_task_config):
mock_task_config.return_value = config = mock.MagicMock()
eng = engine.TaskEngine(mock.MagicMock(), mock.MagicMock(),
def test_validate(self, mock_validate):
config = mock.MagicMock()
eng = engine.TaskEngine(config, mock.MagicMock(),
mock.Mock())
mock_validate = mock.MagicMock()
@ -87,15 +74,6 @@ class TaskEngineTestCase(test.TestCase):
mock_validate.platforms.assert_called_once_with(config)
mock_validate.semantic.assert_called_once_with(config)
def test_validate__wrong_schema(self):
config = {
"wrong": True
}
task = mock.MagicMock()
self.assertRaises(exceptions.InvalidTaskException,
engine.TaskEngine, config, task, mock.Mock())
self.assertTrue(task.set_failed.called)
@mock.patch("rally.task.engine.TaskConfig")
def test_validate__wrong_syntax(self, mock_task_config):
task = mock.MagicMock()
@ -424,7 +402,6 @@ class TaskEngineTestCase(test.TestCase):
])
@mock.patch("rally.task.engine.objects.task.Task.get_status")
@mock.patch("rally.task.engine.TaskConfig")
@mock.patch("rally.task.engine.LOG")
@mock.patch("rally.task.engine.ResultConsumer")
@mock.patch("rally.task.engine.context.Context")
@ -435,8 +412,7 @@ class TaskEngineTestCase(test.TestCase):
def test_run_exception_is_logged(
self, mock_context_manager_setup, mock_context_manager_cleanup,
mock_scenario_runner, mock_scenario, mock_context,
mock_result_consumer, mock_log, mock_task_config,
mock_task_get_status):
mock_result_consumer, mock_log, mock_task_get_status):
scenario_cls = mock_scenario.get.return_value
scenario_cls.get_default_context.return_value = {}
@ -459,9 +435,10 @@ class TaskEngineTestCase(test.TestCase):
contexts={"context_a": {"b": 2}},
position=2)]}]
mock_task_config.return_value = mock_task_instance
eng = engine.TaskEngine(mock.MagicMock(), mock.MagicMock(),
mock.MagicMock())
deployment = fakes.FakeDeployment(
uuid="deployment_uuid", admin={"foo": "admin"})
eng = engine.TaskEngine(mock_task_instance, mock.MagicMock(),
deployment)
eng.run()
self.assertEqual(2, mock_log.exception.call_count)
@ -482,14 +459,14 @@ class TaskEngineTestCase(test.TestCase):
mock_result_consumer.is_task_in_aborting_status.side_effect = [False,
False,
True]
config = {
config = engine.TaskConfig({
"a.task": [{"runner": {"type": "a", "b": 1},
"description": "foo"}],
"b.task": [{"runner": {"type": "a", "b": 1},
"description": "bar"}],
"c.task": [{"runner": {"type": "a", "b": 1},
"description": "xxx"}]
}
})
fake_runner_cls = mock.MagicMock()
fake_runner = mock.MagicMock()
fake_runner_cls.return_value = fake_runner
@ -521,11 +498,11 @@ class TaskEngineTestCase(test.TestCase):
mock_context_manager_setup, mock_context_manager_cleanup,
mock_result_consumer, mock_task_get_status):
task = mock.MagicMock(spec=objects.Task)
config = {
config = engine.TaskConfig({
"a.task": [{"runner": {"type": "a", "b": 1}}],
"b.task": [{"runner": {"type": "a", "b": 1}}],
"c.task": [{"runner": {"type": "a", "b": 1}}]
}
})
fake_runner_cls = mock.MagicMock()
fake_runner = mock.MagicMock()
fake_runner_cls.return_value = fake_runner
@ -553,11 +530,11 @@ class TaskEngineTestCase(test.TestCase):
subtask_obj = task.add_subtask.return_value
subtask_obj.add_workload.side_effect = MyException()
mock_result_consumer.is_task_in_aborting_status.return_value = False
config = {
config = engine.TaskConfig({
"a.task": [{"runner": {"type": "a", "b": 1}}],
"b.task": [{"runner": {"type": "a", "b": 1}}],
"c.task": [{"runner": {"type": "a", "b": 1}}]
}
})
fake_runner_cls = mock.MagicMock()
fake_runner = mock.MagicMock()
fake_runner_cls.return_value = fake_runner
@ -956,6 +933,13 @@ class ResultConsumerTestCase(test.TestCase):
class TaskConfigTestCase(test.TestCase):
def test_init_empty_config(self):
config = None
exception = self.assertRaises(Exception, # noqa
engine.TaskConfig, config)
self.assertIn("Input task is empty", str(exception))
@mock.patch("jsonschema.validate")
def test_validate_json(self, mock_validate):
config = {}

66
tests/unit/test_api.py
View File

@ -67,10 +67,12 @@ class TaskAPITestCase(test.TestCase):
mock_api.endpoint_url = None
self.task_inst = api._Task(mock_api)
@mock.patch("rally.task.engine.TaskConfig")
@mock.patch("rally.api.objects.Task")
@mock.patch("rally.api.objects.Deployment.get")
@mock.patch("rally.api.engine.TaskEngine")
def test_validate(self, mock_task_engine, mock_deployment_get, mock_task):
def test_validate(self, mock_task_engine, mock_deployment_get, mock_task,
mock_task_config):
fake_deployment = fakes.FakeDeployment(
uuid="deployment_uuid_1", admin="fake_admin", users=["fake_user"])
mock_deployment_get.return_value = fake_deployment
@ -82,7 +84,8 @@ class TaskAPITestCase(test.TestCase):
config="config")
mock_task_engine.assert_called_once_with(
"config", mock_task.return_value, fake_deployment),
mock_task_config.return_value, mock_task.return_value,
fake_deployment),
mock_task_engine.return_value.validate.assert_called_once_with()
mock_task.assert_called_once_with(
@ -106,8 +109,8 @@ class TaskAPITestCase(test.TestCase):
config="config",
task=task_uuid)
mock_task_engine.assert_called_once_with("config", fake_task,
fake_deployment)
mock_task_engine.assert_called_once_with(
mock_task_config.return_value, fake_task, fake_deployment)
mock_task_engine.return_value.validate.assert_called_once_with()
self.assertFalse(mock_task.called)
@ -130,8 +133,8 @@ class TaskAPITestCase(test.TestCase):
config="config",
task_instance=task_instance)
mock_task_engine.assert_called_once_with("config", fake_task,
fake_deployment)
mock_task_engine.assert_called_once_with(
mock_task_config.return_value, fake_task, fake_deployment)
mock_task_engine.return_value.validate.assert_called_once_with()
self.assertFalse(mock_task.called)
@ -141,6 +144,18 @@ class TaskAPITestCase(test.TestCase):
mock_task.get.assert_called_once_with(task_instance["uuid"])
#######################################################################
# The case #4 -- TaskConfig returns error
#######################################################################
mock_task_config.side_effect = Exception("Who is a good boy?! Woof.")
e = self.assertRaises(exceptions.InvalidTaskException,
self.task_inst.validate,
deployment=fake_deployment["uuid"],
config="config",
task_instance=task_instance)
self.assertIn("Who is a good boy?! Woof.", "%s" % e)
@mock.patch("rally.api.objects.Task")
@mock.patch("rally.api.objects.Deployment",
return_value=fakes.FakeDeployment(uuid="deployment_uuid"))
@ -246,17 +261,19 @@ class TaskAPITestCase(test.TestCase):
self.task_inst.create, deployment=deployment_id,
tags=["a"])
@mock.patch("rally.task.engine.TaskConfig")
@mock.patch("rally.api.objects.Task")
@mock.patch("rally.api.objects.Deployment.get")
@mock.patch("rally.api.engine.TaskEngine")
def test_start(self, mock_task_engine, mock_deployment_get,
mock_task):
mock_task, mock_task_config):
fake_task = fakes.FakeTask(uuid="some_uuid")
fake_task.get_status = mock.Mock()
mock_task.return_value = fake_task
mock_deployment_get.return_value = fakes.FakeDeployment(
uuid="deployment_uuid", admin="fake_admin", users=["fake_user"],
status=consts.DeployStatus.DEPLOY_FINISHED)
task_config_instance = mock_task_config.return_value
self.assertEqual(
(fake_task["uuid"], fake_task.get_status.return_value),
@ -265,16 +282,20 @@ class TaskAPITestCase(test.TestCase):
config="config")
)
mock_task_engine.assert_has_calls([
mock.call("config", mock_task.return_value,
mock_deployment_get.return_value,
abort_on_sla_failure=False),
mock.call().validate(),
mock.call().run()
])
mock_task_engine.assert_called_once_with(
task_config_instance,
mock_task.return_value,
mock_deployment_get.return_value,
abort_on_sla_failure=False
)
task_engine = mock_task_engine.return_value
task_engine.validate.assert_called_once_with()
task_engine.run.assert_called_once_with()
mock_task.assert_called_once_with(
deployment_uuid=mock_deployment_get.return_value["uuid"])
deployment_uuid=mock_deployment_get.return_value["uuid"],
title=task_config_instance.title,
description=task_config_instance.description)
mock_deployment_get.assert_called_once_with(
mock_deployment_get.return_value["uuid"])
@ -315,12 +336,13 @@ class TaskAPITestCase(test.TestCase):
config="config",
task=fake_task["uuid"])
@mock.patch("rally.task.engine.TaskConfig")
@mock.patch("rally.api.objects.Task")
@mock.patch("rally.api.objects.Deployment.get")
@mock.patch("rally.api.engine.TaskEngine")
@mock.patch("rally.api.CONF", spec=cfg.CONF)
def test_start_exception(self, mock_conf, mock_task_engine,
mock_deployment_get, mock_task):
mock_deployment_get, mock_task, mock_task_config):
fake_deployment = fakes.FakeDeployment(
status=consts.DeployStatus.DEPLOY_FINISHED,
name="foo", uuid="deployment_uuid")
@ -332,6 +354,18 @@ class TaskAPITestCase(test.TestCase):
fake_deployment.update_status.assert_called_once_with(
consts.DeployStatus.DEPLOY_INCONSISTENT)
@mock.patch("rally.api.objects.Task")
@mock.patch("rally.api.objects.Deployment.get")
@mock.patch("rally.api.engine.TaskEngine")
@mock.patch("rally.api.CONF", spec=cfg.CONF)
def test_start_with_wrong_config(self, mock_conf, mock_task_engine,
mock_deployment_get, mock_task):
mock_deployment_get.return_value = {
"status": consts.DeployStatus.DEPLOY_FINISHED}
self.assertRaises(exceptions.InvalidTaskException,
self.task_inst.start,
deployment="deployment_uuid", config="config")
@ddt.data(True, False)
@mock.patch("rally.api.time")
@mock.patch("rally.api.objects.Task")