openstack
/
fuel-web
Code Issues Proposed changes

Fixed condition evaluation for task 

The condition for task was checked before evaluation of
YAQL expressions.
Also fixed that extra attributes of task were passed to astute.

Change-Id: Iaed23a8d0f263eef5d56281ee383328a6f0a98cc
Closes-Bug: 1563016
This commit is contained in:
Bulat Gaifullin 2016-03-28 22:28:02 +03:00
parent f374b47a1b
commit eb1ca9e695
3 changed files with 77 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

63
nailgun/nailgun/lcm/task_serializer.py
View File

@ -57,7 +57,7 @@ class Context(object):
self._yaql_context = yaql_ext.create_context(
add_serializers=True, add_datadiff=True
)
self.yaql_expressions_cache = {}
self._yaql_expressions_cache = {}
def get_new_data(self, node_id):
return self._transaction.get_new_data(node_id)
@ -66,7 +66,7 @@ class Context(object):
context = self._yaql_context.create_child_context()
context['$%new'] = self._transaction.get_new_data(node_id)
context['$%old'] = self._transaction.get_old_data(node_id)
cache = self.yaql_expressions_cache
cache = self._yaql_expressions_cache
def evaluate(expression):
try:
@ -112,6 +112,12 @@ class Context(object):
@six.add_metaclass(abc.ABCMeta)
class DeploymentTaskSerializer(object):
fields = frozenset((
'id', 'type', 'parameters', 'fail_on_error',
'requires', 'required_for',
'cross_depends', 'cross_depended_by',
))
@abc.abstractmethod
def serialize(self, node_id):
"""Serialize task in expected by orchestrator format.
@ -122,6 +128,15 @@ class DeploymentTaskSerializer(object):
:param node_id: the target node_id
"""
@classmethod
def get_required_fields(cls, task, fields=None):
"""Gets only specified fields from task.
:param task: the serialized task
:param fields: the list of fields for including
"""
return {k: task.get(k) for k in (fields or cls.fields)}
class NoopTaskSerializer(DeploymentTaskSerializer):
def __init__(self, context, task_template):
@ -129,34 +144,25 @@ class NoopTaskSerializer(DeploymentTaskSerializer):
self.context = context
def serialize(self, node_id):
return {
'id': self.task_template['id'],
'type': consts.ORCHESTRATOR_TASK_TYPES.skipped,
'fail_on_error': False,
'requires': self.task_template.get('requires'),
'required_for': self.task_template.get('required_for'),
'cross_depends': self.task_template.get('cross_depends'),
'cross_depended_by': self.task_template.get('cross_depended_by'),
}
task = self.get_required_fields(
self.task_template, self.fields - {'parameters'}
)
task['type'] = consts.ORCHESTRATOR_TASK_TYPES.skipped
task['fail_on_error'] = False
return task
class DefaultTaskSerializer(NoopTaskSerializer):
hidden_attributes = ('roles', 'role', 'groups', 'condition')
def should_execute(self, node_id):
if 'condition' not in self.task_template:
return True
# the utils.traverse removes all '.value' attributes.
# the option prune_value_attribute is used
# to keep backward compatibility
interpreter = self.context.get_legacy_interpreter(node_id)
return interpreter(self.task_template['condition'])
def should_execute(self, task, node_id):
condition = task.get('condition', True)
if isinstance(condition, six.string_types):
# string condition interprets as legacy condition
# and it should be evaluated
interpreter = self.context.get_legacy_interpreter(node_id)
return interpreter(condition)
return condition
def serialize(self, node_id):
if not self.should_execute(node_id):
return super(DefaultTaskSerializer, self).serialize(node_id)
task = utils.traverse(
self.task_template,
utils.text_format_safe,
@ -165,11 +171,12 @@ class DefaultTaskSerializer(NoopTaskSerializer):
'yaql_exp': self.context.get_yaql_interpreter(node_id)
}
)
if not self.should_execute(task, node_id):
return super(DefaultTaskSerializer, self).serialize(node_id)
task.setdefault('parameters', {}).setdefault('cwd', '/')
task.setdefault('fail_on_error', True)
for attr in self.hidden_attributes:
task.pop(attr, None)
return task
return self.get_required_fields(task)
def handle_unsupported(_, task_template):

14
nailgun/nailgun/test/unit/test_lcm_task_serializers.py
View File

@ -131,7 +131,9 @@ class TestDefaultTaskSerializer(BaseUnitTest):
}
serializer = self.serializer_class(self.context, task_template)
for node_id, result in expected:
self.assertEqual(result, serializer.should_execute(node_id))
self.assertEqual(result, serializer.should_execute(
task_template, node_id
))
def test_should_execute_legacy_condition_for_settings(self):
self.check_condition(
@ -172,7 +174,9 @@ class TestDefaultTaskSerializer(BaseUnitTest):
'path': '/etc/{CLUSTER_ID}/astute.yaml'
},
'requires': ['deploy_start'],
'required_for': ['deploy_end']
'required_for': ['deploy_end'],
'cross_depends': [],
'cross_depended_by': [],
}
serializer = self.serializer_class(self.context, task_template)
serialized = serializer.serialize('1')
@ -193,7 +197,11 @@ class TestDefaultTaskSerializer(BaseUnitTest):
'cmd': "cat /etc/astute.yaml | awk '{ print $1 }'",
'cwd': '/tmp/'
},
'fail_on_error': False
'fail_on_error': False,
'required_for': None,
'requires': None,
'cross_depends': [],
'cross_depended_by': []
}
serializer = self.serializer_class(self.context, task_template)
serialized = serializer.serialize('1')

