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

[sonic-utilities] managementVRF cli support(l3mdev) #463

Merged
merged 12 commits into from
Oct 4, 2019
136 changes: 134 additions & 2 deletions config/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import json
import subprocess
import netaddr
import logging
import logging.handlers
jleveque marked this conversation as resolved.
Show resolved Hide resolved
import re
import syslog

Expand Down Expand Up @@ -889,6 +891,79 @@ def del_vlan_member(ctx, vid, interface_name):
db.set_entry('VLAN', vlan_name, vlan)
db.set_entry('VLAN_MEMBER', (vlan_name, interface_name), None)

def mvrf_restart_interfaces_config_ntp():
prsunny marked this conversation as resolved.
Show resolved Hide resolved
"""Restart interfaces-config service and NTP service"""
cmd="service ntp stop"
os.system (cmd)
cmd="systemctl restart interfaces-config"
os.system (cmd)
cmd="service ntp start"
os.system (cmd)

prsunny marked this conversation as resolved.
Show resolved Hide resolved
def vrf_add_management_vrf():
"""Enable management vrf"""

config_db = ConfigDBConnector()
config_db.connect()
entry = config_db.get_entry('MGMT_VRF_CONFIG', "vrf_global")
if entry and entry['mgmtVrfEnabled'] == 'true' :
click.echo("ManagementVRF is already Enabled.")
return None
config_db.mod_entry('MGMT_VRF_CONFIG',"vrf_global",{"mgmtVrfEnabled": "true"})
# To move eth0 to management VRF, interfaces-config service has be to be
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move this comments to the below function. This is repeated below as well. Also please follow the multi-line comment format

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually the comments are slightly different for enable and disable conditions. That is why it was provided at this place. Now, the comments are moved inside the function and reworded to suit for both enable and disable conditions. By multi-line, we assume you meant the usage of """, which is also done now.

# restarted. /etc/network/interfaces file is recreated and the networking
# service is restarted which moves the eth0 to management VRF.
# NTP service should also be restarted to rerun the NTP service inside
# mvrf using cgexec. When the NTP service is restarted, service init
# file /etc/init.d/ntp takes care of using cgexec.
mvrf_restart_interfaces_config_ntp()

prsunny marked this conversation as resolved.
Show resolved Hide resolved
def vrf_delete_management_vrf():
"""Disable management vrf"""

config_db = ConfigDBConnector()
config_db.connect()
entry = config_db.get_entry('MGMT_VRF_CONFIG', "vrf_global")
if not entry or entry['mgmtVrfEnabled'] == 'false' :
click.echo("ManagementVRF is already Disabled.")
return None
config_db.mod_entry('MGMT_VRF_CONFIG',"vrf_global",{"mgmtVrfEnabled": "false"})
# To move back eth0 to default VRF, interfaces-config service has be to be
# restarted. /etc/network/interfaces file is recreated and the networking
# service is restarted which moves the eth0 to default VRF.
# NTP service should also be restarted to rerun the NTP service inside
# default VRF without cgexec.
mvrf_restart_interfaces_config_ntp()

#
# 'vrf' group ('config vrf ...')
#

@config.group('vrf')
def vrf():
"""VRF-related configuration tasks"""
pass

@vrf.command('add')
@click.argument('vrfname', metavar='<vrfname>. Type mgmt for management VRF', required=True)
@click.pass_context
def vrf_add (ctx, vrfname):
"""VRF ADD"""
jleveque marked this conversation as resolved.
Show resolved Hide resolved
if vrfname == 'mgmt' or vrfname == 'management':
vrf_add_management_vrf()
else:
click.echo("Creation of data vrf={} is not yet supported".format(vrfname))

@vrf.command('del')
@click.argument('vrfname', metavar='<vrfname>. Type mgmt for management VRF', required=False)
@click.pass_context
def vrf_del (ctx, vrfname):
"""VRF Delete"""
jleveque marked this conversation as resolved.
Show resolved Hide resolved
if vrfname == 'mgmt' or vrfname == 'management':
vrf_delete_management_vrf()
else:
click.echo("Deletion of data vrf={} is not yet supported".format(vrfname))

@vlan.group('dhcp_relay')
@click.pass_context
def vlan_dhcp_relay(ctx):
Expand Down Expand Up @@ -1112,6 +1187,21 @@ def speed(ctx, interface_name, interface_speed, verbose):
command += " -vv"
run_command(command, display_cmd=verbose)

def _get_all_mgmtinterface_keys():
"""Returns list of strings containing mgmt interface keys
"""
config_db = ConfigDBConnector()
config_db.connect()
return config_db.get_table('MGMT_INTERFACE').keys()

jleveque marked this conversation as resolved.
Show resolved Hide resolved

def restart_interfaces_config_ntp_config():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the difference between this and mvrf_restart_interfaces_config_ntp? Can we use the same?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a difference. One is restarting the "ntp-config" service and the other is restarting "ntp" service. "ntp-config" service will regenerate the ntp.conf file, which is required when the IP address for eth0 is modified. Same is not required when VRF is enabled or disabled.

"""Add or remove IP address"""
cmd="systemctl restart interfaces-config"
os.system (cmd)
cmd="systemctl restart ntp-config"
os.system (cmd)

#
# 'ip' subgroup ('config interface ip ...')
#
Expand All @@ -1129,8 +1219,9 @@ def ip(ctx):
@ip.command()
@click.argument('interface_name', metavar='<interface_name>', required=True)
@click.argument("ip_addr", metavar="<ip_addr>", required=True)
@click.argument('gw', metavar='<default gateway IP address>', required=False)
@click.pass_context
def add(ctx, interface_name, ip_addr):
def add(ctx, interface_name, ip_addr, gw):
"""Add an IP address towards the interface"""
config_db = ctx.obj["config_db"]
if get_interface_naming_mode() == "alias":
Expand All @@ -1147,6 +1238,38 @@ def add(ctx, interface_name, ip_addr):
else:
config_db.set_entry("INTERFACE", (interface_name, ip_addr), {"NULL": "NULL"})
config_db.set_entry("INTERFACE", interface_name, {"NULL": "NULL"})
elif interface_name == 'eth0':

# Configuring more than 1 IPv4 or more than 1 IPv6 address fails.
# Allow only one IPv4 and only one IPv6 address to be configured for IPv6.
# If a row already exist, overwrite it (by doing delete and add).
mgmtintf_key_list = _get_all_mgmtinterface_keys()

for key in mgmtintf_key_list:
# For loop runs for max 2 rows, once for IPv4 and once for IPv6.
# No need to capture the exception since the ip_addr is already validated earlier
ip_input = ipaddress.ip_interface(ip_addr)
current_ip = ip = ipaddress.ip_interface(key[1])
if (ip_input.version == 6) and (current_ip.version == 6):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (ip_input.version == current_ip.version) will cover both cases right?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it will cover. Changed.

# If user has configured IPv6 address and the already available row is also IPv6, delete it here.
config_db.set_entry("MGMT_INTERFACE", ("eth0", key[1]), None)
elif (ip_input.version != 6) and (current_ip.version != 6):
# If user has configured IPv4 address and the already available row is also IPv4, delete it here.
config_db.set_entry("MGMT_INTERFACE", ("eth0", key[1]), None)

# Set the new row with new value
if not gw:
config_db.set_entry("MGMT_INTERFACE", (interface_name, ip_addr), {"NULL": "NULL"})
else:
config_db.set_entry("MGMT_INTERFACE", (interface_name, ip_addr), {"gwaddr": gw})
# Restart the interfaces-config service which regenerates the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned, don't have to repeat this comments. You can move it to the function.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

# /etc/network/interfaces file and restarts the networking service
# to make the new IP address effective for eth0.
# "ntp-config" service should also be restarted based on the new
# eth0 IP address since the ntp.conf (generated from ntp.conf.j2) is
# made to listen on specific eth0 IP address.
restart_interfaces_config_ntp_config()

elif interface_name.startswith("PortChannel"):
if VLAN_SUB_INTERFACE_SEPARATOR in interface_name:
config_db.set_entry("VLAN_SUB_INTERFACE", interface_name, {"admin_status": "up"})
Expand Down Expand Up @@ -1190,6 +1313,15 @@ def remove(ctx, interface_name, ip_addr):
else:
config_db.set_entry("INTERFACE", (interface_name, ip_addr), None)
if_table = "INTERFACE"
elif interface_name == 'eth0':
config_db.set_entry("MGMT_INTERFACE", (interface_name, ip_addr), None)
# Restart the interfaces-config service which regenerates the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove comments

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

# /etc/network/interfaces file and restarts the networking service
# to remove it from the system.
# "ntp-config" service should also be restarted since the ntp.conf
# (generated from ntp.conf.j2) had earlier been made to listen on
# previously configured eth0 IP address. Reset it back.
restart_interfaces_config_ntp_config()
elif interface_name.startswith("PortChannel"):
if VLAN_SUB_INTERFACE_SEPARATOR in interface_name:
config_db.set_entry("VLAN_SUB_INTERFACE", (interface_name, ip_addr), None)
Expand All @@ -1206,7 +1338,7 @@ def remove(ctx, interface_name, ip_addr):
ctx.fail("'interface_name' is not valid. Valid names [Ethernet/PortChannel/Vlan/Loopback]")
except ValueError:
ctx.fail("'ip_addr' is not valid.")

exists = False
if if_table:
interfaces = config_db.get_table(if_table)
Expand Down
70 changes: 70 additions & 0 deletions show/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,76 @@ def ndp(ip6address, iface, verbose):

run_command(cmd, display_cmd=verbose)

def is_mgmt_vrf_enabled(ctx):
"""Check if management VRF is enabled"""
if ctx.invoked_subcommand is None:
cmd = 'sonic-cfggen -d --var-json "MGMT_VRF_CONFIG"'

p = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
res = p.communicate()
if p.returncode == 0 :
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra space after 0

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

p = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
mvrf_dict = json.loads(p.stdout.read())

# if the mgmtVrfEnabled attribute is configured, check the value
# and print Enabled or Disabled accordingly.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want to rephrase the comment here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

if 'mgmtVrfEnabled' in mvrf_dict['vrf_global']:
if (mvrf_dict['vrf_global']['mgmtVrfEnabled'] == "true"):
#ManagementVRF is enabled. Return True.
return True
return False

#
# 'mgmt-vrf' group ("show mgmt-vrf ...")
#

@cli.group('mgmt-vrf', invoke_without_command=True)
@click.argument('routes', required=False)
@click.pass_context
def mgmt_vrf(ctx,routes):
"""Show management VRF attributes"""

prsunny marked this conversation as resolved.
Show resolved Hide resolved
if is_mgmt_vrf_enabled(ctx) is False:
click.echo("\nKVSK:ManagementVRF : Disabled")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

KVSK??

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

return
else:
if routes is None:
click.echo("\nManagementVRF : Enabled")
click.echo("\nManagement VRF interfaces in Linux:")
cmd = "ip -d link show mgmt"
run_command(cmd)
cmd = "ip link show vrf mgmt"
run_command(cmd)
else:
click.echo("\nRoutes in Management VRF Routing Table:")
cmd = "ip route show table 5000"
run_command(cmd)

#
# 'management_interface' group ("show management_interface ...")
#

@cli.group(cls=AliasedGroup, default_if_no_args=False)
def management_interface():
"""Show management interface parameters"""
pass

# 'address' subcommand ("show management_interface address")
@management_interface.command()
def address ():
"""Show IP address configured for management interface"""

config_db = ConfigDBConnector()
config_db.connect()
header = ['IFNAME', 'IP Address', 'PrefixLen',]
body = []

# Fetching data from config_db for MGMT_INTERFACE
mgmt_ip_data = config_db.get_table('MGMT_INTERFACE')
for key in natsorted(mgmt_ip_data.keys()):
click.echo("Management IP address = {0}".format(key[1]))
click.echo("Management NetWork Default Gateway = {0}".format(mgmt_ip_data[key]['gwaddr']))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Network

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


#
# 'interfaces' group ("show interfaces ...")
#
Expand Down