Fix expected neighbor when multiple ports connect to same neighbor (s…

…onic-net#1162)


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

Signed-off-by: Guohan Lu <lguohan@gmail.com>

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 '/'