openstack
/
os-net-config
Code Issues Proposed changes

Enhance and fix NIC mapping feature 

The original implementation supported a strictly numeric mapping scheme
(e.g. nic1, nic2) that could misbehave if an active NIC was not listed in the
user's mapping file. This change fixes the misbehavior, and enhances the
feature by not requiring NIC aliases follow the numeric mapping scheme. This
allows the user to choose meaningful names for the NIC aliases.

NIC mapping now happens in two steps:

1) Process any user supplied mappings
   - NIC alias does not need to follow the numeric "nicN" scheme
   - Existing validation rules apply: mappings for inactive NICs are ignored
     (but logged), and duplicate mappings are rejected
2) Generate default mappings as needed
   - Only applies to active NICs that aren't already mapped
   - Follows the numeric scheme (nic1, nic2) using the NIC number based on
     the list of active NICs
   - No default mapping is assigned if another NIC is already using that
     numeric alias

Change-Id: I6943623a51702349f6a7dcf2de8a8429078a3ab0
Closes-Bug: 1612723
This commit is contained in:
Alan Bishop 2016-08-05 16:16:14 -04:00
parent 9e1a613204
commit c6e21c8bf6
5 changed files with 96 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
etc/os-net-config/samples/mapping_mnemonic.yaml Normal file
View File

@ -0,0 +1,8 @@
# See the mapping.yaml sample for an overview of the mapping functionality.
# This shows the use of mnemonic aliases instead of the default nicN aliases
# in configs. It can be used with the -m option to override the default
# mapping of the nicN aliases.
interface_mapping:
provision: em1
bond0p1: em2
bond0p2: 12:34:56:de:f0:12

62
os_net_config/objects.py
View File

@ -23,7 +23,7 @@ from os_net_config import utils
logger = logging.getLogger(__name__)
_NUMBERED_NICS = None
_MAPPED_NICS = None
class InvalidConfigException(ValueError):
@ -71,21 +71,17 @@ def _get_required_field(json, name, object_name):
return field
def _numbered_nics(nic_mapping=None):
def _mapped_nics(nic_mapping=None):
mapping = nic_mapping or {}
global _NUMBERED_NICS
if _NUMBERED_NICS:
return _NUMBERED_NICS
_NUMBERED_NICS = {}
count = 0
global _MAPPED_NICS
if _MAPPED_NICS:
return _MAPPED_NICS
_MAPPED_NICS = {}
active_nics = utils.ordered_active_nics()
for nic in active_nics:
count += 1
nic_alias = "nic%i" % count
nic_mapped = mapping.get(nic_alias, nic)
# The mapping is either invalid, or specifies a mac
for nic_alias, nic_mapped in mapping.items():
if nic_mapped not in active_nics:
# The mapping is either invalid, or specifies a mac
is_mapping_valid = False
for active in active_nics:
try:
active_mac = utils.interface_mac(active)
@ -94,25 +90,39 @@ def _numbered_nics(nic_mapping=None):
if nic_mapped == active_mac:
logger.debug("%s matches device %s" % (nic_mapped, active))
nic_mapped = active
is_mapping_valid = True
break
else:
if not is_mapping_valid:
# The mapping can't specify a non-active or non-existent nic
logger.warning('interface %s is not in an active nic (%s)'
logger.warning('interface %s is not an active nic (%s)'
% (nic_mapped, ', '.join(active_nics)))
continue
# Duplicate mappings are not allowed
if nic_mapped in _NUMBERED_NICS.values():
if nic_mapped in _MAPPED_NICS.values():
msg = ('interface %s already mapped, '
'check mapping file for duplicates'
% nic_mapped)
raise InvalidConfigException(msg)
_NUMBERED_NICS[nic_alias] = nic_mapped
_MAPPED_NICS[nic_alias] = nic_mapped
logger.info("%s mapped to: %s" % (nic_alias, nic_mapped))
if not _NUMBERED_NICS:
# Add default numbered mappings, but do not overwrite existing entries
for nic_mapped in set(active_nics).difference(set(_MAPPED_NICS.values())):
nic_alias = "nic%i" % (active_nics.index(nic_mapped) + 1)
if nic_alias in _MAPPED_NICS:
logger.warning("no mapping for interface %s because "
"%s is mapped to %s"
% (nic_mapped, nic_alias, _MAPPED_NICS[nic_alias]))
else:
_MAPPED_NICS[nic_alias] = nic_mapped
logger.info("%s mapped to: %s" % (nic_alias, nic_mapped))
if not _MAPPED_NICS:
logger.warning('No active nics found.')
return _NUMBERED_NICS
return _MAPPED_NICS
class Route(object):
@ -158,18 +168,18 @@ class _BaseOpts(object):
addresses = addresses or []
routes = routes or []
dns_servers = dns_servers or []
numbered_nic_names = _numbered_nics(nic_mapping)
mapped_nic_names = _mapped_nics(nic_mapping)
self.hwaddr = None
self.hwname = None
self.renamed = False
if name in numbered_nic_names:
if name in mapped_nic_names:
if persist_mapping:
self.name = name
self.hwname = numbered_nic_names[name]
self.hwname = mapped_nic_names[name]
self.hwaddr = utils.interface_mac(self.hwname)
self.renamed = True
else:
self.name = numbered_nic_names[name]
self.name = mapped_nic_names[name]
else:
self.name = name
@ -297,9 +307,9 @@ class Vlan(_BaseOpts):
dns_servers)
self.vlan_id = int(vlan_id)
numbered_nic_names = _numbered_nics(nic_mapping)
if device in numbered_nic_names:
self.device = numbered_nic_names[device]
mapped_nic_names = _mapped_nics(nic_mapping)
if device in mapped_nic_names:
self.device = mapped_nic_names[device]
else:
self.device = device

