From 8f88e7877835f7baa981e978f0d8f665fb6264c5 Mon Sep 17 00:00:00 2001 From: Ana Krivokapic Date: Fri, 18 May 2018 22:11:10 +0200 Subject: [PATCH] Add support for custom validations This patch introduces support for running custom validations by changing the behavior of the validations actions ListValidationsAction, ListGroupsAction and RunValidationAction. Until now, these actions sourced validations from a directory on disk. Now, these action are sourcing validations from the plan container subdirectory (custom validations), or, if this is not available, from the Swift container holding the default validations. Change-Id: I9e9131b355312c53f12d154976d5d9cd706cc338 Implements: blueprint custom-validations Depends-On: I338e139fa770ebb7bdcc1c0afb79eec062fada8b --- tripleo_common/actions/validations.py | 29 ++++- tripleo_common/constants.py | 3 + .../tests/actions/test_validations.py | 53 +++++--- .../tests/utils/test_validations.py | 116 ++++++++++++++---- tripleo_common/utils/swift.py | 28 ++++- tripleo_common/utils/validations.py | 111 +++++++++++++---- workbooks/validations.yaml | 9 +- 7 files changed, 272 insertions(+), 77 deletions(-) diff --git a/tripleo_common/actions/validations.py b/tripleo_common/actions/validations.py index e6d7def9b..3b37c6a88 100644 --- a/tripleo_common/actions/validations.py +++ b/tripleo_common/actions/validations.py @@ -12,9 +12,12 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +import six + from mistral_lib import actions from mistralclient.api import base as mistralclient_api from oslo_concurrency.processutils import ProcessExecutionError +from swiftclient import exceptions as swiftexceptions from tripleo_common.actions import base from tripleo_common import constants @@ -82,19 +85,34 @@ class Enabled(base.TripleOAction): class ListValidationsAction(base.TripleOAction): """Return a set of TripleO validations""" - def __init__(self, groups=None): + def __init__(self, plan=constants.DEFAULT_CONTAINER_NAME, groups=None): super(ListValidationsAction, self).__init__() self.groups = groups + self.plan = plan def run(self, context): - return utils.load_validations(groups=self.groups) + swift = self.get_object_client(context) + try: + return utils.load_validations( + swift, plan=self.plan, groups=self.groups) + except swiftexceptions.ClientException as err: + msg = "Error loading validations from Swift: %s" % err + return actions.Result(error={"msg": six.text_type(msg)}) class ListGroupsAction(base.TripleOAction): """Return a set of TripleO validation groups""" + def __init__(self, plan=constants.DEFAULT_CONTAINER_NAME): + super(ListGroupsAction, self).__init__() + self.plan = plan def run(self, context): - validations = utils.load_validations() + swift = self.get_object_client(context) + try: + validations = utils.load_validations(swift, plan=self.plan) + except swiftexceptions.ClientException as err: + msg = "Error loading validations from Swift: %s" % err + return actions.Result(error={"msg": six.text_type(msg)}) return { group for validation in validations for group in validation['groups'] @@ -110,13 +128,16 @@ class RunValidationAction(base.TripleOAction): def run(self, context): mc = self.get_workflow_client(context) + swift = self.get_object_client(context) + identity_file = None try: env = mc.environments.get('ssh_keys') private_key = env.variables['private_key'] identity_file = utils.write_identity_file(private_key) - stdout, stderr = utils.run_validation(self.validation, + stdout, stderr = utils.run_validation(swift, + self.validation, identity_file, self.plan, context) diff --git a/tripleo_common/constants.py b/tripleo_common/constants.py index c798ca75b..413584b7a 100644 --- a/tripleo_common/constants.py +++ b/tripleo_common/constants.py @@ -47,6 +47,9 @@ CONFIG_CONTAINER_NAME = 'overcloud-config' #: The default name to use for the container for validations VALIDATIONS_CONTAINER_NAME = 'tripleo-validations' +#: The name of the plan subdirectory that holds custom validations +CUSTOM_VALIDATIONS_FOLDER = 'custom-validations' + #: The default key to use for updating parameters in plan environment. DEFAULT_PLAN_ENV_KEY = 'parameter_defaults' diff --git a/tripleo_common/tests/actions/test_validations.py b/tripleo_common/tests/actions/test_validations.py index 342b5192c..d0cac5b1d 100644 --- a/tripleo_common/tests/actions/test_validations.py +++ b/tripleo_common/tests/actions/test_validations.py @@ -113,47 +113,64 @@ class Enabled(base.TestCase): class ListValidationsActionTest(base.TestCase): @mock.patch('tripleo_common.utils.validations.load_validations') - def test_run_default(self, mock_load_validations): + @mock.patch('tripleo_common.actions.base.TripleOAction.get_object_client') + def test_run_default(self, mock_get_object_client, mock_load_validations): mock_ctx = mock.MagicMock() + swiftclient = mock.MagicMock(url='http://swift:8080/v1/AUTH_test') + mock_get_object_client.return_value = swiftclient mock_load_validations.return_value = 'list of validations' - action = validations.ListValidationsAction() + + action = validations.ListValidationsAction(plan='overcloud') self.assertEqual('list of validations', action.run(mock_ctx)) - mock_load_validations.assert_called_once_with(groups=None) + mock_load_validations.assert_called_once_with( + mock_get_object_client(), plan='overcloud', groups=None) @mock.patch('tripleo_common.utils.validations.load_validations') - def test_run_groups(self, mock_load_validations): + @mock.patch('tripleo_common.actions.base.TripleOAction.get_object_client') + def test_run_groups(self, mock_get_object_client, mock_load_validations): mock_ctx = mock.MagicMock() + swiftclient = mock.MagicMock(url='http://swift:8080/v1/AUTH_test') + mock_get_object_client.return_value = swiftclient mock_load_validations.return_value = 'list of validations' - action = validations.ListValidationsAction(groups=['group1', - 'group2']) + + action = validations.ListValidationsAction( + plan='overcloud', groups=['group1', 'group2']) self.assertEqual('list of validations', action.run(mock_ctx)) - mock_load_validations.assert_called_once_with(groups=['group1', - 'group2']) + mock_load_validations.assert_called_once_with( + mock_get_object_client(), plan='overcloud', + groups=['group1', 'group2']) class ListGroupsActionTest(base.TestCase): @mock.patch('tripleo_common.utils.validations.load_validations') - def test_run(self, mock_load_validations): + @mock.patch('tripleo_common.actions.base.TripleOAction.get_object_client') + def test_run(self, mock_get_object_client, mock_load_validations): mock_ctx = mock.MagicMock() + swiftclient = mock.MagicMock(url='http://swift:8080/v1/AUTH_test') + mock_get_object_client.return_value = swiftclient mock_load_validations.return_value = [ test_validations.VALIDATION_GROUPS_1_2_PARSED, test_validations.VALIDATION_GROUP_1_PARSED, test_validations.VALIDATION_WITH_METADATA_PARSED] - action = validations.ListGroupsAction() - self.assertEqual(set(['group1', 'group2']), action.run(mock_ctx)) - mock_load_validations.assert_called_once_with() + + action = validations.ListGroupsAction(plan='overcloud') + self.assertEqual({'group1', 'group2'}, action.run(mock_ctx)) + mock_load_validations.assert_called_once_with( + mock_get_object_client(), plan='overcloud') class RunValidationActionTest(base.TestCase): @mock.patch( 'tripleo_common.actions.base.TripleOAction.get_workflow_client') + @mock.patch('tripleo_common.actions.base.TripleOAction.get_object_client') @mock.patch('tripleo_common.utils.validations.write_identity_file') @mock.patch('tripleo_common.utils.validations.cleanup_identity_file') @mock.patch('tripleo_common.utils.validations.run_validation') def test_run(self, mock_run_validation, mock_cleanup_identity_file, - mock_write_identity_file, get_workflow_client_mock): + mock_write_identity_file, mock_get_object_client, + get_workflow_client_mock): mock_ctx = mock.MagicMock() mistral = mock.MagicMock() get_workflow_client_mock.return_value = mistral @@ -161,6 +178,8 @@ class RunValidationActionTest(base.TestCase): mistral.environments.get.return_value = environment(variables={ 'private_key': 'shhhh' }) + swiftclient = mock.MagicMock(url='http://swift:8080/v1/AUTH_test') + mock_get_object_client.return_value = swiftclient mock_write_identity_file.return_value = 'identity_file_path' mock_run_validation.return_value = 'output', 'error' action = validations.RunValidationAction('validation') @@ -173,6 +192,7 @@ class RunValidationActionTest(base.TestCase): self.assertEqual(expected, action.run(mock_ctx)) mock_write_identity_file.assert_called_once_with('shhhh') mock_run_validation.assert_called_once_with( + mock_get_object_client(), 'validation', 'identity_file_path', constants.DEFAULT_CONTAINER_NAME, @@ -182,11 +202,13 @@ class RunValidationActionTest(base.TestCase): @mock.patch( 'tripleo_common.actions.base.TripleOAction.get_workflow_client') + @mock.patch('tripleo_common.actions.base.TripleOAction.get_object_client') @mock.patch('tripleo_common.utils.validations.write_identity_file') @mock.patch('tripleo_common.utils.validations.cleanup_identity_file') @mock.patch('tripleo_common.utils.validations.run_validation') def test_run_failing(self, mock_run_validation, mock_cleanup_identity_file, - mock_write_identity_file, get_workflow_client_mock): + mock_write_identity_file, mock_get_object_client, + get_workflow_client_mock): mock_ctx = mock.MagicMock() mistral = mock.MagicMock() get_workflow_client_mock.return_value = mistral @@ -194,6 +216,8 @@ class RunValidationActionTest(base.TestCase): mistral.environments.get.return_value = environment(variables={ 'private_key': 'shhhh' }) + swiftclient = mock.MagicMock(url='http://swift:8080/v1/AUTH_test') + mock_get_object_client.return_value = swiftclient mock_write_identity_file.return_value = 'identity_file_path' mock_run_validation.side_effect = ProcessExecutionError( stdout='output', stderr='error') @@ -207,6 +231,7 @@ class RunValidationActionTest(base.TestCase): self.assertEqual(expected, action.run(mock_ctx)) mock_write_identity_file.assert_called_once_with('shhhh') mock_run_validation.assert_called_once_with( + mock_get_object_client(), 'validation', 'identity_file_path', constants.DEFAULT_CONTAINER_NAME, diff --git a/tripleo_common/tests/utils/test_validations.py b/tripleo_common/tests/utils/test_validations.py index d8ad2b8b5..ea389a8cc 100644 --- a/tripleo_common/tests/utils/test_validations.py +++ b/tripleo_common/tests/utils/test_validations.py @@ -22,6 +22,28 @@ from tripleo_common.tests import base from tripleo_common.utils import validations +VALIDATION_DEFAULT = """--- +- hosts: overcloud + vars: + metadata: + name: First validation + description: Default validation + tasks: + - name: Ping the nodes + ping: +""" + +VALIDATION_CUSTOM = """--- +- hosts: overcloud + vars: + metadata: + name: First validation + description: Custom validation + tasks: + - name: Ping the nodes + ping: +""" + VALIDATION_GROUP_1 = """--- - hosts: overcloud vars: @@ -138,45 +160,83 @@ class LoadValidationsTest(base.TestCase): value = validations.get_remaining_metadata(validation) self.assertEqual({}, value) - @mock.patch('glob.glob') - def test_load_validations_no_group(self, mock_glob): - mock_glob.return_value = ['VALIDATION_GROUP_1', - 'VALIDATION_WITH_METADATA'] - mock_open_context = mock.mock_open() - mock_open_context().read.side_effect = [VALIDATION_GROUP_1, - VALIDATION_WITH_METADATA] + @mock.patch('tripleo_common.actions.base.TripleOAction.get_object_client') + def test_load_validations_no_group(self, mock_get_object_client): + swiftclient = mock.MagicMock(url='http://swift:8080/v1/AUTH_test') + swiftclient.get_container.side_effect = ( + ({}, []), # no custom validations + ({}, + [{'name': 'VALIDATION_GROUP_1.yaml', 'groups': ['group1']}, + {'name': 'VALIDATION_WITH_METADATA.yaml'}])) + swiftclient.get_object.side_effect = ( + ({}, VALIDATION_GROUP_1), + ({}, VALIDATION_WITH_METADATA), + ) + mock_get_object_client.return_value = swiftclient - with mock.patch('tripleo_common.utils.validations.open', - mock_open_context): - my_validations = validations.load_validations() + my_validations = validations.load_validations( + mock_get_object_client(), plan='overcloud') expected = [VALIDATION_GROUP_1_PARSED, VALIDATION_WITH_METADATA_PARSED] self.assertEqual(expected, my_validations) - @mock.patch('glob.glob') - def test_load_validations_group(self, mock_glob): - mock_glob.return_value = ['VALIDATION_GROUPS_1_2', - 'VALIDATION_GROUP_1', - 'VALIDATION_WITH_METADATA'] - mock_open_context = mock.mock_open() - mock_open_context().read.side_effect = [VALIDATION_GROUPS_1_2, - VALIDATION_GROUP_1, - VALIDATION_WITH_METADATA] + @mock.patch('tripleo_common.actions.base.TripleOAction.get_object_client') + def test_load_validations_group(self, mock_get_object_client): + swiftclient = mock.MagicMock(url='http://swift:8080/v1/AUTH_test') + swiftclient.get_container.side_effect = ( + ({}, []), # no custom validations + ({}, + [ + {'name': 'VALIDATION_GROUPS_1_2.yaml', + 'groups': ['group1', 'group2']}, + {'name': 'VALIDATION_GROUP_1.yaml', 'groups': ['group1']}, + {'name': 'VALIDATION_WITH_METADATA.yaml'} + ] + ) + ) + swiftclient.get_object.side_effect = ( + ({}, VALIDATION_GROUPS_1_2), + ({}, VALIDATION_GROUP_1), + ({}, VALIDATION_WITH_METADATA), + ) + mock_get_object_client.return_value = swiftclient - with mock.patch('tripleo_common.utils.validations.open', - mock_open_context): - my_validations = validations.load_validations(groups=['group1']) + my_validations = validations.load_validations( + mock_get_object_client(), plan='overcloud', groups=['group1']) expected = [VALIDATION_GROUPS_1_2_PARSED, VALIDATION_GROUP_1_PARSED] self.assertEqual(expected, my_validations) + @mock.patch('tripleo_common.actions.base.TripleOAction.get_object_client') + def test_load_validations_custom_gets_picked_over_default( + self, mock_get_object_client): + swiftclient = mock.MagicMock(url='http://swift:8080/v1/AUTH_test') + swiftclient.get_container.side_effect = ( + ({}, [{'name': 'FIRST_VALIDATION.yaml'}]), + ({}, [{'name': 'FIRST_VALIDATION.yaml'}]) + ) + swiftclient.get_object.side_effect = ( + ({}, VALIDATION_CUSTOM), + ({}, VALIDATION_DEFAULT) + ) + mock_get_object_client.return_value = swiftclient + + my_validations = validations.load_validations( + mock_get_object_client(), plan='overcloud') + + self.assertEqual(len(my_validations), 1) + self.assertEqual('Custom validation', my_validations[0]['description']) + class RunValidationTest(base.TestCase): - @mock.patch('tripleo_common.utils.validations.find_validation') + @mock.patch('tripleo_common.actions.base.TripleOAction.get_object_client') + @mock.patch('tripleo_common.utils.validations.download_validation') @mock.patch('oslo_concurrency.processutils.execute') def test_run_validation(self, mock_execute, - mock_find_validation): + mock_download_validation, mock_get_object_client): + swiftclient = mock.MagicMock(url='http://swift:8080/v1/AUTH_test') + mock_get_object_client.return_value = swiftclient Ctx = namedtuple('Ctx', 'auth_uri user_name auth_token project_name') mock_ctx = Ctx( auth_uri='auth_uri', @@ -185,9 +245,10 @@ class RunValidationTest(base.TestCase): project_name='project_name' ) mock_execute.return_value = 'output' - mock_find_validation.return_value = 'validation_path' + mock_download_validation.return_value = 'validation_path' - result = validations.run_validation('validation', 'identity_file', + result = validations.run_validation(mock_get_object_client(), + 'validation', 'identity_file', 'plan', mock_ctx) self.assertEqual('output', result) mock_execute.assert_called_once_with( @@ -201,7 +262,8 @@ class RunValidationTest(base.TestCase): 'identity_file', 'plan' ) - mock_find_validation.assert_called_once_with('validation') + mock_download_validation.assert_called_once_with( + mock_get_object_client(), 'plan', 'validation') class RunPatternValidatorTest(base.TestCase): diff --git a/tripleo_common/utils/swift.py b/tripleo_common/utils/swift.py index 9dc640dcd..ba2d13c9e 100644 --- a/tripleo_common/utils/swift.py +++ b/tripleo_common/utils/swift.py @@ -14,6 +14,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import dateutil.parser import logging import os import tempfile @@ -53,22 +54,39 @@ def delete_container(swiftclient, name): LOG.info(six.text_type(e)) -def download_container(swiftclient, container, dest): +def download_container(swiftclient, container, dest, + overwrite_only_newer=False): """Download the contents of a Swift container to a directory""" objects = swiftclient.get_container(container)[1] for obj in objects: + is_newer = False filename = obj['name'] contents = swiftclient.get_object(container, filename)[1] path = os.path.join(dest, filename) dirname = os.path.dirname(path) + already_exists = os.path.exists(path) - if not os.path.exists(dirname): - os.makedirs(dirname) + if already_exists: + last_mod_swift = int(dateutil.parser.parse( + obj['last_modified']).strftime('%s')) + last_mod_disk = int(os.path.getmtime(path)) - with open(path, 'w') as f: - f.write(contents) + if last_mod_swift > last_mod_disk: + is_newer = True + + # write file if `overwrite_only_newer` is not set, + # or if file does not exist at destination, + # or if we found a newer file at source + if (not overwrite_only_newer + or not already_exists + or (overwrite_only_newer and is_newer)): + if not os.path.exists(dirname): + os.makedirs(dirname) + + with open(path, 'w') as f: + f.write(contents) def create_container(swiftclient, container): diff --git a/tripleo_common/utils/validations.py b/tripleo_common/utils/validations.py index 7c78028e7..4d49af664 100644 --- a/tripleo_common/utils/validations.py +++ b/tripleo_common/utils/validations.py @@ -12,7 +12,6 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. -import glob import logging import os import re @@ -20,8 +19,10 @@ import tempfile import yaml from oslo_concurrency import processutils +from swiftclient import exceptions as swiftexceptions from tripleo_common import constants +import tripleo_common.utils.swift as swift_utils LOG = logging.getLogger(__name__) @@ -42,26 +43,63 @@ def get_validation_metadata(validation, key): LOG.exception("Failed to get validation metadata.") -def load_validations(groups=None): - '''Loads all validations.''' - paths = glob.glob('{}/*.yaml'.format(constants.DEFAULT_VALIDATIONS_PATH)) +def _get_validations_from_swift(swift, container, objects, groups, results, + skip_existing=False): + existing_ids = [validation['id'] for validation in results] + + for obj in objects: + validation_id, ext = os.path.splitext(obj['name']) + if ext != '.yaml': + continue + + if skip_existing and validation_id in existing_ids: + continue + + contents = swift.get_object(container, obj['name'])[1] + validation = yaml.safe_load(contents) + validation_groups = get_validation_metadata(validation, 'groups') or [] + + if not groups or set.intersection(set(groups), set(validation_groups)): + results.append({ + 'id': validation_id, + 'name': get_validation_metadata(validation, 'name'), + 'groups': get_validation_metadata(validation, 'groups'), + 'description': get_validation_metadata(validation, + 'description'), + 'metadata': get_remaining_metadata(validation) + }) + + return results + + +def load_validations(swift, plan, groups=None): + """Loads all validations. + + Retrieves all of default and custom validations for a given plan and + returns a list of dicts, with each dict representing a single validation. + If both a default and a custom validation with the same name are found, + the custom validation is picked. + """ results = [] - for validation_path in sorted(paths): - with open(validation_path) as f: - validation = yaml.safe_load(f.read()) - validation_groups = get_validation_metadata(validation, 'groups') \ - or [] - if not groups or \ - set.intersection(set(groups), set(validation_groups)): - results.append({ - 'id': os.path.splitext( - os.path.basename(validation_path))[0], - 'name': get_validation_metadata(validation, 'name'), - 'groups': get_validation_metadata(validation, 'groups'), - 'description': get_validation_metadata(validation, - 'description'), - 'metadata': get_remaining_metadata(validation) - }) + + # Get custom validations first + container = plan + + try: + objects = swift.get_container( + container, prefix=constants.CUSTOM_VALIDATIONS_FOLDER)[1] + except swiftexceptions.ClientException: + pass + else: + results = _get_validations_from_swift( + swift, container, objects, groups, results) + + # Get default validations + container = constants.VALIDATIONS_CONTAINER_NAME + objects = swift.get_container(container)[1] + results = _get_validations_from_swift(swift, container, objects, groups, + results, skip_existing=True) + return results @@ -73,11 +111,36 @@ def get_remaining_metadata(validation): return dict() -def find_validation(validation): - return '{}/{}.yaml'.format(constants.DEFAULT_VALIDATIONS_PATH, validation) +def download_validation(swift, plan, validation): + """Downloads validations from Swift to a temporary location""" + dst_dir = '/tmp/{}-validations'.format(plan) + + # Download the whole default validations container + swift_utils.download_container( + swift, + constants.VALIDATIONS_CONTAINER_NAME, + dst_dir, + overwrite_only_newer=True + ) + + filename = '{}.yaml'.format(validation) + swift_path = os.path.join(constants.CUSTOM_VALIDATIONS_FOLDER, filename) + dst_path = os.path.join(dst_dir, filename) + + # If a custom validation with that name exists, get it from the plan + # container and override. Otherwise, the default one will be used. + try: + contents = swift.get_object(plan, swift_path)[1] + except swiftexceptions.ClientException: + pass + else: + with open(dst_path, 'w') as f: + f.write(contents) + + return dst_path -def run_validation(validation, identity_file, plan, context): +def run_validation(swift, validation, identity_file, plan, context): return processutils.execute( '/usr/bin/sudo', '-u', 'validations', 'OS_AUTH_URL={}'.format(context.auth_uri), @@ -85,7 +148,7 @@ def run_validation(validation, identity_file, plan, context): 'OS_AUTH_TOKEN={}'.format(context.auth_token), 'OS_TENANT_NAME={}'.format(context.project_name), '/usr/bin/run-validation', - find_validation(validation), + download_validation(swift, plan, validation), identity_file, plan ) diff --git a/workbooks/validations.yaml b/workbooks/validations.yaml index 64a4915bc..d3c2e56a5 100644 --- a/workbooks/validations.yaml +++ b/workbooks/validations.yaml @@ -121,7 +121,7 @@ workflows: find_validations: on-success: notify_running - action: tripleo.validations.list_validations groups=<% $.group_names %> + action: tripleo.validations.list_validations plan=<% $.plan %> groups=<% $.group_names %> publish: validations: <% task().result %> @@ -168,6 +168,7 @@ workflows: list: input: - group_names: [] + - plan: overcloud output: validations: <% $.validations %> tags: @@ -175,7 +176,7 @@ workflows: tasks: find_validations: on-success: send_message - action: tripleo.validations.list_validations groups=<% $.group_names %> + action: tripleo.validations.list_validations plan=<% $.plan %> groups=<% $.group_names %> publish: status: SUCCESS message: <% task().result %> @@ -195,13 +196,15 @@ workflows: validations: <% $.get('validations', []) %> list_groups: + input: + - plan: overcloud output: groups: <% task(find_groups).result %> tags: - tripleo-common-managed tasks: find_groups: - action: tripleo.validations.list_groups + action: tripleo.validations.list_groups plan=<% $.plan %> add_validation_ssh_key_parameter: input: