[config] Add support of PortChannels to portconfig

[ghost]

- What I did Resolves https://github.com/Azure/sonic-buildimage/issues/6430 Added support of PortChannels to portconfig script. Added mocking of redis into portconfig script. Added unit tests for subcommand config interface mtu.

- How I did it Added checking of prefix in name of port, passed to the script. If it is EthernetXX, the script uses table name PORT to operate with DB, if it is PortChannelXX, then the script will use table name PORTCHANNEL, otherwise it will throw an exception to notify that port type is wrong.

- How to verify it

1. sudo config portchannel add PortChannel0001
2. sudo config interface mtu PortChannel0001 1510
3. see current MTU of the PortChannel with ip link show PortChannel0001

Test result in case the portchannel has no members:

admin@sonic:~$ ip link show PortChannel0011
83: PortChannel0011: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 9000 qdisc noqueue state DOWN mode DEFAULT group default qlen 1000
    link/ether 1c:34:da:eb:ca:80 brd ff:ff:ff:ff:ff:ff
admin@sonic:~$ show int 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
-----  ---------------  -----------  -------
 0011  PortChannel0011  LACP(A)(Dw)  N/A
admin@sonic:~$ sudo config int mtu PortChannel0011 512
admin@sonic:~$ ip link show PortChannel0011
83: PortChannel0011: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 512 qdisc noqueue state DOWN mode DEFAULT group default qlen 1000
    link/ether 1c:34:da:eb:ca:80 brd ff:ff:ff:ff:ff:ff
admin@sonic:~$ sudo config portchannel member add PortChannel0011 Ethernet12
Usage: config portchannel member add [OPTIONS] <portchannel_name> <port_name>
Try "config portchannel member add -h" for help.

Error: Port MTU of Ethernet12 is different than the PortChannel0011 MTU size
admin@sonic:~$ sudo config int mtu Ethernet12 512
admin@sonic:~$ sudo config portchannel member add PortChannel0011 Ethernet12
admin@sonic:~$ show int 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
-----  ---------------  -----------  -------------
 0011  PortChannel0011  LACP(A)(Dw)  Ethernet12(D)

Test result in case the portchannel has a member:

admin@sonic:~$ show int 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
-----  ---------------  -----------  -------------
 0011  PortChannel0011  LACP(A)(Dw)  Ethernet12(D)
admin@sonic:~$ 
admin@sonic:~$ sudo config int mtu PortChannel0011 9100
admin@sonic:~$ ip link show PortChannel0011
83: PortChannel0011: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 9100 qdisc noqueue state DOWN mode DEFAULT group default qlen 1000
    link/ether 1c:34:da:eb:ca:80 brd ff:ff:ff:ff:ff:ff
admin@sonic:~$ ip link show Ethernet12
87: Ethernet12: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 9100 qdisc pfifo_fast master PortChannel0011 state DOWN mode DEFAULT group default qlen 1000
    link/ether 1c:34:da:eb:ca:80 brd ff:ff:ff:ff:ff:ff

- Previous command output (if the output of a command-line utility has changed)

- New command output (if the output of a command-line utility has changed) if to try to pass some trash as port argument, like this sudo config interface mtu Unknown0002 9100, you will receive error in console output: Invalid port type specified

[anamehra]

Diff looks good, works fine with Ethernet and PortChannel ports, will need this for 201911. Thanks

[gechiang]

@maksymbelei95 What if the port channel although configured but has no member ports in it? Can you try it out and see what is the end result? also, it is a policy that all code changes require unit test code that goes with it. Please include the unit test code for this change. Thanks!

[ghost]

@gechiang

What if the port channel although configured but has no member ports in it?

I have tested the case. Here is my output:

admin@sonic:~$ ip link show PortChannel0011
83: PortChannel0011: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 9000 qdisc noqueue state DOWN mode DEFAULT group default qlen 1000
    link/ether 1c:34:da:eb:ca:80 brd ff:ff:ff:ff:ff:ff
admin@sonic:~$ show int 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
-----  ---------------  -----------  -------
 0011  PortChannel0011  LACP(A)(Dw)  N/A
admin@sonic:~$ sudo config int mtu PortChannel0011 512
admin@sonic:~$ ip link show PortChannel0011
83: PortChannel0011: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 512 qdisc noqueue state DOWN mode DEFAULT group default qlen 1000
    link/ether 1c:34:da:eb:ca:80 brd ff:ff:ff:ff:ff:ff
admin@sonic:~$ sudo config portchannel member add PortChannel0011 Ethernet12
Usage: config portchannel member add [OPTIONS] <portchannel_name> <port_name>
Try "config portchannel member add -h" for help.

Error: Port MTU of Ethernet12 is different than the PortChannel0011 MTU size
admin@sonic:~$ sudo config int mtu Ethernet12 512
admin@sonic:~$ sudo config portchannel member add PortChannel0011 Ethernet12
admin@sonic:~$ show int 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
-----  ---------------  -----------  -------------
 0011  PortChannel0011  LACP(A)(Dw)  Ethernet12(D)