12
os_net_config/tests/base.py
View File

@ -29,19 +29,19 @@ _TRUE_VALUES = ('True', 'true', '1', 'yes')
class TestCase(testtools.TestCase):
"""Test case base class for all unit tests."""
stub_numbered_nics = True
stub_mapped_nics = True
def setUp(self):
"""Run before each test method to initialize test environment."""
super(TestCase, self).setUp()
self.stubs = stubout.StubOutForTesting()
self.stubbed_numbered_nics = {}
self.stubbed_mapped_nics = {}
def dummy_numbered_nics(nic_mapping=None):
return self.stubbed_numbered_nics
if self.stub_numbered_nics:
self.stubs.Set(objects, '_numbered_nics', dummy_numbered_nics)
def dummy_mapped_nics(nic_mapping=None):
return self.stubbed_mapped_nics
if self.stub_mapped_nics:
self.stubs.Set(objects, '_mapped_nics', dummy_mapped_nics)
test_timeout = os.environ.get('OS_TEST_TIMEOUT', 0)
try:

2
os_net_config/tests/test_impl_ifcfg.py
View File

@ -429,7 +429,7 @@ class TestIfcfgNetConfig(base.TestCase):
self.stubs.Set(utils, 'interface_mac', test_interface_mac)
nic_mapping = {'nic1': 'em1'}
self.stubbed_numbered_nics = nic_mapping
self.stubbed_mapped_nics = nic_mapping
v4_addr = objects.Address('192.168.1.2/24')
interface = objects.Interface('nic1', addresses=[v4_addr],
nic_mapping=nic_mapping,

77
os_net_config/tests/test_objects.py
View File

@ -131,9 +131,9 @@ class TestInterface(base.TestCase):
self.assertEqual(["1.2.3.4"], interface1.dns_servers)
def test_from_json_dhcp_nic1(self):
def dummy_numbered_nics(nic_mapping=None):
def dummy_mapped_nics(nic_mapping=None):
return {"nic1": "em3"}
self.stubs.Set(objects, '_numbered_nics', dummy_numbered_nics)
self.stubs.Set(objects, '_mapped_nics', dummy_mapped_nics)
data = '{"type": "interface", "name": "nic1", "use_dhcp": true}'
interface = objects.object_from_json(json.loads(data))
@ -179,9 +179,9 @@ class TestVlan(base.TestCase):
self.assertTrue(vlan.use_dhcp)
def test_from_json_dhcp_nic1(self):
def dummy_numbered_nics(nic_mapping=None):
def dummy_mapped_nics(nic_mapping=None):
return {"nic1": "em4"}
self.stubs.Set(objects, '_numbered_nics', dummy_numbered_nics)
self.stubs.Set(objects, '_mapped_nics', dummy_mapped_nics)
data = '{"type": "vlan", "device": "nic1", "vlan_id": 16,' \
'"use_dhcp": true}'
@ -213,9 +213,9 @@ class TestBridge(base.TestCase):
self.assertEqual("br-foo", interface1.bridge_name)
def test_from_json_dhcp_with_nic1(self):
def dummy_numbered_nics(nic_mapping=None):
def dummy_mapped_nics(nic_mapping=None):
return {"nic1": "em5"}
self.stubs.Set(objects, '_numbered_nics', dummy_numbered_nics)
self.stubs.Set(objects, '_mapped_nics', dummy_mapped_nics)
data = """{
"type": "ovs_bridge",
@ -289,9 +289,9 @@ class TestLinuxBridge(base.TestCase):
self.assertEqual("br-foo", interface1.linux_bridge_name)
def test_from_json_dhcp_with_nic1(self):
def dummy_numbered_nics(nic_mapping=None):
def dummy_mapped_nics(nic_mapping=None):
return {"nic1": "em5"}
self.stubs.Set(objects, '_numbered_nics', dummy_numbered_nics)
self.stubs.Set(objects, '_mapped_nics', dummy_mapped_nics)
data = """{
"type": "linux_bridge",
@ -508,9 +508,9 @@ class TestBond(base.TestCase):
def test_from_json_dhcp_with_nic1_nic2(self):
def dummy_numbered_nics(nic_mapping=None):
def dummy_mapped_nics(nic_mapping=None):
return {"nic1": "em1", "nic2": "em2"}
self.stubs.Set(objects, '_numbered_nics', dummy_numbered_nics)
self.stubs.Set(objects, '_mapped_nics', dummy_mapped_nics)
data = """{
"type": "ovs_bond",
@ -588,9 +588,9 @@ class TestLinuxBond(base.TestCase):
def test_from_json_dhcp_with_nic1_nic2(self):
def dummy_numbered_nics(nic_mapping=None):
def dummy_mapped_nics(nic_mapping=None):
return {"nic1": "em1", "nic2": "em2"}
self.stubs.Set(objects, '_numbered_nics', dummy_numbered_nics)
self.stubs.Set(objects, '_mapped_nics', dummy_mapped_nics)
data = """{
"type": "ovs_bond",
@ -719,9 +719,9 @@ class TestIbInterface(base.TestCase):
self.assertEqual(["1.2.3.4"], ib_interface1.dns_servers)
def test_from_json_dhcp_nic1(self):
def dummy_numbered_nics(nic_mapping=None):
def dummy_mapped_nics(nic_mapping=None):
return {"nic1": "ib0"}
self.stubs.Set(objects, '_numbered_nics', dummy_numbered_nics)
self.stubs.Set(objects, '_mapped_nics', dummy_mapped_nics)
data = '{"type": "ib_interface", "name": "nic1", "use_dhcp": true}'
ib_interface = objects.object_from_json(json.loads(data))
@ -756,52 +756,65 @@ class TestIbInterface(base.TestCase):
self.assertEqual("192.0.2.1/24", route1.ip_netmask)
class TestNumberedNicsMapping(base.TestCase):
class TestNicMapping(base.TestCase):
# We want to test the function, not the dummy..
stub_numbered_nics = False
stub_mapped_nics = False
def tearDown(self):
super(TestNumberedNicsMapping, self).tearDown()
objects._NUMBERED_NICS = None
super(TestNicMapping, self).tearDown()
objects._MAPPED_NICS = None
def _stub_active_nics(self, nics):
def dummy_ordered_active_nics():
return nics
self.stubs.Set(utils, 'ordered_active_nics', dummy_ordered_active_nics)
def test_numbered_nics_default(self):
def test_mapped_nics_default(self):
self._stub_active_nics(['em1', 'em2'])
expected = {'nic1': 'em1', 'nic2': 'em2'}
self.assertEqual(expected, objects._numbered_nics())
self.assertEqual(expected, objects._mapped_nics())
def test_numbered_nics_mapped(self):
def test_mapped_nics_mapped(self):
self._stub_active_nics(['em1', 'em2'])
mapping = {'nic1': 'em2', 'nic2': 'em1'}
expected = {'nic1': 'em2', 'nic2': 'em1'}
self.assertEqual(expected, objects._numbered_nics(nic_mapping=mapping))
self.assertEqual(expected, objects._mapped_nics(nic_mapping=mapping))
def test_numbered_nics_mapped_partial(self):
def test_mapped_nics_mapped_partial(self):
self._stub_active_nics(['em1', 'em2', 'em3', 'em4'])
mapping = {'nic1': 'em2', 'nic2': 'em1'}
expected = {'nic1': 'em2', 'nic2': 'em1', 'nic3': 'em3', 'nic4': 'em4'}
self.assertEqual(expected, objects._numbered_nics(nic_mapping=mapping))
self.assertEqual(expected, objects._mapped_nics(nic_mapping=mapping))
def test_numbered_nics_map_error_notactive(self):
def test_mapped_nics_mapped_partial_reordered(self):
self._stub_active_nics(['em1', 'em2', 'em3', 'em4'])
mapping = {'nic1': 'em1', 'nic2': 'em3'}
expected = {'nic1': 'em1', 'nic2': 'em3', 'nic4': 'em4'}
self.assertEqual(expected, objects._mapped_nics(nic_mapping=mapping))
def test_mapped_nics_mapped_unnumbered(self):
self._stub_active_nics(['em1', 'em2', 'em3', 'em4'])
mapping = {'John': 'em1', 'Paul': 'em2', 'George': 'em3'}
expected = {'John': 'em1', 'Paul': 'em2', 'George': 'em3',
'nic4': 'em4'}
self.assertEqual(expected, objects._mapped_nics(nic_mapping=mapping))
def test_mapped_nics_map_error_notactive(self):
self._stub_active_nics(['em1', 'em2'])
mapping = {'nic1': 'em3', 'nic2': 'em1'}
expected = {'nic2': 'em1'}
self.assertEqual(expected, objects._numbered_nics(nic_mapping=mapping))
self.assertEqual(expected, objects._mapped_nics(nic_mapping=mapping))
def test_numbered_nics_map_error_duplicate(self):
def test_mapped_nics_map_error_duplicate(self):
self._stub_active_nics(['em1', 'em2'])
mapping = {'nic1': 'em1', 'nic2': 'em1'}
err = self.assertRaises(objects.InvalidConfigException,
objects._numbered_nics, nic_mapping=mapping)
objects._mapped_nics, nic_mapping=mapping)
expected = 'em1 already mapped, check mapping file for duplicates'
self.assertIn(expected, six.text_type(err))
def test_numbered_nics_map_mac(self):
def test_mapped_nics_map_mac(self):
def dummy_interface_mac(name):
mac_map = {'em1': '12:34:56:78:9a:bc',
'em2': '12:34:56:de:f0:12'}
@ -810,10 +823,10 @@ class TestNumberedNicsMapping(base.TestCase):
self._stub_active_nics(['em1', 'em2'])
mapping = {'nic1': '12:34:56:de:f0:12', 'nic2': '12:34:56:78:9a:bc'}
expected = {'nic1': 'em2', 'nic2': 'em1'}
self.assertEqual(expected, objects._numbered_nics(nic_mapping=mapping))
self.assertEqual(expected, objects._mapped_nics(nic_mapping=mapping))
def test_numbered_nics_no_active(self):
def test_mapped_nics_no_active(self):
self._stub_active_nics([])
expected = {}
# This only emits a warning, so it should still work
self.assertEqual(expected, objects._numbered_nics())
self.assertEqual(expected, objects._mapped_nics())