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

Mgmt vrf namespace2 #431

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion config/aaa.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,16 @@ def add(address, timeout, key, auth_type, port, pri, use_mgmt_vrf):
if key is not None:
data['passkey'] = key
if use_mgmt_vrf :
data['vrf'] = "mgmt"
entry = config_db.get_entry('MGMT_VRF_CONFIG',"vrf_global")
if not entry or entry['mgmtVrfEnabled'] == 'false' :
# Either VRF entry does not exist or it is disabled.
# Silenty ignore the --use-mgmt-vrf if VRF is not enabled
data['vrf'] = "None"
else:
data['vrf'] = "mgmt"

else:
data['vrf'] = "None"
config_db.set_entry('TACPLUS_SERVER', address, data)
tacacs.add_command(add)

Expand Down
208 changes: 207 additions & 1 deletion config/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,16 @@
import json
import subprocess
import netaddr
import logging
import logging.handlers
import re
import syslog

import sonic_platform
from swsssdk import ConfigDBConnector
from natsort import natsorted
from minigraph import parse_device_desc_xml
from sonic_snmp_trap_conf import snmptrap_del_old_nat_rule

import aaa
import mlnx
Expand Down Expand Up @@ -735,6 +738,143 @@ def del_vlan_member(ctx, vid, interface_name):
db.set_entry('VLAN_MEMBER', (vlan_name, interface_name), None)


def vrf_add_management_vrf():
"""Enable management vrf"""

cmd = 'sonic-cfggen -d --var-json "MGMT_VRF_CONFIG"'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@pavel-shirshov
pavel-shirshov 11 days ago Contributor
Is it possible to read this address from the config DB?
Why do you read using sonic-cfggen, but write directly to DB?

Yes we can read it from config DB

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@pavel-shirshov : Addressed your comment. Removed cfggen and used config DB Read.

p = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
res = p.communicate()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@pavel-shirshov
pavel-shirshov 11 days ago Contributor
I'd extract subprocess.Popen chunk as a function, otherwise you have a lot of duplicate code

Will change

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@pavel-shirshov : Addressed your comment. Removed cfggen and used config DB Read. Hence, there is no duplicate code for Popen chunk.

if p.returncode == 0 :
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 modify only if it is changed.
if 'mgmtVrfEnabled' in mvrf_dict['vrf_global']:
if (mvrf_dict['vrf_global']['mgmtVrfEnabled'] == "true"):
click.echo("ManagementVRF is already Enabled.")
return None

config_db = ConfigDBConnector()
config_db.connect()
config_db.mod_entry('MGMT_VRF_CONFIG',"vrf_global",{"mgmtVrfEnabled": "true"})


def vrf_delete_management_vrf():
"""Disable management vrf"""

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 :
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 modify only if it is changed.
if 'mgmtVrfEnabled' in mvrf_dict['vrf_global']:
if (mvrf_dict['vrf_global']['mgmtVrfEnabled'] == "false"):
click.echo("ManagementVRF is already Disabled.")
return None
else:
click.echo("ManagementVRF is already Disabled.")
return None

config_db = ConfigDBConnector()
config_db.connect()
config_db.mod_entry('MGMT_VRF_CONFIG',"vrf_global",{"mgmtVrfEnabled": "false"})


#
# '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"""
if vrfname == 'mgmt' or vrfname == 'management':
pavel-shirshov marked this conversation as resolved.
Show resolved Hide resolved
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"""
if vrfname == 'mgmt' or vrfname == 'management':
vrf_delete_management_vrf()
else:
click.echo("Deletion of data vrf={} is not yet supported".format(vrfname))


@config.command('clear_mgmt')
@click.pass_context
def clear_mgmt(ctx):
MGMT_TABLE_NAMES = [
'MGMT_INTERFACE',
'MGMT_VRF_CONFIG']
config_db = ConfigDBConnector()
config_db.connect()
for mgmt_table in MGMT_TABLE_NAMES:
config_db.delete_table(mgmt_table)

@config.group()
def snmptrap():
"""SNMP Traps Related Task"""
pass

@snmptrap.command('modify')
@click.argument('ver', metavar='<SNMP Version>', type=click.Choice(['1', '2', '3']), required=True)
@click.argument('serverip', metavar='<SNMP TRAP SERVER IP Address>', required=True)
@click.argument('trapport', metavar='<SNMP Trap Server port, default 162>', required=False)
@click.option('-m', '--use-mgmt-vrf', help="Management vrf, default is no vrf", is_flag=True)
@click.pass_context
def modify_snmptrap_server(ctx, ver, serverip, trapport, use_mgmt_vrf):
"""Add the SNMP Trap server configuration"""
if not trapport:
trapport = 162

config_db = ConfigDBConnector()
config_db.connect()
if use_mgmt_vrf :
entry = config_db.get_entry('MGMT_VRF_CONFIG', "vrf_global")
if entry == {}:
click.echo('Cannot set SNMPTrap server using --use-mgmt-vrf when management VRF is not yet configured.')
return
if entry['mgmtVrfEnabled'] == "false" :
click.echo('Cannot set SNMPTrap server using --use-mgmt-vrf when management VRF is not yet enabled.')
return
snmptrap_del_old_nat_rule (ver, serverip, trapport, use_mgmt_vrf)
if ver == "1":
if use_mgmt_vrf:
config_db.mod_entry('SNMP_TRAP_CONFIG',"v1TrapDest",{"DestIp": serverip, "DestPort": trapport, "vrf": "mgmt"})
else:
config_db.set_entry('SNMP_TRAP_CONFIG',"v1TrapDest",{"DestIp": serverip, "DestPort": trapport})
elif ver == "2":
if use_mgmt_vrf:
config_db.mod_entry('SNMP_TRAP_CONFIG',"v2TrapDest",{"DestIp": serverip, "DestPort": trapport, "vrf":"mgmt"})
else:
config_db.set_entry('SNMP_TRAP_CONFIG',"v2TrapDest",{"DestIp": serverip, "DestPort": trapport})
else:
if use_mgmt_vrf:
config_db.mod_entry('SNMP_TRAP_CONFIG',"v3TrapDest",{"DestIp": serverip, "DestPort": trapport, "vrf":"mgmt"})
else:
config_db.set_entry('SNMP_TRAP_CONFIG',"v3TrapDest",{"DestIp": serverip, "DestPort": trapport})
cmd="systemctl restart snmp"
os.system (cmd)



#
# 'bgp' group ('config bgp ...')
#
Expand Down Expand Up @@ -860,6 +1000,42 @@ def speed(ctx, 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()

def is_address_in_network(network, address):
"""
Determine whether the provided address is within a network range.

