Skip to content

Fix test case: generic_config_updater/test_eth_interface.py::test_replace_mtu#5692

Merged
liat-grozovik merged 1 commit intosonic-net:masterfrom
congh-nvidia:generic_config_updater
May 25, 2022
Merged

Fix test case: generic_config_updater/test_eth_interface.py::test_replace_mtu#5692
liat-grozovik merged 1 commit intosonic-net:masterfrom
congh-nvidia:generic_config_updater

Conversation

@congh-nvidia
Copy link
Copy Markdown
Contributor

Description of PR

Summary:
The test_replace_mtu case uses Ethernet0 to do the replacing mtu test. But in some testbeds, Ethernet0 is a member of a port channel. The mtu of port channel member should not be modified directly.

Type of change

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

Back port request

  • 201911
  • 202012

Approach

What is the motivation for this PR?

The test_replace_mtu case uses Ethernet0 to do the replacing mtu test. But in some testbeds, Ethernet0 is a member of a port channel. The mtu of port channel member should not be modified directly(can't config mtu of a port channel member via CLI).
Take a t1-64-lag testbed for example:

admin@r-tigris-26:~$ show interface portchannel
Flags: A - active, I - inactive, Up - up, Dw - Down, N/A - not available,
       S - selected, D - deselected, * - not synced
  No.  Team Dev         Protocol     Ports
-----  ---------------  -----------  ---------------------------
  101  PortChannel101   LACP(A)(Up)  Ethernet68(S)
  102  PortChannel102   LACP(A)(Up)  Ethernet0(S) Ethernet2(S)
  103  PortChannel103   LACP(A)(Up)  Ethernet72(S)
  104  PortChannel104   LACP(A)(Up)  Ethernet74(S)
  105  PortChannel105   LACP(A)(Up)  Ethernet10(S) Ethernet8(S)
  106  PortChannel106   LACP(A)(Up)  Ethernet76(S)
  107  PortChannel107   LACP(A)(Up)  Ethernet78(S)
  108  PortChannel108   LACP(A)(Up)  Ethernet34(S) Ethernet32(S)
  109  PortChannel109   LACP(A)(Up)  Ethernet84(S)
 1010  PortChannel1010  LACP(A)(Up)  Ethernet88(S)
 1011  PortChannel1011  LACP(A)(Up)  Ethernet42(S) Ethernet40(S)
 1012  PortChannel1012  LACP(A)(Up)  Ethernet90(S)
 1013  PortChannel1013  LACP(A)(Up)  Ethernet92(S)
 1014  PortChannel1014  LACP(A)(Up)  Ethernet94(S)
 1015  PortChannel1015  LACP(A)(Up)  Ethernet104(S)
 1016  PortChannel1016  LACP(A)(Up)  Ethernet108(S)
 1017  PortChannel1017  LACP(A)(Up)  Ethernet110(S)
 1018  PortChannel1018  LACP(A)(Up)  Ethernet112(S)
 1019  PortChannel1019  LACP(A)(Up)  Ethernet116(S)
 1020  PortChannel1020  LACP(A)(Up)  Ethernet124(S)
 1021  PortChannel1021  LACP(A)(Up)  Ethernet128(S)
 1022  PortChannel1022  LACP(A)(Up)  Ethernet132(S)
 1023  PortChannel1023  LACP(A)(Up)  Ethernet136(S)
 1024  PortChannel1024  LACP(A)(Up)  Ethernet138(S)
admin@r-tigris-26:~$ sudo config interface mtu Ethernet0 1518
Usage: config interface mtu [OPTIONS] <interface_name> <interface_mtu>
Try "config interface mtu -h" for help.

Error: 'interface_name' is in portchannel!

How did you do it?

  1. Add get_ethernet_port_not_in_portchannel(duthost) to select a ethernet port not in a port channel for the test.
  2. Modify check_interface_status(duthost, field, interface='Ethernet0') to accept a interface name parameter and able to check the specified interface, keeping 'Ethernet0' as the default value for other cases.
  3. Use get_ethernet_port_not_in_portchannel in mtu test to get a port not in a port channel.

How did you verify/test it?

Have run this case on t0-64 and t-64-lag testbeds, all passed.

Any platform specific information?

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

Documentation

…lace_mtu

The case uses Ethernet0 to do the replacing mtu test, but in our community setups, Ethernet0 is a member of a port channel. The mtu of port channel member should not be modified directly.
Add logic to select a ethernet port not in a port channel for the test.
@congh-nvidia congh-nvidia requested a review from a team as a code owner May 20, 2022 04:56
@liat-grozovik
Copy link
Copy Markdown
Collaborator

@yxieca could you please help to review or assing someone to do so?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants