Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support 2 vlans test in test_acl dynamically #16551

Merged
merged 2 commits into from
Jan 20, 2025

Conversation

ZhaohuiS
Copy link
Contributor

@ZhaohuiS ZhaohuiS commented Jan 16, 2025

Description of PR

Summary:
Fixes # (issue)
#16322 can only run test for the first vlan even t0 has 2 vlan config by default, which is not expected.
We expects to run 2 vlans seperately for all acl cases.

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • New Test case
    • Skipped for non-supported platforms
  • Test case improvement

Back port request

  • 202012
  • 202205
  • 202305
  • 202311
  • 202405
  • 202411

Approach

What is the motivation for this PR?

In pytest_generate_tests hook, depends on how many vlan interfaces in topo file, it parametrizes vlan_name dynamically before testing.
It doesn't need to use hardcode to check the topo name then set vlan_name. It's not flexible.

How did you do it?

In this PR, Add t0-118 topo which has 2 vlan config by default.

In test_acl, will choose vlan ip range and rule id separately.
If default_vlan_config == two_vlan_a, the IP range should choose DOWNSTREAM_DST_IP_VLAN and DOWNSTREAM_DST_IP_VLAN2000 separately for each vlan.
And also chooses different rule id in case test_dest_ip_match_forwarded and test_dest_ip_match_dropped for 2 vlan config.

How did you verify/test it?

verified on two vlan testbed on 202405 image, 384 all passed

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
-------------------------------------------------------------------------------------------------- live log sessionfinish ---------------------------------------------------------------------------------------------------
12:46:19 __init__.pytest_terminal_summary         L0067 INFO   | Can not get Allure report URL. Please check logs
=============================================================================== 384 passed, 1216 skipped, 1545 warnings in 4036.92s (1:07:16) ===============================================================================
DEBUG:tests.conftest:[log_custom_msg] item: <Function test_icmp_match_forwarded[ipv6-ingress-uplink->downlink-default-Vlan2000]>

Also verified on one vlan testbed on 202405 image, 192 all passed

================================================================================ 192 passed, 608 skipped, 777 warnings in 2099.37s (0:34:59) ================================================================================
DEBUG:tests.conftest:[log_custom_msg] item: <Function test_icmp_match_forwarded[ipv6-ingress-uplink->downlink-default-Vlan1000]>
DEBUG:tests.conftest:append custom_msg: {'dut_check_result': {'core_dump_check_pass': True, 'config_db_check_pass': False}}
INFO:root:Can not get Allure report URL. Please check logs
zhaohuisun@sonic-mgmt-zhaohuisun:/data/sonic-mgmt-int/tests$ 

Any platform specific information?

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

Documentation

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ZhaohuiS ZhaohuiS requested a review from yaqiangz January 16, 2025 12:02
@ZhaohuiS ZhaohuiS changed the title Add t0-118 topo and support its 2 vlans test in test_acl Add t0-118 topo and support its 2 vlans test in test_acl dynamically Jan 16, 2025
@ZhaohuiS ZhaohuiS changed the title Add t0-118 topo and support its 2 vlans test in test_acl dynamically Add t0-118 topo and support 2 vlans test in test_acl dynamically Jan 16, 2025
@ZhaohuiS ZhaohuiS changed the title Add t0-118 topo and support 2 vlans test in test_acl dynamically support 2 vlans test in test_acl dynamically Jan 16, 2025
@ZhaohuiS ZhaohuiS removed request for wangxin and yxieca January 16, 2025 12:53
Copy link
Contributor

@yaqiangz yaqiangz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ZhaohuiS ZhaohuiS requested a review from StormLiangMS January 17, 2025 09:31
Copy link
Collaborator

@StormLiangMS StormLiangMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mssonicbld
Copy link
Collaborator

@ZhaohuiS PR conflicts with 202405 branch

@mssonicbld
Copy link
Collaborator

@ZhaohuiS PR conflicts with 202311 branch

@mssonicbld
Copy link
Collaborator

@ZhaohuiS PR conflicts with 202411 branch

@mssonicbld
Copy link
Collaborator

@ZhaohuiS PR conflicts with 202305 branch

@StormLiangMS
Copy link
Collaborator

@ZhaohuiS all cherry pick conflicted, could you help to take a look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment