openstack
/
tripleo-validations
Code Issues Proposed changes

Add jsonschema validation for os-net-config data 

The current network environment validation (see change
Ie6bdfcaa78cf831bcb182a2de9fc813ae2fb83be) only performs basic
checks of NIC config files, e.g. checking the multiplicity of bonds
and interfaces on OvS bridges. This patch leverages the jsonschema
validation added to os-net-config in change
Ie4a905863b2d46c88d9cd6c3afc50e7d0a877090 to validate the network
environment configs against a full schema, catching a larger class of
errors (typos, missing required parameters, etc.) for all network
elements (e.g. linux bridges, OvS tunnels) configurable through
os-net-config.

Change-Id: Ic16ee0bc353c46f8fe512454176a07ee95347346
Depends-On: Ie4a905863b2d46c88d9cd6c3afc50e7d0a877090
Signed-off-by: Frank A. Zdarsky <fzdarsky@redhat.com>
This commit is contained in:
Frank A. Zdarsky 2016-08-04 20:24:37 +02:00
parent ce35d5fc2b
commit e6ebdd37bd
4 changed files with 84 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
releasenotes/notes/network-environment-validation-ce98d775d9e1b17f.yaml Normal file
View File

@ -0,0 +1,5 @@
---
features:
- |
Updated the network environment validation to use the new schema-based
validation method that would detect more error cases.

1
requirements.txt
View File

@ -6,3 +6,4 @@ pbr!=2.1.0,>=2.0.0 # Apache-2.0
oslo.config!=4.3.0,!=4.4.0,>=4.0.0 # Apache-2.0
keystoneauth1>=2.21.0 # Apache-2.0
python-heatclient>=1.6.1 # Apache-2.0
os-net-config>=7.1.0 # Apache-2.0

90
tripleo_validations/tests/library/test_network_environment.py
View File

@ -165,8 +165,7 @@ class TestNicConfigs(base.TestCase):
nic_data = self.nic_data(None)
errors = validation.check_nic_configs("controller.yaml", nic_data)
self.assertEqual(len(errors), 1)
self.assertEqual("The 'network_config' property of 'foo' must be"
" a list.", errors[0])
self.assertIn("'foo' must be a list", errors[0])
def test_bridge_has_type(self):
nic_data = self.nic_data([{
@ -175,7 +174,15 @@ class TestNicConfigs(base.TestCase):
}])
errors = validation.check_nic_configs("controller.yaml", nic_data)
self.assertEqual(len(errors), 1)
self.assertIn('must have a type', errors[0])
self.assertIn("'type' is a required property", errors[0])
def test_bridge_is_of_known_type(self):
nic_data = self.nic_data([{
'type': 'intreface'
}])
errors = validation.check_nic_configs("controller.yaml", nic_data)
self.assertEqual(len(errors), 1)
self.assertIn("{'type': 'intreface'} is not valid", errors[0])
def test_bridge_has_name(self):
nic_data = self.nic_data([{
@ -184,7 +191,18 @@ class TestNicConfigs(base.TestCase):
}])
errors = validation.check_nic_configs("controller.yaml", nic_data)
self.assertEqual(len(errors), 1)
self.assertIn('must have a name', errors[0])
self.assertIn("'name' is a required property", errors[0])
def test_bridge_has_only_known_properties(self):
nic_data = self.nic_data([{
'type': 'ovs_bridge',
'name': 'storage',
'member': [],
}])
errors = validation.check_nic_configs("controller.yaml", nic_data)
self.assertEqual(len(errors), 1)
self.assertIn("Additional properties are not allowed"
" ('member' was unexpected)", errors[0])
def test_ovs_bridge_has_members(self):
nic_data = self.nic_data([{
@ -194,7 +212,8 @@ class TestNicConfigs(base.TestCase):
}])
errors = validation.check_nic_configs("controller.yaml", nic_data)
self.assertEqual(len(errors), 1)
self.assertIn("must contain a 'members' list", errors[0])
self.assertIn("members/type: 'None' is not of type 'array'",
errors[0])
def test_ovs_bridge_members_dict(self):
nic_data = self.nic_data([{
@ -203,42 +222,55 @@ class TestNicConfigs(base.TestCase):
'members': [None],
}])
errors = validation.check_nic_configs("controller.yaml", nic_data)
self.assertEqual(len(errors), 2)
self.assertIn("must be a dictionary.", errors[0])
self.assertIn("at least 1 interface", errors[1])
self.assertEqual(len(errors), 1)
self.assertIn("members/items/oneOf: None is not valid under any"
" of the given schemas", errors[0])
def test_bonds_have_type(self):
def test_bonds_have_known_type(self):
nic_data = self.nic_data([{
'type': 'ovs_bridge',
'type': 'magic_bridge',
'name': 'storage',
'members': [{}],
}])
errors = validation.check_nic_configs("controller.yaml", nic_data)
self.assertEqual(len(errors), 2)
self.assertIn("must have a type.", errors[0])
self.assertIn("at least 1 interface", errors[1])
self.assertEqual(len(errors), 1)
self.assertIn("members/items/oneOf: {} is not valid under any"
" of the given schemas", errors[0])
def test_more_than_one_bond(self):
nic_data = self.nic_data([{
'type': 'ovs_bridge',
'name': 'storage',
'members': [
{'type': 'ovs_bond'},
{'type': 'ovs_bond'},
{
'type': 'ovs_bond',
'name': 'bond0',
'members': [
{'type': 'interface', 'name': 'eth0'},
{'type': 'interface', 'name': 'eth1'},
]
}, {
'type': 'ovs_bond',
'name': 'bond1',
'members': [
{'type': 'interface', 'name': 'eth2'},
{'type': 'interface', 'name': 'eth3'},
]
},
],
}])
errors = validation.check_nic_configs("controller.yaml", nic_data)
self.assertEqual(len(errors), 1)
self.assertIn('Invalid bonding: There are 2 bonds for bridge storage',
errors[0])
self.assertIn('Invalid bonding: There are >= 2 bonds for bridge '
'storage', errors[0])
def test_multiple_interfaces_without_bond(self):
nic_data = self.nic_data([{
'type': 'ovs_bridge',
'name': 'storage',
'members': [
{'type': 'interface'},
{'type': 'interface'},
{'type': 'interface', 'name': 'eth0'},
{'type': 'interface', 'name': 'eth1'},
],
}])
errors = validation.check_nic_configs("controller.yaml", nic_data)
@ -251,7 +283,7 @@ class TestNicConfigs(base.TestCase):
'type': 'ovs_bridge',
'name': 'storage',
'members': [
{'type': 'interface'},
{'type': 'interface', 'name': 'eth0'},
],
}])
errors = validation.check_nic_configs("controller.yaml", nic_data)
@ -262,20 +294,28 @@ class TestNicConfigs(base.TestCase):
'type': 'ovs_bridge',
'name': 'storage',
'members': [
{'type': 'ovs_bond'},
{'type': 'ovs_bond', 'name': 'bond0', 'members': []},
],
}])
errors = validation.check_nic_configs("controller.yaml", nic_data)
self.assertEqual([], errors)
self.assertEqual(len(errors), 1)
self.assertIn('members/minItems: [] is too short', errors[0])
def test_one_bond_multiple_interfaces(self):
nic_data = self.nic_data([{
'type': 'ovs_bridge',
'name': 'storage',
'members': [
{'type': 'ovs_bond'},
{'type': 'interface'},
{'type': 'interface'},
{
'type': 'ovs_bond',
'name': 'bond0',
'members': [
{'type': 'interface', 'name': 'eth2'},
{'type': 'interface', 'name': 'eth3'},
]
},
{'type': 'interface', 'name': 'eth0'},
{'type': 'interface', 'name': 'eth1'},
],
}])
errors = validation.check_nic_configs("controller.yaml", nic_data)

41
validations/library/network_environment.py
View File

@ -23,6 +23,7 @@ import yaml
import six
from ansible.module_utils.basic import AnsibleModule
from os_net_config import validator
def open_network_environment_files(netenv_path, template_files):
@ -149,47 +150,31 @@ def check_nic_configs(path, nic_data):
# Not all resources contain a network config:
if not bridges:
continue
# Validate the os_net_config object against the schema
v_errors = validator.validate_config(bridges, path)
errors.extend(v_errors)
if len(v_errors) > 0:
continue
# If we get here, the nic config file conforms to the schema and
# there is no more need to check for existence and type of
# properties.
for bridge in bridges:
if 'type' not in bridge:
errors.append("The bridge item {} in {} {} must have a type."
.format(bridge, name, path))
continue
if 'name' not in bridge:
errors.append("The bridge item {} in {} {} must have a name."
.format(bridge, name, path))
continue
if bridge['type'] == 'ovs_bridge':
bond_count = 0
interface_count = 0
if not isinstance(bridge.get('members'), collections.Iterable):
errors.append(
"OVS bridge {} in {} {} must contain a 'members' list."
.format(bridge, name, path))
continue
for bridge_member in bridge['members']:
if not isinstance(bridge_member, collections.Mapping):
errors.append(
"The {} bridge member in {} {} must be a "
"dictionary."
.format(bridge_member, name, path))
continue
if 'type' not in bridge_member:
errors.append(
"The {} bridge member in {} {} must have a type."
.format(bridge_member, name, path))
continue
if bridge_member['type'] in ('ovs_bond', 'ovs_dpdk_bond'):
bond_count += 1
elif bridge_member['type'] == 'interface':
interface_count += 1
else:
# TODO(mandre) should we add an error for unknown
# bridge member types?
pass
if bond_count == 2:
if bond_count >= 2:
errors.append(
'Invalid bonding: There are 2 bonds for'
'Invalid bonding: There are >= 2 bonds for'
' bridge {} of resource {} in {}'.format(
bridge['name'], name, path))
if bond_count == 0 and interface_count > 1: