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

3 files changed, 52 insertions, 39 deletions

diff --git a/os_net_config/tests/base.py b/os_net_config/tests/base.py
index d5adc66..55e0b61 100644
--- a/os_net_config/tests/base.py
+++ b/os_net_config/tests/base.py
@@ -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:
diff --git a/os_net_config/tests/test_impl_ifcfg.py b/os_net_config/tests/test_impl_ifcfg.py
index 467db65..9b24717 100644
--- a/os_net_config/tests/test_impl_ifcfg.py
+++ b/os_net_config/tests/test_impl_ifcfg.py
@@ -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,
diff --git a/os_net_config/tests/test_objects.py b/os_net_config/tests/test_objects.py
index bbff1f3..7e8441c 100644
--- a/os_net_config/tests/test_objects.py
+++ b/os_net_config/tests/test_objects.py
@@ -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())