From 8dbad440dbd7868361113d11aa09feea8ab205fb Mon Sep 17 00:00:00 2001 From: Andrey Kurilin Date: Sun, 22 Mar 2020 15:06:53 +0200 Subject: [PATCH] Deprecate rally task results command This command produces task results in old format that missis a lot of information. For backward compatibility, the new task results exporter is introduced. Change-Id: I28880642e370513c2430e8e0f67dd7a622023a92 --- CHANGELOG.rst | 3 + rally/cli/commands/task.py | 71 +-------- .../task/exporters/old_json_results.py | 121 ++++++++++++++ tests/ci/cover.sh | 2 +- tests/functional/test_cli_task.py | 3 +- tests/unit/cli/commands/test_task.py | 29 +++- .../unit/plugins/task/exporters/dummy_data.py | 120 ++++++++++++++ .../unit/plugins/task/exporters/test_html.py | 61 +------- .../task/exporters/test_json_exporter.py | 10 +- .../task/exporters/test_old_json_results.py | 148 ++++++++++++++++++ 10 files changed, 437 insertions(+), 131 deletions(-) create mode 100644 rally/plugins/task/exporters/old_json_results.py create mode 100644 tests/unit/plugins/task/exporters/dummy_data.py create mode 100644 tests/unit/plugins/task/exporters/test_old_json_results.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index d6fe9a1e66..f47e287475 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -42,6 +42,9 @@ Changed Deprecated ~~~~~~~~~~ +* Command *rally task results* is deprecated. Use *rally task report --json* + instead. + * Module *rally.common.sshutils* is deprecated. Use *rally.utils.sshutils* instead. diff --git a/rally/cli/commands/task.py b/rally/cli/commands/task.py index 18f5ce1117..38b07a5d23 100644 --- a/rally/cli/commands/task.py +++ b/rally/cli/commands/task.py @@ -16,7 +16,6 @@ """Rally command: task""" from __future__ import print_function -import collections import datetime as dt import itertools import json @@ -544,70 +543,16 @@ class TaskCommands(object): @envutils.with_default_task_id @cliutils.suppress_warnings def results(self, api, task_id=None): - """Display raw task results. - - This will produce a lot of output data about every iteration. - """ - - task = api.task.get(task_id=task_id, detailed=True) - finished_statuses = (consts.TaskStatus.FINISHED, - consts.TaskStatus.ABORTED) - if task["status"] not in finished_statuses: - print("Task status is %s. Results available when it is one of %s." - % (task["status"], ", ".join(finished_statuses))) + """DEPRECATED since Rally 3.0.0.""" + LOG.warning("CLI method `rally task results` is deprecated since " + "Rally 3.0.0 and will be removed soon. " + "Use `rally task report --json` instead.") + try: + self.export(api, tasks=[task_id], output_type="old-json-results") + except exceptions.RallyException as e: + print(e.format_message()) return 1 - # TODO(chenhb): Ensure `rally task results` puts out old format. - for workload in itertools.chain( - *[s["workloads"] for s in task["subtasks"]]): - for itr in workload["data"]: - itr["atomic_actions"] = collections.OrderedDict( - tutils.WrapperForAtomicActions( - itr["atomic_actions"]).items() - ) - - results = [] - for w in itertools.chain(*[s["workloads"] for s in task["subtasks"]]): - w["runner"]["type"] = w["runner_type"] - - def port_hook_cfg(h): - h["config"] = { - "name": h["config"]["action"][0], - "args": h["config"]["action"][1], - "description": h["config"].get("description", ""), - "trigger": {"name": h["config"]["trigger"][0], - "args": h["config"]["trigger"][1]} - } - return h - - hooks = [port_hook_cfg(h) for h in w["hooks"]] - - created_at = dt.datetime.strptime(w["created_at"], - "%Y-%m-%dT%H:%M:%S") - created_at = created_at.strftime("%Y-%d-%mT%H:%M:%S") - - results.append({ - "key": { - "name": w["name"], - "description": w["description"], - "pos": w["position"], - "kw": { - "args": w["args"], - "runner": w["runner"], - "context": w["contexts"], - "sla": w["sla"], - "hooks": [h["config"] for h in w["hooks"]], - } - }, - "result": w["data"], - "sla": w["sla_results"].get("sla", []), - "hooks": hooks, - "load_duration": w["load_duration"], - "full_duration": w["full_duration"], - "created_at": created_at}) - - print(json.dumps(results, sort_keys=False, indent=4)) - @cliutils.args("--deployment", dest="deployment", type=str, metavar="", required=False, help="UUID or name of a deployment.") diff --git a/rally/plugins/task/exporters/old_json_results.py b/rally/plugins/task/exporters/old_json_results.py new file mode 100644 index 0000000000..2fb3381c37 --- /dev/null +++ b/rally/plugins/task/exporters/old_json_results.py @@ -0,0 +1,121 @@ +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import collections +import datetime as dt +import itertools +import json + +from rally import consts +from rally import exceptions +from rally.task import exporter + + +def _to_old_atomic_actions_format(atomic_actions): + """Convert atomic actions to old format. """ + old_style = collections.OrderedDict() + for action in atomic_actions: + duration = action["finished_at"] - action["started_at"] + if action["name"] in old_style: + name_template = action["name"] + " (%i)" + i = 2 + while name_template % i in old_style: + i += 1 + old_style[name_template % i] = duration + else: + old_style[action["name"]] = duration + return old_style + + +@exporter.configure("old-json-results") +class OldJSONExporter(exporter.TaskExporter): + """Generates task report in JSON format as old `rally task results`.""" + def __init__(self, *args, **kwargs): + super(OldJSONExporter, self).__init__(*args, **kwargs) + if len(self.tasks_results) != 1: + raise exceptions.RallyException( + f"'{self.get_fullname()}' task exporter can be used only for " + f"a one task.") + self.task = self.tasks_results[0] + + def _get_report(self): + results = [] + for w in itertools.chain(*[s["workloads"] + for s in self.task["subtasks"]]): + for itr in w["data"]: + itr["atomic_actions"] = _to_old_atomic_actions_format( + itr["atomic_actions"] + ) + + w["runner"]["type"] = w["runner_type"] + + def port_hook_cfg(h): + h["config"] = { + "name": h["config"]["action"][0], + "args": h["config"]["action"][1], + "description": h["config"].get("description", ""), + "trigger": {"name": h["config"]["trigger"][0], + "args": h["config"]["trigger"][1]} + } + return h + + hooks = [port_hook_cfg(h) for h in w["hooks"]] + + created_at = dt.datetime.strptime(w["created_at"], + "%Y-%m-%dT%H:%M:%S") + created_at = created_at.strftime("%Y-%d-%mT%H:%M:%S") + + results.append({ + "key": { + "name": w["name"], + "description": w["description"], + "pos": w["position"], + "kw": { + "args": w["args"], + "runner": w["runner"], + "context": w["contexts"], + "sla": w["sla"], + "hooks": [h["config"] for h in w["hooks"]], + } + }, + "result": w["data"], + "sla": w["sla_results"].get("sla", []), + "hooks": hooks, + "load_duration": w["load_duration"], + "full_duration": w["full_duration"], + "created_at": created_at}) + + return results + + def generate(self): + if len(self.tasks_results) != 1: + raise exceptions.RallyException( + f"'{self.get_fullname()}' task exporter can be used only for " + f"a one task.") + + finished_statuses = (consts.TaskStatus.FINISHED, + consts.TaskStatus.ABORTED) + if self.task["status"] not in finished_statuses: + raise exceptions.RallyException( + f"Task status is {self.task['status']}. Results available " + f"when it is one of {', '.join(finished_statuses)}." + ) + + results = json.dumps(self._get_report(), sort_keys=False, indent=4) + + if self.output_destination: + return {"files": {self.output_destination: results}, + "open": "file://" + self.output_destination} + else: + return {"print": results} diff --git a/tests/ci/cover.sh b/tests/ci/cover.sh index 6c3675d5a6..59b888e0c9 100755 --- a/tests/ci/cover.sh +++ b/tests/ci/cover.sh @@ -15,7 +15,7 @@ # License for the specific language governing permissions and limitations # under the License. -ALLOWED_EXTRA_MISSING=400 +ALLOWED_EXTRA_MISSING=4 show_diff () { head -1 $1 diff --git a/tests/functional/test_cli_task.py b/tests/functional/test_cli_task.py index b118cc8688..1508706564 100644 --- a/tests/functional/test_cli_task.py +++ b/tests/functional/test_cli_task.py @@ -365,7 +365,8 @@ class TaskTestCase(testtools.TestCase): rally("task start --task %s" % config.filename) task_result_file = rally.gen_report_path(suffix="results") self.addCleanup(os.remove, task_result_file) - rally("task results", report_path=task_result_file, raw=True) + rally("task results", report_path=task_result_file, raw=True, + getjson=True) html_report = rally.gen_report_path(extension="html") rally("task report --html-static %s --out %s" diff --git a/tests/unit/cli/commands/test_task.py b/tests/unit/cli/commands/test_task.py index 33ad27dc7c..076cefdb11 100644 --- a/tests/unit/cli/commands/test_task.py +++ b/tests/unit/cli/commands/test_task.py @@ -573,11 +573,24 @@ class TaskCommandsTestCase(test.TestCase): "contexts": {"users": {}}, "data": data or []}]}]} - @mock.patch("rally.cli.commands.task.json.dumps") + @mock.patch("rally.plugins.task.exporters.old_json_results.json.dumps") def test_results(self, mock_json_dumps): + + mock_json_dumps.return_value = "" + + def fake_export(tasks, output_type, output_dest=None): + tasks = [self.fake_api.task.get(u, detailed=True) for u in tasks] + return self.real_api.task.export(tasks, output_type, output_dest) + + self.fake_api.task.export.side_effect = fake_export + task_id = "foo_task_id" - task_obj = self._make_task(data=[{"atomic_actions": {"foo": 1.1}}]) + task_obj = self._make_task( + data=[{"atomic_actions": [{"name": "foo", + "started_at": 0, + "finished_at": 1.1}]}] + ) task_obj["subtasks"][0]["workloads"][0]["hooks"] = [{ "config": { "action": ("foo", "arg"), @@ -585,6 +598,7 @@ class TaskCommandsTestCase(test.TestCase): }, "summary": {"success": 1}} ] + task_obj["uuid"] = task_id def fix_r(workload): cfg = workload["runner"] @@ -633,9 +647,16 @@ class TaskCommandsTestCase(test.TestCase): @mock.patch("rally.cli.commands.task.sys.stdout") def test_results_no_data(self, mock_stdout): + def fake_export(tasks, output_type, output_dest=None): + tasks = [self.fake_api.task.get(u, detailed=True) for u in tasks] + return self.real_api.task.export(tasks, output_type, output_dest) + + self.fake_api.task.export.side_effect = fake_export + task_id = "foo" - self.fake_api.task.get.return_value = self._make_task( - status=consts.TaskStatus.CRASHED) + task_obj = self._make_task(status=consts.TaskStatus.CRASHED) + task_obj["uuid"] = task_id + self.fake_api.task.get.return_value = task_obj self.assertEqual(1, self.task.results(self.fake_api, task_id)) diff --git a/tests/unit/plugins/task/exporters/dummy_data.py b/tests/unit/plugins/task/exporters/dummy_data.py new file mode 100644 index 0000000000..e922b0982b --- /dev/null +++ b/tests/unit/plugins/task/exporters/dummy_data.py @@ -0,0 +1,120 @@ +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + + +def get_tasks_results(): + task_id = "2fa4f5ff-7d23-4bb0-9b1f-8ee235f7f1c8" + workload = {"uuid": "uuid", + "name": "CinderVolumes.list_volumes", + "description": "List all volumes.", + "created_at": "2017-06-04T05:14:44", + "updated_at": "2017-06-04T05:15:14", + "task_uuid": task_id, + "position": 0, + "data": [ + { + "duration": 0.2504892349243164, + "timestamp": 1584551892.7336202, + "idle_duration": 0.0, + "error": [], + "output": { + "additive": [], + "complete": [] + }, + "atomic_actions": [ + { + "name": "foo", + "children": [ + { + "name": "bar", + "children": [], + "started_at": 1584551892.733688, + "finished_at": 1584551892.984079 + } + ], + "started_at": 1584551892.7336745, + "finished_at": 1584551892.9840994 + } + ] + }, + { + "duration": 0.25043749809265137, + "timestamp": 1584551892.7363858, + "idle_duration": 0.0, + "error": [], + "output": { + "additive": [], + "complete": [] + }, + "atomic_actions": [ + { + "name": "foo", + "children": [ + { + "name": "bar", + "children": [], + "started_at": 1584551892.7364488, + "finished_at": 1584551892.9867969 + } + ], + "started_at": 1584551892.736435, + "finished_at": 1584551892.9868152 + } + ] + } + ], + "full_duration": 29.969523191452026, + "load_duration": 2.03029203414917, + "hooks": [], + "runner": {}, + "runner_type": "runner_type", + "args": {}, + "contexts": {}, + "contexts_results": [], + "min_duration": 0.0, + "max_duration": 1.0, + "start_time": 0, + "statistics": {}, + "failed_iteration_count": 0, + "total_iteration_count": 10, + "sla": {}, + "sla_results": {"sla": []}, + "pass_sla": True + } + task = { + "uuid": task_id, + "title": "task", + "description": "description", + "status": "finished", + "env_uuid": "env-uuid", + "env_name": "env-name", + "tags": [], + "created_at": "2017-06-04T05:14:44", + "updated_at": "2017-06-04T05:15:14", + "pass_sla": True, + "task_duration": 2.0, + "subtasks": [ + {"uuid": "subtask_uuid", + "title": "subtask", + "description": "description", + "status": "finished", + "run_in_parallel": True, + "created_at": "2017-06-04T05:14:44", + "updated_at": "2017-06-04T05:15:14", + "sla": {}, + "duration": 29.969523191452026, + "task_uuid": task_id, + "workloads": [workload]} + ]} + return [task] diff --git a/tests/unit/plugins/task/exporters/test_html.py b/tests/unit/plugins/task/exporters/test_html.py index b851842f24..38c8859171 100644 --- a/tests/unit/plugins/task/exporters/test_html.py +++ b/tests/unit/plugins/task/exporters/test_html.py @@ -16,73 +16,18 @@ import os from unittest import mock from rally.plugins.task.exporters import html +from tests.unit.plugins.task.exporters import dummy_data from tests.unit import test PATH = "rally.plugins.task.exporters.html" -def get_tasks_results(): - task_id = "2fa4f5ff-7d23-4bb0-9b1f-8ee235f7f1c8" - workload = {"uuid": "uuid", - "name": "CinderVolumes.list_volumes", - "description": "List all volumes.", - "created_at": "2017-06-04T05:14:44", - "updated_at": "2017-06-04T05:15:14", - "task_uuid": task_id, - "position": 0, - "data": {"raw": []}, - "full_duration": 29.969523191452026, - "load_duration": 2.03029203414917, - "hooks": [], - "runner": {}, - "runner_type": "runner_type", - "args": {}, - "contexts": {}, - "contexts_results": [], - "min_duration": 0.0, - "max_duration": 1.0, - "start_time": 0, - "statistics": {}, - "failed_iteration_count": 0, - "total_iteration_count": 10, - "sla": {}, - "sla_results": {"sla": []}, - "pass_sla": True - } - task = { - "uuid": task_id, - "title": "task", - "description": "description", - "status": "finished", - "env_uuid": "env-uuid", - "env_name": "env-name", - "tags": [], - "created_at": "2017-06-04T05:14:44", - "updated_at": "2017-06-04T05:15:14", - "pass_sla": True, - "task_duration": 2.0, - "subtasks": [ - {"uuid": "subtask_uuid", - "title": "subtask", - "description": "description", - "status": "finished", - "run_in_parallel": True, - "created_at": "2017-06-04T05:14:44", - "updated_at": "2017-06-04T05:15:14", - "sla": {}, - "duration": 29.969523191452026, - "task_uuid": task_id, - "workloads": [workload]} - ]} - return [task] - - class HTMLExporterTestCase(test.TestCase): @mock.patch("%s.plot.plot" % PATH, return_value="html") def test_generate(self, mock_plot): - tasks_results = get_tasks_results() - tasks_results.extend(get_tasks_results()) + tasks_results = dummy_data.get_tasks_results() + tasks_results.extend(dummy_data.get_tasks_results()) reporter = html.HTMLExporter(tasks_results, None) reporter._generate_results = mock.MagicMock() diff --git a/tests/unit/plugins/task/exporters/test_json_exporter.py b/tests/unit/plugins/task/exporters/test_json_exporter.py index b442cf0e06..47671f2dcd 100644 --- a/tests/unit/plugins/task/exporters/test_json_exporter.py +++ b/tests/unit/plugins/task/exporters/test_json_exporter.py @@ -18,7 +18,7 @@ from unittest import mock from rally.common import version as rally_version from rally.plugins.task.exporters import json_exporter -from tests.unit.plugins.task.exporters import test_html +from tests.unit.plugins.task.exporters import dummy_data from tests.unit import test PATH = "rally.plugins.task.exporters.json_exporter" @@ -27,9 +27,11 @@ PATH = "rally.plugins.task.exporters.json_exporter" class JSONExporterTestCase(test.TestCase): def test__generate_tasks(self): - tasks_results = test_html.get_tasks_results() + tasks_results = dummy_data.get_tasks_results() reporter = json_exporter.JSONExporter(tasks_results, None) + w_data = tasks_results[0]["subtasks"][0]["workloads"][0]["data"] + self.assertEqual([ collections.OrderedDict([ ("uuid", "2fa4f5ff-7d23-4bb0-9b1f-8ee235f7f1c8"), @@ -65,7 +67,7 @@ class JSONExporterTestCase(test.TestCase): ("load_duration", 2.03029203414917), ("full_duration", 29.969523191452026), ("statistics", {}), - ("data", {"raw": []}), + ("data", w_data), ("failed_iteration_count", 0), ("total_iteration_count", 10), ("created_at", "2017-06-04T05:14:44"), @@ -86,7 +88,7 @@ class JSONExporterTestCase(test.TestCase): @mock.patch("%s.dt" % PATH) def test_generate(self, mock_dt, mock_json_dumps): mock_dt.datetime.utcnow.return_value = dt.datetime.utcnow() - tasks_results = test_html.get_tasks_results() + tasks_results = dummy_data.get_tasks_results() # print reporter = json_exporter.JSONExporter(tasks_results, None) diff --git a/tests/unit/plugins/task/exporters/test_old_json_results.py b/tests/unit/plugins/task/exporters/test_old_json_results.py new file mode 100644 index 0000000000..d3fbf992fc --- /dev/null +++ b/tests/unit/plugins/task/exporters/test_old_json_results.py @@ -0,0 +1,148 @@ +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import collections +from unittest import mock + +from rally import exceptions +from rally.plugins.task.exporters import old_json_results +from tests.unit.plugins.task.exporters import dummy_data +from tests.unit import test + +PATH = "rally.plugins.task.exporters.old_json_results" + + +class OldJSONResultsTestCase(test.TestCase): + + def test___init__(self): + old_json_results.OldJSONExporter([1], None) + + self.assertRaises( + exceptions.RallyException, + old_json_results.OldJSONExporter, [1, 2], None + ) + + @mock.patch("%s.json.dumps" % PATH, return_value="json") + def test_generate(self, mock_json_dumps): + tasks_results = dummy_data.get_tasks_results() + + # print + exporter = old_json_results.OldJSONExporter(tasks_results, None) + exporter._get_report = mock.MagicMock() + + self.assertEqual({"print": "json"}, exporter.generate()) + + exporter._get_report.assert_called_once_with() + mock_json_dumps.assert_called_once_with( + exporter._get_report.return_value, sort_keys=False, indent=4) + + # export to file + exporter = old_json_results.OldJSONExporter( + tasks_results, output_destination="path") + exporter._get_report = mock.MagicMock() + self.assertEqual({"files": {"path": "json"}, + "open": "file://path"}, exporter.generate()) + + def test__get_report(self): + tasks_results = dummy_data.get_tasks_results() + exporter = old_json_results.OldJSONExporter(tasks_results, None) + + self.assertEqual( + [ + { + "created_at": "2017-04-06T05:14:44", + "full_duration": 29.969523191452026, + "hooks": [], + "key": { + "description": "List all volumes.", + "kw": {"args": {}, + "context": {}, + "hooks": [], + "runner": {"type": "runner_type"}, + "sla": {}}, + "name": "CinderVolumes.list_volumes", + "pos": 0 + }, + "load_duration": 2.03029203414917, + "result": [ + { + "atomic_actions": collections.OrderedDict([ + ("foo", 0.250424861907959)]), + "duration": 0.2504892349243164, + "error": [], + "idle_duration": 0.0, + "output": {"additive": [], "complete": []}, + "timestamp": 1584551892.7336202 + }, + { + "atomic_actions": collections.OrderedDict([ + ("foo", 0.250380277633667)]), + "duration": 0.25043749809265137, + "error": [], + "idle_duration": 0.0, + "output": {"additive": [], "complete": []}, + "timestamp": 1584551892.7363858}], + "sla": [] + } + ], exporter._get_report()) + + def test__to_old_atomic_actions_format(self): + actions = [ + { + "name": "foo", + "started_at": 0, + "finished_at": 1, + "children": [] + }, + { + "name": "foo", + "started_at": 1, + "finished_at": 2, + "children": [] + }, + { + "name": "foo", + "started_at": 2, + "finished_at": 3, + "children": [] + }, + { + "name": "bar", + "started_at": 3, + "finished_at": 5, + "children": [ + { + "name": "xxx", + "started_at": 3, + "finished_at": 4, + "children": [] + }, + { + "name": "xxx", + "started_at": 4, + "finished_at": 5, + "children": [] + } + ] + } + ] + + actual = old_json_results._to_old_atomic_actions_format(actions) + self.assertIsInstance(actual, collections.OrderedDict) + # it is easier to compare list instead of constructing an ordered dict + actual = list(actual.items()) + self.assertEqual( + [("foo", 1), ("foo (2)", 1), ("foo (3)", 1), ("bar", 2)], + actual + )