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.

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

[VRF] Update vrf add, del commands for duplicate/non-existing VRFs #2467

Merged
merged 1 commit into from
Nov 29, 2022

Conversation

mdanish-kh
Copy link
Contributor

@mdanish-kh mdanish-kh commented Nov 1, 2022

Signed-off-by: Muhammad Danish [email protected]

What I did

  • Throw an error in case user wants to add a duplicate VRF.
  • Throw an error if user wants to delete a VRF that does not exist.
  • Update is_vrf_exists function to use vrf_name == management as a valid vrf name as well.
  • Update grammar for vrf_name is invalid message.
  • Renamed tests/show_vrf_test.py -> tests/vrf_test.py to correctly represent the file contents.
  • Add test cases for the added/modified lines.

The motivation is to make the VRF commands consistent with other SONiC CLI commands e.g., VLAN commands that throw an error in case the user tries to add a duplicate VLAN or tries to remove a VLAN that doesn't exist. VRF bind also throws an error if you try to bind to a non-existing VRF. This PR aims to add the same behavior in VRF add and del commands for consistency and to prevent unnecessary function calls.

How I did it

  • Added ctx.fail() messages.

How to verify it

All the tests pass locally.

mdanish@745742a118f8:/sonic/src/sonic-utilities$ pytest-3 -vv tests/vrf_test.py
==================================================================== test session starts =====================================================================
platform linux -- Python 3.9.2, pytest-6.0.2, py-1.10.0, pluggy-0.13.0 -- /usr/bin/python3
cachedir: .pytest_cache
rootdir: /sonic/src/sonic-utilities/tests, configfile: pytest.ini
plugins: pyfakefs-5.0.0, cov-2.10.1
collected 4 items                                                                                                                                            

tests/vrf_test.py::TestShowVrf::test_vrf_show PASSED                                                                                                   [ 25%]
tests/vrf_test.py::TestShowVrf::test_vrf_bind_unbind PASSED                                                                                            [ 50%]
tests/vrf_test.py::TestShowVrf::test_vrf_add_del PASSED                                                                                                [ 75%]
tests/vrf_test.py::TestShowVrf::test_invalid_vrf_name PASSED                                                                                           [100%]

===================================================================== 4 passed in 0.38s ======================================================================
mdanish@745742a118f8:/sonic/src/sonic-utilities$ 

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

CASE 1
admin@sonic:~$ sudo config vrf add Vrf1
admin@sonic:~$ sudo config vrf add Vrf1
CASE 2
admin@sonic:~$ show vrf
VRF    Interfaces
-----  ------------
Vrf1
admin@sonic:~$ sudo config vrf del Vrf1
VRF Vrf1 deleted and all associated IP addresses removed.
admin@sonic:~$ sudo config vrf del Vrf1
VRF Vrf1 deleted and all associated IP addresses removed.

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

CASE 1 (MODIFIED)
admin@sonic:~$ sudo config vrf add Vrf1
admin@sonic:~$ sudo config vrf add Vrf1
Usage: config vrf add [OPTIONS] <vrf_name>
Try "config vrf add -h" for help.

Error: VRF Vrf1 already exists!
admin@sonic:~$ 
CASE 2 (MODIFIED)
admin@sonic:~$ show vrf
VRF    Interfaces
-----  ------------
Vrf1
admin@sonic:~$ sudo config vrf del Vrf1
VRF Vrf1 deleted and all associated IP addresses removed.
admin@sonic:~$ sudo config vrf del Vrf1
Usage: config vrf del [OPTIONS] <vrf_name>
Try "config vrf del -h" for help.

Error: VRF Vrf1 does not exist!

@mdanish-kh mdanish-kh changed the title Update vrf add, del commands for duplicate/non-existing VRFs [VRF] Update vrf add, del commands for duplicate/non-existing VRFs Nov 5, 2022
@mdanish-kh
Copy link
Contributor Author

