Give more informative message for deployment graph
Sometimes we have situation when requires or required_for uses task id which not described in system. In such case nailgun should show places where it happens Change-Id: I8004e18a396328f3051f4878da138c7a286450f0 Closes-Bug: #1477258
This commit is contained in:
parent
3732cfb6de
commit
647c6734ec
|
@ -28,11 +28,10 @@ class GraphTasksValidator(BasicValidator):
|
|||
def validate_update(cls, data, instance):
|
||||
parsed = cls.validate(data)
|
||||
cls.validate_schema(parsed, tasks.TASKS_SCHEMA)
|
||||
graph = deployment_graph.DeploymentGraph()
|
||||
graph.add_tasks(parsed)
|
||||
if not graph.is_acyclic():
|
||||
raise errors.InvalidData(
|
||||
"Tasks can not be processed because it contains cycles in it.")
|
||||
graph_validator = deployment_graph.DeploymentGraphValidator(
|
||||
parsed)
|
||||
graph_validator.check()
|
||||
|
||||
return parsed
|
||||
|
||||
|
||||
|
|
|
@ -21,6 +21,7 @@ except ImportError:
|
|||
from ordereddict import OrderedDict
|
||||
|
||||
import networkx as nx
|
||||
import six
|
||||
|
||||
from nailgun import consts
|
||||
from nailgun.errors import errors
|
||||
|
@ -428,3 +429,38 @@ class AstuteGraph(object):
|
|||
|
||||
priority.one_by_one(serialized)
|
||||
return serialized
|
||||
|
||||
|
||||
class DeploymentGraphValidator(object):
|
||||
|
||||
def __init__(self, tasks):
|
||||
self.graph = DeploymentGraph()
|
||||
self.graph.add_tasks(tasks)
|
||||
|
||||
def check(self):
|
||||
if not self.graph.is_acyclic():
|
||||
raise errors.InvalidData(
|
||||
"Tasks can not be processed because it contains cycles in it.")
|
||||
|
||||
non_existing_tasks = []
|
||||
invalid_tasks = []
|
||||
|
||||
for node_key, node_value in six.iteritems(self.graph.node):
|
||||
if not node_value.get('id'):
|
||||
successors = self.graph.successors(node_key)
|
||||
predecessors = self.graph.predecessors(node_key)
|
||||
|
||||
neighbors = successors + predecessors
|
||||
|
||||
non_existing_tasks.append(node_key)
|
||||
invalid_tasks.extend(neighbors)
|
||||
|
||||
if non_existing_tasks:
|
||||
raise errors.InvalidData(
|
||||
"Tasks '{non_existing_tasks}' can't be in requires"
|
||||
"|required_for|groups|tasks for [{invalid_tasks}]"
|
||||
" because they don't exist in the graph".format(
|
||||
non_existing_tasks=', '.join(
|
||||
str(x) for x in sorted(non_existing_tasks)),
|
||||
invalid_tasks=', '.join(
|
||||
str(x) for x in sorted(set(invalid_tasks)))))
|
||||
|
|
|
@ -803,6 +803,7 @@ class CheckBeforeDeploymentTask(object):
|
|||
cls._check_public_network(task)
|
||||
cls._check_vmware_consistency(task)
|
||||
cls._validate_network_template(task)
|
||||
cls._check_deployment_graph_for_correctness(task)
|
||||
|
||||
if objects.Release.is_external_mongo_enabled(task.cluster.release):
|
||||
cls._check_mongo_nodes(task)
|
||||
|
@ -1047,6 +1048,16 @@ class CheckBeforeDeploymentTask(object):
|
|||
'configuration template').format(error_roles)
|
||||
raise errors.NetworkTemplateMissingNetRoles(error_msg)
|
||||
|
||||
@classmethod
|
||||
def _check_deployment_graph_for_correctness(self, task):
|
||||
"""Check that deployment graph hasn't not existing dependencies(
|
||||
such as requires|required_for|tasks|groups)
|
||||
"""
|
||||
deployment_tasks = objects.Cluster.get_deployment_tasks(task.cluster)
|
||||
graph_validator = deployment_graph.DeploymentGraphValidator(
|
||||
deployment_tasks)
|
||||
graph_validator.check()
|
||||
|
||||
|
||||
class DumpTask(object):
|
||||
@classmethod
|
||||
|
|
|
@ -607,9 +607,9 @@ class EnvironmentManager(object):
|
|||
{
|
||||
'id': 'role-name',
|
||||
'type': 'group',
|
||||
'role': '[role-name]',
|
||||
'requires': '[controller]',
|
||||
'required_for': '[deploy_end]',
|
||||
'role': ['role-name'],
|
||||
'requires': ['controller'],
|
||||
'required_for': ['deploy_end'],
|
||||
'parameters': {
|
||||
'strategy': {
|
||||
'type': 'parallel'
|
||||
|
|
|
@ -34,18 +34,30 @@ class BaseGraphTasksTests(BaseIntegrationTest):
|
|||
|
||||
def get_correct_tasks(self):
|
||||
yaml_tasks = """
|
||||
- id: deploy_start
|
||||
type: stage
|
||||
requires: [pre_deployment_end]
|
||||
- id: deploy_end
|
||||
type: stage
|
||||
requires: [deploy_start]
|
||||
- id: pre_deployment_start
|
||||
type: stage
|
||||
- id: pre_deployment_end
|
||||
type: stage
|
||||
requires: [pre_deployment_start]
|
||||
- id: primary-controller
|
||||
type: group
|
||||
role: [primary-controller]
|
||||
required_for: [deploy]
|
||||
required_for: [deploy_end]
|
||||
requires: [deploy_start]
|
||||
parameters:
|
||||
strategy:
|
||||
type: one_by_one
|
||||
- id: controller
|
||||
- id: test-controller
|
||||
type: group
|
||||
role: [primary-controller]
|
||||
role: [test-controller]
|
||||
requires: [primary-controller]
|
||||
required_for: [deploy]
|
||||
required_for: [deploy_end]
|
||||
parameters:
|
||||
strategy:
|
||||
type: parallel
|
||||
|
@ -55,8 +67,8 @@ class BaseGraphTasksTests(BaseIntegrationTest):
|
|||
|
||||
def get_corrupted_tasks(self):
|
||||
yaml_tasks = """
|
||||
- id: primary-controller
|
||||
required_for: [deploy]
|
||||
- id: test-controller
|
||||
required_for: [deploy_end]
|
||||
parameters:
|
||||
strategy:
|
||||
type: one_by_one
|
||||
|
@ -65,12 +77,24 @@ class BaseGraphTasksTests(BaseIntegrationTest):
|
|||
|
||||
def get_tasks_with_cycles(self):
|
||||
yaml_tasks = """
|
||||
- id: primary-controller
|
||||
- id: test-controller-1
|
||||
type: role
|
||||
requires: [controller]
|
||||
- id: controller
|
||||
requires: [test-controller-2]
|
||||
- id: test-controller-2
|
||||
type: role
|
||||
requires: [primary-controller]
|
||||
requires: [test-controller-1]
|
||||
"""
|
||||
return yaml.load(yaml_tasks)
|
||||
|
||||
def get_tasks_with_incorrect_dependencies(self):
|
||||
yaml_tasks = """
|
||||
- id: test-controller
|
||||
type: group
|
||||
role: [test-controller]
|
||||
required_for: [non_existing_stage]
|
||||
parameters:
|
||||
strategy:
|
||||
type: one_by_one
|
||||
"""
|
||||
return yaml.load(yaml_tasks)
|
||||
|
||||
|
@ -121,6 +145,22 @@ class TestReleaseGraphHandler(BaseGraphTasksTests):
|
|||
)
|
||||
self.assertEqual(resp.status_code, 400)
|
||||
|
||||
def test_upload_tasks_with_incorrect_dependencies(self):
|
||||
tasks = self.get_tasks_with_incorrect_dependencies()
|
||||
resp = self.app.put(
|
||||
reverse('ReleaseDeploymentTasksHandler',
|
||||
kwargs={'obj_id': self.cluster.release_id}),
|
||||
params=jsonutils.dumps(tasks),
|
||||
headers=self.default_headers,
|
||||
expect_errors=True
|
||||
)
|
||||
|
||||
self.assertEqual(resp.status_code, 400)
|
||||
self.assertEqual(
|
||||
"Tasks 'non_existing_stage' can't be in requires|required_for|"
|
||||
"groups|tasks for [test-controller] because they don't exist in "
|
||||
"the graph", resp.json_body['message'])
|
||||
|
||||
def test_post_tasks(self):
|
||||
resp = self.app.post(
|
||||
reverse('ReleaseDeploymentTasksHandler',
|
||||
|
@ -194,6 +234,21 @@ class TestClusterGraphHandler(BaseGraphTasksTests):
|
|||
)
|
||||
self.assertEqual(resp.status_code, 400)
|
||||
|
||||
def test_upload_tasks_with_incorrect_dependencies(self):
|
||||
tasks = self.get_tasks_with_incorrect_dependencies()
|
||||
resp = self.app.put(
|
||||
reverse('ClusterDeploymentTasksHandler',
|
||||
kwargs={'obj_id': self.cluster.id}),
|
||||
params=jsonutils.dumps(tasks),
|
||||
headers=self.default_headers,
|
||||
expect_errors=True
|
||||
)
|
||||
self.assertEqual(resp.status_code, 400)
|
||||
self.assertEqual(
|
||||
"Tasks 'non_existing_stage' can't be in requires|required_for|"
|
||||
"groups|tasks for [test-controller] because they don't exist in "
|
||||
"the graph", resp.json_body['message'])
|
||||
|
||||
def test_post_tasks(self):
|
||||
resp = self.app.post(
|
||||
reverse('ClusterDeploymentTasksHandler',
|
||||
|
|
|
@ -20,6 +20,7 @@ from itertools import groupby
|
|||
import mock
|
||||
import yaml
|
||||
|
||||
from nailgun.errors import errors
|
||||
from nailgun.orchestrator import deployment_graph
|
||||
from nailgun.orchestrator import graph_configuration
|
||||
from nailgun.test import base
|
||||
|
@ -699,3 +700,65 @@ class TestIncludeSkipped(base.BaseTestCase):
|
|||
self.assertItemsEqual(
|
||||
subgraph.nodes(),
|
||||
[t['id'] for t in self.tasks])
|
||||
|
||||
|
||||
class TestDeploymentGraphValidator(base.BaseTestCase):
|
||||
|
||||
def test_validation_pass_with_existing_dependencies(self):
|
||||
yaml_tasks = """
|
||||
- id: deploy_end
|
||||
type: stage
|
||||
- id: pre_deployment_start
|
||||
type: stage
|
||||
- id: test-controller
|
||||
type: group
|
||||
role: [test-controller]
|
||||
requires: [pre_deployment_start]
|
||||
required_for: [deploy_end]
|
||||
parameters:
|
||||
strategy:
|
||||
type: parallel
|
||||
amount: 2
|
||||
"""
|
||||
tasks = yaml.load(yaml_tasks)
|
||||
graph_validator = deployment_graph.DeploymentGraphValidator(tasks)
|
||||
graph_validator.check()
|
||||
|
||||
def test_validation_failed_with_not_existing_dependencies(self):
|
||||
dependencies_types = ['requires', 'required_for', 'groups', 'tasks']
|
||||
|
||||
for dependency_type in dependencies_types:
|
||||
yaml_tasks = """
|
||||
- id: test-controller
|
||||
type: group
|
||||
role: [test-controlle]
|
||||
{dependency_type}: [non_existing_stage]
|
||||
parameters:
|
||||
strategy:
|
||||
type: one_by_one
|
||||
""".format(dependency_type=dependency_type)
|
||||
tasks = yaml.load(yaml_tasks)
|
||||
graph_validator = deployment_graph.DeploymentGraphValidator(tasks)
|
||||
|
||||
with self.assertRaisesRegexp(
|
||||
errors.InvalidData,
|
||||
"Tasks 'non_existing_stage' can't be in requires|"
|
||||
"required_for|groups|tasks for \['test-controller'\] "
|
||||
"because they don't exist in the graph"):
|
||||
graph_validator.check()
|
||||
|
||||
def test_validation_failed_with_cycling_dependencies(self):
|
||||
yaml_tasks = """
|
||||
- id: test-controller-1
|
||||
type: role
|
||||
requires: [test-controller-2]
|
||||
- id: test-controller-2
|
||||
type: role
|
||||
requires: [test-controller-1]
|
||||
"""
|
||||
tasks = yaml.load(yaml_tasks)
|
||||
graph_validator = deployment_graph.DeploymentGraphValidator(tasks)
|
||||
with self.assertRaisesRegexp(
|
||||
errors.InvalidData,
|
||||
"Tasks can not be processed because it contains cycles in it"):
|
||||
graph_validator.check()
|
||||
|
|
|
@ -15,6 +15,7 @@
|
|||
|
||||
from mock import patch
|
||||
from oslo_serialization import jsonutils
|
||||
import yaml
|
||||
|
||||
from nailgun import consts
|
||||
from nailgun.db.sqlalchemy.models import Task
|
||||
|
@ -390,3 +391,69 @@ class TestCheckBeforeDeploymentTask(BaseTestCase):
|
|||
errors.NetworkCheckError,
|
||||
CheckBeforeDeploymentTask._check_public_network,
|
||||
self.task)
|
||||
|
||||
def test_check_deployment_graph_with_correct_data(self):
|
||||
correct_yaml_tasks = """
|
||||
- id: test-controller
|
||||
type: group
|
||||
role: [test-controller]
|
||||
requires: [primary-controller]
|
||||
required_for: [deploy_end]
|
||||
parameters:
|
||||
strategy:
|
||||
type: parallel
|
||||
amount: 2
|
||||
"""
|
||||
tasks = yaml.load(correct_yaml_tasks)
|
||||
deployment_tasks = objects.Cluster.get_deployment_tasks(self.cluster)
|
||||
deployment_tasks.extend(tasks)
|
||||
objects.Cluster.update(
|
||||
self.cluster,
|
||||
{'deployment_tasks': deployment_tasks})
|
||||
CheckBeforeDeploymentTask._check_deployment_graph_for_correctness(
|
||||
self.task)
|
||||
|
||||
def test_check_deployment_graph_with_incorrect_dependencies_data(self):
|
||||
incorrect_dependencies_yaml_tasks = """
|
||||
- id: test-controller
|
||||
type: group
|
||||
role: [primary-controller]
|
||||
required_for: [non_existing_stage]
|
||||
parameters:
|
||||
strategy:
|
||||
type: one_by_one
|
||||
"""
|
||||
tasks = yaml.load(incorrect_dependencies_yaml_tasks)
|
||||
deployment_tasks = objects.Cluster.get_deployment_tasks(self.cluster)
|
||||
deployment_tasks.extend(tasks)
|
||||
objects.Cluster.update(
|
||||
self.cluster,
|
||||
{'deployment_tasks': deployment_tasks})
|
||||
with self.assertRaisesRegexp(
|
||||
errors.InvalidData,
|
||||
"Tasks 'non_existing_stage' can't be in requires|required_for|"
|
||||
"groups|tasks for \['test-controller'\] because they don't "
|
||||
"exist in the graph"):
|
||||
CheckBeforeDeploymentTask._check_deployment_graph_for_correctness(
|
||||
self.task)
|
||||
|
||||
def test_check_deployment_graph_with_cycling_dependencies_data(self):
|
||||
incorrect_cycle_yaml_tasks = """
|
||||
- id: test-controller-1
|
||||
type: role
|
||||
requires: [test-controller-2]
|
||||
- id: test-controller-2
|
||||
type: role
|
||||
requires: [test-controller-1]
|
||||
"""
|
||||
tasks = yaml.load(incorrect_cycle_yaml_tasks)
|
||||
deployment_tasks = objects.Cluster.get_deployment_tasks(self.cluster)
|
||||
deployment_tasks.extend(tasks)
|
||||
objects.Cluster.update(
|
||||
self.cluster,
|
||||
{'deployment_tasks': deployment_tasks})
|
||||
with self.assertRaisesRegexp(
|
||||
errors.InvalidData,
|
||||
"Tasks can not be processed because it contains cycles in it"):
|
||||
CheckBeforeDeploymentTask._check_deployment_graph_for_correctness(
|
||||
self.task)
|
||||
|
|
Loading…
Reference in New Issue