Merge pull request #2981 from anarkiwi/hwtestrollback

Revert "Merge pull request #2957 from lantz/random-ports" due to complex breakage

clib/clib_mininet_test_main.py

@@ -201,13 +201,13 @@ def check_dependencies():
     return True
 
 
-def make_suite(tc_class, hw_config, root_tmpdir, ports_sock, max_test_load, port_base):
+def make_suite(tc_class, hw_config, root_tmpdir, ports_sock, max_test_load):
     """Compose test suite based on test class names."""
     testloader = unittest.TestLoader()
     testnames = testloader.getTestCaseNames(tc_class)
     suite = unittest.TestSuite()
     for name in testnames:
-        suite.addTest(tc_class(name, hw_config, root_tmpdir, ports_sock, max_test_load, port_base))
+        suite.addTest(tc_class(name, hw_config, root_tmpdir, ports_sock, max_test_load))
     return suite
 
 
@@ -361,7 +361,7 @@ def max_loadavg():
 
 
 def expand_tests(module, requested_test_classes, excluded_test_classes,
-                 hw_config, root_tmpdir, ports_sock, port_base, serial):
+                 hw_config, root_tmpdir, ports_sock, serial):
     sanity_test_suites = []
     single_test_suites = []
     parallel_test_suites = []
@@ -379,7 +379,7 @@ def expand_tests(module, requested_test_classes, excluded_test_classes,
                 continue
             print('adding test %s' % test_name)
             test_suite = make_suite(
-                test_obj, hw_config, root_tmpdir, ports_sock, max_loadavg(), port_base)
+                test_obj, hw_config, root_tmpdir, ports_sock, max_loadavg())
             if test_name.startswith('FaucetSanity'):
                 sanity_test_suites.append(test_suite)
             else:
@@ -585,8 +585,7 @@ def clean_test_dirs(root_tmpdir, all_successful, sanity, keep_logs, dumpfail):
 
 
 def run_tests(module, hw_config, requested_test_classes, dumpfail,
-              keep_logs, port_base, serial, excluded_test_classes,
-              report_json_filename):
+              keep_logs, serial, excluded_test_classes, report_json_filename):
     """Actually run the test suites, potentially in parallel."""
     if hw_config is not None:
         print('Testing hardware, forcing test serialization')
@@ -602,7 +601,7 @@ def run_tests(module, hw_config, requested_test_classes, dumpfail,
     print('test ports server started')
     sanity_tests, single_tests, parallel_tests = expand_tests(
         module, requested_test_classes, excluded_test_classes,
-        hw_config, root_tmpdir, ports_sock, port_base, serial)
+        hw_config, root_tmpdir, ports_sock, serial)
     resultclass = FaucetCleanupResult
     if keep_logs:
         resultclass = FaucetResult
@@ -638,9 +637,6 @@ def parse_args():
         '-k', '--keep_logs', action='store_true', help='keep logs even for OK tests')
     parser.add_argument(
         '-n', '--nocheck', action='store_true', help='skip dependency check')
-    parser.add_argument(
-        '-p', '--ports', type=str, default='random',
-        help='OvS ports or starting port [1,2,3,4 | 1 | random] (random)')
     parser.add_argument(
         '-i', '--integration', default=True, action='store_true', help='run integration tests')
     parser.add_argument(
@@ -655,9 +651,7 @@ def parse_args():
 
     try:
         args, requested_test_classes = parser.parse_known_args(sys.argv[1:])
-        if args.ports != 'random':
-            args.ports = [int(x) for x in args.ports.split(',')]
-    except(KeyError, IndexError, ValueError):
+    except(KeyError, IndexError):
         parser.print_usage()
         sys.exit(-1)
 
@@ -667,16 +661,17 @@ def parse_args():
         excluded_test_classes = args.x.split(',')
     return (
         requested_test_classes, args.clean, args.dumpfail,
-        args.keep_logs, args.nocheck, args.ports, args.serial,
+        args.keep_logs, args.nocheck, args.serial,
         excluded_test_classes, report_json_filename)
 
 
 def test_main(module):
     """Test main."""
     setLogLevel('error')
     print('testing module %s' % module)
-    (requested_test_classes, clean, dumpfail, keep_logs, nocheck, port_base,
+    (requested_test_classes, clean, dumpfail, keep_logs, nocheck,
      serial, excluded_test_classes, report_json_filename) = parse_args()
+
     if clean:
         print('Cleaning up test interfaces, processes and openvswitch '
               'configuration from previous test runs')
@@ -690,11 +685,6 @@ def test_main(module):
                   'list in header of this script')
             sys.exit(-1)
     hw_config = import_hw_config()
-    if port_base == 'random':
-        # A random range to allocate OvS ports from
-        port_base = random.sample(range(1, 1024), 16)
-        print("random port base: -p", ",".join(str(x) for x in port_base))
     run_tests(
         module, hw_config, requested_test_classes, dumpfail,
-        keep_logs, port_base, serial, excluded_test_classes, report_json_filename,
-    )
+        keep_logs, serial, excluded_test_classes, report_json_filename)

clib/clib_mininet_tests.py

@@ -49,7 +49,6 @@ def setUp(self):
             n_extended=self.N_EXTENDED, e_cls=self.EXTENDED_CLS,
             tmpdir=self.tmpdir, links_per_host=self.LINKS_PER_HOST,
             hw_dpid=self.hw_dpid)
-        self.port_map = self.create_port_map(self.dpid)
         self.start_net()
 
     def test_ping_all(self):

clib/mininet_test_base.py

@@ -32,7 +32,6 @@
 
 from clib import mininet_test_util
 from clib import mininet_test_topo
-from clib.mininet_test_topo import FaucetSwitchTopo
 from clib.tcpdump_helper import TcpdumpHelper
 
 MAX_TEST_VID = 512
@@ -120,13 +119,14 @@ class FaucetTestBase(unittest.TestCase):
     event_sock = None
     faucet_config_path = None
 
-    def __init__(self, name, config, root_tmpdir, ports_sock, max_test_load, port_base=None):
+    def __init__(self, name, config, root_tmpdir, ports_sock, max_test_load):
         super(FaucetTestBase, self).__init__(name)
         self.config = config
         self.root_tmpdir = root_tmpdir
         self.ports_sock = ports_sock
         self.max_test_load = max_test_load
-        self.port_base = port_base
+        start_port = mininet_test_topo.SWITCH_START_PORT
+        self.port_map = {'port_%u' % (port + 1): port + start_port for port in range(4)}
 
     def rand_dpid(self):
         reserved_range = 100
@@ -331,8 +331,7 @@ def _stop_net(self):
     def setUp(self):
         self.tmpdir = self._tmpdir_name()
         self._set_static_vars()
-        self.topo_class = partial(
-            mininet_test_topo.FaucetSwitchTopo, port_base=self.port_base)
+        self.topo_class = mininet_test_topo.FaucetSwitchTopo
         if self.hw_switch:
             self.hw_dpid = mininet_test_util.str_int_dpid(self.dpid)
             self.dpid = self.hw_dpid
@@ -423,8 +422,6 @@ def _attach_physical_switch(self):
 
     def start_net(self):
         """Start Mininet network."""
-        if not self.port_map:
-            self.port_map = self.create_port_map(self.dpid)
         controller_intf = 'lo'
         controller_ipv6 = False
         if self.hw_switch:
@@ -467,12 +464,6 @@ def _start_check(self):
                             port, port_name)
         return self._start_gauge_check()
 
-    def create_port_map(self, dpid):
-        """Return a port map {'port_1': port...} for a dpid in self.topo"""
-        ports = self.topo.dpid_ports(dpid)
-        port_map = {'port_%d' % (i+1): port for i, port in enumerate(ports)}
-        return port_map
-
     def _start_faucet(self, controller_intf, controller_ipv6):
         last_error_txt = ''
         assert self.net is None # _start_faucet() can't be multiply called

clib/mininet_test_topo.py

@@ -1,6 +1,5 @@
 """Topology components for FAUCET Mininet unit tests."""
 
-from collections import defaultdict
 import os
 import socket
 import string
@@ -154,62 +153,28 @@ def _add_extended_host(self, sid_prefix, host_n, e_cls, tmpdir):
     def _add_faucet_switch(self, sid_prefix, dpid, ovs_type, switch_cls):
         """Add a FAUCET switch."""
         switch_name = 's%s' % sid_prefix
-        self._dpid_names[dpid] = switch_name
         return self.addSwitch(
             name=switch_name,
             cls=switch_cls,
             datapath=ovs_type,
             dpid=mininet_test_util.mininet_dpid(dpid))
 
