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.

Sign up for GitHub

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

Jump to bottom

[show] Fix int status of LAGs, configured as Vlan members #1478

Merged
merged 2 commits into from
Mar 24, 2021
Merged

[show] Fix int status of LAGs, configured as Vlan members #1478

merged 2 commits into from
Mar 24, 2021

Conversation

ghost
Copy link

@ghost ghost commented Mar 4, 2021 •
edited by liat-grozovik
Loading

Pass missed argument to function "appl_db_portchannel_status_get" in "intfutil" script to provide correct information in "Vlan" column of command "show int status" for LAG interfaces.

Signed-off-by: Maksym Belei Maksym_Belei@jabil.com

What I did

Resolves sonic-net/sonic-buildimage#6928
Fixed status of LAG interfaces in Vlan column. LAG interfaces, configured as a Vlan member, should be seen as trunk interface.

How I did it

Missed argument "self.combined_int_to_vlan_po_dict" passed to function "appl_db_portchannel_status_get"
in "intfutil" script.

How to verify it

admin@sonic:~$ show int status | grep PortChannel0011
      Ethernet4          4,5,6,7      N/A   9100    N/A     etp2  PortChannel0011    down       up             N/A         N/A
      Ethernet8        8,9,10,11      N/A   9100    N/A     etp3  PortChannel0011      up       up  QSFP+ or later         N/A
PortChannel0011              N/A        G   9100    N/A      N/A           routed    down       up             N/A         N/A
admin@sonic:~$ sudo config vlan member add 11 PortChannel0011
admin@sonic:~$ show vlan brief | grep 11
|        11 |              | PortChannel0011 | tagged         |                       | disabled    |
admin@sonic:~$ show int status | grep PortChannel0011
      Ethernet4          4,5,6,7      N/A   9100    N/A     etp2  PortChannel0011    down       up             N/A         N/A
      Ethernet8        8,9,10,11      N/A   9100    N/A     etp3  PortChannel0011      up       up  QSFP+ or later         N/A
PortChannel0011              N/A        G   9100    N/A      N/A            trunk    down       up             N/A         N/A
admin@sonic:~$ sudo config vlan member del 11 PortChannel0011
admin@sonic:~$ show vlan brief | grep 11
|        11 |              |         |                |                       | disabled    |
admin@sonic:~$ show int status | grep PortChannel0011
      Ethernet4          4,5,6,7      N/A   9100    N/A     etp2  PortChannel0011    down       up             N/A         N/A
      Ethernet8        8,9,10,11      N/A   9100    N/A     etp3  PortChannel0011      up       up  QSFP+ or later         N/A
PortChannel0011              N/A        G   9100    N/A      N/A           routed    down       up             N/A         N/A

@ghost ghost mentioned this pull request Mar 4, 2021
Closed
prsunny
prsunny previously approved these changes Mar 6, 2021
View reviewed changes
Copy link
Contributor

@prsunny prsunny left a comment

Choose a reason for hiding this comment

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

lgtm

@prsunny prsunny requested a review from qiluo-msft March 6, 2021 00:51
qiluo-msft
qiluo-msft previously approved these changes Mar 6, 2021
View reviewed changes
@lguohan
Copy link
Contributor

lguohan commented Mar 11, 2021

can you update the unit test?

lguohan
lguohan requested changes Mar 11, 2021
View reviewed changes
Copy link
Contributor

@lguohan lguohan left a comment

Choose a reason for hiding this comment

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

need unit test

@ghost ghost dismissed stale reviews from qiluo-msft and prsunny via 0a6169e March 15, 2021 12:14
@ghost ghost force-pushed the fix_lag_int_status branch 2 times, most recently from 0a6169e to 3dc34bb Compare March 15, 2021 13:01
@ghost ghost requested a review from lguohan March 15, 2021 15:47
@ghost
Copy link
Author

ghost commented Mar 15, 2021

@lguohan, I have added changes in unit tests for intfutil to the PR.

