test/lib/exabgp: Use exabgpcli to add/del routes

Currently, to advertise or withdraw routes with ExaBGPContainer, we need
to configure static routes via config file and restart ExaBGP daemon.
In other words, we can NOT send withdrawing advertisement with
ExaBGPContainer. Also, restating ExaBGP daemon frequently can make
scenario test unstable, and it should be avoided.

On the other hand, with ExaBGP version 4.0.5 or later(*), we can
advertise or withdraw routes using "exabgpcli" without restating ExaBGP
daemon (or writing application which calls ExaBGP's APIs).

This patch fixes to use "exabgpcli" and reduces the number of restating
ExaBGP daemon.

Note: According to this change, adding routes into ExaBGP should be
called after adding neighbor.

(*): "exabgpcli" is introduced at version 4.0.2, but has some bugs
related to Python 3 compatibility and FlowSpec rules combinations, then
we need to use version 4.0.5 or later.

Signed-off-by: IWASE Yusuke <iwase.yusuke0@gmail.com>

5 files changed, 196 insertions, 154 deletions

diff --git a/test/lib/exabgp.py b/test/lib/exabgp.py
index 3bb9d207..10406b6b 100644
--- a/test/lib/exabgp.py
+++ b/test/lib/exabgp.py
@@ -15,66 +15,59 @@
 
 from __future__ import absolute_import
 
-from itertools import chain
-import time
-
 from fabric import colors
-from fabric.api import local
 
 from lib.base import (
     BGPContainer,
     CmdBuffer,
     try_several_times,
+    wait_for_completion,
 )
 
 
 class ExaBGPContainer(BGPContainer):
 
-    SHARED_VOLUME = '/root/shared_volume'
+    SHARED_VOLUME = '/shared_volume'
+    PID_FILE = '/var/run/exabgp.pid'
 
-    def __init__(self, name, asn, router_id, ctn_image_name='osrg/exabgp',
-                 exabgp_path=''):
-        super(ExaBGPContainer, self).__init__(name, asn, router_id,
-                                              ctn_image_name)
+    def __init__(self, name, asn, router_id, ctn_image_name='osrg/exabgp:4.0.5'):
+        super(ExaBGPContainer, self).__init__(name, asn, router_id, ctn_image_name)
         self.shared_volumes.append((self.config_dir, self.SHARED_VOLUME))
-        self.exabgp_path = exabgp_path
+
+    def _pre_start_exabgp(self):
+        # Create named pipes for "exabgpcli"
+        named_pipes = '/run/exabgp.in /run/exabgp.out'
+        self.local('mkfifo {0}'.format(named_pipes), capture=True)
+        self.local('chmod 777 {0}'.format(named_pipes), capture=True)
 
     def _start_exabgp(self):
         cmd = CmdBuffer(' ')
         cmd << 'env exabgp.log.destination={0}/exabgpd.log'.format(self.SHARED_VOLUME)
-        cmd << "exabgp.tcp.bind='0.0.0.0' exabgp.tcp.port=179"
-        cmd << './exabgp/sbin/exabgp {0}/exabgpd.conf'.format(self.SHARED_VOLUME)
+        cmd << 'exabgp.daemon.user=root'
+        cmd << 'exabgp.daemon.pid={0}'.format(self.PID_FILE)
+        cmd << 'exabgp.tcp.bind="0.0.0.0" exabgp.tcp.port=179'
+        cmd << 'exabgp {0}/exabgpd.conf'.format(self.SHARED_VOLUME)
         self.local(str(cmd), detach=True)
 
-    def _update_exabgp(self):
-        if self.exabgp_path == '':
-            return
-        c = CmdBuffer()
-        c << '#!/bin/bash'
-
-        remotepath = '/root/exabgp'
-        localpath = self.exabgp_path
-        local('cp -r {0} {1}'.format(localpath, self.config_dir))
-        c << 'cp {0}/etc/exabgp/exabgp.env {1}'.format(remotepath, self.SHARED_VOLUME)
-        c << 'sed -i -e \'s/all = false/all = true/g\' {0}/exabgp.env'.format(self.SHARED_VOLUME)
-        c << 'cp -r {0}/exabgp {1}'.format(self.SHARED_VOLUME,
-                                           remotepath[:-1 * len('exabgp')])
-        c << 'cp {0}/exabgp.env {1}/etc/exabgp/'.format(self.SHARED_VOLUME, remotepath)
-        cmd = 'echo "{0:s}" > {1}/update.sh'.format(c, self.config_dir)
-        local(cmd, capture=True)
-        cmd = 'chmod 755 {0}/update.sh'.format(self.config_dir)
-        local(cmd, capture=True)
-        cmd = '{0}/update.sh'.format(self.SHARED_VOLUME)
-        self.local(cmd)
+    def _wait_for_boot(self):
+        def _f():
+            ret = self.local('exabgpcli version > /dev/null 2>&1; echo $?', capture=True)
+            return ret == '0'
+
+        return wait_for_completion(_f)
 
     def run(self):
         super(ExaBGPContainer, self).run()
-        self._update_exabgp()
-        self._start_exabgp()
+        self._pre_start_exabgp()
+        # To start ExaBGP, it is required to configure neighbor settings, so
+        # here does not start ExaBGP yet.
+        # self._start_exabgp()
         return self.WAIT_FOR_BOOT
 
     def create_config(self):
-        cmd = CmdBuffer()
+        # Manpage of exabgp.conf(5):
+        # https://github.com/Exa-Networks/exabgp/blob/master/doc/man/exabgp.conf.5
+        cmd = CmdBuffer('\n')
         for peer, info in self.peers.iteritems():
             cmd << 'neighbor {0} {{'.format(info['neigh_addr'].split('/')[0])
             cmd << '    router-id {0};'.format(self.router_id)
@@ -82,78 +75,22 @@ class ExaBGPContainer(BGPContainer):
             cmd << '    local-as {0};'.format(self.asn)
             cmd << '    peer-as {0};'.format(peer.asn)
 
+            caps = []
             if info['as2']:
+                caps.append('        asn4 disable;')
+            if info['addpath']:
+                caps.append('        add-path send/receive;')
+            if caps:
                 cmd << '    capability {'
-                cmd << '        asn4 disable;'
+                for cap in caps:
+                    cmd << cap
                 cmd << '    }'
 
             if info['passwd']:
-                cmd << '    md5 "{0}";'.format(info['passwd'])
+                cmd << '    md5-password "{0}";'.format(info['passwd'])
 
             if info['passive']:
                 cmd << '    passive;'
-
-            if info['addpath']:
-                cmd << '    add-path send/receive;'
-
-            routes = [r for r in chain.from_iterable(self.routes.itervalues()) if r['rf'] == 'ipv4' or r['rf'] == 'ipv6']
-
-            if len(routes) > 0:
-                cmd << '    static {'
-                for route in routes:
-                    r = CmdBuffer(' ')
-                    nexthop = info['local_addr'].split('/')[0]
-                    if route['next-hop']:
-                        nexthop = route['next-hop']
-                    r << '      route {0} next-hop {1}'.format(route['prefix'], nexthop)
-                    if route['as-path']:
-                        r << 'as-path [{0}]'.format(' '.join(str(i) for i in route['as-path']))
-                    if route['community']:
-                        r << 'community [{0}]'.format(' '.join(c for c in route['community']))
-                    if route['med']:
-                        r << 'med {0}'.format(route['med'])
-                    if route['local-pref']:
-                        r << 'local-preference {0}'.format(route['local-pref'])
-                    if route['extended-community']:
-                        r << 'extended-community [{0}]'.format(route['extended-community'])
-                    if route['attr']:
-                        r << 'attribute [ {0} ]'.format(route['attr'])
-                    if route['identifier']:
-                        r << 'path-information {0}'.format(route['identifier'])
-
-                    cmd << '{0};'.format(str(r))
-                cmd << '    }'
-
-            routes = self._extract_routes(['ipv4-flowspec', 'ipv6-flowspec'])
-            for key, routes in routes.items():
-                cmd << '    flow {'
-                for route in routes:
-                    cmd << '        route {0} {{'.format(key)
-                    cmd << '            match {'
-                    for match in route['matchs']:
-                        cmd << '                {0};'.format(match)
-#                    cmd << '                source {0};'.format(route['prefix'])
-#                    cmd << '                destination 192.168.0.1/32;'
-#                    cmd << '                destination-port =3128 >8080&<8088;'
-#                    cmd << '                source-port >1024;'
-#                    cmd << '                port =14 =15 >10&<156;'
-#                    cmd << '                protocol udp;' # how to specify multiple ip protocols
-#                    cmd << '                packet-length >1000&<2000;'
-#                    cmd << '                tcp-flags !syn;'
-                    cmd << '            }'
-                    cmd << '            then {'
-                    for then in route['thens']:
-                        cmd << '                {0};'.format(then)
-#                    cmd << '                accept;'
-#                    cmd << '                discard;'
-#                    cmd << '                rate-limit 9600;'
-#                    cmd << '                redirect 1.2.3.4:100;'
-#                    cmd << '                redirect 100:100;'
-#                    cmd << '                mark 10;'
-#                    cmd << '                action sample-terminal;'
-                    cmd << '            }'
-                    cmd << '        }'
-                cmd << '    }'
             cmd << '}'
 
         with open('{0}/exabgpd.conf'.format(self.config_dir), 'w') as f:
@@ -161,27 +98,137 @@ class ExaBGPContainer(BGPContainer):
             print colors.yellow(str(cmd))
             f.write(str(cmd))
 
+    def _is_running(self):
+        ret = self.local("test -f {0}; echo $?".format(self.PID_FILE), capture=True)
+        return ret == '0'
+
     def reload_config(self):
-        if len(self.peers) == 0:
+        if not self.peers:
             return
 
         def _reload():
-            def _is_running():
-                ps = self.local('ps', capture=True)
-                running = False
-                for line in ps.split('\n')[1:]:
-                    if 'python' in line:
-                        running = True
-                return running
-            if _is_running():
-                self.local('/usr/bin/pkill python -SIGUSR1')
+            if self._is_running():
+                self.local('/usr/bin/pkill --pidfile {0} && rm -f {0}'.format(self.PID_FILE), capture=True)
             else:
                 self._start_exabgp()
-            time.sleep(1)
-            if not _is_running():
-                raise RuntimeError()
+                self._wait_for_boot()
+
+            if not self._is_running():
+                raise RuntimeError('Could not start ExaBGP')
+
         try_several_times(_reload)
 
+    def _construct_ip_unicast(self, path):
+        cmd = CmdBuffer(' ')
+        cmd << str(path['prefix'])
+        if path['next-hop']:
+            cmd << 'next-hop {0}'.format(path['next-hop'])
+        else:
+            cmd << 'next-hop self'
+        return str(cmd)
+
+    def _construct_flowspec(self, path):
+        cmd = CmdBuffer(' ')
+        cmd << '{ match {'
+        for match in path['matchs']:
+            cmd << '{0};'.format(match)
+        cmd << '} then {'
+        for then in path['thens']:
+            cmd << '{0};'.format(then)
+        cmd << '} }'
+        return str(cmd)
+
+    def _construct_path_attributes(self, path):
+        cmd = CmdBuffer(' ')
+        if path['as-path']:
+            cmd << 'as-path [{0}]'.format(' '.join(str(i) for i in path['as-path']))
+        if path['med']:
+            cmd << 'med {0}'.format(path['med'])
+        if path['local-pref']:
+            cmd << 'local-preference {0}'.format(path['local-pref'])
+        if path['community']:
+            cmd << 'community [{0}]'.format(' '.join(c for c in path['community']))
+        if path['extended-community']:
+            cmd << 'extended-community [{0}]'.format(path['extended-community'])
+        if path['attr']:
+            cmd << 'attribute [ {0} ]'.format(path['attr'])
+        return str(cmd)
+
+    def _construct_path(self, path, rf='ipv4', is_withdraw=False):
+        cmd = CmdBuffer(' ')
+
+        if rf in ['ipv4', 'ipv6']:
+            cmd << 'route'
+            cmd << self._construct_ip_unicast(path)
+        elif rf in ['ipv4-flowspec', 'ipv6-flowspec']:
+            cmd << 'flow route'
+            cmd << self._construct_flowspec(path)
+        else:
+            raise ValueError('unsupported address family: %s' % rf)
+
+        if path['identifier']:
+            cmd << 'path-information {0}'.format(path['identifier'])
+
+        if not is_withdraw:
+            # Withdrawal should not require path attributes
+            cmd << self._construct_path_attributes(path)
+
+        return str(cmd)
+
+    def add_route(self, route, rf='ipv4', attribute=None, aspath=None,
+                  community=None, med=None, extendedcommunity=None,
+                  nexthop=None, matchs=None, thens=None,
+                  local_pref=None, identifier=None, reload_config=False):
+        if not self._is_running():
+            raise RuntimeError('ExaBGP is not yet running')
+
+        self.routes.setdefault(route, [])
+        path = {
+            'prefix': route,
+            'rf': rf,
+            'attr': attribute,
+            'next-hop': nexthop,
+            'as-path': aspath,
+            'community': community,
+            'med': med,
+            'local-pref': local_pref,
+            'extended-community': extendedcommunity,
+            'identifier': identifier,
+            'matchs': matchs,
+            'thens': thens,
+        }
+
+        cmd = CmdBuffer(' ')
+        cmd << "exabgpcli 'announce"
+        cmd << self._construct_path(path, rf=rf)
+        cmd << "'"
+        self.local(str(cmd), capture=True)
+
+        self.routes[route].append(path)
+
+    def del_route(self, route, identifier=None, reload_config=False):
+        if not self._is_running():
+            raise RuntimeError('ExaBGP is not yet running')
+
+        path = None
+        new_paths = []
+        for p in self.routes.get(route, []):
+            if p['identifier'] != identifier:
+                new_paths.append(p)
+            else:
+                path = p
+        if not path:
+            return
+
+        rf = path['rf']
+        cmd = CmdBuffer(' ')
+        cmd << "exabgpcli 'withdraw"
+        cmd << self._construct_path(path, rf=rf, is_withdraw=True)
+        cmd << "'"
+        self.local(str(cmd), capture=True)
+
+        self.routes[route] = new_paths
+
 
 class RawExaBGPContainer(ExaBGPContainer):
     def __init__(self, name, config, ctn_image_name='osrg/exabgp',
diff --git a/test/scenario_test/flow_spec_test.py b/test/scenario_test/flow_spec_test.py
index d4b401a7..0a94962f 100644
--- a/test/scenario_test/flow_spec_test.py
+++ b/test/scenario_test/flow_spec_test.py
@@ -65,26 +65,6 @@ class FlowSpecTest(unittest.TestCase):
         initial_wait_time = max(ctn.run() for ctn in ctns)
         time.sleep(initial_wait_time)
 
-        # Add FlowSpec routes into ExaBGP.
-        # Note: Currently, ExaBGPContainer only supports to add routes by
-        # reloading configuration, so we add routes here.
-        cls.e1.add_route(
-            route='ipv4/dst/src',
-            rf='ipv4-flowspec',
-            matchs=[
-                'destination 12.1.0.0/24',
-                'source 12.2.0.0/24',
-            ],
-            thens=['discard'])
-        cls.e1.add_route(
-            route='ipv6/dst/src',
-            rf='ipv6-flowspec',
-            matchs=[
-                'destination 2002:1::/64/10',
-                'source 2002:2::/64/15',
-            ],
-            thens=['discard'])
-
         # Add FlowSpec routes into GoBGP.
         cls.g1.add_route(
             route='ipv4/all',
@@ -122,9 +102,25 @@ class FlowSpecTest(unittest.TestCase):
         cls.g1.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=cls.e1)
         cls.g1.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=cls.y1)
 
+        # Add FlowSpec routes into ExaBGP.
+        cls.e1.add_route(
+            route='ipv4/dst/src',
+            rf='ipv4-flowspec',
+            matchs=[
+                'destination 12.1.0.0/24',
+                'source 12.2.0.0/24',
+            ],
+            thens=['discard'])
+        cls.e1.add_route(
+            route='ipv6/dst/src',
+            rf='ipv6-flowspec',
+            matchs=[
+                'destination 2002:1::/64/10',
+                'source 2002:2::/64/15',
+            ],
+            thens=['discard'])
+
         # Add FlowSpec routes into YABGP.
-        # Note: Currently, YABGPContainer only supports to add routes via
-        # REST API after connection established, so we add routes here.
         cls.y1.add_route(
             route='ipv4/all',
             rf='ipv4-flowspec',
diff --git a/test/scenario_test/global_policy_test.py b/test/scenario_test/global_policy_test.py
index 39074f3a..ff534d8f 100644
--- a/test/scenario_test/global_policy_test.py
+++ b/test/scenario_test/global_policy_test.py
@@ -61,13 +61,7 @@ class GoBGPTestBase(unittest.TestCase):
         qs = [q1, q2, q3]
         ctns = [g1, q1, q2, q3]
 
-        # advertise a route from q1, q2, q3
-        for idx, q in enumerate(qs):
-            route = '10.0.{0}.0/24'.format(idx + 1)
-            q.add_route(route)
-
         initial_wait_time = max(ctn.run() for ctn in ctns)
-
         time.sleep(initial_wait_time)
 
         g1.local('gobgp global policy export add default reject')
@@ -76,6 +70,11 @@ class GoBGPTestBase(unittest.TestCase):
             g1.add_peer(q)
             q.add_peer(g1)
 
+        # advertise a route from q1, q2, q3
+        for idx, q in enumerate(qs):
+            route = '10.0.{0}.0/24'.format(idx + 1)
+            q.add_route(route)
+
         cls.gobgp = g1
         cls.quaggas = {'q1': q1, 'q2': q2, 'q3': q3}
 
@@ -90,10 +89,10 @@ class GoBGPTestBase(unittest.TestCase):
 
     def test_03_add_peer(self):
         q = ExaBGPContainer(name='q4', asn=65004, router_id='192.168.0.5')
-        q.add_route('10.10.0.0/24')
         time.sleep(q.run())
         self.gobgp.add_peer(q)
         q.add_peer(self.gobgp)
+        q.add_route('10.10.0.0/24')
         self.gobgp.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=q)
         self.quaggas['q4'] = q
         for q in self.quaggas.itervalues():
diff --git a/test/scenario_test/route_server_as2_test.py b/test/scenario_test/route_server_as2_test.py
index bcff8f74..d60b4b21 100644
--- a/test/scenario_test/route_server_as2_test.py
+++ b/test/scenario_test/route_server_as2_test.py
@@ -53,16 +53,7 @@ class GoBGPTestBase(unittest.TestCase):
             for i in range(4)]
         ctns = [g1] + rs_clients
 
