Fix expected neighbor when multiple ports connect to same neighbor (sonic-net#1162)

Previous code only enumerate on distinct neighbors, thus only one
port is shown for a neighbor.

Signed-off-by: Guohan Lu <[email protected]>

Author: lguohan

show/interfaces/__init__.py

@@ -249,25 +249,29 @@ def expected(db, interfacename):
     header = ['LocalPort', 'Neighbor', 'NeighborPort', 'NeighborLoopback', 'NeighborMgmt', 'NeighborType']
     body = []
     if interfacename:
-        for device in natsorted(neighbor_metadata_dict.keys()):
-            if device2interface_dict[device]['localPort'] == interfacename:
-                body.append([device2interface_dict[device]['localPort'],
-                             device,
-                             device2interface_dict[device]['neighborPort'],
-                             neighbor_metadata_dict[device]['lo_addr'],
-                             neighbor_metadata_dict[device]['mgmt_addr'],
-                             neighbor_metadata_dict[device]['type']])
-        if len(body) == 0:
-            click.echo("No neighbor information available for interface {}".format(interfacename))
-            return
-    else:
-        for device in natsorted(neighbor_metadata_dict.keys()):
-            body.append([device2interface_dict[device]['localPort'],
+        try:
+            device = neighbor_dict[interfacename]['name']
+            body.append([interfacename,
                          device,
-                         device2interface_dict[device]['neighborPort'],
+                         neighbor_dict[interfacename]['port'],
                          neighbor_metadata_dict[device]['lo_addr'],
                          neighbor_metadata_dict[device]['mgmt_addr'],
                          neighbor_metadata_dict[device]['type']])
+        except KeyError:
+            click.echo("No neighbor information available for interface {}".format(interfacename))
+            return
+    else:
+        for port in natsorted(neighbor_dict.keys()):
+            try:
+                device = neighbor_dict[port]['name']
+                body.append([port,
+                             device,
+                             neighbor_dict[port]['port'],
+                             neighbor_metadata_dict[device]['lo_addr'],
+                             neighbor_metadata_dict[device]['mgmt_addr'],
+                             neighbor_metadata_dict[device]['type']])
+            except KeyError:
+                pass
 
     click.echo(tabulate(body, header))
 
@@ -387,7 +391,7 @@ def errors(verbose, period, namespace, display):
     cmd = "portstat -e"
     if period is not None:
         cmd += " -p {}".format(period)
-    
+
     cmd += " -s {}".format(display)
     if namespace is not None:
         cmd += " -n {}".format(namespace)

tests/conftest.py

@@ -70,6 +70,11 @@ def setup_single_broacom_asic():
     config.asic_type = mock.MagicMock(return_value="broadcom")
     config._get_device_type = mock.MagicMock(return_value="ToRRouter")
 
+@pytest.fixture
+def setup_t1_topo():
+    mock_tables.dbconnector.topo = "t1"
+    yield
+    mock_tables.dbconnector.topo = None
 
 @pytest.fixture
 def setup_single_bgp_instance(request):

tests/interfaces_test.py

@@ -57,18 +57,67 @@
 Ethernet124  ARISTA04T1  Ethernet1       None                10.250.0.54     LeafRouter
 """
 
+show_interfaces_neighbor_expected_output_t1="""\
+LocalPort    Neighbor    NeighborPort    NeighborLoopback    NeighborMgmt    NeighborType
+-----------  ----------  --------------  ------------------  --------------  --------------
+Ethernet0    ARISTA01T2  Ethernet1       None                172.16.137.56   SpineRouter
+Ethernet4    ARISTA01T2  Ethernet2       None                172.16.137.56   SpineRouter
+Ethernet8    ARISTA03T2  Ethernet1       None                172.16.137.57   SpineRouter
+Ethernet12   ARISTA03T2  Ethernet2       None                172.16.137.57   SpineRouter
+Ethernet16   ARISTA05T2  Ethernet1       None                172.16.137.58   SpineRouter
+Ethernet20   ARISTA05T2  Ethernet2       None                172.16.137.58   SpineRouter
+Ethernet24   ARISTA07T2  Ethernet1       None                172.16.137.59   SpineRouter
+Ethernet28   ARISTA07T2  Ethernet2       None                172.16.137.59   SpineRouter
+Ethernet32   ARISTA09T2  Ethernet1       None                172.16.137.60   SpineRouter
+Ethernet36   ARISTA09T2  Ethernet2       None                172.16.137.60   SpineRouter
+Ethernet40   ARISTA11T2  Ethernet1       None                172.16.137.61   SpineRouter
+Ethernet44   ARISTA11T2  Ethernet2       None                172.16.137.61   SpineRouter
+Ethernet48   ARISTA13T2  Ethernet1       None                172.16.137.62   SpineRouter
+Ethernet52   ARISTA13T2  Ethernet2       None                172.16.137.62   SpineRouter
+Ethernet56   ARISTA15T2  Ethernet1       None                172.16.137.63   SpineRouter
+Ethernet60   ARISTA15T2  Ethernet2       None                172.16.137.63   SpineRouter
+Ethernet64   ARISTA01T0  Ethernet1       None                172.16.137.64   ToRRouter
+Ethernet68   ARISTA02T0  Ethernet1       None                172.16.137.65   ToRRouter
+Ethernet72   ARISTA03T0  Ethernet1       None                172.16.137.66   ToRRouter
+Ethernet76   ARISTA04T0  Ethernet1       None                172.16.137.67   ToRRouter
+Ethernet80   ARISTA05T0  Ethernet1       None                172.16.137.68   ToRRouter
+Ethernet84   ARISTA06T0  Ethernet1       None                172.16.137.69   ToRRouter
+Ethernet88   ARISTA07T0  Ethernet1       None                172.16.137.70   ToRRouter
+Ethernet92   ARISTA08T0  Ethernet1       None                172.16.137.71   ToRRouter
+Ethernet96   ARISTA09T0  Ethernet1       None                172.16.137.72   ToRRouter
+Ethernet100  ARISTA10T0  Ethernet1       None                172.16.137.73   ToRRouter
+Ethernet104  ARISTA11T0  Ethernet1       None                172.16.137.74   ToRRouter
+Ethernet108  ARISTA12T0  Ethernet1       None                172.16.137.75   ToRRouter
+Ethernet112  ARISTA13T0  Ethernet1       None                172.16.137.76   ToRRouter
+Ethernet116  ARISTA14T0  Ethernet1       None                172.16.137.77   ToRRouter
+Ethernet120  ARISTA15T0  Ethernet1       None                172.16.137.78   ToRRouter
+Ethernet124  ARISTA16T0  Ethernet1       None                172.16.137.79   ToRRouter
+"""
+
 show_interfaces_neighbor_expected_output_Ethernet112="""\
 LocalPort    Neighbor    NeighborPort    NeighborLoopback    NeighborMgmt    NeighborType
 -----------  ----------  --------------  ------------------  --------------  --------------
 Ethernet112  ARISTA01T1  Ethernet1       None                10.250.0.51     LeafRouter
 """
 
+show_interfaces_neighbor_expected_output_t1_Ethernet0="""\
+LocalPort    Neighbor    NeighborPort    NeighborLoopback    NeighborMgmt    NeighborType
+-----------  ----------  --------------  ------------------  --------------  --------------
+Ethernet0    ARISTA01T2  Ethernet1       None                172.16.137.56   SpineRouter
+"""
+
 show_interfaces_neighbor_expected_output_etp29="""\
 LocalPort    Neighbor    NeighborPort    NeighborLoopback    NeighborMgmt    NeighborType
 -----------  ----------  --------------  ------------------  --------------  --------------
 etp29        ARISTA01T1  Ethernet1       None                10.250.0.51     LeafRouter
 """
 
+show_interfaces_neighbor_expected_output_t1_Ethernet1_1="""\
+LocalPort    Neighbor    NeighborPort    NeighborLoopback    NeighborMgmt    NeighborType
+-----------  ----------  --------------  ------------------  --------------  --------------
+Ethernet1/1  ARISTA01T2  Ethernet1       None                172.16.137.56   SpineRouter
+"""
+
 show_interfaces_portchannel_output="""\
 Flags: A - active, I - inactive, Up - up, Dw - Down, N/A - not available,
        S - selected, D - deselected, * - not synced
@@ -166,6 +215,15 @@ def test_show_interfaces_neighbor_expected(self):
         assert result.exit_code == 0
         assert result.output == show_interfaces_neighbor_expected_output
 
+    def test_show_interfaces_neighbor_expected_t1(self, setup_t1_topo):
+        runner = CliRunner()
+        result = runner.invoke(show.cli.commands["interfaces"].commands["neighbor"].commands["expected"], [])
+        print(result.exit_code)
+        print(result.output)
+        # traceback.print_tb(result.exc_info[2])
+        assert result.exit_code == 0
+        assert result.output == show_interfaces_neighbor_expected_output_t1
+
     def test_show_interfaces_neighbor_expected_Ethernet112(self):
         runner = CliRunner()
         result = runner.invoke(show.cli.commands["interfaces"].commands["neighbor"].commands["expected"], ["Ethernet112"])
@@ -175,6 +233,15 @@ def test_show_interfaces_neighbor_expected_Ethernet112(self):
         assert result.exit_code == 0
         assert result.output == show_interfaces_neighbor_expected_output_Ethernet112
 
+    def test_show_interfaces_neighbor_expected_t1_Ethernet0(self, setup_t1_topo):
+        runner = CliRunner()
+        result = runner.invoke(show.cli.commands["interfaces"].commands["neighbor"].commands["expected"], ["Ethernet0"])
+        print(result.exit_code)
+        print(result.output)
+        # traceback.print_tb(result.exc_info[2])
+        assert result.exit_code == 0
+        assert result.output == show_interfaces_neighbor_expected_output_t1_Ethernet0
+
     def test_show_interfaces_neighbor_expected_etp29(self):
         runner = CliRunner()
         os.environ['SONIC_CLI_IFACE_MODE'] = "alias"
@@ -186,6 +253,17 @@ def test_show_interfaces_neighbor_expected_etp29(self):
         assert result.exit_code == 0
         assert result.output == show_interfaces_neighbor_expected_output_etp29
 
+    def test_show_interfaces_neighbor_expected_t1_Ethernet1_1(self, setup_t1_topo):
+        runner = CliRunner()
+        os.environ['SONIC_CLI_IFACE_MODE'] = "alias"
+        result = runner.invoke(show.cli.commands["interfaces"].commands["neighbor"].commands["expected"], ["Ethernet1/1"])
+        os.environ['SONIC_CLI_IFACE_MODE'] = "default"
+        print(result.exit_code)
+        print(result.output)
+        # traceback.print_tb(result.exc_info[2])
+        assert result.exit_code == 0
+        assert result.output == show_interfaces_neighbor_expected_output_t1_Ethernet1_1
+
     def test_show_interfaces_neighbor_expected_Ethernet0(self):
         runner = CliRunner()
         result = runner.invoke(show.cli.commands["interfaces"].commands["neighbor"].commands["expected"], ["Ethernet0"])

tests/mock_tables/dbconnector.py

@@ -9,6 +9,8 @@
 from swsssdk import SonicDBConfig, SonicV2Connector
 from swsssdk.interface import redis
 
+topo = None
+
 def clean_up_config():
     # Set SonicDBConfig variables to initial state
     # so that it can be loaded with single or multiple
@@ -20,7 +22,7 @@ def clean_up_config():
 def load_namespace_config():
     # To support multi asic testing
     # SonicDBConfig load_sonic_global_db_config
-    # is invoked to load multiple namespaces 
+    # is invoked to load multiple namespaces
     clean_up_config()
     SonicDBConfig.load_sonic_global_db_config(
         global_db_file_path=os.path.join(
@@ -37,7 +39,8 @@ def load_database_config():
 _old_connect_SonicV2Connector = SonicV2Connector.connect
 
 def connect_SonicV2Connector(self, db_name, retry_on=True):
-
+    # add topo to kwargs for testing different topology
+    self.dbintf.redis_kwargs['topo'] = topo
     # add the namespace to kwargs for testing multi asic
     self.dbintf.redis_kwargs['namespace'] = self.namespace
     # Mock DB filename for unit-test
@@ -79,23 +82,32 @@ def __init__(self, *args, **kwargs):
         super(SwssSyncClient, self).__init__(strict=True, *args, **kwargs)
         # Namespace is added in kwargs specifically for unit-test
         # to identify the file path to load the db json files.
+        topo = kwargs.pop('topo')
         namespace = kwargs.pop('namespace')
         db_name = kwargs.pop('db_name')
         fname = db_name.lower() + ".json"
         self.pubsub = MockPubSub()
-        
+
         if namespace is not None and namespace is not multi_asic.DEFAULT_NAMESPACE:
             fname = os.path.join(INPUT_DIR, namespace, fname)
+        elif topo is not None:
+            fname = os.path.join(INPUT_DIR, topo, fname)
         else:
             fname = os.path.join(INPUT_DIR, fname)
-        
 
         if os.path.exists(fname):
             with open(fname) as f:
                 js = json.load(f)
-                for h, table in js.items():
-                    for k, v in table.items():
-                        self.hset(h, k, v)
+                for k, v in js.items():
+                    if v.has_key('expireat') and v.has_key('ttl') and v.has_key('type') and v.has_key('value'):
+                        # database is in redis-dump format
+                        if v['type'] == 'hash':
+                            # ignore other types for now since sonic has hset keys only in the db
+                            for attr, value in v['value'].items():
+                                self.hset(k, attr, value)
+                    else:
+                        for attr, value in v.items():
+                            self.hset(k, attr, value)
 
     # Patch mockredis/mockredis/client.py
     # The official implementation will filter out keys with a slash '/'