os_net_config should map nics that are down if nic is in mapping file

Currently os-net_config will map nics from a user-supplied mapping file
only if the nic is active (operstate = up).  This can cause problems
if a nic is in a bond and one of the bond's nics has no carrier.
This fix will map the nic from the mapping file if the nic is defined
on the system, regardless of the operstate status.

The fix implements a new function to return a list of available nics
(no check of operstate) for use if a mapping file is supplied.  The
list of active nics must still be used in the default case when
numbering nics (no mapping file supplied).  There is also some
cleanup to check if a user-supplied mac is in the mapping file before
attempting to convert the mac to a nic.

Change-Id: Ia5d8c8b49b7ac0b51ee42a754f06e5e53587a5f6
Closes-Bug: 1679787
(cherry picked from commit b905e0d803)
This commit is contained in:
Bob Fournier 2017-04-04 13:58:43 -04:00 committed by Dan Sneddon
parent 9e3d128985
commit 621fa2a6af
4 changed files with 103 additions and 40 deletions

View File

@ -85,37 +85,49 @@ def _mapped_nics(nic_mapping=None):
if _MAPPED_NICS:
return _MAPPED_NICS
_MAPPED_NICS = {}
active_nics = utils.ordered_active_nics()
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)
except IOError:
continue
if nic_mapped == active_mac:
logger.debug("%s matches device %s" % (nic_mapped, active))
nic_mapped = active
is_mapping_valid = True
break
if not is_mapping_valid:
# The mapping can't specify a non-active or non-existent nic
logger.warning('interface %s is not an active nic (%s)'
% (nic_mapped, ', '.join(active_nics)))
if mapping:
# If mapping file provided, nics need not be active
available_nics = utils.ordered_available_nics()
for nic_alias, nic_mapped in mapping.items():
if netaddr.valid_mac(nic_mapped):
# If 'nic' is actually a mac address, retrieve actual nic name
for nic in available_nics:
try:
mac = utils.interface_mac(nic)
except IOError:
continue
if nic_mapped == mac:
logger.debug("%s matches device %s" %
(nic_mapped, nic))
nic_mapped = nic
break
else:
# The mac could not be found on this system
logger.error('mac %s not found in available nics (%s)'
% (nic_mapped, ', '.join(available_nics)))
continue
elif nic_mapped not in available_nics:
# nic doesn't exist on this system
logger.error('nic %s not found in available nics (%s)'
% (nic_mapped, ', '.join(available_nics)))
continue
# Duplicate mappings are not allowed
if nic_mapped in _MAPPED_NICS.values():
msg = ('interface %s already mapped, '
'check mapping file for duplicates'
% nic_mapped)
raise InvalidConfigException(msg)
# Duplicate mappings are not allowed
if nic_mapped in _MAPPED_NICS.values():
msg = ('interface %s already mapped, '
'check mapping file for duplicates'
% nic_mapped)
raise InvalidConfigException(msg)
_MAPPED_NICS[nic_alias] = nic_mapped
logger.info("%s mapped to: %s" % (nic_alias, nic_mapped))
_MAPPED_NICS[nic_alias] = nic_mapped
logger.info("%s in mapping file mapped to: %s"
% (nic_alias, nic_mapped))
# nics not in mapping file must be active in order to be mapped
active_nics = utils.ordered_active_nics()
# Add default numbered mappings, but do not overwrite existing entries
for nic_mapped in set(active_nics).difference(set(_MAPPED_NICS.values())):

View File

