From 14354b222a66c7d09ec32b4e4f66a9ef45a1e79a Mon Sep 17 00:00:00 2001 From: Jiri Podivin Date: Fri, 14 Oct 2022 16:44:02 +0200 Subject: [PATCH] Adjusting node-disks validation for new authentication procedures Validation will now only verify primary disk of the node. Description of the role and validation itself now includes further instructions and explanation. Validation failure message now recommends introspection as a next step. node-disks.py module, associated tests and documentation were deleted Signed-off-by: Jiri Podivin Change-Id: I68b83bc9c035a4f3cfc04afb098ceb04de602691 --- doc/source/modules/modules-node_disks.rst | 14 -- library/node_disks.py | 161 ------------------ playbooks/node-disks.yaml | 6 +- roles/node_disks/defaults/main.yml | 2 - roles/node_disks/tasks/main.yml | 64 ++++--- roles/node_disks/vars/main.yml | 2 +- .../tests/library/test_node_disks.py | 148 ---------------- 7 files changed, 46 insertions(+), 351 deletions(-) delete mode 100644 doc/source/modules/modules-node_disks.rst delete mode 100644 library/node_disks.py delete mode 100644 roles/node_disks/defaults/main.yml delete mode 100644 tripleo_validations/tests/library/test_node_disks.py diff --git a/doc/source/modules/modules-node_disks.rst b/doc/source/modules/modules-node_disks.rst deleted file mode 100644 index 7d7afa527..000000000 --- a/doc/source/modules/modules-node_disks.rst +++ /dev/null @@ -1,14 +0,0 @@ -=================== -Module - node_disks -=================== - - -This module provides for the following ansible plugin: - - * node_disks - - -.. ansibleautoplugin:: - :module: library/node_disks.py - :documentation: true - :examples: true diff --git a/library/node_disks.py b/library/node_disks.py deleted file mode 100644 index 7f1110051..000000000 --- a/library/node_disks.py +++ /dev/null @@ -1,161 +0,0 @@ -#!/usr/bin/env python -# Copyright 2016 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. -"""node_disks module -Used by the 'node_disks' validation. -""" - -from ansible.module_utils.basic import AnsibleModule # noqa -from yaml import safe_load as yaml_safe_load - -DOCUMENTATION = ''' ---- -module: node_disks -short_description: Check disks, flavors and root device hints -description: - - Check if each node has a root device hint set if there is more - than one disk and compare flavors to disk sizes. - - Used by the 'node_disks' validation. -options: - nodes: - required: true - description: - - A list of nodes - type: list - flavors: - required: true - description: - - A list of flavors - type: dict - introspection_data: - required: true - description: - - Introspection data for all nodes - type: list - -author: "Florian Fuchs " -''' - -EXAMPLES = ''' -- hosts: undercloud - tasks: - - name: Check node disks - node_disks: - nodes: "{{ lookup('ironic_nodes') }}" - flavors: "{{ lookup('nova_flavors') }}" - introspection_data: "{{ lookup('introspection_data', - auth_url=auth_url.value, password=password.value) }}" -''' - - -IGNORE_BYTE_MAX = 4294967296 - -ONE_DISK_TOO_SMALL_ERROR = """\ -The node {} only has one disk and it's too small for the "{}" flavor""" - -NO_RDH_SMALLEST_DISK_TOO_SMALL_ERROR = ( - '{} has more than one disk available for deployment and no ' - 'root device hints set. The disk that will be used is too small ' - 'for the flavor with the largest disk requirement ("{}").') - - -def _get_minimum_disk_size(flavors): - min_gb = 0 - name = 'n.a.' - for key, val in flavors.items(): - disk_gb = val['disk'] - if disk_gb > min_gb: - min_gb = disk_gb - name = key - # convert GB to bytes to compare to introspection data - return name, min_gb * 1073741824 - - -def _get_smallest_disk(disks): - smallest = disks[0] - for disk in disks[1:]: - if disk['size'] < smallest['size']: - smallest = disk - return smallest - - -def _has_root_device_hints(node_name, node_data): - rdh = node_data.get( - node_name, {}).get('properties', {}).get('root_device') - return rdh is not None - - -def validate_node_disks(nodes, flavors, introspection_data): - """Validate root device hints using introspection data. - - :param nodes: Ironic nodes - :param introspection_data: Introspection data for all nodes - :returns warnings: List of warning messages - errors: List of error messages - """ - errors = [] - warnings = [] - # Get the name of the flavor with the largest disk requirement, - # which defines the minimum disk size. - max_disk_flavor, min_disk_size = _get_minimum_disk_size(flavors) - - for node, content in introspection_data.items(): - disks = content.get('inventory', {}).get('disks') - valid_disks = [disk for disk in disks - if disk['size'] > IGNORE_BYTE_MAX] - - root_device_hints = _has_root_device_hints(node, nodes) - smallest_disk = _get_smallest_disk(valid_disks) - - if len(valid_disks) == 1: - if smallest_disk.get('size', 0) < min_disk_size: - errors.append(ONE_DISK_TOO_SMALL_ERROR.format( - node, max_disk_flavor)) - elif not root_device_hints and len(valid_disks) > 1: - if smallest_disk.get('size', 0) < min_disk_size: - errors.append(NO_RDH_SMALLEST_DISK_TOO_SMALL_ERROR.format( - node, max_disk_flavor)) - else: - warnings.append('{} has more than one disk available for ' - 'deployment'.format(node)) - - return errors, warnings - - -def main(): - module = AnsibleModule( - argument_spec=yaml_safe_load(DOCUMENTATION)['options'] - ) - - nodes = {node['name']: node for node in module.params.get('nodes')} - flavors = module.params.get('flavors') - introspection_data = {name: content for (name, content) in - module.params.get('introspection_data')} - - errors, warnings = validate_node_disks(nodes, - flavors, - introspection_data) - - if errors: - module.fail_json(msg="\n".join(errors)) - elif warnings: - module.exit_json(warnings="\n".join(warnings)) - else: - module.exit_json(msg="Root device hints are either set or not " - "necessary.") - - -if __name__ == '__main__': - main() diff --git a/playbooks/node-disks.yaml b/playbooks/node-disks.yaml index 5ecaaa4a1..a5a376409 100644 --- a/playbooks/node-disks.yaml +++ b/playbooks/node-disks.yaml @@ -1,10 +1,11 @@ --- -- hosts: undercloud +- hosts: localhost vars: metadata: name: Check node disk configuration description: | - Check node disk numbers and sizes and whether root device hints are set. + Check node primary/root disk sizes and compare with flavor expectations. + Fitness of other disks can be checked manually using introspection. groups: - pre-deployment categories: @@ -15,6 +16,5 @@ - undercloud products: - tripleo - ironic_conf_file: "/var/lib/config-data/puppet-generated/ironic/etc/ironic/ironic.conf" roles: - node_disks diff --git a/roles/node_disks/defaults/main.yml b/roles/node_disks/defaults/main.yml deleted file mode 100644 index afea282bc..000000000 --- a/roles/node_disks/defaults/main.yml +++ /dev/null @@ -1,2 +0,0 @@ ---- -ironic_conf_file: "/var/lib/config-data/puppet-generated/ironic/etc/ironic/ironic.conf" diff --git a/roles/node_disks/tasks/main.yml b/roles/node_disks/tasks/main.yml index 30f73291d..17bb83ab4 100644 --- a/roles/node_disks/tasks/main.yml +++ b/roles/node_disks/tasks/main.yml @@ -1,25 +1,45 @@ --- -- name: Get Ironic Inspector swift auth_url - become: true - validations_read_ini: - path: "{{ ironic_conf_file }}" - section: inspector - key: auth_url - register: ironic_auth_url +- openstack.cloud.compute_flavor_info: + cloud: overcloud + register: result -- name: Get Ironic Inspector swift password - become: true - validations_read_ini: - path: "{{ ironic_conf_file }}" - section: inspector - key: password - register: ironic_password - no_log: true +- name: Set required disk size + set_fact: + disk_sizes: "{{ result | community.general.json_query('openstack_flavors[*].{disk : disk, name : name}') }}" -- name: Check node disks - node_disks: - nodes: "{{ lookup('ironic_nodes', wantlist=True) }}" - flavors: "{{ lookup('nova_flavors', wantlist=True) }}" - introspection_data: "{{ lookup('introspection_data', - auth_url=ironic_auth_url.value, - password=ironic_password.value) }}" +- name: Set minimum required disk size + set_fact: + min_disk_size: "{{ disk_sizes | max(attribute='disk') }}" + +- name: Get list of baremetal nodes + openstack.cloud.baremetal_node_info: + cloud: undercloud + register: baremetal_nodes + +- name: Get baremetal node details + openstack.cloud.baremetal_node_info: + cloud: undercloud + node: "{{ item }}" + with_items: "{{ baremetal_nodes | community.general.json_query('baremetal_nodes[*].name') }}" + register: result + +- name: Set baremetal node details + set_fact: + baremetal_node_details: "{{ result.results | community.general.json_query('[*].baremetal_nodes[0]') }}" + +- name: Compare expected with available disk sizes + set_fact: + nodes_with_undersized_disks: "{{ baremetal_node_details | community.general.json_query('[?to_number(properties.local_gb) < to_number(min_disk_size) ]') }}" + +- fail: + msg: | + Following nodes have less storage available on their primary disks than requested by the flavor {{ min_disk_size.name }} : + {{ nodes_with_undersized_disks }} + Use node introspection to verify presence of possible additional disks. + when: nodes_with_undersized_disks | length > 0 + +- name: Print recommendation for further steps + debug: + msg: | + Validation was successful. Root disk is sufficient for all flavors. + Use node introspection to verify presence of possible additional disks. diff --git a/roles/node_disks/vars/main.yml b/roles/node_disks/vars/main.yml index 43481e75e..f9057cc81 100644 --- a/roles/node_disks/vars/main.yml +++ b/roles/node_disks/vars/main.yml @@ -2,6 +2,6 @@ metadata: name: Check node disk configuration description: > - Check node disk numbers and sizes and whether root device hints are set. + Check node root disk sizes against available flavors. groups: - pre-deployment diff --git a/tripleo_validations/tests/library/test_node_disks.py b/tripleo_validations/tests/library/test_node_disks.py deleted file mode 100644 index ad80dab0b..000000000 --- a/tripleo_validations/tests/library/test_node_disks.py +++ /dev/null @@ -1,148 +0,0 @@ -# -*- coding: utf-8 -*- - -# 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. - -try: - from unittest import mock -except ImportError: - import mock - -from tripleo_validations.tests import base -from tripleo_validations.tests import fakes - -from library.node_disks import _get_smallest_disk -from library.node_disks import _has_root_device_hints -from library.node_disks import validate_node_disks - - -# node_1: 2 disks, 1 larger than 4GB (50GB) -# node_2: 3 disks, 2 larger than 4GB (50GB, 10GB) -# node_2: 3 disks, 2 larger than 4GB (50GB, 10GB) -INTROSPECTION_DATA = { - 'node_1': { - "inventory": { - "disks": [ - {"name": "disk-1", "size": 53687091200}, - {"name": "disk-2", "size": 4294967296}, - ] - } - }, - 'node_2': { - "inventory": { - "disks": [ - {"name": "disk-1", "size": 53687091200}, - {"name": "disk-2", "size": 10737418240}, - {"name": "disk-3", "size": 4294967296}, - ] - } - }, - 'node_3': { - "inventory": { - "disks": [ - {"name": "disk-1", "size": 53687091200}, - {"name": "disk-2", "size": 10737418240}, - {"name": "disk-3", "size": 4294967296}, - ] - } - }, -} - -# small: fits nodes with disks >= 10GB -# large: fits nodes with disks >= 50GB -FLAVOR_DATA = { - "small": { - "disk": 10, - "name": "small" - }, - "large": { - "disk": 50, - "name": "large" - }, -} - -# node_1: no root device set, one disk > 4GB, fits both flavors -# node_2: root device set to name of disk-2 which fits both flavors -# node_3: no root device set, small disk only fits small flavor -NODE_DATA = { - "node_1": {"properties": {}}, - "node_2": { - "properties": { - "root_device": { - "wwn": "0x4000cca77fc4dba1" - } - } - }, - "node_3": {"properties": {}}, -} - - -class TestGetSmallestDisk(base.TestCase): - - def test_get_correct_disk(self): - introspection_data = INTROSPECTION_DATA['node_2'] - smallest_disk = _get_smallest_disk( - introspection_data['inventory']['disks']) - self.assertEqual(smallest_disk['size'], 4294967296) - - -class TestHasRootDeviceHints(base.TestCase): - - def test_detect_root_device_hints(self): - self.assertTrue(_has_root_device_hints('node_2', NODE_DATA)) - - def test_detect_no_root_device_hints(self): - self.assertFalse(_has_root_device_hints('node_1', NODE_DATA)) - - -class TestValidateRootDeviceHints(base.TestCase): - - def setUp(self): - super(TestValidateRootDeviceHints, self).setUp() - - def test_node_1_no_warning(self): - introspection_data = { - 'node_1': INTROSPECTION_DATA['node_1'] - } - errors, warnings = validate_node_disks({}, - FLAVOR_DATA, - introspection_data) - self.assertEqual([[], []], [errors, warnings]) - - def test_small_flavor_no_hints_warning(self): - introspection_data = { - 'node_2': INTROSPECTION_DATA['node_2'] - } - flavors = { - "small": FLAVOR_DATA['small'] - } - expected_warnings = [ - 'node_2 has more than one disk available for deployment', - ] - errors, warnings = validate_node_disks({}, - flavors, - introspection_data) - self.assertEqual([[], expected_warnings], [errors, warnings]) - - def test_large_flavor_no_hints_error(self): - introspection_data = { - 'node_3': INTROSPECTION_DATA['node_3'] - } - expected_errors = [ - 'node_3 has more than one disk available for deployment and no ' - 'root device hints set. The disk that will be used is too small ' - 'for the flavor with the largest disk requirement ("large").', - ] - errors, warnings = validate_node_disks({}, - FLAVOR_DATA, - introspection_data) - self.assertEqual([expected_errors, []], [errors, warnings])