Add retry and timeout for VPP interface discoveryopnfv-6.2.0opnfv-6.1.0opnfv-6.0.0stable/fraser

VPP sometimes takes some time to come up, this causes VPP interface detection
to fail. This patch adds retries for interface discovery so we can properly
identify state of VPP interfaces.

Also contains minor fixes for regex for VPP configs generation code to prevent
cases where we might edit configs that are commented out.

Change-Id: I915d5455acb8d496438b9c9e851639d3a43e6fa9
Signed-off-by: Feng Pan <fpan@redhat.com>
(cherry picked from commit 1c50db100a7b7ff797a5ab7e1a6f7567adeb658e)

2 files changed, 102 insertions, 36 deletions

diff --git a/os_net_config/tests/test_utils.py b/os_net_config/tests/test_utils.py
index e09b6f7..10ee3b1 100644
--- a/os_net_config/tests/test_utils.py
+++ b/os_net_config/tests/test_utils.py
@@ -14,6 +14,7 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import mock
 import os
 import os.path
 import random
@@ -354,6 +355,12 @@ class TestUtils(base.TestCase):
         self.assertRaises(utils.VppException,
                           utils._get_vpp_interface_name, '0000:09.0')
 
+    @mock.patch('os_net_config.utils.processutils.execute',
+                return_value=('', None))
+    def test_get_vpp_interface_name_multiple_iterations(self, mock_execute):
+        self.assertIsNone(utils._get_vpp_interface_name('0000:00:09.0', 2, 1))
+        self.assertEqual(4, mock_execute.call_count)
+
     def test_generate_vpp_config(self):
         tmpdir = tempfile.mkdtemp()
         config_path = os.path.join(tmpdir, 'startup.conf')