:param network (str): CIDR presentation format. For example,
'192.168.1.0/24'.
:param address: An individual IPv4 or IPv6 address without a net
mask or subnet prefix. For example, '192.168.1.1'.
:returns boolean: Flag indicating whether address is in network.
"""
try:
network = netaddr.IPNetwork(network)
# There wont be any exception if the IPNetwork is valid
except (netaddr.core.AddrFormatError, ValueError):
raise ValueError("Network (%s) is not in CIDR presentation format" %
network)

try:
address = netaddr.IPAddress(address)
# There wont be any exception if the IPAddress is valid
except (netaddr.core.AddrFormatError, ValueError):
raise ValueError("Address (%s) is not in correct presentation format" %
address)

if address in network:
return True
else:
return False
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@pavel-shirshov
pavel-shirshov 11 days ago Contributor
return address in network ?
This function checks if the address is in network range or not so returning true/false.


#
# 'ip' subgroup ('config interface ip ...')
#
Expand All @@ -876,14 +1052,42 @@ def ip(ctx):

@ip.command()
@click.argument("ip_addr", metavar="<ip_addr>", required=True)
@click.argument('gw1', metavar='<default gateway IP address>', required=False)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@pavel-shirshov
pavel-shirshov 11 days ago Contributor
should we use gw here? Why gw1?

We will change

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@pavel-shirshov : Addressed your comment. Changed gw1 to gw.

@click.pass_context
def add(ctx, ip_addr):
def add(ctx, ip_addr, gw1):
"""Add an IP address towards the interface"""
config_db = ctx.obj["config_db"]
interface_name = ctx.obj["interface_name"]

if interface_name.startswith("Ethernet"):
config_db.set_entry("INTERFACE", (interface_name, ip_addr), {"NULL": "NULL"})
elif interface_name.startswith("eth0"):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@pavel-shirshov
pavel-shirshov 11 days ago Contributor
It could match interface 'eth01'. I think it's better to compare to 'eth' here

Will address this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@pavel-shirshov : Addressed your comment. Changed "startswith eth0", to "exact match for eth0"

# Validate the ip/mask
ipaddress= ip_addr.split("/")
if is_address_in_network(ip_addr, ipaddress[0]) == False:
click.echo ("Its an invalid ip/mask value")
return

# 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.
if ':' in ip_addr and ':' in key[1]:
# 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 ':' not in ip_addr and ':' not in key[1]:
# If user has configured IPv4 address and the already available row is also IPv6, delete it here.
config_db.set_entry("MGMT_INTERFACE", ("eth0", key[1]), None)

# Set the new row with new value
if not gw1:
config_db.set_entry("MGMT_INTERFACE", (interface_name, ip_addr), {"NULL": "NULL"})
else:
config_db.set_entry("MGMT_INTERFACE", (interface_name, ip_addr), {"gwaddr": gw1})

elif interface_name.startswith("PortChannel"):
config_db.set_entry("PORTCHANNEL_INTERFACE", (interface_name, ip_addr), {"NULL": "NULL"})
elif interface_name.startswith("Vlan"):
Expand All @@ -903,6 +1107,8 @@ def remove(ctx, ip_addr):

if interface_name.startswith("Ethernet"):
config_db.set_entry("INTERFACE", (interface_name, ip_addr), None)
elif interface_name.startswith("eth0"):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@pavel-shirshov
pavel-shirshov 11 days ago Contributor
interface_name ==

Will address this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@pavel-shirshov : Addressed your comment. Changed "startswith eth0", to "exact match for eth0"

config_db.set_entry("MGMT_INTERFACE", (interface_name, ip_addr), None)
elif interface_name.startswith("PortChannel"):
config_db.set_entry("PORTCHANNEL_INTERFACE", (interface_name, ip_addr), None)
elif interface_name.startswith("Vlan"):
Expand Down
59 changes: 59 additions & 0 deletions config/sonic_snmp_trap_conf.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
#!/usr/bin/python

'''
Motivation: Backend API implementation for manipulation of the SNMP configuration file.

Functionalities:
Add trap recipient host(s) config entries
modifiy/delete a trap recipient host entry
'''

import os
import sys
import click
import json
import subprocess
import fileinput
import netaddr
import syslog
import logging
import logging.handlers
from swsssdk import ConfigDBConnector

SNMP_SERVER_LOCAL_IP="127.100.100.1"


def snmptrap_del_old_nat_rule (ver, hostaddr, port, use_mgmt_vrf):
"""
Function to set snmp-server configuration for the destination to
send the notifications to.
"""
syslog.syslog(syslog.LOG_DEBUG, "SNMP " + \
"mod_cfg() : received : " + \
" snmp version - %s"%(ver) + \
" host address - port %s %s"%(hostaddr, port))
try :
# use_mgmt_vrf is valid only when vrf is enabled .
# If VRF enabled, delete old NAT rule based on old serverip/port and add new NAT rule
if use_mgmt_vrf:
#vrf is enabled. Get the local_port based on version number. For v1 serverIP, local port 62101 is used
# for v2 serverIP, local port 62102 is used. For v3, 62103 is used.
local_port = "6210{0}".format(ver)
# Get the previously configured serverip and delete the corresponding iptables rule before adding the new rule.
TrapVar = "v{0}TrapDest".format(ver)
config_db = ConfigDBConnector()
config_db.connect()
snmp_config=config_db.get_entry('SNMP_TRAP_CONFIG',TrapVar)
if snmp_config:
old_server_ip = snmp_config['DestIp']
old_server_port = snmp_config['DestPort']
cmd = "ip netns exec mgmt iptables -t nat -D PREROUTING -i if1 -p udp -d {0} --dport {1} -j DNAT --to-destination {2}:{3}".format(SNMP_SERVER_LOCAL_IP, local_port, old_server_ip, old_server_port)
syslog.syslog(syslog.LOG_DEBUG, "Deleting iptables rule for old SNMPv{0} Trap Destination Config. Rule={1}".format(ver,cmd))
os.system(cmd)


except :
syslog.syslog(syslog.LOG_ERR, "Exception in modifying the snmp trap destination")
return 1
return 0

Loading