[VRF]: submit vrf CLI #392 by tylerlinp · Pull Request #558 · sonic-net/sonic-utilities

tylerlinp · 2019-06-19T11:33:20Z

- What I did
Submit VRF CLI according to vrf HLD #392

- How I did it

- How to verify it
Pass the test on nephos lab

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

show ip interfaces
Interface    IPv4 address/mask    Admin/Oper
-----------  -------------------  ------------
Ethernet0    1.1.1.1/24           up/up

show ipv6 interfaces
Interface    IPv6 address/mask                       Admin/Oper
-----------  --------------------------------------  ------------
Ethernet0    fe80::6eec:5aff:fe08:164b%Ethernet0/64  up/up

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

show ip interfaces
Interface    Master    IPv4 address/mask    Admin/Oper
-----------  --------  -------------------  ------------
Ethernet0    Vrf-blue  1.1.1.1/24           up/up

show ipv6 interfaces
Interface    Master    IPv6 address/mask                       Admin/Oper
-----------  --------  --------------------------------------  ------------
Ethernet0    Vrf-blue  fe80::6eec:5aff:fe08:164b%Ethernet0/64  up/up

-->

1. support bind to a new vrf even when had binded or ipaddress 2. do nothing when bind to same vrf or unbind at unbinded status

venkatmahalingam · 2019-09-16T04:59:50Z

show/main.py

 def interfaces():
    """Show interfaces IPv6 address"""
-    header = ['Interface', 'IPv6 address/mask', 'Admin/Oper']
+    header = ['Interface', 'Master', 'IPv6 address/mask', 'Admin/Oper']


Can we have 'VRF' header instead of 'Master'?

I think 'Master' may be more precise, while device name contains Vrf prefix either.

config/main.py

venkatmahalingam · 2019-09-16T05:25:53Z

config/main.py

+
+    table_name = get_interface_table_name(interface_name)
+    if table_name == "":
+        ctx.fail("'interface_name' is not valid. Valid names [Ethernet/PortChannel/Vlan/Loopback]")


Do we need to add any support for mgmt intf bind to mgmt VRF (if not present already) as part of this pull request or that should be handled separately?

Now mgmt intf vrf has its own CLI.

venkatmahalingam · 2019-09-16T05:28:08Z

config/main.py

+    state_db = SonicV2Connector(host='127.0.0.1')
+    state_db.connect(state_db.STATE_DB, False)
+    _hash = '{}{}'.format('INTERFACE_TABLE|', interface_name)
+    while state_db.get(state_db.STATE_DB, _hash, "state") == "ok":


Why do we need this handling? dont we handle this in VRF-mgrd?

In intfmgrd. It is a good question. The codes here is to assure DEL op has been handled, or else the new SET op will overwite DEL op and then we have to handle complex intf-move-vrf in intfsorch.

config/main.py

venkatmahalingam · 2019-09-16T05:49:56Z

config/main.py

+#
+# 'vrf' subgroup ('config interface vrf ...')
+#
+


Hope we delete the dependent IP, VRF configs upon deleting the portchannel and Vlan...etc? or we expect the user the clean-up the dependent configs before deleting the interface?

Deleting dependent configs implicitly can help user to simplify operation, actually it makes CLI implementation more complex.

prsunny · 2019-10-08T19:00:01Z

@tylerlinp , can you rebase to latest code. MGMT VRF PR is merged, so request to resolve conflicts.

tylerlinp · 2019-10-12T08:12:06Z

retest this please

shine4chen · 2019-10-12T14:36:51Z

retest this please

tylerlinp · 2019-10-15T02:57:47Z

retest this please

keboliu · 2019-10-25T07:44:37Z

config/main.py

-                config_db.set_entry("INTERFACE", interface_name, {"NULL": "NULL"})
-        elif interface_name == 'eth0':
+
+        if interface_name == 'eth0':


suggest to don't hardcode here and below

It has nothing to do with this PR, it is just merged from master. If to change, a new PR is needed. I think mgmt intf should be handled in another command.

prsunny · 2019-11-06T21:23:54Z

@tylerlinp , can you please resolve conflict?

tylerlinp · 2019-11-07T07:14:19Z

retest this please

tylerlinp · 2019-11-07T09:48:50Z

sonic-swss-build/264 build failed and sonic-swss-build/260 not including PR swss/943, so this PR test failed.

prsunny · 2019-11-12T03:09:53Z

retest this please

prsunny · 2019-11-13T18:21:27Z

retest this please

prsunny · 2019-11-13T21:06:36Z

retest this please

prsunny · 2019-11-14T00:05:28Z

@tylerlinp , can you please check the VS test failures

tylerlinp · 2019-11-25T10:02:22Z

retest this please

tylerlinp · 2019-12-03T10:25:56Z

@cony7642 Oh, that's a issue after this PR merged. I ignored same command defined in 2 PRs.
@prsunny My suggestion is change mgmt vrf commands

config vrf add mgmt
config vrf del mgmt

to

config mgmt-vrf add mgmt
config mgmt-vrf del mgmt

and that just like show commands.

Tyler Li added 3 commits June 11, 2019 05:21

Support VRF

72df947

VRF CLI revise interface bind/unbind to vrf

ba45568

1. support bind to a new vrf even when had binded or ipaddress 2. do nothing when bind to same vrf or unbind at unbinded status

Merge branch 'master' of https://github.com/Azure/sonic-utilities

d4fe120

dawnbeauty mentioned this pull request Aug 12, 2019

[tests/vrf] Submit VRF testcases according to vrf test plan sonic-net/sonic-mgmt#1065

Merged

1 task

venkatmahalingam reviewed Sep 16, 2019

View reviewed changes

config/main.py Show resolved Hide resolved

venkatmahalingam reviewed Sep 16, 2019

View reviewed changes

config/main.py Show resolved Hide resolved

venkatmahalingam reviewed Sep 16, 2019

View reviewed changes

Merge https://github.com/Azure/sonic-utilities

0be6eb1

tylerlinp force-pushed the master branch from 62d91c2 to 0be6eb1 Compare September 25, 2019 03:47

Revise according to review comments

96e7fa9

Merge https://github.com/Azure/sonic-utilities

1599f62

keboliu reviewed Oct 25, 2019

View reviewed changes

Merge https://github.com/Azure/sonic-utilities

c7702f8

prsunny approved these changes Nov 26, 2019

View reviewed changes

prsunny merged commit 1b5679c into sonic-net:master Nov 26, 2019

abdosi added the Enhancement label Jan 1, 2020

Conversation

tylerlinp commented Jun 19, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

venkatmahalingam Sep 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

prsunny commented Oct 8, 2019

Uh oh!

tylerlinp commented Oct 12, 2019

Uh oh!

shine4chen commented Oct 12, 2019

Uh oh!

tylerlinp commented Oct 15, 2019

Uh oh!

keboliu Oct 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

prsunny commented Nov 6, 2019

Uh oh!

tylerlinp commented Nov 7, 2019

Uh oh!

tylerlinp commented Nov 7, 2019

Uh oh!

prsunny commented Nov 12, 2019

Uh oh!

prsunny commented Nov 13, 2019

Uh oh!

prsunny commented Nov 13, 2019

Uh oh!

prsunny commented Nov 14, 2019

Uh oh!

tylerlinp commented Nov 25, 2019

Uh oh!

tylerlinp commented Dec 3, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

venkatmahalingam Sep 16, 2019 •

edited

Loading

keboliu Oct 25, 2019 •

edited

Loading