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

Fix validation of input of config mirror_session add #2162

Merged
merged 1 commit into from
May 12, 2022
Merged

Fix validation of input of config mirror_session add #2162

merged 1 commit into from
May 12, 2022

Conversation

bingwang-ms
Copy link
Contributor

Signed-off-by: bingwang wang.bing@microsoft.com

What I did

FIX sonic-net/sonic-buildimage#10795

queue = 0 is not accepted by the validation of input because the follow check
https://github.com/Azure/sonic-utilities/blob/bce4694d3ad4454fc7181b99433457e76ab9e0a0/config/main.py#L2028-L2029

When addressing the issue, I saw similar issue in the logic of handling gre_type. The issue is also fixed.

How I did it

Change if queue to if queue is not None

How to verify it

Verified by UT

python setup.py test --addopts "tests/config_mirror_session_test.py -v" 
========================================================================================= test session starts =========================================================================================
collected 3 items                                                                                                                                                                                     

tests/config_mirror_session_test.py::test_mirror_session_add PASSED                                                                                                                             [ 33%]
tests/config_mirror_session_test.py::test_mirror_session_erspan_add PASSED                                                                                                                      [ 66%]
tests/config_mirror_session_test.py::test_mirror_session_span_add PASSED                                                                                                                        [100%]

========================================================================================== 3 passed in 0.96s ==========================================================================================

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)

@bingwang-ms
Accept 0 for queue and dscp
1196558 
Signed-off-by: bingwang <wang.bing@microsoft.com>
@bingwang-ms bingwang-ms mentioned this pull request May 11, 2022
Closed
@bingwang-ms bingwang-ms merged commit fac1769 into sonic-net:master May 12, 2022
@yozhao101 yozhao101 mentioned this pull request May 12, 2022
Closed
judyjoseph pushed a commit that referenced this pull request May 16, 2022
@bingwang-ms @judyjoseph
Accept 0 for queue and dscp (#2162)
0225195 
Signed-off-by: bingwang <wang.bing@microsoft.com>
@qiluo-msft
Copy link
Contributor

@bingwang-ms In future when you merge, please make sure copy PR title to commit title.

@qiluo-msft
Copy link
Contributor

This commit could not be cleanly cherry-picked to 202012. Please submit another PR.

dprital added a commit to dprital/sonic-buildimage that referenced this pull request May 25, 2022
@dprital
[submodule] Update sonic-utilities submodule
657dd81 
Update sonic-utilities submodule pointer to include the following:
* [GCU] Handling type1 lists ([sonic-net#2171](sonic-net/sonic-utilities#2171))
* [yang] extend ConfigMgmt constructor to pass YANG options ([sonic-net#2118](sonic-net/sonic-utilities#2118))
* [dump] implement ACL modules ([sonic-net#2153](sonic-net/sonic-utilities#2153))
* show commands for SYSTEM READY ([sonic-net#1851](sonic-net/sonic-utilities#1851))
* [GCU] Handling non-compliant leaf-list with string values ([sonic-net#2174](sonic-net/sonic-utilities#2174))
* Add sonic-delayed.target to Application Extension .timer file generator ([sonic-net#2176](sonic-net/sonic-utilities#2176))
* [portconfig] Allow to configure interface mtu for physical ports ([#l](https://github.com/Azure/sonic-utilities/pull/l))
* Broadcast Unknown-multicast and Unknown-unicast Storm-control  ([sonic-net#928](sonic-net/sonic-utilities#928))
* sonic-utils: initial support for link-training ([sonic-net#2071](sonic-net/sonic-utilities#2071))
* [portchannel] Added ACL/PBH binding checks to the port before getting added to portchannel ([sonic-net#2151](sonic-net/sonic-utilities#2151))
* Modify override testcase to cover PORT admin_status ([sonic-net#2165](sonic-net/sonic-utilities#2165))
* [GCU] Validate peer_group_range ip_range are correct ([sonic-net#2145](sonic-net/sonic-utilities#2145))
* [auto-ts] add memory check ([sonic-net#2116](sonic-net/sonic-utilities#2116))
* support new interface types CR8/SR8/KR8/LR8 which are brougnt by SAI V.1.10.2 ([sonic-net#2167](sonic-net/sonic-utilities#2167))
* [scripts/fast-reboot] Add option to include ssd-upgrader-part boot option with SONiC partition ([sonic-net#2150](sonic-net/sonic-utilities#2150))
* [config reload] Fix invalid rstrip. ([sonic-net#2157](sonic-net/sonic-utilities#2157))
* Accept 0 for queue and dscp ([sonic-net#2162](sonic-net/sonic-utilities#2162))

Signed-off-by: dprital <drorp@nvidia.com>
@dprital dprital mentioned this pull request May 25, 2022
Closed
6 tasks
@dprital
Copy link
Collaborator

dprital commented May 25, 2022

@qiluo-msft - Can you please cherry pick this PR to 202012 ?

@qiluo-msft
Copy link
Contributor

Check my above comment.

@dprital
Copy link
Collaborator

dprital commented May 25, 2022

Check my above comment.

Thanks.
@bingwang-ms - Can you please ?

@bingwang-ms
Copy link
Contributor Author

Check my above comment.

Thanks. @bingwang-ms - Can you please ?

Sure, will file another PR.

@bingwang-ms bingwang-ms mentioned this pull request May 26, 2022
Merged
@bingwang-ms
Copy link
Contributor Author

cherry-pick PR created https://github.com/Azure/sonic-utilities/pull/2187/files

This was referenced Jun 9, 2022
Closed
Closed
@yxieca
Copy link
Contributor

yxieca commented Nov 10, 2022

@bingwang-ms looks like there is merge conflict in 202205 as well. Can you open an PR for 202205?

@bingwang-ms bingwang-ms mentioned this pull request Nov 11, 2022
Merged
@bingwang-ms
Copy link
Contributor Author

@bingwang-ms looks like there is merge conflict in 202205 as well. Can you open an PR for 202205?

Sure, opened another PR for 202205 branch #2494

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

@arfeigin arfeigin arfeigin approved these changes

@lolyu lolyu lolyu approved these changes

Assignees
No one assigned
Labels
Cherry Pick Conflict_202205 Included in 202111 Branch Included in 202205 Branch Request for 202111 Branch Request for 202205 Branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[mirroring] Queue value 0 is missing after validation in config mirror-session command
7 participants
@bingwang-ms @qiluo-msft @dprital @yxieca @lolyu @arfeigin @judyjoseph