Skip to content

Issue #22420: Modify 'config route add' command not to include empty … #3862

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

anders-nexthop
Copy link

@anders-nexthop anders-nexthop commented Apr 23, 2025

What I did

The config route commands accept a nexthop-vrf as an argument. When that argument is not specified, the commands use the empty string as the default. This violates the yang model, which only accepts vrf names of the form (default, mgmt, or Vrf*). In the case where a nexthop-vrf is not defined, we should fall back to using the same vrf given for the prefix we are adding. If that is not specified, then both prefix and nexthop will use the default vrf.

Fixes: #22420

How I did it

Modify the add_route() and del_route() codepaths from sonic-utilites/config/main.py to set nexthop-vrf to whatever the vrf is for the associated prefix whenever it is not explicitly specified.

How to verify it

All unit tests pass (including all tests in static_route_test.py).

I don't think the output of any show commands changes because of this, but it's possible there are combinations of different VRFs which would show up differently. Before this merges I'll set up some routes and nexthops with different VRFs and make sure any changes there may be are shown here.

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)

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@anders-nexthop anders-nexthop force-pushed the anders.nexthop-vrf-bug branch from bc05ba7 to e06623d Compare April 23, 2025 08:25
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@anders-nexthop anders-nexthop force-pushed the anders.nexthop-vrf-bug branch from e06623d to 87311d7 Compare April 23, 2025 08:52
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@anders-nexthop anders-nexthop marked this pull request as ready for review April 23, 2025 09:04
…elements (sonic-net#12)

'config route add' fills out missing 'nexthop-vrf' elements with the
empty string (''). This is not valid in the yang model; vrf names can
only be 'default', 'mgmt', or 'VRF<somestring>'.
@anders-nexthop anders-nexthop force-pushed the anders.nexthop-vrf-bug branch from 87311d7 to 15f0abc Compare April 23, 2025 16:33
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vmittal-msft vmittal-msft requested a review from prsunny May 7, 2025 15:11
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.

Bug: 'config route add' incorrectly adds an empty string for default vrf
2 participants