From 7d71eb6d03a7010965ef6123630a375b32c5aecd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Harald=20Jens=C3=A5s?= Date: Mon, 9 Nov 2020 15:37:27 +0100 Subject: [PATCH] Introduce role/instance 'networks' key Introduce the 'networks' key in be baremetal YAML definition. Networks can be defined as VIF (ironic attached nic) or as a non-attached "virtual nic" via the 'vif' boolean. The default_network 'ctlplane' is updated to have 'vif': True. Role default networks are merged with instance networks. Partial-Implements: blueprint network-data-v2-ports Change-Id: Ib7931cae079f923a66b412dc5664d1b119580182 --- .../module_utils/baremetal_deploy.py | 102 +++++++-- .../modules/tripleo_baremetal_expand_roles.py | 3 + .../module_utils/test_baremetal_deploy.py | 204 +++++++++++++++++- 3 files changed, 283 insertions(+), 26 deletions(-) diff --git a/tripleo_ansible/ansible_plugins/module_utils/baremetal_deploy.py b/tripleo_ansible/ansible_plugins/module_utils/baremetal_deploy.py index 37917f892..ac02d49d1 100644 --- a/tripleo_ansible/ansible_plugins/module_utils/baremetal_deploy.py +++ b/tripleo_ansible/ansible_plugins/module_utils/baremetal_deploy.py @@ -14,7 +14,7 @@ # License for the specific language governing permissions and limitations # under the License. - +from copy import deepcopy as dcopy import jsonschema import metalsmith @@ -44,6 +44,18 @@ _NIC_SCHEMA = { 'additionalProperties': False } +_NETWORK_SCHEMA = { + 'type': 'object', + 'properties': { + 'network': {'type': 'string'}, + 'port': {'type': 'string'}, + 'fixed_ip': {'type': 'string'}, + 'subnet': {'type': 'string'}, + 'vif': {'type': 'boolean'} + }, + 'additionalProperties': False +} + _INSTANCE_SCHEMA = { 'type': 'object', 'properties': { @@ -57,10 +69,10 @@ _INSTANCE_SCHEMA = { 'image': _IMAGE_SCHEMA, 'name': {'type': 'string'}, 'netboot': {'type': 'boolean'}, - 'nics': { - 'type': 'array', - 'items': _NIC_SCHEMA - }, + 'nics': {'type': 'array', + 'items': _NIC_SCHEMA}, + 'networks': {'type': 'array', + 'items': _NETWORK_SCHEMA}, 'passwordless_sudo': {'type': 'boolean'}, 'profile': {'type': 'string'}, 'provisioned': {'type': 'boolean'}, @@ -84,6 +96,21 @@ _INSTANCES_SCHEMA = { } """JSON schema of the instances list.""" +_no_nics = dcopy(_INSTANCE_SCHEMA) +_no_networks = dcopy(_INSTANCE_SCHEMA) +del _no_nics['properties']['nics'] +del _no_networks['properties']['networks'] + +_ROLE_DEFAULTS_SCHEMA = { + 'anyOf': [_no_nics, _no_networks] +} +"""JSON schema of the role defaults.""" + +_INSTANCES_INPUT_SCHEMA = { + 'type': 'array', + 'items': {'anyOf': [_no_nics, _no_networks]}, +} +"""JSON schema of the instances input.""" _ROLES_INPUT_SCHEMA = { 'type': 'array', @@ -93,8 +120,8 @@ _ROLES_INPUT_SCHEMA = { 'name': {'type': 'string'}, 'hostname_format': {'type': 'string'}, 'count': {'type': 'integer', 'minimum': 0}, - 'defaults': _INSTANCE_SCHEMA, - 'instances': _INSTANCES_SCHEMA, + 'defaults': _ROLE_DEFAULTS_SCHEMA, + 'instances': _INSTANCES_INPUT_SCHEMA, }, 'additionalProperties': False, 'required': ['name'], @@ -110,19 +137,27 @@ class BaremetalDeployException(Exception): def expand(roles, stack_name, expand_provisioned=True, default_image=None, default_network=None, user_name=None, ssh_public_keys=None): + def _remove_vif_key(nets): + for net in nets: + net.pop('vif', None) + for role in roles: - role.setdefault('defaults', {}) + defaults = role.setdefault('defaults', {}) if default_image: - role['defaults'].setdefault('image', default_image) - if default_network: - role['defaults'].setdefault('nics', default_network) + defaults.setdefault('image', default_image) if ssh_public_keys: - role['defaults'].setdefault('ssh_public_keys', ssh_public_keys) + defaults.setdefault('ssh_public_keys', ssh_public_keys) if user_name: - role['defaults'].setdefault('user_name', user_name) + defaults.setdefault('user_name', user_name) + if default_network: + default_networks = defaults.setdefault('networks', []) + default_networks.extend([x for x in default_network + if x not in default_networks]) for inst in role.get('instances', []): - for k, v in role['defaults'].items(): + merge_networks_defaults(defaults, inst) + + for k, v in defaults.items(): inst.setdefault(k, v) # Set the default hostname now for duplicate hostname @@ -177,8 +212,8 @@ def expand(roles, stack_name, expand_provisioned=True, default_image=None, hostname_format) # ensure each instance has a unique non-empty hostname - # and a hostname map entry. Also build a list of indexes - # for unprovisioned instances + # and a hostname map entry and add nics entry for vif networks. + # Also build a list of indexes for unprovisioned instances index = 0 for inst in role_instances: provisioned = inst.get('provisioned', True) @@ -203,6 +238,12 @@ def expand(roles, stack_name, expand_provisioned=True, default_image=None, unprovisioned_indexes.append( potential_gen_names[hostname]) + vif_networks = [x for x in dcopy(inst.get('networks', [])) + if x.get('vif')] + if vif_networks: + _remove_vif_key(vif_networks) + inst.setdefault('nics', vif_networks) + if unprovisioned_indexes: parameter_defaults['%sRemovalPolicies' % name] = [{ 'resource_list': unprovisioned_indexes @@ -222,7 +263,7 @@ def expand(roles, stack_name, expand_provisioned=True, default_image=None, parameter_defaults['%sCount' % name] = ( provisioned_count) - validate_instances(instances) + validate_instances(instances, _INSTANCES_SCHEMA) if expand_provisioned: env = {'parameter_defaults': parameter_defaults} else: @@ -230,8 +271,27 @@ def expand(roles, stack_name, expand_provisioned=True, default_image=None, return instances, env +def merge_networks_defaults(defaults, instance): + d_networks = defaults.get('networks', []) + i_networks = instance.get('networks', []) + if not d_networks: + return + + i_dict = {x['network']: x for x in i_networks} + d_dict = {x['network']: x for x in d_networks} + + # only merge networks not already defined on the instance + for key in d_dict: + if key not in i_dict: + i_networks.append(d_dict[key]) + + # only set non-empty networks value on the instance + if i_networks: + instance['networks'] = i_networks + + def check_existing(instances, provisioner, baremetal): - validate_instances(instances) + validate_instances(instances, _INSTANCES_SCHEMA) # Due to the name shadowing we should import other way import importlib @@ -319,8 +379,8 @@ def build_hostname(hostname_format, index, stack): return gen_name -def validate_instances(instances): - jsonschema.validate(instances, _INSTANCES_SCHEMA) +def validate_instances(instances, schema): + jsonschema.validate(instances, schema) hostnames = set() names = set() for inst in instances: @@ -366,7 +426,7 @@ def validate_roles(roles): raise ValueError("%s: cannot specify provisioned in defaults" % name) if 'instances' in item: - validate_instances(item['instances']) + validate_instances(item['instances'], _INSTANCES_INPUT_SCHEMA) def get_source(instance): diff --git a/tripleo_ansible/ansible_plugins/modules/tripleo_baremetal_expand_roles.py b/tripleo_ansible/ansible_plugins/modules/tripleo_baremetal_expand_roles.py index 447f8c84e..4ad776cc5 100644 --- a/tripleo_ansible/ansible_plugins/modules/tripleo_baremetal_expand_roles.py +++ b/tripleo_ansible/ansible_plugins/modules/tripleo_baremetal_expand_roles.py @@ -99,6 +99,7 @@ options: suboptions: dict default: - network: ctlplane + vif: true default_image: description: - Default image @@ -191,11 +192,13 @@ EXAMPLES = ''' defaults: image: href: overcloud-full + networks: [] - name: Compute count: 3 defaults: image: href: overcloud-full + networks: [] state: present stack_name: overcloud register: tripleo_baremetal_instances diff --git a/tripleo_ansible/tests/plugins/module_utils/test_baremetal_deploy.py b/tripleo_ansible/tests/plugins/module_utils/test_baremetal_deploy.py index e3274f0c5..cd8f17e7c 100644 --- a/tripleo_ansible/tests/plugins/module_utils/test_baremetal_deploy.py +++ b/tripleo_ansible/tests/plugins/module_utils/test_baremetal_deploy.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. +import jsonschema import metalsmith from unittest import mock from openstack import exceptions as sdk_exc @@ -56,10 +57,45 @@ class TestBaremetalDeployUtils(base.TestCase): ) ) + def test_merge_networks_defaults(self): + # Network defined only in role defaults is appended + defaults = {'networks': [{'network': 'role_net'}]} + instance = {'networks': [{'network': 'instance_net'}]} + bd.merge_networks_defaults(defaults, instance) + self.assertEqual({'networks': [{'network': 'instance_net'}, + {'network': 'role_net'}]}, instance) + + # Network defined in both role defaults and instance is not appended + instance = {'networks': [{'network': 'instance_net'}, + {'network': 'role_net'}]} + bd.merge_networks_defaults(defaults, instance) + self.assertEqual({'networks': [{'network': 'instance_net'}, + {'network': 'role_net'}]}, instance) + + # Network defined in role defaults and in instance with richer data + # is not appended. + instance = {'networks': [{'network': 'instance_net'}, + {'network': 'role_net', 'port': 'port_uuid'}]} + bd.merge_networks_defaults(defaults, instance) + self.assertEqual({'networks': [{'network': 'instance_net'}, + {'network': 'role_net', + 'port': 'port_uuid'}]}, instance) + + # Network defined in role defaults with richer data compared to the + # instance is not appended. + defaults = {'networks': [{'network': 'role_net', + 'subnet': 'subnet_name'}]} + instance = {'networks': [{'network': 'instance_net'}, + {'network': 'role_net'}]} + bd.merge_networks_defaults(defaults, instance) + self.assertEqual({'networks': [{'network': 'instance_net'}, + {'network': 'role_net'}]}, instance) + class TestExpandRoles(base.TestCase): default_image = {'href': 'overcloud-full'} + default_network = [{'network': 'ctlplane', 'vif': True}] def test_simple(self): roles = [ @@ -93,6 +129,149 @@ class TestExpandRoles(base.TestCase): }, environment['parameter_defaults']) + def test_default_network(self): + roles = [ + {'name': 'Compute'}, + {'name': 'Controller'}, + ] + instances, environment = bd.expand( + roles, 'overcloud', True, self.default_image, self.default_network + ) + self.assertEqual( + [ + {'hostname': 'overcloud-novacompute-0', + 'image': {'href': 'overcloud-full'}, + 'networks': [{'network': 'ctlplane', 'vif': True}], + 'nics': [{'network': 'ctlplane'}]}, + {'hostname': 'overcloud-controller-0', + 'image': {'href': 'overcloud-full'}, + 'networks': [{'network': 'ctlplane', 'vif': True}], + 'nics': [{'network': 'ctlplane'}]}, + ], + instances) + + def test_networks_set_no_default_network(self): + roles = [ + {'name': 'Compute', + 'defaults': { + 'networks': [ + {'network': 'some_net', 'vif': True}, + ]} + }, + {'name': 'Controller', + 'defaults': { + 'networks': [ + {'network': 'some_net', 'vif': True}, + ]} + }, + ] + instances, environment = bd.expand( + roles, 'overcloud', True, self.default_image, None + ) + self.assertEqual( + [ + {'hostname': 'overcloud-novacompute-0', + 'image': {'href': 'overcloud-full'}, + 'networks': [{'network': 'some_net', 'vif': True}], + 'nics': [{'network': 'some_net'}]}, + {'hostname': 'overcloud-controller-0', + 'image': {'href': 'overcloud-full'}, + 'networks': [{'network': 'some_net', 'vif': True}], + 'nics': [{'network': 'some_net'}]}, + ], + instances) + + def test_networks_set_default_appended(self): + roles = [ + {'name': 'Compute', + 'defaults': { + 'networks': [ + {'network': 'foo', 'subnet': 'foo_subnet'}, + ]} + }, + {'name': 'Controller', + 'defaults': { + 'networks': [ + {'network': 'foo', 'subnet': 'foo_subnet'}, + ]} + }, + ] + instances, environment = bd.expand( + roles, 'overcloud', True, self.default_image, self.default_network + ) + self.assertEqual( + [ + {'hostname': 'overcloud-novacompute-0', + 'image': {'href': 'overcloud-full'}, + 'networks': [{'network': 'foo', 'subnet': 'foo_subnet'}, + {'network': 'ctlplane', 'vif': True}], + 'nics': [{'network': 'ctlplane'}]}, + {'hostname': 'overcloud-controller-0', + 'image': {'href': 'overcloud-full'}, + 'networks': [{'network': 'foo', 'subnet': 'foo_subnet'}, + {'network': 'ctlplane', 'vif': True}], + 'nics': [{'network': 'ctlplane'}]}, + ], + instances) + + def test_networks_vif_set_default_appended(self): + roles = [ + {'name': 'Compute', + 'defaults': { + 'networks': [ + {'network': 'foo', 'subnet': 'foo_subnet', 'vif': True}, + ]} + }, + {'name': 'Controller', + 'defaults': { + 'networks': [ + {'network': 'foo', 'subnet': 'foo_subnet', 'vif': True}, + ]} + }, + ] + instances, environment = bd.expand( + roles, 'overcloud', True, self.default_image, self.default_network + ) + self.assertEqual( + [ + {'hostname': 'overcloud-novacompute-0', + 'image': {'href': 'overcloud-full'}, + 'networks': [ + {'network': 'foo', 'subnet': 'foo_subnet', 'vif': True}, + {'network': 'ctlplane', 'vif': True} + ], + 'nics': [{'network': 'foo', 'subnet': 'foo_subnet'}, + {'network': 'ctlplane'}], + }, + {'hostname': 'overcloud-controller-0', + 'image': {'href': 'overcloud-full'}, + 'networks': [ + {'network': 'foo', 'subnet': 'foo_subnet', 'vif': True}, + {'network': 'ctlplane', 'vif': True} + ], + 'nics': [ + {'network': 'foo', 'subnet': 'foo_subnet'}, + {'network': 'ctlplane'} + ]}, + ], + instances) + + def test_networks_nics_are_mutually_exclusive(self): + # Neither 'nics' nor 'networks' - OK + roles = [{'name': 'Compute', 'defaults': {}}] + bd.expand(roles, 'overcloud', True, self.default_image) + # 'networks' but not 'nics' - OK + roles = [{'name': 'Compute', 'defaults': {'networks': []}}] + bd.expand(roles, 'overcloud', True, self.default_image) + # 'nics' but not 'networks' - OK + roles = [{'name': 'Compute', 'defaults': {'nics': []}}] + bd.expand(roles, 'overcloud', True, self.default_image) + # 'networks' and 'nics' - mutually exclusive, Raises ValidationError + roles = [{'name': 'Compute', 'defaults': {'networks': [], 'nics': []}}] + self.assertRaises( + jsonschema.exceptions.ValidationError, + bd.expand, roles, 'overcloud', True, self.default_image) + def test_image_in_defaults(self): roles = [{ 'name': 'Controller', @@ -200,15 +379,22 @@ class TestExpandRoles(base.TestCase): 'name': 'Controller', 'count': 2, 'defaults': { - 'profile': 'control' + 'profile': 'control', + 'networks': [ + {'network': 'foo', 'subnet': 'foo_subnet'}, + ] }, 'instances': [{ 'hostname': 'controller-X.example.com', - 'profile': 'control-X' + 'profile': 'control-X', + 'networks': [ + {'network': 'inst_net', 'fixed_ip': '10.1.1.1'} + ] }, { 'name': 'node-0', 'traits': ['CUSTOM_FOO'], - 'nics': [{'subnet': 'leaf-2'}]}, + 'networks': [{'network': 'some_net', 'subnet': 'leaf-2', + 'vif': True}]}, ]}, ] instances, environment = bd.expand( @@ -222,12 +408,20 @@ class TestExpandRoles(base.TestCase): 'image': {'href': 'overcloud-full'}}, {'hostname': 'controller-X.example.com', 'image': {'href': 'overcloud-full'}, - 'profile': 'control-X'}, + 'profile': 'control-X', + 'networks': [{'fixed_ip': '10.1.1.1', 'network': 'inst_net'}, + {'network': 'foo', 'subnet': 'foo_subnet'}], + }, # Name provides the default for hostname later on. {'name': 'node-0', 'profile': 'control', 'hostname': 'node-0', + 'networks': [ + {'network': 'some_net', 'subnet': 'leaf-2', 'vif': True}, + {'network': 'foo', 'subnet': 'foo_subnet'}, + ], 'image': {'href': 'overcloud-full'}, - 'traits': ['CUSTOM_FOO'], 'nics': [{'subnet': 'leaf-2'}]}, + 'traits': ['CUSTOM_FOO'], + 'nics': [{'network': 'some_net', 'subnet': 'leaf-2'}]}, ], instances) self.assertEqual(