@@ -406,7 +413,7 @@ dpdk {
             return None, None
         self.stubs.Set(processutils, 'execute', test_execute)
 
-        def test_get_vpp_interface_name(pci_dev):
+        def test_get_vpp_interface_name(pci_dev, tries, timeout):
             return 'GigabitEthernet0/9/0'
 
         self.stubs.Set(utils, '_get_vpp_interface_name',
diff --git a/os_net_config/utils.py b/os_net_config/utils.py
index 27e888d..8207c71 100644
--- a/os_net_config/utils.py
+++ b/os_net_config/utils.py
@@ -18,10 +18,14 @@ import glob
 import logging
 import os
 import re
+import time
 import yaml
+import json
 
 from oslo_concurrency import processutils
+import subprocess
 
+HIERADATA_FILE = '/etc/puppet/hieradata/common.json'
 
 logger = logging.getLogger(__name__)
 _SYS_CLASS_NET = '/sys/class/net'
@@ -59,6 +63,12 @@ def write_yaml_config(filepath, data):
         yaml.dump(data, f, default_flow_style=False)
 
 
+def write_json_config(filepath, data): 
+    ensure_directory_presence(filepath) 
+    with open(filepath, 'w') as f: 
+        json.dump(data, f, indent=2) 
+
+
 def ensure_directory_presence(filepath):
     dir_path = os.path.dirname(filepath)
     if not os.path.exists(dir_path):
@@ -321,6 +331,23 @@ def _get_dpdk_mac_address(name):
             return item['mac_address']
 
 
+def write_hiera(filename, data): 
+    """Writes a dictionary as hiera variables to a file. 
+ 
+    If file already exists, the data will be appended to the file. 
+    :param filename:  file to write the hiera data to 
+    :param data: dictionary of key,value pairs to write as hiera variables 
+    :return: None 
+    """ 
+    if not isinstance(data, dict): 
+        raise TypeError('data type must be dictionary') 
+ 
+    current_content = get_file_data(filename) or "{}" 
+    current_data = json.loads(current_content) 
+    current_data.update(data) 
+    write_json_config(HIERADATA_FILE, current_data) 
+
+
 def restart_vpp(vpp_interfaces):
     for vpp_int in vpp_interfaces:
         if 'vfio-pci' in vpp_int.uio_driver:
@@ -329,49 +356,68 @@ def restart_vpp(vpp_interfaces):
     processutils.execute('systemctl', 'restart', 'vpp')
 
 
-def _get_vpp_interface_name(pci_addr):
+def _get_vpp_interface_name(pci_addr, tries=10, timeout=5):
     """Get VPP interface name from a given PCI address
 
     From a running VPP instance, attempt to find the interface name from
     a given PCI address of a NIC.
 
-    VppException will be raised if pci_addr is not formatted correctly.
-    ProcessExecutionError will be raised if VPP interface mapped to pci_addr
-    is not found.
-
     :param pci_addr: PCI address to lookup, in the form of DDDD:BB:SS.F, where
                      - DDDD = Domain
                      - BB = Bus Number
                      - SS = Slot number
                      - F = Function
+    :param tries: Number of tries for getting vppctl output. Defaults to 1.
+    :param timeout: Timeout in seconds between tries. Defaults to 5.
     :return: VPP interface name. None if an interface is not found.
     """
     if not pci_addr:
         return None
 
-    try:
-        processutils.execute('systemctl', 'is-active', 'vpp')
-        out, err = processutils.execute('vppctl', 'show', 'interfaces')
-        m = re.search(r':([0-9a-fA-F]{2}):([0-9a-fA-F]{2}).([0-9a-fA-F])',
-                      pci_addr)
-        if m:
-            formatted_pci = "%x/%x/%x" % (int(m.group(1), 16),
-                                          int(m.group(2), 16),
-                                          int(m.group(3), 16))
-        else:
-            raise VppException('Invalid PCI address format: %s' % pci_addr)
+    for _ in range(tries):
+        try:
+            timestamp = time.time()
+            # processutils.execute() seems to fail repeatedly despite
+            # the command's output showing the VPP interface. Probably
+            # an issue with the oslo_concurrency module.
+            #
+            # For the time being, replace processutils.execute with
+            # the simple subprocess.check_output which seems to work
+            # fine.
+            #
+            # TODO(onong): find out why this happens
+            # processutils.execute('systemctl', 'is-active', 'vpp')
+            # out, err = processutils.execute('vppctl', 'show', 'interface')
+            # logger.debug("vppctl show interface\n%s\n%s\n" % (out, err))
+            out = None
+            try:
+                out = subprocess.check_output(['vppctl', 'show', 'interface'])
+            except:
+                # the loop will try again
+                pass
+            if out:
+                logger.debug("vppctl show interface\n%s\n" % out)
+                m = re.search(r':([0-9a-fA-F]{2}):([0-9a-fA-F]{2}).([0-9a-fA-F])',
+                              pci_addr)
+                if m:
+                    formatted_pci = "%x/%x/%x" % (int(m.group(1), 16),
+                                                  int(m.group(2), 16),
+                                                  int(m.group(3), 16))
+                else:
+                    raise VppException('Invalid PCI address format: %s' % pci_addr)
 
-        m = re.search(r'^(\w+%s)\s+' % formatted_pci, out, re.MULTILINE)
-        if m:
-            logger.debug('VPP interface found: %s' % m.group(1))
-            return m.group(1)
-        else:
-            logger.debug('Interface with pci address %s not bound to VPP'
-                         % pci_addr)
-            return None
-    except processutils.ProcessExecutionError:
-        logger.debug('Interface with pci address %s not bound to vpp' %
-                     pci_addr)
+                m = re.search(r'^(\w+%s)\s+' % formatted_pci, out, re.MULTILINE)
+                if m:
+                    logger.debug('VPP interface found: %s' % m.group(1))
+                    return m.group(1)
+        except processutils.ProcessExecutionError:
+            pass
+
+        time.sleep(max(0, (timestamp + timeout) - time.time()))
+    else:
+        logger.info('Interface with pci address %s not bound to vpp' %
+                    pci_addr)
+        return None
 
 
 def generate_vpp_config(vpp_config_path, vpp_interfaces):
@@ -417,11 +463,12 @@ def generate_vpp_config(vpp_config_path, vpp_interfaces):
             # If such config line is found, we will replace the line with
             # appropriate configuration, otherwise, add a new config line
             # in 'dpdk' section of the config.
-            m = re.search(r'^\s*dev\s+%s\s*(\{[^}]*\})?\s*'
+            m = re.search(r'^\s*dev\s+%s\s*(\{[^}]*\})?\s*$'
                           % vpp_interface.pci_dev, data,
                           re.IGNORECASE | re.MULTILINE)
             if m:
-                data = re.sub(m.group(0), '  dev %s\n' % int_cfg, data)
+                data = re.sub(m.group(0), '  dev %s\n' % int_cfg, data,
+                              flags=re.MULTILINE)
             else:
                 data = re.sub(r'(^\s*dpdk\s*\{)',
                               r'\1\n  dev %s\n' % int_cfg,
@@ -434,13 +481,15 @@ def generate_vpp_config(vpp_config_path, vpp_interfaces):
                 # configuration, otherwise, add a new line in 'dpdk' section.
                 m = re.search(r'^\s*uio-driver.*$', data, re.MULTILINE)
                 if m:
-                    data = re.sub(m.group(0), r'  uio-driver %s'
-                                  % vpp_interface.uio_driver, data)
+                    data = re.sub(r'^\s*uio-driver.*$', '  uio-driver %s'
+                                  % vpp_interface.uio_driver, data,
+                                  flags=re.MULTILINE)
                 else:
-                    data = re.sub(r'(dpdk\s*\{)',
+                    data = re.sub(r'(^\s*dpdk\s*\{)',
                                   r'\1\n  uio-driver %s'
                                   % vpp_interface.uio_driver,
-                                  data)
+                                  data,
+                                  flags=re.MULTILINE)
         else:
             logger.debug('pci address not found for interface %s, may have'
                          'already been bound to vpp' % vpp_interface.name)
@@ -490,9 +539,18 @@ def update_vpp_mapping(vpp_interfaces):
         # for some reason, we will restart VPP and try again. Currently
         # only trying one more time, can turn into a retry_counter if needed
         # in the future.
-        for i in range(2):
-            vpp_name = _get_vpp_interface_name(vpp_int.pci_dev)
+        for i in range(5):
+            vpp_name = _get_vpp_interface_name(vpp_int.pci_dev,
+                                               tries=12, timeout=5)
             if not vpp_name:
+                # VPP refuses to add the interface if it is in UP state.
+                # This is a hack for that.
+                # TODO(onong): Find out why. Also, there was a patch to fix this.
+                try:
+                    subprocess.call(["ip", "link", "set", vpp_int.name, "down"])
+                except:
+                    # here's hoping that ip link does not fail
+                    pass
                 restart_vpp(vpp_interfaces)
             else:
                 break
@@ -513,6 +571,7 @@ def update_vpp_mapping(vpp_interfaces):
                        vpp_int.uio_driver))
         _update_dpdk_map(vpp_int.name, vpp_int.pci_dev, vpp_int.hwaddr,
                          vpp_int.uio_driver)
+        write_hiera(HIERADATA_FILE, {vpp_int.name: vpp_name})
         # Enable VPP service to make the VPP interface configuration
         # persistent.
         processutils.execute('systemctl', 'enable', 'vpp')