Skip to content

Bug: test_bgp_bbr.py check_other_vms doesn't populate results dict, causing test to always pass #21954

@markx-arista

Description

@markx-arista

Is it platform specific

generic

Importance or Severity

HIgh

Description of the bug

#18300 modified the function check_other_vms and doesn't populate results dictionary, and vm_route returned by it is ignored by parallel_run:

     # Check route on other VMs
@@ -378,23 +399,45 @@ def check_other_vms(nbrhosts, setup, route, accepted=True, node=None, results=No
         vm_route['failed'] = False
         vm_route['message'] = 'Checking route {} on {} passed'.format(str(route), node)
         if accepted:
-            if route.prefix not in list(vm_route['vrfs']['default']['bgpRouteEntries'].keys()):
-                vm_route['failed'] = True
-                vm_route['message'] = 'No route for {} found on {}'.format(route.prefix, node)
-            else:
-                tor_route_aspath = vm_route['vrfs']['default']['bgpRouteEntries'][route.prefix]['bgpRoutePaths'][0]\
-                    ['asPathEntry']['asPath']   # noqa:E211
-                # Route path from other VMs: -> DUT(T1) -> TOR1 -> aspath(other T1 -> DUMMY_ASN1)
-                tor_route_aspath_expected = '{} {} {}'.format(dut_asn, tor1_asn, route.aspath)
-                if tor_route_aspath != tor_route_aspath_expected:
+            tor_route_aspath = None
+            if isinstance(nbrhosts[node]['host'], EosHost):
+                if route.prefix not in list(vm_route['vrfs']['default']['bgpRouteEntries'].keys()):
+                    vm_route['failed'] = True
+                    vm_route['message'] = 'No route for {} found on {}'.format(route.prefix, node)
+                else:
+                    tor_route_aspath = vm_route['vrfs']['default']['bgpRouteEntries'][route.prefix]['bgpRoutePaths'][0]\
+                    ['asPathEntry']['asPath']   # noqa E211
+            elif isinstance(nbrhosts[node]['host'], SonicHost):
+                if vm_route and 'paths' in vm_route:
+                    tor_route_aspath = vm_route['paths'][0]['aspath']['string']
+                else:
                     vm_route['failed'] = True
-                    vm_route['message'] = 'On {} expected aspath: {}, actual aspath: {}'\
-                        .format(node, tor_route_aspath_expected, tor_route_aspath)
+                    vm_route['message'] = 'No route for {} found on {}'.format(route.prefix, node)
+            else:
+                vm_route['failed'] = True
+                logging.error('Unknown host type {} for {}'.format(type(nbrhosts[node]['host']), node))
+                return
+
+            # Route path from other VMs: -> DUT(T1) -> TOR1 -> aspath(other T1 -> DUMMY_ASN1)
+            tor_route_aspath_expected = '{} {} {}'.format(dut_asn, tor1_asn, route.aspath)
+            if tor_route_aspath != tor_route_aspath_expected:
+                vm_route['failed'] = True
+                vm_route['message'] = 'On {} expected aspath: {}, actual aspath: {}'\
+                    .format(node, tor_route_aspath_expected, tor_route_aspath)
         else:
-            if route.prefix in list(vm_route['vrfs']['default']['bgpRouteEntries'].keys()):
+            if isinstance(nbrhosts[node]['host'], EosHost):
+                if route.prefix in list(vm_route['vrfs']['default']['bgpRouteEntries'].keys()):
+                    vm_route['failed'] = True
+                    vm_route['message'] = 'No route {} expected on {}'.format(route.prefix, node)
+            elif isinstance(nbrhosts[node]['host'], SonicHost):
+                if vm_route and 'paths' in vm_route:
+                    vm_route['failed'] = True
+                    vm_route['message'] = 'No route {} expected on {}'.format(route.prefix, node)
+            else:
                 vm_route['failed'] = True
-                vm_route['message'] = 'No route {} expected on {}'.format(route.prefix, node)
-        results[node] = vm_route
+                logging.error('Unknown host type {} for {}'.format(type(nbrhosts[node]['host']), node))
+                return
+        return vm_route

The results dict remains empty after parallel_run, and route validation failures are never detected , so the test always passes:

    results = parallel_run(check_other_vms, (nbrhosts, setup, route), {'accepted': accepted},
                           other_vms, timeout=120, concurrent_tasks=6)

    failed_results = {}
    for node, result in list(results.items()):
        if result['failed']:
            failed_results[node] = result

    pytest_assert(not failed_results, 'Checking route {} failed, failed_results: {}'
                  .format(str(route), json.dumps(failed_results, indent=2)))

Steps to Reproduce

Run the test, print results after parallel_run, it will be empty.

Actual Behavior and Expected Behavior

results dictionary shouldn't be empty and check_other_vms should detect route validation failures.

Relevant log output

Output of show version

Attach files (if any)

No response

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions