diff --git a/nailgun/nailgun/db/sqlalchemy/models/task.py b/nailgun/nailgun/db/sqlalchemy/models/task.py index f6d85534d5..583b563db7 100644 --- a/nailgun/nailgun/db/sqlalchemy/models/task.py +++ b/nailgun/nailgun/db/sqlalchemy/models/task.py @@ -74,11 +74,11 @@ class Task(Base): self.status ) - def create_subtask(self, name): + def create_subtask(self, name, **kwargs): if not name: raise ValueError("Subtask name not specified") - task = Task(name=name, cluster=self.cluster) + task = Task(name=name, cluster=self.cluster, **kwargs) self.subtasks.append(task) db().commit() diff --git a/nailgun/nailgun/task/helpers.py b/nailgun/nailgun/task/helpers.py index be2e905317..8a5111cfee 100644 --- a/nailgun/nailgun/task/helpers.py +++ b/nailgun/nailgun/task/helpers.py @@ -225,21 +225,27 @@ class TaskHelper(object): subtasks ) if subtasks_with_progress: - task.progress = int( - round( - sum( - [s.weight * s.progress for s - in subtasks_with_progress] - ) / - sum( - [s.weight for s - in subtasks_with_progress] - ), 0) + task.progress = cls._calculate_parent_task_progress( + subtasks_with_progress ) else: task.progress = 0 db().commit() + @classmethod + def _calculate_parent_task_progress(cls, subtasks_list): + return int( + round( + sum( + [s.weight * s.progress for s + in subtasks_list] + ) / + sum( + [s.weight for s + in subtasks_list] + ), 0) + ) + @classmethod def update_cluster_status(cls, uuid): task = db().query(Task).filter_by(uuid=uuid).first() diff --git a/nailgun/nailgun/task/manager.py b/nailgun/nailgun/task/manager.py index 3d59e93de8..45d7450dc4 100644 --- a/nailgun/nailgun/task/manager.py +++ b/nailgun/nailgun/task/manager.py @@ -161,7 +161,10 @@ class ApplyChangesTaskManager(TaskManager): task_deletion, task_provision, task_deployment = None, None, None if nodes_to_delete: - task_deletion = supertask.create_subtask("node_deletion") + # For more accurate progress calulation + task_weight = 0.4 + task_deletion = supertask.create_subtask("node_deletion", + weight=task_weight) logger.debug("Launching deletion task: %s", task_deletion.uuid) self._call_silently(task_deletion, tasks.DeletionTask) @@ -169,9 +172,11 @@ class ApplyChangesTaskManager(TaskManager): TaskHelper.update_slave_nodes_fqdn(nodes_to_provision) logger.debug("There are nodes to provision: %s", " ".join([n.fqdn for n in nodes_to_provision])) - task_provision = supertask.create_subtask("provision") + # For more accurate progress calulation - task_provision.weight = 0.4 + task_weight = 0.4 + task_provision = supertask.create_subtask("provision", + weight=task_weight) provision_message = self._call_silently( task_provision, tasks.ProvisionTask, diff --git a/nailgun/nailgun/test/integration/test_rpc_consumer.py b/nailgun/nailgun/test/integration/test_rpc_consumer.py index 4e15780654..013e6ec488 100644 --- a/nailgun/nailgun/test/integration/test_rpc_consumer.py +++ b/nailgun/nailgun/test/integration/test_rpc_consumer.py @@ -16,6 +16,7 @@ import json from mock import patch +import random import uuid from nailgun.db.sqlalchemy.models import Attributes @@ -26,6 +27,7 @@ from nailgun.db.sqlalchemy.models import Node from nailgun.db.sqlalchemy.models import Notification from nailgun.db.sqlalchemy.models import Task from nailgun.rpc import receiver as rcvr +from nailgun.task import helpers from nailgun.test.base import BaseIntegrationTest from nailgun.test.base import reverse @@ -745,6 +747,103 @@ class TestConsumer(BaseIntegrationTest): self.assertEqual(task.progress, 20) self.assertEqual(task.status, "running") + def test_node_deletion_subtask_progress(self): + supertask = Task( + uuid=str(uuid.uuid4()), + name="super", + status="running" + ) + + self.db.add(supertask) + self.db.commit() + + task_deletion = supertask.create_subtask("node_deletion") + task_provision = supertask.create_subtask("provision", weight=0.4) + + subtask_progress = random.randint(1, 20) + + deletion_kwargs = {'task_uuid': task_deletion.uuid, + 'progress': subtask_progress} + provision_kwargs = {'task_uuid': task_provision.uuid, + 'progress': subtask_progress} + + def progress_difference(): + self.receiver.provision_resp(**provision_kwargs) + + self.db.refresh(task_provision) + self.assertEqual(task_provision.progress, subtask_progress) + + self.db.refresh(supertask) + progress_before_delete_subtask = supertask.progress + + self.receiver.remove_nodes_resp(**deletion_kwargs) + + self.db.refresh(task_deletion) + self.assertEqual(task_deletion.progress, subtask_progress) + + self.db.refresh(supertask) + progress_after_delete_subtask = supertask.progress + + return abs(progress_after_delete_subtask - + progress_before_delete_subtask) + + without_coeff = progress_difference() + + task_deletion.progress = 0 + task_deletion.weight = 0.5 + self.db.merge(task_deletion) + + task_provision.progress = 0 + self.db.merge(task_provision) + + supertask.progress = 0 + self.db.merge(supertask) + + self.db.commit() + + with_coeff = progress_difference() + + # some freaking magic is here but haven't found + # better way to test what is already working + self.assertTrue((without_coeff / with_coeff) < 2) + + def test_proper_progress_calculation(self): + supertask = Task( + uuid=str(uuid.uuid4()), + name="super", + status="running" + ) + + self.db.add(supertask) + self.db.commit() + + subtask_weight = 0.4 + task_deletion = supertask.create_subtask("node_deletion", + weight=subtask_weight) + task_provision = supertask.create_subtask("provision", + weight=subtask_weight) + + subtask_progress = random.randint(1, 20) + + deletion_kwargs = {'task_uuid': task_deletion.uuid, + 'progress': subtask_progress} + provision_kwargs = {'task_uuid': task_provision.uuid, + 'progress': subtask_progress} + + self.receiver.provision_resp(**provision_kwargs) + self.receiver.remove_nodes_resp(**deletion_kwargs) + + self.db.refresh(task_deletion) + self.db.refresh(task_provision) + self.db.refresh(supertask) + + calculated_progress = helpers.\ + TaskHelper._calculate_parent_task_progress( + [task_deletion, task_provision] + ) + + self.assertEqual(supertask.progress, calculated_progress) + def test_error_node_progress(self): self.env.create( cluster_kwargs={},