@prsunny @dgsudharsan @qiluo-msft Can someone kindly take a look please?

@mdanish-kh
Copy link
Contributor Author

@prsunny Can you please sign this off?

@prsunny prsunny merged commit 263810b into sonic-net:master Nov 29, 2022
@mdanish-kh mdanish-kh deleted the vrf-add-del-update branch November 29, 2022 14:53
preetham-singh pushed a commit to preetham-singh/sonic-utilities that referenced this pull request Dec 6, 2022
…et#2467)

*[VRF] Update vrf add, del commands for duplicate/non-existing VRFs sonic-net#2467

*What I did
Throw an error in case user wants to add a duplicate VRF.
Throw an error if user wants to delete a VRF that does not exist.
Update is_vrf_exists function to use vrf_name == management as a valid vrf name as well.
Update grammar for vrf_name is invalid message.
Renamed tests/show_vrf_test.py -> tests/vrf_test.py to correctly represent the file contents.
Add test cases for the added/modified lines.
dprital added a commit to dprital/sonic-buildimage that referenced this pull request Dec 6, 2022
Update sonic-utilities submodule pointer to include the following:
* ca9a020 [generate_dump] [Mellanox] Fix the duplicate dfw dump collection problem by adding symlinks ([sonic-net#2536](sonic-net/sonic-utilities#2536))
* 92c7001 [config] Add check in config interface ip command to block if the interface is portchannel member ([sonic-net#2539](sonic-net/sonic-utilities#2539))
* e8130f5 [system-health] Improve code structure of system health CLIs ([sonic-net#2453](sonic-net/sonic-utilities#2453))
* 00c01b3 Transceiver eeprom dom CLI modification to show output from TRANSCEIVER_DOM_THRESHOLD table ([sonic-net#2535](sonic-net/sonic-utilities#2535))
* 42f51c2 sonic-utilities: Update config reload() to verify formatting of an input file ([sonic-net#2529](sonic-net/sonic-utilities#2529))
* a5e1e2b [GCU] Add RemoveCreateOnlyDependency Validator/Generator ([sonic-net#2500](sonic-net/sonic-utilities#2500))
* 6411b52 [QoS] Introduce delay to the qos reload flow ([sonic-net#2503](sonic-net/sonic-utilities#2503))
* fce7ec3 Use github code scanning instead of LGTM ([sonic-net#2530](sonic-net/sonic-utilities#2530))
* 91bd6de Change show kube command default value of insecure key to True ([sonic-net#2517](sonic-net/sonic-utilities#2517))
* c44c584 Add db_migrator_constants.py script to setup.py ([sonic-net#2534](sonic-net/sonic-utilities#2534))
* 6a3238e [drop counters] Fix CLI script for unconfigured PGs ([sonic-net#2518](sonic-net/sonic-utilities#2518))
* 263810b Update vrf add, del commands for duplicate/non-existing VRFs ([sonic-net#2467](sonic-net/sonic-utilities#2467))
* addae73 Port 202012 DB migration changes to newer branches ([sonic-net#2515](sonic-net/sonic-utilities#2515))
* 2af8cfa [VXLAN]Fixing traceback in show remotemac when mac moves during command execution ([sonic-net#2506](sonic-net/sonic-utilities#2506))

Signed-off-by: dprital <[email protected]>
StormLiangMS pushed a commit that referenced this pull request Dec 30, 2022
*[VRF] Update vrf add, del commands for duplicate/non-existing VRFs #2467

*What I did
Throw an error in case user wants to add a duplicate VRF.
Throw an error if user wants to delete a VRF that does not exist.
Update is_vrf_exists function to use vrf_name == management as a valid vrf name as well.
Update grammar for vrf_name is invalid message.
Renamed tests/show_vrf_test.py -> tests/vrf_test.py to correctly represent the file contents.
Add test cases for the added/modified lines.
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.

3 participants