sonic-utilities icon indicating copy to clipboard operation
sonic-utilities copied to clipboard

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

Open mdanish-kh opened this issue 2 years ago • 1 comments

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 avatar Nov 01 '22 19:11 mdanish-kh

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

mdanish-kh avatar Nov 05 '22 11:11 mdanish-kh

@prsunny Can you please sign this off?

mdanish-kh avatar Nov 15 '22 19:11 mdanish-kh