Another case(the port channel has a member):

admin@sonic:~$ show int 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
-----  ---------------  -----------  -------------
 0011  PortChannel0011  LACP(A)(Dw)  Ethernet12(D)
admin@sonic:~$ 
admin@sonic:~$ sudo config int mtu PortChannel0011 9100
admin@sonic:~$ ip link show PortChannel0011
83: PortChannel0011: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 9100 qdisc noqueue state DOWN mode DEFAULT group default qlen 1000
    link/ether 1c:34:da:eb:ca:80 brd ff:ff:ff:ff:ff:ff
admin@sonic:~$ ip link show Ethernet12
87: Ethernet12: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 9100 qdisc pfifo_fast master PortChannel0011 state DOWN mode DEFAULT group default qlen 1000
    link/ether 1c:34:da:eb:ca:80 brd ff:ff:ff:ff:ff:ff

Both cases look good.

also, it is a policy that all code changes require unit test code that goes with it. Please include the unit test code for this change. Thanks!

Sure. But, as I mentioned before in the related ticket, I found out that there are no unit tests, related to config interface commands. I thought it is related to https://github.com/Azure/sonic-utilities/issues/1301, isn't it?

[gechiang]

@maksymbelei95 Thanks for the added result of various config scenario. Please update/add those under "How to verify it" of this PR. As for the testcase, can you please refer to this PR which has both multi-asic support and single asic support for config: https://github.com/Azure/sonic-utilities/pull/1248

Please follow the above PR and add the necessary test cases. Thanks!

[ghost]

retest this please

[ghost]

@gechiang, @judyjoseph, I have added a related unit test cases to the PR. Could I ask you to review them?

[ghost]

@gechiang, @judyjoseph, could you review the PR?

[judyjoseph]

/AzurePipelines run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).

[ghost]

@gechiang, @judyjoseph, I have rebased the PR, unit tests done successfully. Could your check the PR?

[judyjoseph]

@maksymbelei95, Could you please resolve this conflict before we merge it in - thanks.

[gechiang]

@maksymbelei95 It appears that if a port is already a member of portchannel, user can still configure its MTU on its own? If there is a conflict between portchannel configured MTU and that of its member I believe the portchannel will force its value to its member. but after that if another MTU configured to one of its member with conflicting value, I believe it will again becomes a conflict. I think if we decide to support MTU configured on port-channel, we must add check to disallow user from configuring MTU onto the individual LAG members. Also, when the port is being converted to LAG, we should check that if its MTU is at default value and if it is not, we should disallow the user from adding it to portchannel until that is corrected and then let it inherit what is already configured for the LAG itself. We also need to consider what if a member is deleted from port-channel, what would its MTU be? Retain what it inherited from the port-channel? or go back to default MTU? I think all these requires some answer before we should allow MTU configuration for portchannel. Can you work on these so that we have a solid coverage of all these scenarios? Thanks!

[NGorb-jabil]

@gechiang I have checked all cases you mentioned above. User unable to add port into port-channel with mtu different from port-channel mtu, also its impossible to change MTU onto the individual LAG members. If port is deleted from port-channel, it inherits port-channel MTU. Please, could your check last changes ?

[lgtm-com[bot]]

This pull request fixes 1 alert when merging f35327586450378678f7633fd2aae05112b299bd into 9017d99f8dc67883c03a13e6c96c9a3e48d0c7f5 - view on LGTM.com

fixed alerts:

• 1 for Unused import

[lgtm-com[bot]]

This pull request fixes 1 alert when merging cfb23f605a979679cc2b53cd4ca4915352de65ee into 9017d99f8dc67883c03a13e6c96c9a3e48d0c7f5 - view on LGTM.com

fixed alerts:

• 1 for Unused import

[KonstiantynHalushka]

@judyjoseph Please run this PR again for testing

[lgtm-com[bot]]

This pull request fixes 1 alert when merging e063ecabacdf223af155bc79f78914ee02891a48 into 63a525711d1e41ffdda0f4d627b23c0612b3262b - view on LGTM.com

fixed alerts:

• 1 for Unused import

[KonstiantynHalushka]

fixed alert about unused import found by bot is not an error. It is designed for unit tests.

[pshulik]

@judyjoseph Could you please review the changes

[pshulik]

Dear @gechiang and @anamehra. Could you please review the changes

[TafkaMax]

Why is this not yet merged???

show interfaces 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
-----  -------------  -----------  -------
   01  PortChannel01  LACP(A)(Dw)
   02  PortChannel02  LACP(A)(Dw)

sudo config interface mtu PortChannel02 1500
Invalid port PortChannel02

 sudo config portchannel member add PortChannel02 Ethernet0
Usage: config portchannel member add [OPTIONS] <portchannel_name> <port_name>
Try "config portchannel member add -h" for help.

Error: Port MTU of Ethernet0 is different than the PortChannel02 MTU size