-        # advertise a route from route-server-clients
-        for idx, rs_client in enumerate(rs_clients):
-            route = '10.0.{0}.0/24'.format(idx + 1)
-            rs_client.add_route(route)
-            if idx < 2:
-                route = '10.0.10.0/24'
-            rs_client.add_route(route)
-
         initial_wait_time = max(ctn.run() for ctn in ctns)
-
         time.sleep(initial_wait_time)
 
         for i, rs_client in enumerate(rs_clients):
@@ -72,6 +63,14 @@ class GoBGPTestBase(unittest.TestCase):
                 as2 = True
             rs_client.add_peer(g1, as2=as2)
 
+        # advertise a route from route-server-clients
+        for idx, rs_client in enumerate(rs_clients):
+            route = '10.0.{0}.0/24'.format(idx + 1)
+            rs_client.add_route(route)
+            if idx < 2:
+                route = '10.0.10.0/24'
+            rs_client.add_route(route)
+
         cls.gobgp = g1
         cls.quaggas = {x.name: x for x in rs_clients}
 
diff --git a/test/scenario_test/route_server_test2.py b/test/scenario_test/route_server_test2.py
index e60e264e..61c546b8 100644
--- a/test/scenario_test/route_server_test2.py
+++ b/test/scenario_test/route_server_test2.py
@@ -43,7 +43,6 @@ class GoBGPTestBase(unittest.TestCase):
         g1 = GoBGPContainer(name='g1', asn=65000, router_id='192.168.0.1',
                             ctn_image_name=gobgp_ctn_image_name,
                             log_level=parser_option.gobgp_log_level)
-
         g2 = GoBGPContainer(name='g2', asn=65001, router_id='192.168.0.2',
                             ctn_image_name=gobgp_ctn_image_name)
         e1 = ExaBGPContainer(name='e1', asn=65002, router_id='192.168.0.3')
@@ -55,8 +54,10 @@ class GoBGPTestBase(unittest.TestCase):
         time.sleep(initial_wait_time)
 
         for cli in cls.clients.values():
-            g1.add_peer(cli, is_rs_client=True, passwd='passwd', passive=True, prefix_limit=10)
-            cli.add_peer(g1, passwd='passwd')
+            # Omit "passwd" to avoid a issue on ExaBGP version 4.0.5:
+            # https://github.com/Exa-Networks/exabgp/issues/766
+            g1.add_peer(cli, is_rs_client=True, passive=True, prefix_limit=10)
+            cli.add_peer(g1)
 
         # advertise a route from route-server-clients
         g2.add_route('10.0.0.0/24')
@@ -66,7 +67,7 @@ class GoBGPTestBase(unittest.TestCase):
 
     # test each neighbor state is turned establish
     def test_01_neighbor_established(self):
-        for cli in self.clients.itervalues():
+        for cli in self.clients.values():
             self.gobgp.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=cli)
 
     def test_02_add_neighbor(self):