From 7a4c92e7171d642a1e1fce324441710ad4d44ce9 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Sat, 19 May 2018 18:45:36 +0300 Subject: Change: use build graphs in build resources, instead of action list --- ick2/__init__.py | 2 + ick2/build.py | 53 ++++++++++++ ick2/build_tests.py | 120 +++++++++++++++++++++++++++ ick2/buildgraph.py | 102 +++++++++++++++++++++++ ick2/buildgraph_tests.py | 206 +++++++++++++++++++++++++++++++++++++++++++++++ ick2/projectapi.py | 26 ++++-- ick2/workapi.py | 62 +++++++------- ick2/workapi_tests.py | 5 ++ 8 files changed, 540 insertions(+), 36 deletions(-) create mode 100644 ick2/build.py create mode 100644 ick2/build_tests.py create mode 100644 ick2/buildgraph.py create mode 100644 ick2/buildgraph_tests.py (limited to 'ick2') diff --git a/ick2/__init__.py b/ick2/__init__.py index 18615a4..59344a3 100644 --- a/ick2/__init__.py +++ b/ick2/__init__.py @@ -22,6 +22,8 @@ from .persistent import ( resource_from_dict, ) from .trans import TransactionalState +from .build import Build, WrongBuildStatusChange +from .buildgraph import BuildGraph from .exceptions import ( BadUpdate, ExistsAlready, diff --git a/ick2/build.py b/ick2/build.py new file mode 100644 index 0000000..cfb1f71 --- /dev/null +++ b/ick2/build.py @@ -0,0 +1,53 @@ +# Copyright (C) 2018 Lars Wirzenius +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + + +import ick2 + + +class Build: + + acceptable = { + 'triggered': ['building'], + 'building': ['done', 'failed'], + } + + def __init__(self, resource): + self.resource = resource + self.graph = ick2.BuildGraph(graph=self.resource.get('graph', {})) + self.graph.set_observer(self.update_graph_in_resource) + + def get_status(self): + return self.resource['status'] + + def set_status(self, status): + current = self.get_status() + if status not in self.acceptable[current]: + raise WrongBuildStatusChange(current, status) + self.resource['status'] = status + + def get_graph(self): + return self.graph + + def update_graph_in_resource(self): + self.resource['graph'] = self.graph.get_actions() + + +class WrongBuildStatusChange(Exception): + + def __init__(self, current, new): + super().__init__( + 'Unacceptable build status change from {} to {}'.format( + current, new)) diff --git a/ick2/build_tests.py b/ick2/build_tests.py new file mode 100644 index 0000000..47b2db8 --- /dev/null +++ b/ick2/build_tests.py @@ -0,0 +1,120 @@ +# Copyright (C) 2018 Lars Wirzenius +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + + +import unittest + + +import ick2 + + +class BuildTests(unittest.TestCase): + + def setUp(self): + as_dict = { + 'status': 'triggered', + } + self.resource = ick2.resource_from_dict(as_dict) + + def test_sets_status_from_triggered_only_when_acceptable(self): + build = ick2.Build(self.resource) + + with self.assertRaises(ick2.WrongBuildStatusChange): + build.set_status('triggered') + self.assertEqual(build.get_status(), 'triggered') + + with self.assertRaises(ick2.WrongBuildStatusChange): + build.set_status('done') + self.assertEqual(build.get_status(), 'triggered') + + with self.assertRaises(ick2.WrongBuildStatusChange): + build.set_status('failed') + self.assertEqual(build.get_status(), 'triggered') + + build.set_status('building') + self.assertEqual(build.get_status(), 'building') + + def test_refuses_changing_status_from_building_when_unacceptable(self): + build = ick2.Build(self.resource) + build.set_status('building') + + with self.assertRaises(ick2.WrongBuildStatusChange): + build.set_status('triggered') + self.assertEqual(build.get_status(), 'building') + + with self.assertRaises(ick2.WrongBuildStatusChange): + build.set_status('building') + self.assertEqual(build.get_status(), 'building') + + def test_changes_status_from_building_to_done(self): + build = ick2.Build(self.resource) + build.set_status('building') + build.set_status('done') + self.assertEqual(build.get_status(), 'done') + + def test_changes_status_from_building_to_failed(self): + build = ick2.Build(self.resource) + build.set_status('building') + build.set_status('failed') + self.assertEqual(build.get_status(), 'failed') + + def test_has_empty_build_graph_initially(self): + build = ick2.Build(self.resource) + graph = build.get_graph() + self.assertTrue(isinstance(graph, ick2.BuildGraph)) + self.assertEqual(graph.get_actions(), {}) + + def test_has_build_graph_from_resource(self): + self.resource['graph'] = { + 1: { + 'action': {'action': 'foo'}, + 'status': 'ready', + 'depends': [], + }, + } + build = ick2.Build(self.resource) + graph = build.get_graph() + self.assertEqual(graph.get_actions(), self.resource['graph']) + + def test_appending_actions_to_build_graph_shows_in_build_resource(self): + build = ick2.Build(self.resource) + graph = build.get_graph() + action_id = graph.append_action({'action': 'foo'}) + self.assertEqual( + self.resource['graph'], + { + action_id: { + 'action': {'action': 'foo'}, + 'status': 'ready', + 'depends': [], + }, + } + ) + + def test_updating_actions_in_build_graph_changes_build_resource(self): + build = ick2.Build(self.resource) + graph = build.get_graph() + action_id = graph.append_action({'action': 'foo'}) + graph.set_action_status(action_id, 'building') + self.assertEqual( + self.resource['graph'], + { + action_id: { + 'action': {'action': 'foo'}, + 'status': 'building', + 'depends': [], + }, + } + ) diff --git a/ick2/buildgraph.py b/ick2/buildgraph.py new file mode 100644 index 0000000..ddcfbfb --- /dev/null +++ b/ick2/buildgraph.py @@ -0,0 +1,102 @@ +# Copyright (C) 2018 Lars Wirzenius +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + + +import copy + + +class BuildGraph: + + def __init__(self, graph=None): + self.observer = None + self.actions = graph or {} + self.idgen = IdGenerator(self.actions.keys()) + + def set_observer(self, observer): + self.observer = observer + + def get_actions(self): + return copy.deepcopy(self.actions) + + def get_action(self, action_id): + return self.actions[action_id]['action'] + + def get_action_status(self, action_id): + return self.actions[action_id]['status'] + + def set_action_status(self, action_id, status): + self.actions[action_id]['status'] = status + self.trigger_observer() + + def unblock(self): + blocked_ids = self.find_actions('blocked') + for blocked_id in blocked_ids: + blocked = self.actions[blocked_id] + if self.is_unblockable(blocked): + self.set_action_status(blocked_id, 'ready') + + def is_unblockable(self, action): + return all( + self.get_action_status(dep) == 'done' + for dep in action['depends'] + ) + + def trigger_observer(self): + if self.observer is not None: + self.observer() + + def append_action(self, action): + prev_id, action_id = self.idgen.next_id() + + graph_node = { + 'action': copy.deepcopy(action), + } + + if not self.actions: + graph_node['status'] = 'ready' + graph_node['depends'] = [] + else: + graph_node['status'] = 'blocked' + graph_node['depends'] = [prev_id] + + self.actions[action_id] = graph_node + self.trigger_observer() + return action_id + + def append_pipeline(self, pipeline): + for action in pipeline.get('actions', []): + self.append_action(action) + + def find_actions(self, status): + return [ + action_id + for action_id, action in self.actions.items() + if action['status'] == status + ] + + +class IdGenerator: + + def __init__(self, action_ids): + self.current = 0 + if action_ids: + action_ids = [int(an_id) for an_id in action_ids] + action_ids.sort() + self.current = action_ids[-1] + + def next_id(self): + prev = self.current + self.current += 1 + return str(prev), str(self.current) diff --git a/ick2/buildgraph_tests.py b/ick2/buildgraph_tests.py new file mode 100644 index 0000000..a81d2c6 --- /dev/null +++ b/ick2/buildgraph_tests.py @@ -0,0 +1,206 @@ +# Copyright (C) 2018 Lars Wirzenius +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + + +import unittest + + +import ick2 + + +class BuildGraphTests(unittest.TestCase): + + def test_has_no_actions_initially(self): + graph = ick2.BuildGraph() + self.assertEqual(graph.get_actions(), {}) + + def test_initialises_from_existing_graph(self): + as_dict = { + '1': { + 'action': {'action': 'foo'}, + 'status': 'ready', + 'depends': [], + }, + '2': { + 'action': {'action': 'bar'}, + 'status': 'blocked', + 'depends': ['1'], + }, + } + graph = ick2.BuildGraph(graph=as_dict) + self.assertEqual(graph.get_actions(), as_dict) + + def test_appends_first_action(self): + action = { + 'action': 'foo', + } + + graph = ick2.BuildGraph() + action_id = graph.append_action(action) + self.assertEqual(action_id, '1') + self.assertEqual( + graph.get_actions(), + { + action_id: { + 'action': {'action': 'foo'}, + 'status': 'ready', + 'depends': [] + } + } + ) + self.assertEqual(graph.get_action(action_id), action) + + def test_appends_second_action(self): + action1 = { + 'action': 'foo', + } + action2 = { + 'action': 'bar', + } + + graph = ick2.BuildGraph() + action_id1 = graph.append_action(action1) + action_id2 = graph.append_action(action2) + self.assertEqual(action_id1, '1') + self.assertEqual(action_id2, '2') + self.assertEqual( + graph.get_actions(), + { + action_id1: { + 'action': {'action': 'foo'}, + 'status': 'ready', + 'depends': [], + }, + action_id2: { + 'action': {'action': 'bar'}, + 'status': 'blocked', + 'depends': [action_id1], + }, + } + ) + + def test_changes_action_status(self): + action = { + 'action': 'foo', + } + + graph = ick2.BuildGraph() + action_id = graph.append_action(action) + self.assertEqual(graph.get_action_status(action_id), 'ready') + + graph.set_action_status(action_id, 'building') + self.assertEqual(graph.get_action_status(action_id), 'building') + + def test_appends_pipeline_actions(self): + pipeline_as_dict = { + 'actions': [ + { + 'action': 'foo', + }, + { + 'action': 'bar', + }, + ], + } + pipeline = ick2.resource_from_dict(pipeline_as_dict) + graph = ick2.BuildGraph() + graph.append_pipeline(pipeline) + self.assertEqual( + graph.get_actions(), + { + '1': { + 'action': {'action': 'foo'}, + 'depends': [], + 'status': 'ready', + }, + '2': { + 'action': {'action': 'bar'}, + 'depends': ['1'], + 'status': 'blocked', + }, + } + ) + + def test_appending_action_triggers_observer(self): + + def observer(): + setattr(self, 'observed', True) + + action = {} + + setattr(self, 'observed', False) + graph = ick2.BuildGraph() + graph.set_observer(observer) + self.assertFalse(getattr(self, 'observed')) + action_id = graph.append_action(action) + self.assertTrue(getattr(self, 'observed')) + + def test_finds_actions(self): + pipeline_as_dict = { + 'actions': [ + { + 'action': 'foo', + }, + { + 'action': 'bar', + }, + ], + } + pipeline = ick2.resource_from_dict(pipeline_as_dict) + graph = ick2.BuildGraph() + graph.append_pipeline(pipeline) + + self.assertEqual(graph.find_actions('no-such-state'), []) + self.assertEqual(graph.find_actions('ready'), ['1']) + self.assertEqual(graph.find_actions('blocked'), ['2']) + + def test_doesnt_unblock_when_deps_are_not_done(self): + pipeline_as_dict = { + 'actions': [ + { + 'action': 'foo', + }, + { + 'action': 'bar', + }, + ], + } + pipeline = ick2.resource_from_dict(pipeline_as_dict) + graph = ick2.BuildGraph() + graph.append_pipeline(pipeline) + + graph.unblock() + self.assertEqual(graph.get_action_status('1'), 'ready') + self.assertEqual(graph.get_action_status('2'), 'blocked') + + def test_unblocks_when_deps_are_done(self): + pipeline_as_dict = { + 'actions': [ + { + 'action': 'foo', + }, + { + 'action': 'bar', + }, + ], + } + pipeline = ick2.resource_from_dict(pipeline_as_dict) + graph = ick2.BuildGraph() + graph.append_pipeline(pipeline) + + graph.set_action_status('1', 'done') + graph.unblock() + self.assertEqual(graph.get_action_status('1'), 'done') + self.assertEqual(graph.get_action_status('2'), 'ready') diff --git a/ick2/projectapi.py b/ick2/projectapi.py index 22a5bb6..ffa46bb 100644 --- a/ick2/projectapi.py +++ b/ick2/projectapi.py @@ -63,11 +63,6 @@ class ProjectAPI(ick2.ResourceApiBase): with self._trans.new('builds', build_id) as build: parameters = project.get('parameters', {}) - create_workspace = { - 'action': 'create_workspace', - 'where': 'host', - } - actions = [create_workspace] + self._get_actions(project) build.from_dict({ 'build_id': build_id, 'build_number': build_no, @@ -76,9 +71,26 @@ class ProjectAPI(ick2.ResourceApiBase): 'project': project['project'], 'parameters': parameters, 'status': 'triggered', - 'actions': actions, - 'current_action': 0, + 'graph': {}, }) + + create_workspace = { + 'action': 'create_workspace', + 'where': 'host', + } + + build_obj = ick2.Build(build) + graph = build_obj.get_graph() + graph.append_action(create_workspace) + for action in self._get_actions(project): + graph.append_action(action) + + with self._trans.new('log', build_id) as r: + r.from_dict({ + 'build_id': build_id, + 'log': '', + }) + return build_id, build_no def _pick_build_number(self, project): # pragma: no cover diff --git a/ick2/workapi.py b/ick2/workapi.py index d254515..a2d76d9 100644 --- a/ick2/workapi.py +++ b/ick2/workapi.py @@ -51,17 +51,23 @@ class WorkAPI(ick2.APIbase): build['status'] = 'building' build['worker'] = worker_id - self._start_log(build_id) + build_obj = ick2.Build(build) + graph = build_obj.get_graph() + action_id = self._pick_next_action(graph) + if action_id is None: # pragma: no cover + return {} + + graph.set_action_status(action_id, 'building') + action = graph.get_action(action_id) - actions = build['actions'] - current_action = build['current_action'] doing = { 'build_id': build_id, 'build_number': build['build_number'], 'worker': worker_id, 'project': build['project'], 'parameters': build['parameters'], - 'step': actions[current_action], + 'action_id': action_id, + 'step': action, 'log': build['log'], } @@ -102,13 +108,11 @@ class WorkAPI(ick2.APIbase): return build['build_id'] return None - def _start_log(self, build_id): - ick2.log.log('info', msg_text='Starting new log', build_id=build_id) - with self._trans.new('log', build_id) as r: - r.from_dict({ - 'build_id': build_id, - 'log': '', - }) + def _pick_next_action(self, graph): + action_ids = graph.find_actions('ready') + if not action_ids: # pragma: no cover + return None + return action_ids[0] def update_work(self, update, **kwargs): try: @@ -121,24 +125,25 @@ class WorkAPI(ick2.APIbase): with self._trans.modify('workers', worker_id) as worker: with self._trans.modify('builds', build_id) as build: + build_obj = ick2.Build(build) + graph = build_obj.get_graph() doing = worker.get('doing', {}) self._check_work_update(doing, update) self._append_to_build_log(update) + action_id = doing['action_id'] if exit_code is not None: if exit_code == 0: - action = self._move_to_next_action(build) - if action is None: - doing = {} - self._finish_build(build, exit_code) - else: - doing['step'] = action + self._finish_action(graph, action_id, 'done') + if self._build_finished(graph): + self._finish_build(build, 0) worker.from_dict({ 'worker': worker_id, - 'doing': doing, + 'doing': {}, }) elif exit_code is not None: + graph.set_action_status(action_id, 'failed') self._finish_build(build, exit_code) worker.from_dict({ 'worker': worker_id, @@ -160,22 +165,21 @@ class WorkAPI(ick2.APIbase): with self._trans.modify('log', build_id) as log: for stream in ['stdout', 'stderr']: text = update.get(stream, '') - if text is not None: - log['log'] += text + log['log'] = log.get('log', '') + text - def _move_to_next_action(self, build): - actions = build['actions'] - current_action = build['current_action'] - if current_action + 1 >= len(actions): - return None + def _finish_action(self, graph, action_id, status): + graph.set_action_status(action_id, status) + if status == 'done': + graph.unblock() - index = current_action + 1 - build['current_action'] = index - return actions[index] + def _build_finished(self, graph): + return ( + graph.find_actions('ready') == [] and + graph.find_actions('blocked') == [] + ) def _finish_build(self, build, exit_code): build['status'] = exit_code - build['current_action'] = None def create(self, body, **kwargs): # pragma: no cover pass diff --git a/ick2/workapi_tests.py b/ick2/workapi_tests.py index 05e91a0..b4d72a7 100644 --- a/ick2/workapi_tests.py +++ b/ick2/workapi_tests.py @@ -90,6 +90,7 @@ class WorkAPITests(unittest.TestCase): 'parameters': { 'foo': 'bar', }, + 'action_id': '1', 'step': { 'action': 'create_workspace', 'where': 'host', @@ -124,6 +125,7 @@ class WorkAPITests(unittest.TestCase): 'parameters': { 'foo': 'bar', }, + 'action_id': '1', 'step': { 'action': 'create_workspace', 'where': 'host', @@ -156,6 +158,7 @@ class WorkAPITests(unittest.TestCase): # We should get the next step now. got = work.get_work(claims=claims) + expected['action_id'] = '2' expected['step'] = {'shell': 'step-1', 'where': 'host'} self.assertEqual(got, expected) @@ -164,6 +167,7 @@ class WorkAPITests(unittest.TestCase): work.update_work(done) # We should get the next step now. + expected['action_id'] = '3' expected['step'] = {'shell': 'step-2', 'where': 'host'} self.assertEqual(work.get_work(claims=claims), expected) @@ -189,6 +193,7 @@ class WorkAPITests(unittest.TestCase): 'parameters': { 'foo': 'bar', }, + 'action_id': '1', 'step': { 'action': 'create_workspace', 'where': 'host', -- cgit v1.2.1