From 9750f945db25d1405312a6df738d7c6aaeb1a7f0 Mon Sep 17 00:00:00 2001 From: Jeremy Freudberg Date: Thu, 20 Jun 2019 23:54:32 +0300 Subject: [PATCH] Support multi-arch in deploy image validations Previously: - expect one kernel image and one ramdisk image - expect every overcloud node to use the same kernel and and ramdisk Now: - expect any number of kernel/ramdisk images - expect each overcloud node to use the most suitable kernel/ramdisk in accordance with arch/platform of that node (but recognizing that, for any given node, the form that the "most suitable" image takes may vary) Currently only Glance images are considered; support for file images as introduced in Train will come in a subsequent patch. Blueprint: multiarch-support Change-Id: I5a49b60fd32d9032b4dec1a01a3fdad6fddf42aa --- library/check_ironic_boot_config.py | 186 ++++++++++++++++++ playbooks/deployment-images.yaml | 16 -- playbooks/ironic-boot-configuration.yaml | 4 +- ...nt-images_multi-arch-031eea343453e67c.yaml | 16 ++ roles/deployment-images/defaults/main.yml | 3 - roles/deployment-images/tasks/main.yml | 22 --- roles/deployment-images/vars/main.yml | 10 - .../defaults/main.yml | 4 +- .../ironic-boot-configuration/tasks/main.yml | 33 +--- .../library/test_check_ironic_boot_config.py | 133 +++++++++++++ 10 files changed, 345 insertions(+), 82 deletions(-) create mode 100644 library/check_ironic_boot_config.py delete mode 100644 playbooks/deployment-images.yaml create mode 100644 releasenotes/notes/deployment-images_multi-arch-031eea343453e67c.yaml delete mode 100644 roles/deployment-images/defaults/main.yml delete mode 100644 roles/deployment-images/tasks/main.yml delete mode 100644 roles/deployment-images/vars/main.yml create mode 100644 tripleo_validations/tests/library/test_check_ironic_boot_config.py diff --git a/library/check_ironic_boot_config.py b/library/check_ironic_boot_config.py new file mode 100644 index 000000000..c1bd23399 --- /dev/null +++ b/library/check_ironic_boot_config.py @@ -0,0 +1,186 @@ +#!/usr/bin/env python +# Copyright 2019 Red Hat, Inc. +# All Rights Reserved. +# +# 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 collections + +from ansible.module_utils.basic import AnsibleModule # noqa + +DOCUMENTATION = ''' +--- +module: check_ironic_boot_config +short_description: > + - Check that overcloud nodes have the correct associated ramdisk and kernel + image +description: > + - Each overcloud node needs to have the correct associated ramdisk and + kernel image according to its architecture and platform. When it does + appear that the correct image is associated, we also need to check that + there is only image in Glance with that name. +options: + images: + required: true + description: + - A list of images from Glance + type: list + nodes: + required: true + description: + - A list of nodes from Ironic + type: list + deploy_kernel_name_base: + required: true + description: + - Base name of kernel image + type: string + deploy_ramdisk_name_base: + required: true + description: + - Base name of ramdisk image + type: string + +author: Jeremy Freudberg +''' + +EXAMPLES = ''' +- hosts: undercloud + tasks: + - name: Check Ironic boot config + check_ironic_boot_config: + images: "{{ lookup('glance_images', wantlist=True) }}" + nodes: "{{ lookup('ironic_nodes', wantlist=True) }}" + deploy_kernel_name_base: " {{ deploy_kernel_name_base }} " + deploy_ramdisk_name_base: " {{ deploy_ramdisk_name_base }} " +''' + + +def _name_helper(basename, arch=None, platform=None): + # TODO(jfreud): add support for non-Glance ironic-python-agent images + # TODO(jfreud): delete in favor of (eventual) tripleo-common equivalent + if arch and platform: + basename = platform + '-' + arch + '-' + basename + elif arch: + basename = arch + '-' + basename + return basename + + +def _all_possible_names(arch, platform, image_name_base): + # TODO(jfreud): delete in favor of (eventual) tripleo-common equivalent + if arch: + if platform: + yield _name_helper(image_name_base, arch=arch, platform=platform) + yield _name_helper(image_name_base, arch=arch) + yield _name_helper(image_name_base) + +MISMATCH = ( + "\nNode {} has an incorrectly configured driver_info/deploy_{}. Expected " + "{} but got {}." +) + +NO_CANDIDATES = ( + "\nNode {} has an incorrectly configured driver_info/deploy_{}. Got {}, " + "but cannot validate because could not find any suitable {} images in " + "Glance." +) + +DUPLICATE_NAME = ( + "\nNode {} appears to have a correctly configured driver_info/deploy_{} " + "but the presence of more than one image in Glance with the name '{}' " + "prevents the certainty of this." +) + + +def validate_boot_config(images, + nodes, + deploy_kernel_name_base, + deploy_ramdisk_name_base): + errors = [] + + image_map = {deploy_kernel_name_base: 'kernel', + deploy_ramdisk_name_base: 'ramdisk'} + + image_name_to_id = collections.defaultdict(list) + for image in images: + image_name_to_id[image["name"]].append(image["id"]) + + for image_name_base, image_type in image_map.items(): + for node in nodes: + actual_image_id = ( + node["driver_info"].get("deploy_%s" % image_type, None) + ) + arch = node["properties"].get("cpu_arch", None) + platform = node["extra"].get("tripleo_platform", None) + + candidates = [name for name in + _all_possible_names(arch, platform, image_name_base) + if name in image_name_to_id.keys()] + if not candidates: + errors.append( + NO_CANDIDATES.format( + node["uuid"], + image_type, + actual_image_id, + image_type + ) + ) + continue + + expected_image_name = candidates[0] + expected_image_id = image_name_to_id[expected_image_name][0] + + if expected_image_id != actual_image_id: + errors.append( + MISMATCH.format( + node["uuid"], + image_type, + expected_image_id, + actual_image_id or "None" + ) + ) + elif len(image_name_to_id[expected_image_name]) > 1: + errors.append( + DUPLICATE_NAME.format( + node["uuid"], + image_type, + expected_image_name + ) + ) + + return errors + + +def main(): + module = AnsibleModule(argument_spec=dict( + images=dict(required=True, type='list'), + nodes=dict(required=True, type='list'), + deploy_kernel_name_base=dict(required=True, type='str'), + deploy_ramdisk_name_base=dict(required=True, type='str') + )) + + images = module.params.get('images') + nodes = module.params.get('nodes') + deploy_kernel_name_base = module.params.get('deploy_kernel_name_base') + deploy_ramdisk_name_base = module.params.get('deploy_ramdisk_name_base') + + errors = validate_boot_config( + images, nodes, deploy_kernel_name_base, deploy_ramdisk_name_base) + + if errors: + module.fail_json(msg="".join(errors)) + else: + module.exit_json() + +if __name__ == '__main__': + main() diff --git a/playbooks/deployment-images.yaml b/playbooks/deployment-images.yaml deleted file mode 100644 index 87a787d8b..000000000 --- a/playbooks/deployment-images.yaml +++ /dev/null @@ -1,16 +0,0 @@ ---- -- hosts: undercloud - vars: - metadata: - name: Verify existence of deployment images - description: > - This validation checks that images bm-deploy-kernel and - bm-deploy-ramdisk exist before deploying the overcloud, - and that only one exists by that name. - groups: - - pre-deployment - - pre-upgrade - deploy_kernel_name: "bm-deploy-kernel" - deploy_ramdisk_name: "bm-deploy-ramdisk" - roles: - - deployment-images diff --git a/playbooks/ironic-boot-configuration.yaml b/playbooks/ironic-boot-configuration.yaml index cc3ba8e6a..0a44ff848 100644 --- a/playbooks/ironic-boot-configuration.yaml +++ b/playbooks/ironic-boot-configuration.yaml @@ -8,7 +8,7 @@ groups: - pre-deployment - pre-upgrade - deploy_kernel_name: "bm-deploy-kernel" - deploy_ramdisk_name: "bm-deploy-ramdisk" + deploy_kernel_name_base: "bm-deploy-kernel" + deploy_ramdisk_name_base: "bm-deploy-ramdisk" roles: - ironic-boot-configuration diff --git a/releasenotes/notes/deployment-images_multi-arch-031eea343453e67c.yaml b/releasenotes/notes/deployment-images_multi-arch-031eea343453e67c.yaml new file mode 100644 index 000000000..21cd25c24 --- /dev/null +++ b/releasenotes/notes/deployment-images_multi-arch-031eea343453e67c.yaml @@ -0,0 +1,16 @@ +--- +features: + - | + The behavior of the ``ironic-boot-configuration`` validation has changed + in order to suppport multi-arch. It now checks that each node has the + correct associated ramdisk and kernel image according to the node's + architecture and platform, and, when it does appear that the correct image + is associated, checks that there is only one image in Glance with that + name. Also, the vars ``deploy_kernel_name`` and ``deploy_ramdisk_name`` + have changed to ``deploy_kernel_name_base`` and + ``deploy_ramdisk_name_base`` respectively. +other: + - | + The ``deployment-images`` validation has been removed, as its intended + functionality became inseparable from ``ironic-boot-configuration`` in the + multi-arch case. diff --git a/roles/deployment-images/defaults/main.yml b/roles/deployment-images/defaults/main.yml deleted file mode 100644 index 2d35c2c81..000000000 --- a/roles/deployment-images/defaults/main.yml +++ /dev/null @@ -1,3 +0,0 @@ ---- -deploy_kernel_name: "bm-deploy-kernel" -deploy_ramdisk_name: "bm-deploy-ramdisk" diff --git a/roles/deployment-images/tasks/main.yml b/roles/deployment-images/tasks/main.yml deleted file mode 100644 index fec4c8ccc..000000000 --- a/roles/deployment-images/tasks/main.yml +++ /dev/null @@ -1,22 +0,0 @@ ---- -- name: Fetch deploy kernel by name - set_fact: - deploy_kernel_id: "{{ lookup('glance_images', 'name', ['{{ deploy_kernel_name }}'], wantlist=True) | map(attribute='id') | list }}" - -- name: Fetch deploy ramdisk by name - set_fact: - deploy_ramdisk_id: "{{ lookup('glance_images', 'name', ['{{ deploy_ramdisk_name }}'], wantlist=True) | map(attribute='id') | list }}" - -- name: Fail if image is not found - fail: msg="No image with the name '{{ item.name }}' found - make sure you have uploaded boot images." - failed_when: item.id | length < 1 - with_items: - - { name: '{{ deploy_kernel_name }}', id: '{{ deploy_kernel_id }}' } - - { name: '{{ deploy_ramdisk_name }}', id: '{{ deploy_ramdisk_id }}' } - -- name: Fail if there is more than one image - fail: msg="Please make sure there is only one image named '{{ item.name }}' in glance." - failed_when: item.id | length > 1 - with_items: - - { name: '{{ deploy_kernel_name }}', id: '{{ deploy_kernel_id }}' } - - { name: '{{ deploy_ramdisk_name }}', id: '{{ deploy_ramdisk_id }}' } diff --git a/roles/deployment-images/vars/main.yml b/roles/deployment-images/vars/main.yml deleted file mode 100644 index 22b4420d9..000000000 --- a/roles/deployment-images/vars/main.yml +++ /dev/null @@ -1,10 +0,0 @@ ---- -metadata: - name: Verify existence of deployment images - description: > - This validation checks that images bm-deploy-kernel and - bm-deploy-ramdisk exist before deploying the overcloud, - and that only one exists by that name. - groups: - - pre-deployment - - pre-upgrade diff --git a/roles/ironic-boot-configuration/defaults/main.yml b/roles/ironic-boot-configuration/defaults/main.yml index 2d35c2c81..036f11f4b 100644 --- a/roles/ironic-boot-configuration/defaults/main.yml +++ b/roles/ironic-boot-configuration/defaults/main.yml @@ -1,3 +1,3 @@ --- -deploy_kernel_name: "bm-deploy-kernel" -deploy_ramdisk_name: "bm-deploy-ramdisk" +deploy_kernel_name_base: "bm-deploy-kernel" +deploy_ramdisk_name_base: "bm-deploy-ramdisk" diff --git a/roles/ironic-boot-configuration/tasks/main.yml b/roles/ironic-boot-configuration/tasks/main.yml index 46d607e09..a3ae915f4 100644 --- a/roles/ironic-boot-configuration/tasks/main.yml +++ b/roles/ironic-boot-configuration/tasks/main.yml @@ -1,28 +1,7 @@ --- -- name: Get id for deploy kernel by name - set_fact: - deploy_kernel_id: "{{ lookup('glance_images', 'name', ['{{ deploy_kernel_name }}'], wantlist=True) | map(attribute='id') | join(', ') }}" - -- name: Get id for deploy ramdisk by name - set_fact: - deploy_ramdisk_id: "{{ lookup('glance_images', 'name', ['{{ deploy_ramdisk_name }}'], wantlist=True) | map(attribute='id') | join(', ') }}" - -- name: Get ironic nodes - set_fact: - ironic_nodes: "{{ lookup('ironic_nodes', wantlist=True) }}" - -- name: Check each node for kernel id - fail: - msg: >- - 'Node {{ item.uuid }} has an incorrectly configured driver_info/deploy_kernel. - Expected "{{ deploy_kernel_id }}" but got "{{ item.driver_info.deploy_kernel }}".' - failed_when: item.driver_info.deploy_kernel != deploy_kernel_id - with_items: "{{ ironic_nodes }}" - -- name: Check each node for ramdisk id - fail: - msg: >- - 'Node {{ item.uuid }} has an incorrectly configured driver_info/deploy_ramdisk. - Expected "{{ deploy_ramdisk_id }}" but got "{{ item.driver_info.deploy_ramdisk }}".' - failed_when: item.driver_info.deploy_ramdisk != deploy_ramdisk_id - with_items: "{{ ironic_nodes }}" +- name: Check ironic boot config + check_ironic_boot_config: + images: "{{ lookup('glance_images', wantlist=True) }}" + nodes: "{{ lookup('ironic_nodes', wantlist=True) }}" + deploy_kernel_name_base: "{{ deploy_kernel_name_base }}" + deploy_ramdisk_name_base: "{{ deploy_ramdisk_name_base }}" diff --git a/tripleo_validations/tests/library/test_check_ironic_boot_config.py b/tripleo_validations/tests/library/test_check_ironic_boot_config.py new file mode 100644 index 000000000..d94b682fe --- /dev/null +++ b/tripleo_validations/tests/library/test_check_ironic_boot_config.py @@ -0,0 +1,133 @@ +# Copyright 2019 Red Hat, Inc. +# All Rights Reserved. +# +# 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 library.check_ironic_boot_config as validation +from tripleo_validations.tests import base + +import mock + +KERNEL_IMAGE_ID = 111 +RAMDISK_IMAGE_ID = 112 +KERNEL_NAME_BASE = "bm-deploy-kernel" +RAMDISK_NAME_BASE = "bm-deploy-ramdisk" + + +class TestCheckIronicBootConfig(base.TestCase): + + def _image_helper(self, prefixes): + # first set of images gets the magic ID + yield { + "id": KERNEL_IMAGE_ID, + "name": prefixes[0] + KERNEL_NAME_BASE + } + yield { + "id": RAMDISK_IMAGE_ID, + "name": prefixes[0] + RAMDISK_NAME_BASE + } + if len(prefixes) > 1: + # if there's a second set of images give them some other ID + yield { + "id": KERNEL_IMAGE_ID + 2, + "name": prefixes[1] + KERNEL_NAME_BASE + } + yield { + "id": RAMDISK_IMAGE_ID + 2, + "name": prefixes[1] + RAMDISK_NAME_BASE + } + + def _node_helper(self, arch, platform): + # just create one node + nodes = [ + {"uuid": 222, + "driver_info": + { + "deploy_kernel": KERNEL_IMAGE_ID, # magic ID + "deploy_ramdisk": RAMDISK_IMAGE_ID # magic ID + }, + "properties": {}, + "extra": {}, + } + ] + if arch: + nodes[0]["properties"]["cpu_arch"] = arch + if platform: + nodes[0]["extra"]["tripleo_platform"] = platform + return nodes + + def _do_test_case( + self, image_prefixes, node_arch=None, node_platform=None): + images = self._image_helper(image_prefixes) + nodes = self._node_helper(node_arch, node_platform) + return validation.validate_boot_config( + images, nodes, KERNEL_NAME_BASE, RAMDISK_NAME_BASE) + + def test_successes(self): + self.assertEqual( + [], self._do_test_case(['p9-ppc64le-'], 'ppc64le', 'p9')) + self.assertEqual( + [], self._do_test_case([''], 'ppc64le', 'p9')) + self.assertEqual( + [], self._do_test_case( + ['ppc64le-', 'p8-ppc64le-'], 'ppc64le', 'p9')) + self.assertEqual( + [], self._do_test_case( + ['', 'SB-x86_64-'], 'ppc64le', 'p9')) + self.assertEqual( + [], self._do_test_case([''], 'x86_64')) + self.assertEqual( + [], self._do_test_case([''])) + + @mock.patch('library.check_ironic_boot_config.NO_CANDIDATES') + @mock.patch('library.check_ironic_boot_config.MISMATCH') + def test_errors(self, mismatch, no_candidates): + self._do_test_case(['p8-ppc64le-', 'p9-ppc64le-'], 'ppc64le', 'p9') + mismatch.format.assert_called() + no_candidates.format.assert_not_called() + + mismatch.reset_mock() + no_candidates.reset_mock() + + self._do_test_case(['ppc64le-', 'p9-ppc64le-'], 'ppc64le', 'p9') + mismatch.format.assert_called() + no_candidates.format.assert_not_called() + + mismatch.reset_mock() + no_candidates.reset_mock() + + self._do_test_case(['', 'ppc64le-'], 'ppc64le') + mismatch.format.assert_called() + no_candidates.format.assert_not_called() + + mismatch.reset_mock() + no_candidates.reset_mock() + + self._do_test_case(['p9-ppc64le-', ''], 'ppc64le') + mismatch.format.assert_called() + no_candidates.format.assert_not_called() + + mismatch.reset_mock() + no_candidates.reset_mock() + + self._do_test_case(['p8-ppc64le-'], 'ppc64le', 'p9') + mismatch.format.assert_not_called() + no_candidates.format.assert_called() + + mismatch.reset_mock() + no_candidates.reset_mock() + + self._do_test_case(['p9-ppc64le-', 'x86_64-'], 'ppc64le') + mismatch.format.assert_not_called() + no_candidates.format.assert_called()