[Bug]: srv6/test_srv6_basic_sanity.py failed due to KeyError

[ZhaohuiS]

Issue Description

srv6/test_srv6_basic_sanity.py failed due to KeyError

Results you see

=================================== FAILURES ===================================
_________________________ test_interface_on_each_node __________________________

duthosts = [<MultiAsicSonicHost bjw2-can-8102-6>]
rand_one_dut_hostname = 'bjw2-can-8102-6'
nbrhosts = {'ARISTA01T1': <EosHost VM85115>, 'ARISTA02T1': <EosHost VM85116>, 'ARISTA03T1': <EosHost VM85117>, 'ARISTA04T1': <EosHost VM85118>}

    def test_interface_on_each_node(duthosts, rand_one_dut_hostname, nbrhosts):
        for vm_name in test_vm_names:
>           nbrhost = nbrhosts[vm_name]['host']
E           KeyError: 'PE1'

duthosts   = [<MultiAsicSonicHost bjw2-can-8102-6>]
nbrhosts   = {'ARISTA01T1': <EosHost VM85115>, 'ARISTA02T1': <EosHost VM85116>, 'ARISTA03T1': <EosHost VM85117>, 'ARISTA04T1': <EosHost VM85118>}
rand_one_dut_hostname = 'bjw2-can-8102-6'
vm_name    = 'PE1'

srv6/test_srv6_basic_sanity.py:81: KeyError

Results you expected to see

This case should be passed

Is it platform specific

generic

Relevant log output

srv6.test_srv6_basic_sanity.test_interface_on_each_node | failure | KeyError: 'PE1'
srv6.test_srv6_basic_sanity.test_check_bgp_neighbors | failure | KeyError: 'PE3'

Output of show version

No response

Attach files (if any)

No response

[ZhaohuiS]

#15564

To skip it for non cisco vs topologies.
This script should not run on any topologies.

pytest.mark.topology("any"),

[yxieca]

@yejianquan @r12f is this the best solution? Why didn't the test case come in and limited to applicable platform only?

@ZhaohuiS I think we should have a standard check list for new test cases, when it goes in, it should have clear limited scope for the platform it has been tested on. And expand to other platforms after validation?

[ZhaohuiS]

@yxieca Ye, totally agree. Let me see how to do this kind of standard check in future.

[ZhaohuiS]

Skip it in #15564

[ZhaohuiS]

@yxieca #16418 add reminder for submitter to pay attention on supported platforms.
Sync with Xin, we also need standardization for PR in sonic-mgmt repo, not only for PR contributor but also for reviewer.

[BYGX-wcr]

@eddieruan-alibaba , can you please take a look?

[LARLSN]

This test case only runs on topology vms-kvm-ciscovs-7nodes, uses Cisco vsonic with hwsku cisco-8101-p4-32x100-vs, and vsonic image needs to build from phoenixwing branch. How did you triggered this test ? And why do you need to run this test?

[ZhaohuiS]

@BYGX-wcr I think it is caused by the pytestmark in script. If it's specific for some topology, please don't use any in pytestmark.

pytestmark = [
pytest.mark.disable_loganalyzer, # Disable automatic loganalyzer, since we use it for the test
pytest.mark.topology("any"),
pytest.mark.skip_check_dut_health
]

[BYGX-wcr]

@BYGX-wcr I think it is caused by the pytestmark in script. If it's specific for some topology, please don't use any in pytestmark.

pytestmark = [ pytest.mark.disable_loganalyzer, # Disable automatic loganalyzer, since we use it for the test pytest.mark.topology("any"), pytest.mark.skip_check_dut_health ]

I understand.

Hi @LARLSN , are you part of Alibaba's SONiC team? Please fix this issue and treat the topology marker seriously.

[eddieruan-alibaba]

@BYGX-wcr I assume you just need us to add supported topology list in stead of "any" here, right?

https://github.com/sonic-net/sonic-mgmt/blob/master/tests/srv6/test_srv6_basic_sanity.py#L34

@LARLSN is from Alibaba SONiC team. You could assign this issue to her.

[BYGX-wcr]

Hi @eddieruan-alibaba,

Yes, please use another topo marker instead of "any". Looks like your topology is a bit special. There may not be an existing topo marker for your testcase, so you probably need to define a customized one.

@BYGX-wcr I assume you just need us to add supported topology list in stead of "any" here, right?

https://github.com/sonic-net/sonic-mgmt/blob/master/tests/srv6/test_srv6_basic_sanity.py#L34

@LARLSN is from Alibaba SONiC team. You could assign this issue to her.

[LARLSN]

Hi @eddieruan-alibaba,

Yes, please use another topo marker instead of "any". Looks like your topology is a bit special. There may not be an existing topo marker for your testcase, so you probably need to define a customized one.

@BYGX-wcr I assume you just need us to add supported topology list in stead of "any" here, right?
https://github.com/sonic-net/sonic-mgmt/blob/master/tests/srv6/test_srv6_basic_sanity.py#L34
@LARLSN is from Alibaba SONiC team. You could assign this issue to her.

Hi @BYGX-wcr , I'v added topology marker in this test and added our marker in topo_name_to_type func, I'm not sure if there is anywhere else we need to add our custom marker, could you please help us confirm that, this is pr : #16545, thanks

[BYGX-wcr]

Hi @eddieruan-alibaba,
Yes, please use another topo marker instead of "any". Looks like your topology is a bit special. There may not be an existing topo marker for your testcase, so you probably need to define a customized one.

@BYGX-wcr I assume you just need us to add supported topology list in stead of "any" here, right?
https://github.com/sonic-net/sonic-mgmt/blob/master/tests/srv6/test_srv6_basic_sanity.py#L34
@LARLSN is from Alibaba SONiC team. You could assign this issue to her.

Hi @BYGX-wcr , I'v added topology marker in this test and added our marker in topo_name_to_type func, I'm not sure if there is anywhere else we need to add our custom marker, could you please help us confirm that, this is pr : #16545, thanks

This should be fine. @ZhaohuiS , may you take a look?