-    @staticmethod
-    def port_nums(count, ports=None, start_port=SWITCH_START_PORT):
-        """Return switch port numbers as a tuple of at least count entries;
-           ports = [1, 2, 3...] | [] (range starting from start_port)"""
-        assert ports
-        if not ports:
-            ports = [SWITCH_START_PORT]
-        ports = tuple(ports)
-        length, next_port = len(ports), max(ports) + 1
-        if length < count:
-            ports += tuple(range(next_port, next_port + count - length))
-        return ports
-
-    def _add_links(self, switch, hosts, links_per_host, port_base=None):
-        ports = self.port_nums(len(hosts)*links_per_host, port_base)
-        i = 0
+    def _add_links(self, switch, hosts, links_per_host, start_port):
+        port = start_port
         for host in hosts:
-            for _ in range(links_per_host):
+            for i in range(links_per_host):
                 # Order of switch/host is important, since host may be in a container.
-                self.addLink(switch, host, port1=ports[i], delay=self.DELAY, use_htb=True)
-                i += 1
-        # Next port to use if needed
-        return max(ports) + 1
-
-    def addLink(self, src, dst, **kwargs):
-        """Keep track of switch ports in order"""
-        result = super().addLink(src, dst, **kwargs)
-        sport, dport = kwargs.get('port1'), kwargs.get('port2')
-        for node, port  in ((src, sport), (dst, dport)):
-            if self.isSwitch(node) and port:
-                self._switch_ports[node].append(port)
-        return result
-
-    def dpid_ports(self, dpid):
-        """Return port list for dpid"""
-        switch = self._dpid_names[dpid]
-        return self._switch_ports[switch]
-
-    def __init__(self, *args, **kwargs):
-        self._switch_ports = defaultdict(list)  # ports in order for each switch
-        self._dpid_names = {}  # maps dpids to switch names
-        super().__init__(*args, **kwargs)
+                self.addLink(switch, host, port1=port, delay=self.DELAY, use_htb=True)
+                port += 1
+        return port
 
     def build(self, ovs_type, ports_sock, test_name, dpids,
               n_tagged=0, tagged_vid=100, n_untagged=0, links_per_host=0,
               n_extended=0, e_cls=None, tmpdir=None, hw_dpid=None,
-              host_namespace=None, port_base=None):
+              host_namespace=None, start_port=SWITCH_START_PORT):
         if not host_namespace:
             host_namespace = {}
+
         for dpid in dpids:
             serialno = mininet_test_util.get_serialno(
                 ports_sock, test_name)
@@ -228,7 +193,7 @@ def build(self, ovs_type, ports_sock, test_name, dpids,
                 dpid = remap_dpid
                 switch_cls = NoControllerFaucetSwitch
             switch = self._add_faucet_switch(sid_prefix, dpid, ovs_type, switch_cls)
-            self._add_links(switch, self.hosts(), links_per_host, port_base)
+            self._add_links(switch, self.hosts(), links_per_host, start_port)
 
 
 class FaucetStringOfDPSwitchTopo(FaucetSwitchTopo):
@@ -239,7 +204,7 @@ class FaucetStringOfDPSwitchTopo(FaucetSwitchTopo):
     def build(self, ovs_type, ports_sock, test_name, dpids,
               n_tagged=0, tagged_vid=100, n_untagged=0,
               links_per_host=0, switch_to_switch_links=1,
-              hw_dpid=None, stack_ring=False, port_base=None):
+              hw_dpid=None, stack_ring=False, start_port=SWITCH_START_PORT):
         """
 
                                Hosts
@@ -270,6 +235,7 @@ def addLinks(src, dst, switch_ports): # pylint: disable=invalid-name
                 self.addLink(src, dst, port1=switch_ports[src], port2=switch_ports[dst])
                 switch_ports[src] += 1
                 switch_ports[dst] += 1
+
         first_switch = None
         last_switch = None
         self.switch_to_switch_links = switch_to_switch_links
@@ -293,7 +259,7 @@ def addLinks(src, dst, switch_ports): # pylint: disable=invalid-name
             switch = self._add_faucet_switch(sid_prefix, dpid, ovs_type, switch_cls)
             if first_switch is None:
                 first_switch = switch
-            switch_ports[switch] = self._add_links(switch, hosts, links_per_host, port_base)
+            switch_ports[switch] = self._add_links(switch, hosts, links_per_host, start_port)
             if last_switch is not None:
                 # Add a switch-to-switch link with the previous switch,
                 # if this isn't the first switch in the topology.