qiluo-msft
qiluo-msft previously approved these changes Mar 15, 2021
View reviewed changes
@qiluo-msft
Copy link
Contributor

@maksymbelei95 Is this PR needed on 201911 branch?

@ghost
Copy link
Author

ghost commented Mar 15, 2021

@qiluo-msft, the issue is being occurred on master and 202012 branches, but, did not checked on 201911. I will check and let you know whether the PR is needed on 201911.

@qiluo-msft
Copy link
Contributor

Thanks for checking!

prsunny
prsunny previously approved these changes Mar 16, 2021
View reviewed changes
@ghost
Copy link
Author

ghost commented Mar 16, 2021

@qiluo-msft, regarding 201911. The related line of script intfutil looks right:
https://github.com/Azure/sonic-utilities/blob/201911/scripts/intfutil#L402
I think there is no need to cherry-pick the PR to 201911.

liat-grozovik
liat-grozovik previously approved these changes Mar 16, 2021
View reviewed changes
@qiluo-msft
Copy link
Contributor

@lguohan Could you help review again? Unit test was added.

@ghost
Copy link
Author

ghost commented Mar 23, 2021

@lguohan, could you review the PR?

@prsunny
Copy link
Contributor

prsunny commented Mar 23, 2021

@maksymbelei95 , would you please resolve conflict?

Maksym Belei added 2 commits March 23, 2021 19:19
[show] Fix int status of LAGs, configured as Vlan members
d6e9017 
* Pass missed argument to function "appl_db_portchannel_status_get"
  in "intfutil" script to provide correct information in "Vlan"
  column of command "show int status" for LAG interfaces.

Signed-off-by: Maksym Belei <Maksym_Belei@jabil.com>
[tests] update unit tests for intfutil script
52646d6 
* Updating mocked tables and related unit test
  cases of intfutil to check case, when
  the PortChannel is a member of the Vlan.
  Command 'show int status' should display
  such interface as trunk device, not routed.
* As there are dependencies between test modules,
  updating affected unit test modules. Updated:
  vlan_test.py

Signed-off-by: Maksym Belei <Maksym_Belei@jabil.com>
@ghost ghost dismissed stale reviews from liat-grozovik, prsunny, and qiluo-msft via 52646d6 March 23, 2021 17:50
@ghost ghost force-pushed the fix_lag_int_status branch from 3dc34bb to 52646d6 Compare March 23, 2021 17:50
@ghost
Copy link
Author

ghost commented Mar 23, 2021

@prsunny, done.

@ghost
Copy link
Author

ghost commented Mar 24, 2021

@prsunny, could you merge the PR?

@liat-grozovik liat-grozovik merged commit c7d4947 into sonic-net:master Mar 24, 2021
@liat-grozovik
Copy link
Collaborator

@maksymbelei95 could you please update submodule to get the fix in master?

@ghost
Copy link
Author

ghost commented Mar 24, 2021

@liat-grozovik, created PR with the submodule update: sonic-net/sonic-buildimage#7144

yxieca pushed a commit that referenced this pull request Mar 26, 2021
@yxieca
[show] Fix int status of LAGs, configured as Vlan members (#1478)
2718cd8 
Pass missed argument to function "appl_db_portchannel_status_get" in "intfutil" script to provide correct information in "Vlan" column of command "show int status" for LAG interfaces.

- What I did
Fixed status of LAG interfaces in Vlan column. LAG interfaces, configured as a Vlan member, should be seen as trunk interface.

- How I did it
Missed argument "self.combined_int_to_vlan_po_dict" passed to function "appl_db_portchannel_status_get"
in "intfutil" script.

- How to verify it
admin@sonic:~$ show int status | grep PortChannel0011

Signed-off-by: Maksym Belei <Maksym_Belei@jabil.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Included in 202012 Branch Request for 202012 Branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Trunk LAG interface is listed as routed port in "show interfaces status" output.
5 participants
@lguohan @qiluo-msft @prsunny @liat-grozovik @yxieca