diff --git a/nailgun/nailgun/orchestrator/task_based_deployment.py b/nailgun/nailgun/orchestrator/task_based_deployment.py index f5e8f3524b..1517e32d8b 100644 --- a/nailgun/nailgun/orchestrator/task_based_deployment.py +++ b/nailgun/nailgun/orchestrator/task_based_deployment.py @@ -139,10 +139,30 @@ class TaskProcessor(object): 'required_for', 'cross-depended-by' ) + min_supported_task_version = StrictVersion(consts.TASK_CROSS_DEPENDENCY) + def __init__(self): # stores mapping between ids of generated tasks and origin task self.origin_task_ids = dict() + @classmethod + def ensure_task_based_deploy_allowed(cls, task): + """Raises error if task does not support cross-dependencies. + + :param task: the task instance + :raises: errors.TaskBaseDeploymentNotAllowed + """ + if task.get('type') == consts.ORCHESTRATOR_TASK_TYPES.stage: + return + + task_version = StrictVersion(task.get('version', '1.0.0')) + if task_version < cls.min_supported_task_version: + logger.warning( + "Task '%s' does not supported task based deploy.", + task['id'] + ) + raise errors.TaskBaseDeploymentNotAllowed + def get_origin(self, task_id): """Gets the origin ID of task. @@ -190,12 +210,20 @@ class TaskProcessor(object): task_iter = iter(serialized_tasks) frame = collections.deque(itertools.islice(task_iter, 2), maxlen=2) - if len(frame) < 2: + + # in case if there is no nodes was resolved + # the serializers return empty list of task + if len(frame) == 0: + return + + # check only if task will be add to graph + self.ensure_task_based_deploy_allowed(origin_task) + + if len(frame) == 1: # It is simple case when chain contains only 1 task # do nothing # check that that frame is not empty - if len(frame) == 1: - yield self._convert_task(frame.pop(), origin_task) + yield self._convert_task(frame.pop(), origin_task) return # it is chain of tasks, need to properly handle them @@ -345,8 +373,6 @@ class TaskProcessor(object): class TasksSerializer(object): """The deploy tasks serializer.""" - min_supported_task_version = StrictVersion(consts.TASK_CROSS_DEPENDENCY) - def __init__(self, cluster, nodes): """Initializes. @@ -388,7 +414,6 @@ class TasksSerializer(object): tasks_groups = collections.defaultdict(set) for task in tasks: - self.ensure_task_based_deploy_allowed(task) if task.get('type') == consts.ORCHESTRATOR_TASK_TYPES.group: tasks_for_role = task.get('tasks') if tasks_for_role: @@ -566,17 +591,3 @@ class TasksSerializer(object): "no candidates in nodes '%s'.", name, ", ".join(six.moves.map(str, node_ids)) ) - - @classmethod - def ensure_task_based_deploy_allowed(cls, task): - """Raises error if task is supported task based deployment. - - :param task: the task instance - """ - task_version = StrictVersion(task.get('version', '1.0.0')) - if task_version < cls.min_supported_task_version: - logger.warning( - "Task '%s' does not supported task based deploy.", - task['id'] - ) - raise errors.TaskBaseDeploymentNotAllowed diff --git a/nailgun/nailgun/test/integration/test_task_deploy.py b/nailgun/nailgun/test/integration/test_task_deploy.py index 3bd255a287..5cefa35409 100644 --- a/nailgun/nailgun/test/integration/test_task_deploy.py +++ b/nailgun/nailgun/test/integration/test_task_deploy.py @@ -20,7 +20,7 @@ import mock from nailgun import consts from nailgun.errors import errors from nailgun.objects import Task -from nailgun.orchestrator.task_based_deployment import TasksSerializer +from nailgun.orchestrator.task_based_deployment import TaskProcessor from nailgun.test.base import BaseIntegrationTest from nailgun.test.base import fake_tasks from nailgun.utils import reverse @@ -77,7 +77,7 @@ class TestTaskDeploy(BaseIntegrationTest): args, kwargs = rpc_cast.call_args return args[1][1] - @mock.patch.object(TasksSerializer, "ensure_task_based_deploy_allowed") + @mock.patch.object(TaskProcessor, "ensure_task_based_deploy_allowed") def test_task_deploy_used_if_option_enabled(self, _): self.enable_deploy_task(True) message = self.get_deploy_message() @@ -87,7 +87,7 @@ class TestTaskDeploy(BaseIntegrationTest): message["args"] ) - @mock.patch.object(TasksSerializer, "ensure_task_based_deploy_allowed") + @mock.patch.object(TaskProcessor, "ensure_task_based_deploy_allowed") def test_fallback_to_granular_deploy(self, ensure_allowed): ensure_allowed.side_effect = errors.TaskBaseDeploymentNotAllowed self.enable_deploy_task(True) @@ -110,7 +110,7 @@ class TestTaskDeploy(BaseIntegrationTest): message["args"] ) - @mock.patch.object(TasksSerializer, "ensure_task_based_deploy_allowed") + @mock.patch.object(TaskProcessor, "ensure_task_based_deploy_allowed") @mock.patch('nailgun.plugins.adapters.os.path.exists', return_value=True) @mock.patch('nailgun.plugins.adapters.PluginAdapterBase._load_tasks') def test_task_deploy_with_plugins(self, load_tasks, *_): @@ -144,7 +144,7 @@ class TestTaskDeploy(BaseIntegrationTest): ) @fake_tasks(mock_rpc=False, fake_rpc=False) - @mock.patch.object(TasksSerializer, "ensure_task_based_deploy_allowed") + @mock.patch.object(TaskProcessor, "ensure_task_based_deploy_allowed") @mock.patch('nailgun.rpc.cast') def test_task_deploy_specified_tasks(self, rpc_cast, *_): self.enable_deploy_task(True) diff --git a/nailgun/nailgun/test/unit/test_task_based_deployment.py b/nailgun/nailgun/test/unit/test_task_based_deployment.py index e1c3ac0119..872c24b452 100644 --- a/nailgun/nailgun/test/unit/test_task_based_deployment.py +++ b/nailgun/nailgun/test/unit/test_task_based_deployment.py @@ -74,9 +74,12 @@ class TestTaskSerializers(BaseTestCase): tasks = [ { "id": "test1", "role": ["controller"], - "type": "stage", "version": "2.0.0" + "type": "puppet", "version": "2.0.0", "parameters": {} + }, + { + "id": "test2", "role": ["compute"], "type": "puppet", + "parameters": {} }, - {"id": "test2", "role": ["compute"], "type": "stage"}, ] self.assertRaises( task_based_deployment.errors.TaskBaseDeploymentNotAllowed, @@ -84,8 +87,28 @@ class TestTaskSerializers(BaseTestCase): self.env.clusters[-1], self.env.nodes, tasks ) + def test_serialize_success_if_all_applicable_task_has_version_2(self): + tasks = [ + { + "id": "test1", "role": ["controller"], + "type": "puppet", "version": "2.0.0", "parameters": {} + }, + { + "id": "test2", "role": ["cinder"], "type": "puppet", + "parameters": {} + }, + ] + self.assertNotRaises( + task_based_deployment.errors.TaskBaseDeploymentNotAllowed, + self.serializer.serialize, + self.env.clusters[-1], self.env.nodes, tasks + ) + def test_process_task_de_duplication(self): - task = {"id": "test", "type": "puppet", "parameters": {}} + task = { + "id": "test", "type": "puppet", "parameters": {}, + "version": "2.0.0" + } self.serializer.process_task( task, ["1"], task_based_deployment.NullResolver ) @@ -107,7 +130,8 @@ class TestTaskSerializers(BaseTestCase): def test_process_skipped_task(self): task = { - "id": "test", "type": "puppet", "parameters": {}, 'skipped': True + "id": "test", "type": "puppet", "version": "2.0.0", + "parameters": {}, 'skipped': True, } self.serializer.process_task( task, ["1"], task_based_deployment.NullResolver @@ -148,8 +172,10 @@ class TestTaskSerializers(BaseTestCase): self.serializer.expand_task_groups( {'role': ['task1', 'task2']}, { - 'task1': {'id': 'task1', 'type': 'skipped', 'role': '*'}, - 'task2': {'id': 'task2', 'type': 'skipped', 'role': '*'} + 'task1': {'id': 'task1', 'version': '2.0.0', + 'type': 'skipped', 'role': '*'}, + 'task2': {'id': 'task2', 'version': '2.0.0', + 'type': 'skipped', 'role': '*'} } ) self.assertIn('1', self.serializer.tasks_per_node) @@ -294,23 +320,6 @@ class TestTaskSerializers(BaseTestCase): "not_exists", "1, 2, 3" ) - def test_ensure_task_based_deployment_allowed(self): - self.assertRaises( - task_based_deployment.errors.TaskBaseDeploymentNotAllowed, - self.serializer.ensure_task_based_deploy_allowed, - {'id': 'task'} - ) - self.assertRaises( - task_based_deployment.errors.TaskBaseDeploymentNotAllowed, - self.serializer.ensure_task_based_deploy_allowed, - {'id': 'task', 'version': '1.2.3'} - ) - self.assertNotRaises( - task_based_deployment.errors.TaskBaseDeploymentNotAllowed, - self.serializer.ensure_task_based_deploy_allowed, - {'id': 'task', 'version': consts.TASK_CROSS_DEPENDENCY} - ) - class TestNoopSerializer(BaseTestCase): def setUp(self): @@ -410,8 +419,9 @@ class TestDeploymentTaskSerializer(BaseUnitTest): ) -class TestTaskProcessor(BaseUnitTest): +class TestTaskProcessor(BaseTestCase): def setUp(self): + super(TestTaskProcessor, self).setUp() self.processor = task_based_deployment.TaskProcessor() def test_link_tasks_on_same_node(self): @@ -527,12 +537,14 @@ class TestTaskProcessor(BaseUnitTest): def test_process_tasks_if_not_chain(self): origin_task = { - 'id': 'task', 'requires': ['a'], 'cross-depends': [{'name': 'b'}], + 'id': 'task', 'version': '2.0.0', + 'requires': ['a'], 'cross-depends': [{'name': 'b'}], 'required_for': ['c'], 'cross-depended-by': [{'name': 'd'}] } serialized = iter([{'type': 'puppet'}]) - tasks = self.processor.process_tasks(origin_task, serialized) + tasks = list(self.processor.process_tasks(origin_task, serialized)) + del origin_task['version'] self.assertItemsEqual( [dict(origin_task, type='puppet')], tasks @@ -541,7 +553,8 @@ class TestTaskProcessor(BaseUnitTest): def test_process_if_chain(self): origin_task = { - 'id': 'task', 'requires': ['a'], 'cross-depends': [{'name': 'b'}], + 'id': 'task', 'version': '2.0.0', + 'requires': ['a'], 'cross-depends': [{'name': 'b'}], 'required_for': ['c'], 'cross-depended-by': [{'name': 'd'}] } serialized = iter([ @@ -574,3 +587,25 @@ class TestTaskProcessor(BaseUnitTest): self.assertEqual('task', self.processor.get_origin('task_start')) self.assertEqual('task', self.processor.get_origin('task#1')) self.assertEqual('task', self.processor.get_origin('task_end')) + + def test_ensure_task_based_deployment_allowed(self): + self.assertRaises( + task_based_deployment.errors.TaskBaseDeploymentNotAllowed, + self.processor.ensure_task_based_deploy_allowed, + {'id': 'task'} + ) + self.assertRaises( + task_based_deployment.errors.TaskBaseDeploymentNotAllowed, + self.processor.ensure_task_based_deploy_allowed, + {'id': 'task', 'version': '1.2.3'} + ) + self.assertNotRaises( + task_based_deployment.errors.TaskBaseDeploymentNotAllowed, + self.processor.ensure_task_based_deploy_allowed, + {'id': 'task', 'version': consts.TASK_CROSS_DEPENDENCY} + ) + self.assertNotRaises( + task_based_deployment.errors.TaskBaseDeploymentNotAllowed, + self.processor.ensure_task_based_deploy_allowed, + {'id': 'task', 'type': consts.ORCHESTRATOR_TASK_TYPES.stage} + )