diff --git a/releasenotes/notes/validations-in-workflows-021f93404f3a222e.yaml b/releasenotes/notes/validations-in-workflows-021f93404f3a222e.yaml new file mode 100644 index 000000000..c2c396040 --- /dev/null +++ b/releasenotes/notes/validations-in-workflows-021f93404f3a222e.yaml @@ -0,0 +1,9 @@ +--- +features: + - Pre-deployment checks are now being called in a + workflow. This simplifies the client, and removes + code that does not need to be in the client. +fixes: + - Fixes `bug 1638697 + `__ Moves the + pre-deployment checks to workflows. diff --git a/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py b/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py index 52ae69a17..b85d02282 100644 --- a/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py +++ b/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py @@ -876,37 +876,6 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): mock_create_tempest_deployer_input.assert_called_with() - @mock.patch('tripleoclient.utils.check_nodes_count') - @mock.patch('tripleoclient.utils.check_hypervisor_stats') - @mock.patch('tripleoclient.utils.assign_and_verify_profiles') - @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.' - '_get_default_role_counts') - @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.' - '_check_ironic_boot_configuration') - @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.' - '_collect_flavors') - def test_predeploy_verify_capabilities_hypervisor_stats( - self, mock_collect_flavors, - mock_check_ironic_boot_configuration, - mock_get_default_role_counts, - mock_assign_and_verify_profiles, - mock_check_hypervisor_stats, - mock_check_nodes_count): - self.cmd._predeploy_verify_capabilities = \ - self.real_predeploy_verify_capabilities - - stack = None - parameters = {} - parsed_args = mock.Mock() - mock_assign_and_verify_profiles.return_value = (0, 0) - mock_check_nodes_count.return_value = (True, 0, 0) - - # A None return value here indicates an error - mock_check_hypervisor_stats.return_value = None - self.cmd._predeploy_verify_capabilities( - stack, parameters, parsed_args) - self.assertEqual(1, self.cmd.predeploy_errors) - def test_get_default_role_counts_defaults(self): parsed_args = mock.Mock() parsed_args.roles_file = None diff --git a/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy_validators.py b/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy_validators.py deleted file mode 100644 index 14f1f9c97..000000000 --- a/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy_validators.py +++ /dev/null @@ -1,250 +0,0 @@ -# Copyright 2015 Red Hat, Inc. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# 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 mock -from uuid import uuid4 - -from osc_lib.tests import utils - -from tripleoclient.tests.v1.overcloud_deploy import fakes -from tripleoclient.v1 import overcloud_deploy - - -class TestDeployValidators(fakes.TestDeployOvercloud): - def setUp(self): - super(TestDeployValidators, self).setUp() - - # Get the command object to test - self.cmd = overcloud_deploy.DeployOvercloud(self.app, None) - - @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.' - '_check_node_boot_configuration') - def test_ironic_boot_checks(self, mock_node_boot_check): - class FakeNode(object): - uuid = None - - def __init__(self, uuid): - self.uuid = uuid - - bm_client = self.app.client_manager.baremetal - mock_node = mock.Mock() - bm_client.attach_mock(mock_node, 'node') - - fake_nodes = [FakeNode(uuid) for uuid in ( - '97dd6459-cf2d-4eea-865e-84fee3bf5e6d', - '1867d71b-d0a5-44c6-b83e-ada8b16de556' - )] - # return a list of FakeNodes, replaces bm_client.node.list - mock_maint_nodes = mock.Mock(return_value=fake_nodes) - mock_node.attach_mock(mock_maint_nodes, 'list') - - # get a FakeNode by its UUID, replaces bm_client.node.get - - self.cmd.baremetal_client = bm_client - self.cmd._check_ironic_boot_configuration() - - mock_maint_nodes.assert_called_once_with(detail=True, - maintenance=False) - - def test_image_ids(self): - image_client = self.app.client_manager.image - image_client.images = {} - image_ids = self.cmd._image_ids() - - image_client.images = { - 'bm-deploy-kernel': - mock.Mock(id='fb7a98fb-acb9-43ec-9b93-525d1286f9d8'), - 'bm-deploy-ramdisk': - mock.Mock(id='8558de2e-1b72-4654-8ba9-cceb89e9194e'), - } - - image_ids = self.cmd._image_ids() - self.assertEqual(image_ids, ('fb7a98fb-acb9-43ec-9b93-525d1286f9d8', - '8558de2e-1b72-4654-8ba9-cceb89e9194e')) - - @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.' - '_image_ids', - return_value=('fb7a98fb-acb9-43ec-9b93-525d1286f9d8', - '8558de2e-1b72-4654-8ba9-cceb89e9194e')) - def test_node_boot_checks(self, mock_image_ids): - class FakeNode(object): - uuid = 'fake-node-123' - driver_info = None - properties = None - - node = FakeNode() - node.driver_info = { - 'deploy_kernel': 'fb7a98fb-acb9-43ec-9b93-525d1286f9d8', - 'deploy_ramdisk': '8558de2e-1b72-4654-8ba9-cceb89e9194e', - } - node.properties = { - 'capabilities': 'boot_option:local,profile:foobar' - } - self.cmd._check_node_boot_configuration(node) - self.assertEqual(self.cmd.predeploy_errors, 0) - self.assertEqual(self.cmd.predeploy_warnings, 0) - - node.properties['capabilities'] = 'profile:foobar' - self.cmd._check_node_boot_configuration(node) - self.assertEqual(self.cmd.predeploy_errors, 0) - self.assertEqual(self.cmd.predeploy_warnings, 1) - - node.properties['capabilities'] = 'profile:foobar,boot_option:local' - node.driver_info.pop('deploy_kernel') - self.cmd._check_node_boot_configuration(node) - self.assertEqual(self.cmd.predeploy_errors, 1) - self.assertEqual(self.cmd.predeploy_warnings, 1) - - @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.' - '_image_ids', - return_value=('fb7a98fb-acb9-43ec-9b93-525d1286f9d8', - '8558de2e-1b72-4654-8ba9-cceb89e9194e')) - def test_boot_image_checks(self, mock_image_ids): - self.cmd._check_boot_images() - self.assertEqual(self.cmd.predeploy_errors, 0) - self.assertEqual(self.cmd.predeploy_warnings, 0) - - mock_image_ids.return_value = ( - None, '8558de2e-1b72-4654-8ba9-cceb89e9194e') - self.cmd._check_boot_images() - self.assertEqual(self.cmd.predeploy_errors, 1) - self.assertEqual(self.cmd.predeploy_warnings, 0) - - mock_image_ids.return_value = ( - '8558de2e-1b72-4654-8ba9-cceb89e9194e', None) - self.cmd._check_boot_images() - self.assertEqual(self.cmd.predeploy_errors, 2) - self.assertEqual(self.cmd.predeploy_warnings, 0) - - -class FakeFlavor(object): - name = '' - uuid = '' - - def __init__(self, name): - self.uuid = uuid4() - self.name = name - - def get_keys(self): - return {'capabilities:boot_option': 'local'} - - -class TestCollectFlavors(fakes.TestDeployOvercloud): - def setUp(self): - super(TestCollectFlavors, self).setUp() - self.cmd = overcloud_deploy.DeployOvercloud(self.app, None) - self.arglist = [ - '--block-storage-flavor', 'block', - '--block-storage-scale', '3', - '--ceph-storage-flavor', 'ceph', - '--ceph-storage-scale', '0', - '--compute-flavor', 'compute', - '--compute-scale', '3', - '--control-flavor', 'control', - '--control-scale', '1', - '--swift-storage-flavor', 'swift', - '--swift-storage-scale', '2', - '--templates' - ] - self.verifylist = [ - ('templates', '/usr/share/openstack-tripleo-heat-templates/'), - ] - - self.mock_flavors = mock.Mock() - self.app.client_manager.compute.attach_mock(self.mock_flavors, - 'flavors') - - def test_ok(self): - parsed_args = self.check_parser(self.cmd, self.arglist, - self.verifylist) - - expected_result = { - 'block': (FakeFlavor('block'), 3), - 'compute': (FakeFlavor('compute'), 3), - 'control': (FakeFlavor('control'), 1), - 'swift': (FakeFlavor('swift'), 2) - } - mock_flavor_list = mock.Mock( - return_value=[ - flavor for flavor, scale in expected_result.values() - ] - ) - self.mock_flavors.attach_mock(mock_flavor_list, 'list') - - result = self.cmd._collect_flavors(parsed_args) - self.assertEqual(self.cmd.predeploy_errors, 0) - self.assertEqual(self.cmd.predeploy_warnings, 0) - self.assertEqual(expected_result, result) - - def test_flavor_not_found(self): - parsed_args = self.check_parser(self.cmd, self.arglist, - self.verifylist) - - expected_result = { - 'block': (FakeFlavor('block'), 3), - 'compute': (FakeFlavor('compute'), 3), - 'control': (FakeFlavor('control'), 1), - } - mock_flavor_list = mock.Mock( - return_value=[ - flavor for flavor, scale in expected_result.values() - ] - ) - self.mock_flavors.attach_mock(mock_flavor_list, 'list') - result = self.cmd._collect_flavors(parsed_args) - self.assertEqual(self.cmd.predeploy_errors, 1) - self.assertEqual(self.cmd.predeploy_warnings, 0) - self.assertEqual(expected_result, result) - - def test_same_flavor(self): - self.arglist = [ - '--compute-flavor', 'baremetal', - '--compute-scale', '3', - '--control-flavor', 'baremetal', - '--control-scale', '1', - '--templates' - ] - parsed_args = self.check_parser(self.cmd, self.arglist, - self.verifylist) - - expected_result = { - 'baremetal': (FakeFlavor('baremetal'), 4), - } - mock_flavor_list = mock.Mock( - return_value=[ - flavor for flavor, scale in expected_result.values() - ] - ) - self.mock_flavors.attach_mock(mock_flavor_list, 'list') - - result = self.cmd._collect_flavors(parsed_args) - self.assertEqual(self.cmd.predeploy_errors, 0) - self.assertEqual(self.cmd.predeploy_warnings, 0) - self.assertEqual(expected_result, result) - - def test_error_default(self): - self.check_parser(self.cmd, ['--templates'], - [('validation_errors_fatal', True)]) - - def test_error_nonfatal(self): - self.check_parser(self.cmd, - ['--templates', '--validation-errors-nonfatal'], - [('validation_errors_fatal', False)]) - - def test_error_exclusive(self): - self.assertRaises(utils.ParserException, - self.check_parser, self.cmd, - ['--templates', '--validation-errors-nonfatal', - '--validation-errors-fatal'], []) diff --git a/tripleoclient/v1/overcloud_deploy.py b/tripleoclient/v1/overcloud_deploy.py index 7aa5cf2d6..00e296781 100644 --- a/tripleoclient/v1/overcloud_deploy.py +++ b/tripleoclient/v1/overcloud_deploy.py @@ -32,7 +32,6 @@ from heatclient import exc as hc_exc from osc_lib.command import command from osc_lib import exceptions as oscexc from osc_lib.i18n import _ -from osc_lib import utils as osc_utils from swiftclient.exceptions import ClientException from tripleo_common import update @@ -42,6 +41,7 @@ from tripleoclient import utils from tripleoclient.workflows import deployment from tripleoclient.workflows import parameters as workflow_params from tripleoclient.workflows import plan_management +from tripleoclient.workflows import validations class DeployOvercloud(command.Command): @@ -607,177 +607,27 @@ class DeployOvercloud(command.Command): self.predeploy_warnings = 0 self.log.debug("Starting _pre_verify_capabilities") - self._check_boot_images() + validation_params = { + 'deploy_kernel_name': 'bm-deploy-kernel', + 'deploy_ramdisk_name': 'bm-deploy-ramdisk', + 'roles_info': utils.get_roles_info(parsed_args), + 'stack_id': parsed_args.stack, + 'parameters': parameters, + 'default_role_counts': self._get_default_role_counts(parsed_args), + 'run_validations': True, + 'queue_name': str(uuid.uuid4()), + } - flavors = self._collect_flavors(parsed_args) - - self._check_ironic_boot_configuration() - - errors, warnings = utils.assign_and_verify_profiles( - self.baremetal_client, flavors, - assign_profiles=False, - dry_run=parsed_args.dry_run + errors, warnings = validations.check_predeployment_validations( + self.app.client_manager, + **validation_params ) + self.predeploy_errors += errors self.predeploy_warnings += warnings - self.log.debug("Checking hypervisor stats") - if utils.check_hypervisor_stats(self.compute_client) is None: - self.log.error("Expected hypervisor stats not met") - self.predeploy_errors += 1 - - self.log.debug("Checking nodes count") - default_role_counts = self._get_default_role_counts(parsed_args) - enough_nodes, count, ironic_nodes_count = utils.check_nodes_count( - self.baremetal_client, - stack, - parameters, - default_role_counts - ) - if not enough_nodes: - self.log.error( - "Not enough nodes - available: {0}, requested: {1}".format( - ironic_nodes_count, count)) - self.predeploy_errors += 1 - return self.predeploy_errors, self.predeploy_warnings - __kernel_id = None - __ramdisk_id = None - - def _image_ids(self): - if self.__kernel_id is not None and self.__ramdisk_id is not None: - return self.__kernel_id, self.__ramdisk_id - - kernel_id, ramdisk_id = None, None - try: - kernel_id = osc_utils.find_resource( - self.image_client.images, 'bm-deploy-kernel').id - except AttributeError: - self.log.exception("Please make sure there is only one image " - "named 'bm-deploy-kernel' in glance.") - except oscexc.CommandError: - # kernel_id=None will be returned and an error will be logged from - # self._check_boot_images - pass - - try: - ramdisk_id = osc_utils.find_resource( - self.image_client.images, 'bm-deploy-ramdisk').id - except AttributeError: - self.log.exception("Please make sure there is only one image " - "named 'bm-deploy-ramdisk' in glance.") - except oscexc.CommandError: - # ramdisk_id=None will be returned and an error will be logged from - # self._check_boot_images - pass - - self.log.debug("Using kernel ID: {0} and ramdisk ID: {1}".format( - kernel_id, ramdisk_id)) - - self.__kernel_id = kernel_id - self.__ramdisk_id = ramdisk_id - return kernel_id, ramdisk_id - - def _check_boot_images(self): - kernel_id, ramdisk_id = self._image_ids() - message = ("No image with the name '{}' found - make " - "sure you've uploaded boot images") - if kernel_id is None: - self.predeploy_errors += 1 - self.log.error(message.format('bm-deploy-kernel')) - if ramdisk_id is None: - self.predeploy_errors += 1 - self.log.error(message.format('bm-deploy-ramdisk')) - - def _collect_flavors(self, parsed_args): - """Validate and collect nova flavors in use. - - Ensure that selected flavors (--ROLE-flavor) are valid in nova. - Issue a warning of local boot is not set for a flavor. - - :returns: dictionary flavor name -> (flavor object, scale) - """ - flavors = {f.name: f for f in self.compute_client.flavors.list()} - result = {} - - message = "Provided --{}-flavor, '{}', does not exist" - - for target, (flavor_name, scale) in ( - utils.get_roles_info(parsed_args).items() - ): - if flavor_name is None or not scale: - self.log.debug("--{}-flavor not used".format(target)) - continue - - try: - flavor, old_scale = result[flavor_name] - except KeyError: - pass - else: - result[flavor_name] = (flavor, old_scale + scale) - continue - - try: - flavor = flavors[flavor_name] - except KeyError: - self.predeploy_errors += 1 - self.log.error(message.format(target, flavor_name)) - continue - - if flavor.get_keys().get('capabilities:boot_option', '') \ - != 'local': - self.predeploy_warnings += 1 - self.log.warning( - 'Flavor %s "capabilities:boot_option" is not set to ' - '"local". Nodes must have ability to PXE boot from ' - 'deploy image.', flavor_name) - self.log.warning( - 'Recommended solution: openstack flavor set --property ' - '"cpu_arch"="x86_64" --property ' - '"capabilities:boot_option"="local" ' + flavor_name) - - result[flavor_name] = (flavor, scale) - - return result - - def _check_ironic_boot_configuration(self): - for node in self.baremetal_client.node.list(detail=True, - maintenance=False): - self.log.debug("Checking config for Node {0}".format(node.uuid)) - self._check_node_boot_configuration(node) - - def _check_node_boot_configuration(self, node): - kernel_id, ramdisk_id = self._image_ids() - self.log.debug("Doing boot checks for {}".format(node.uuid)) - message = ("Node uuid={uuid} has an incorrectly configured " - "{property}. Expected \"{expected}\" but got " - "\"{actual}\".") - if node.driver_info.get('deploy_ramdisk') != ramdisk_id: - self.predeploy_errors += 1 - self.log.error(message.format( - uuid=node.uuid, - property='driver_info/deploy_ramdisk', - expected=ramdisk_id, - actual=node.driver_info.get('deploy_ramdisk') - )) - if node.driver_info.get('deploy_kernel') != kernel_id: - self.predeploy_errors += 1 - self.log.error(message.format( - uuid=node.uuid, - property='driver_info/deploy_kernel', - expected=kernel_id, - actual=node.driver_info.get('deploy_kernel') - )) - if 'boot_option:local' not in node.properties.get('capabilities', ''): - self.predeploy_warnings += 1 - self.log.warning(message.format( - uuid=node.uuid, - property='properties/capabilities', - expected='boot_option:local', - actual=node.properties.get('capabilities') - )) - def get_parser(self, prog_name): # add_help doesn't work properly, set it to False: parser = argparse.ArgumentParser( diff --git a/tripleoclient/workflows/validations.py b/tripleoclient/workflows/validations.py new file mode 100644 index 000000000..541c966e5 --- /dev/null +++ b/tripleoclient/workflows/validations.py @@ -0,0 +1,44 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +from tripleoclient.workflows import base + + +def check_predeployment_validations(clients, **workflow_input): + workflow_client = clients.workflow_engine + tripleoclients = clients.tripleoclient + queue_name = workflow_input['queue_name'] + + execution = base.start_workflow( + workflow_client, + 'tripleo.validations.v1.check_pre_deployment_validations', + workflow_input=workflow_input + ) + + errors = [] + warnings = [] + with tripleoclients.messaging_websocket(queue_name) as ws: + for payload in base.wait_for_messages(workflow_client, ws, execution): + if 'message' in payload: + print(payload['message']) + if 'errors' in payload: + errors += payload['errors'] + if 'warnings' in payload: + warnings += payload['warnings'] + + if errors: + print('ERRORS') + print(errors) + if warnings: + print('WARNINGS') + print(warnings) + + return len(errors), len(warnings)