45
nailgun/nailgun/test/unit/test_lcm_transaction_serializer.py
View File

@ -31,6 +31,9 @@ class TestTransactionSerializer(BaseUnitTest):
{
'id': 'task1', 'roles': ['controller'],
'type': 'puppet', 'version': '2.0.0',
'condition': {
'yaql_exp': '$.public_ssl.hostname = localhost'
},
'parameters': {},
'requires': ['task3'],
'required_for': ['task2'],
@ -40,16 +43,26 @@ class TestTransactionSerializer(BaseUnitTest):
{
'id': 'task2', 'roles': ['compute', 'controller'],
'type': 'puppet', 'version': '2.0.0',
'condition': {
'yaql_exp': '$.public_ssl.hostname != localhost'
},
'parameters': {},
'cross_depends': [{'name': 'task3', 'role': 'cinder'}]
},
{
'id': 'task3', 'roles': ['cinder', 'controller'],
'type': 'puppet', 'version': '2.0.0',
'condition': 'settings:public_ssl.hostname != "localhost"',
'parameters': {},
'cross_depends': [{'name': 'task3', 'role': '/.*/'}],
'cross_depended_by': [{'name': 'task2', 'role': 'self'}]
},
{
'id': 'task4', 'roles': ['controller'],
'type': 'puppet', 'version': '2.0.0',
'parameters': {},
'cross_depended_by': [{'name': 'task3'}]
},
]
cls.nodes = [
@ -114,8 +127,8 @@ class TestTransactionSerializer(BaseUnitTest):
self.datadiff(
[
{
'id': 'task1', 'type': 'puppet', 'version': '2.0.0',
'parameters': {}, 'fail_on_error': True,
'id': 'task1', 'type': 'puppet', 'fail_on_error': True,
'parameters': {},
'requires': [
{'node_id': '1', 'name': 'task3'},
{'node_id': '2', 'name': 'task2'},
@ -126,15 +139,13 @@ class TestTransactionSerializer(BaseUnitTest):
]
},
{
'id': 'task2', 'type': 'puppet', 'version': '2.0.0',
'parameters': {}, 'fail_on_error': True,
'id': 'task2', 'type': 'skipped', 'fail_on_error': False,
'requires': [
{'node_id': '3', 'name': 'task3'},
],
},
{
'id': 'task3', 'type': 'puppet', 'version': '2.0.0',
'parameters': {}, 'fail_on_error': True,
'id': 'task3', 'type': 'skipped', 'fail_on_error': False,
'requires': [
{'node_id': '3', 'name': 'task3'},
],
@ -142,6 +153,15 @@ class TestTransactionSerializer(BaseUnitTest):
{'node_id': '1', 'name': 'task2'},
]
},
{
'id': 'task4', 'type': 'puppet', 'fail_on_error': True,
'parameters': {},
'required_for': [
{'node_id': '1', 'name': 'task3'},
{'node_id': '3', 'name': 'task3'},
]
}
],
serialized['1'],
ignore_keys=['parameters', 'fail_on_error'],
@ -151,8 +171,7 @@ class TestTransactionSerializer(BaseUnitTest):
self.datadiff(
[
{
'id': 'task2', 'type': 'puppet', 'version': '2.0.0',
'parameters': {}, 'fail_on_error': True,
'id': 'task2', 'type': 'skipped', 'fail_on_error': True,
'requires': [
{'node_id': '3', 'name': 'task3'},
],
@ -166,8 +185,7 @@ class TestTransactionSerializer(BaseUnitTest):
self.datadiff(
[
{
'id': 'task3', 'type': 'puppet', 'version': '2.0.0',
'parameters': {}, 'fail_on_error': True,
'id': 'task3', 'type': 'skipped', 'fail_on_error': True,
'requires': [
{'node_id': '1', 'name': 'task3'},
]
@ -261,14 +279,13 @@ class TestTransactionSerializer(BaseUnitTest):
self.datadiff(
[
{
'id': 'task2', 'type': 'puppet', 'version': '2.0.0',
'parameters': {}, 'fail_on_error': True,
'id': 'task2', 'type': 'skipped', 'fail_on_error': True,
'requires': [{'name': 'task3', 'node_id': '3'}]
},
{
'id': 'task4', 'type': 'puppet', 'version': '2.0.0',
'parameters': {}, 'fail_on_error': True,
'id': 'task4', 'type': 'puppet', 'fail_on_error': True,
'parameters': {},
'requires': [{'name': 'task2', 'node_id': '4'}]
},