-
Notifications
You must be signed in to change notification settings - Fork 661
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.
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
Add check to not allow deleting PO if its member of vlan. #2141
Conversation
Added check to not allow deleting PO if its member of vlan.
Fixes issue #10388 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add UT for coverage?
I am working on adding UT coverage |
@anilkpan - Any update for this PR ? |
@dprital, I plan to add the tests by the end of the week. |
added test coverage
@anilkpan Can you please review the pipeline failures? |
@dgsudharsan, I am looking at the failures. |
PortChannel creation was successful, but it says portchannel doesn't exist when trying to add to vlan. Not sure how to fix this.
E assert 2 == 0 tests/portchannel_test.py:197: AssertionError 2 Error: PortChannel1005 does not exist |
Were you able to debug further? I believe you need to have portchannel as a member here https://github.com/Azure/sonic-utilities/blob/6a327adf6b55faed8c4f6ddfb8bde6f80efbef2d/tests/mock_tables/config_db.json. The config commands here may not modify the DB. |
@dgsudharsan, Thanks. I will try this. |
* Added check to not allow deleting PO if its member of vlan.
@dgsudharsan this PR cannot be cherry-picked cleanly to 202205. please open PR if needed in 202205. |
@anilkpan Can you please raise a backport PR to 202205 branch? |
@dgsudharsan, will create the PR this week. |
@dgsudharsan , @anilkpan do you have ETA for the 202205 PR? |
@yxieca, I plan to create the PR by Monday. |
Created PR #2237 |
@dgsudharsan, yes it was added by mistake, will remove it. |
What I did
Added check to not allow deleting PortChannel if its member of vlan.
How I did it
Added a check in the command script.
How to verify it
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)
Error: PortChannel1 has vlan Vlan100 configured, remove vlan membership to proceed