@ -845,6 +845,12 @@ class TestNicMapping(base.TestCase):
return nics
self.stubs.Set(utils, 'ordered_active_nics', dummy_ordered_active_nics)
def _stub_available_nics(self, nics):
def dummy_ordered_available_nics():
return nics
self.stubs.Set(utils, 'ordered_available_nics',
dummy_ordered_available_nics)
def test_mapped_nics_default(self):
self._stub_active_nics(['em1', 'em2'])
expected = {'nic1': 'em1', 'nic2': 'em2'}
@ -852,43 +858,56 @@ class TestNicMapping(base.TestCase):
def test_mapped_nics_mapped(self):
self._stub_active_nics(['em1', 'em2'])
self._stub_available_nics(['em1', 'em2'])
mapping = {'nic1': 'em2', 'nic2': 'em1'}
expected = {'nic1': 'em2', 'nic2': 'em1'}
self.assertEqual(expected, objects._mapped_nics(nic_mapping=mapping))
def test_mapped_nics_mapped_partial(self):
self._stub_active_nics(['em1', 'em2', 'em3', 'em4'])
self._stub_available_nics(['em1', 'em2', 'em3', 'em4'])
mapping = {'nic1': 'em2', 'nic2': 'em1'}
expected = {'nic1': 'em2', 'nic2': 'em1', 'nic3': 'em3', 'nic4': 'em4'}
self.assertEqual(expected, objects._mapped_nics(nic_mapping=mapping))
def test_mapped_nics_mapped_partial_reordered(self):
self._stub_active_nics(['em1', 'em2', 'em3', 'em4'])
self._stub_available_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'])
self._stub_available_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._stub_active_nics(['em2'])
self._stub_available_nics(['em1', 'em2', 'em3'])
mapping = {'nic2': 'em1'}
expected = {'nic1': 'em2', 'nic2': 'em1'}
self.assertEqual(expected, objects._mapped_nics(nic_mapping=mapping))
def test_mapped_nics_map_error_duplicate(self):
self._stub_active_nics(['em1', 'em2'])
self._stub_available_nics(['em1', 'em2'])
mapping = {'nic1': 'em1', 'nic2': 'em1'}
err = self.assertRaises(objects.InvalidConfigException,
objects._mapped_nics, nic_mapping=mapping)
expected = 'em1 already mapped, check mapping file for duplicates'
self.assertIn(expected, six.text_type(err))
def test_mapped_nics_map_invalid_nic(self):
self._stub_active_nics(['em1'])
self._stub_available_nics(['em1', 'em2'])
mapping = {'nic1': 'em1', 'nic2': 'foo'}
expected = {'nic1': 'em1'}
self.assertEqual(expected, objects._mapped_nics(nic_mapping=mapping))
def test_mapped_nics_map_mac(self):
def dummy_interface_mac(name):
mac_map = {'em1': '12:34:56:78:9a:bc',
@ -896,10 +915,24 @@ class TestNicMapping(base.TestCase):
return mac_map[name]
self.stubs.Set(utils, 'interface_mac', dummy_interface_mac)
self._stub_active_nics(['em1', 'em2'])
self._stub_available_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._mapped_nics(nic_mapping=mapping))
def test_mapped_nics_map_invalid_mac(self):
def dummy_interface_mac(name):
mac_map = {'em1': '12:34:56:78:9a:bc',
'em2': '12:34:56:de:f0:12'}
return mac_map[name]
self.stubs.Set(utils, 'interface_mac', dummy_interface_mac)
self._stub_active_nics(['em1', 'em2'])
self._stub_available_nics(['em1', 'em2'])
mapping = {'nic1': '12:34:56:de:f0:12', 'nic2': 'aa:bb:cc:dd:ee:ff'}
expected = {'nic1': 'em2'}
self.assertEqual(expected, objects._mapped_nics(nic_mapping=mapping))
def test_mapped_nics_no_active(self):
self._stub_active_nics([])
expected = {}

View File

@ -56,9 +56,9 @@ class TestUtils(base.TestCase):
tmpdir = tempfile.mkdtemp()
self.stubs.Set(utils, '_SYS_CLASS_NET', tmpdir)
def test_is_active_nic(interface_name):
def test_is_available_nic(interface_name, check_active):
return True
self.stubs.Set(utils, '_is_active_nic', test_is_active_nic)
self.stubs.Set(utils, '_is_available_nic', test_is_available_nic)
for nic in ['a1', 'em1', 'em2', 'eth2', 'z1',
'enp8s0', 'enp10s0', 'enp1s0f0']:
@ -186,9 +186,9 @@ class TestUtils(base.TestCase):
tmpdir = tempfile.mkdtemp()
self.stubs.Set(utils, '_SYS_CLASS_NET', tmpdir)
def test_is_active_nic(interface_name):
def test_is_available_nic(interface_name, check_active):
return True
self.stubs.Set(utils, '_is_active_nic', test_is_active_nic)
self.stubs.Set(utils, '_is_available_nic', test_is_available_nic)
for nic in ['a1', 'em1', 'em2', 'eth2', 'z1',
'enp8s0', 'enp10s0', 'enp1s0f0']:

View File

@ -92,20 +92,30 @@ def interface_mac(name):
def _is_active_nic(interface_name):
return _is_available_nic(interface_name, True)
def _is_available_nic(interface_name, check_active=True):
try:
if interface_name == 'lo':
return False
device_dir = _SYS_CLASS_NET + '/%s/device' % interface_name
has_device_dir = os.path.isdir(device_dir)
if not has_device_dir:
return False
operstate = None
with open(_SYS_CLASS_NET + '/%s/operstate' % interface_name, 'r') as f:
operstate = f.read().rstrip().lower()
if check_active and operstate != 'up':
return False
address = None
with open(_SYS_CLASS_NET + '/%s/address' % interface_name, 'r') as f:
address = f.read().rstrip()
if not address:
return False
# If SR-IOV Virtual Functions (VF) are enabled in an interface, there
# will be additional nics created for each VF. It has to be ignored in
@ -114,12 +124,12 @@ def _is_active_nic(interface_name):
# ignored.
vf_path_check = _SYS_CLASS_NET + '/%s/device/physfn' % interface_name
is_sriov_vf = os.path.isdir(vf_path_check)
if (has_device_dir and operstate == 'up' and address and
not is_sriov_vf):
return True
else:
if is_sriov_vf:
return False
# nic is available
return True
except IOError:
return False
@ -136,13 +146,21 @@ def _is_embedded_nic(nic):
return False
def ordered_available_nics():
return _ordered_nics(False)
def ordered_active_nics():
return _ordered_nics(True)
def _ordered_nics(check_active):
embedded_nics = []
nics = []
logger.debug("Finding active nics")
for name in glob.iglob(_SYS_CLASS_NET + '/*'):
nic = name[(len(_SYS_CLASS_NET) + 1):]
if _is_active_nic(nic):
if _is_available_nic(nic, check_active):
if _is_embedded_nic(nic):
logger.debug("%s is an embedded active nic" % nic)
embedded_nics.append(nic)