Merge "Disabled unnecessary checks for version of tasks"

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

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)

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}
+        )