sonic-mgmt icon indicating copy to clipboard operation
sonic-mgmt copied to clipboard

[Enhancement] Test route perf enhancement for backend testbeds

Open developfast opened this issue 1 year ago • 1 comments

Description of PR

Summary: Test enhancement for test_route_perf to accommodate backend testbeds. Fixes # (issue)

Type of change

  • [ ] Bug fix
  • [ ] Testbed and Framework(new/improvement)
  • [X] Test case(new/improvement)

Back port request

  • [ ] 201911
  • [ ] 202012
  • [ ] 202205
  • [X] 202305
  • [X] 202311

Approach

What is the motivation for this PR?

Test enhancement to support backend testbeds. This test currently fails to run on a backend testbed.

How did you do it?

Added valid DUT and PTF port settings in case of backend testbed

How did you verify/test it?

Test passes on both backend testbed and non-backend testbed ========== 2 passed, 71 warnings in 415.82s (0:06:55) ========

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

developfast avatar Apr 15 '24 21:04 developfast

@yxieca can you pls help merge?

developfast avatar Apr 16 '24 03:04 developfast

Hi @developfast @yxieca, this PR is causing failure in test test_duplicate_route.py which is in the PR checker: https://elastictest.org/scheduler/testplan/661f33b8baaf1d86b899274b?testcase=route%2Ftest_duplicate_route.py%7C%7C%7C2&type=console

==================================== ERRORS ====================================
_______ ERROR at setup of test_duplicate_routes[4-Loopback-vlab-01-None] _______

duthosts = [<MultiAsicSonicHost vlab-01>]
enum_rand_one_per_hwsku_frontend_hostname = 'vlab-01'
enum_rand_one_frontend_asic_index = None, ip_versions = 4
interface_types = 'Loopback'

    @pytest.fixture
    def setup_routes(duthosts, enum_rand_one_per_hwsku_frontend_hostname,
                     enum_rand_one_frontend_asic_index, ip_versions, interface_types):
        duthost = duthosts[enum_rand_one_per_hwsku_frontend_hostname]
        cfg_facts = get_cfg_facts(duthost)
        asichost = duthost.asic_instance(enum_rand_one_frontend_asic_index)
        prefixes = []
    
        if interface_types == 'Loopback':
            # Get loopback ips
            intf_ips = get_intf_ips('Loopback', cfg_facts)
            pytest_assert(len(intf_ips) > 0, "No IP configured on Loopback0")
        else:
            # Get vlan ips
            intf_ips = get_intf_ips('Vlan', cfg_facts)
            pytest_assert(len(intf_ips) > 0, "No IP configured on any Vlan")
    
        # Generate interfaces and neighbors
>       intf_neighs, str_intf_nexthop = generate_intf_neigh(
            asichost, 1, ip_versions)
E       TypeError: generate_intf_neigh() missing 2 required positional arguments: 'mg_facts' and 'is_backend_topology'

Could you take a look?

congh-nvidia avatar Apr 17 '24 06:04 congh-nvidia

@yxieca i wonder how come sonic-mgmt pipeline is failing with this PR while the PR was merged probably with no failure

liat-grozovik avatar Apr 17 '24 06:04 liat-grozovik

Hi, I double checked, looks like the test route/test_duplicate_route.py was not running in the checkers of this PR, but it was running in PR https://github.com/sonic-net/sonic-mgmt/pull/12374/

congh-nvidia avatar Apr 17 '24 06:04 congh-nvidia

The route/test_duplicate_route.py is added by this PR https://github.com/sonic-net/sonic-mgmt/pull/12387 by @yutongzhang-microsoft And when 12460 is created and running PR test, 12387 is not merged

yejianquan avatar Apr 17 '24 07:04 yejianquan

But PR test is doing its right thing, if we earlier enable more test modules by #12387, #12460 will be gated out

yejianquan avatar Apr 17 '24 07:04 yejianquan

this is clear. can you please check the failure now or update the test?

liat-grozovik avatar Apr 17 '24 08:04 liat-grozovik

this is clear. can you please check the failure now or update the test?

I checked the failure, #12460 breaks the initialization logic of test_duplicate_route.py Now that #12460 does have a compatible issue, and I believe it will affect our nightly test result. I created a revert PR to resolve the issue https://github.com/sonic-net/sonic-mgmt/pull/12485 so that we can quickly get unblocked. Dev can create another PR to include this change of #12460 and then PR test can help gate out the issue. @developfast Do you agree with this plan?

yejianquan avatar Apr 17 '24 08:04 yejianquan

Sure @yejianquan, will create a new PR - talk about timing lol.

developfast avatar Apr 17 '24 17:04 developfast