[vlan] remove dhcp-relay as dhcp-relay commands will come as a plugin (sonic-net#1378)

- What I did
Remove dhcp relay commands from sonic-utilities. dhcp-relay commands will come as a plugin with dhcp-relay docker installation.
See sonic-net/SONiC#682

- How I did it
Remove dhcp-relay commands from vlan. Make "show vlan brief" command table output extendable.

- How to verify it
Install dhcp-relay docker as app.ext. Verify that "config vlan dhcp-relay" and "show vlan brief" show dhcp data.

Author: stepanblyschak

config/vlan.py

@@ -185,76 +185,3 @@ def del_vlan_member(db, vid, port):
 
     db.cfgdb.set_entry('VLAN_MEMBER', (vlan, port), None)
 
-@vlan.group(cls=clicommon.AbbreviationGroup, name='dhcp_relay')
-def vlan_dhcp_relay():
-    pass
-
-@vlan_dhcp_relay.command('add')
-@click.argument('vid', metavar='<vid>', required=True, type=int)
-@click.argument('dhcp_relay_destination_ip', metavar='<dhcp_relay_destination_ip>', required=True)
-@clicommon.pass_db
-def add_vlan_dhcp_relay_destination(db, vid, dhcp_relay_destination_ip):
-    """ Add a destination IP address to the VLAN's DHCP relay """
-
-    ctx = click.get_current_context()
-
-    if not clicommon.is_ipaddress(dhcp_relay_destination_ip):
-        ctx.fail('{} is invalid IP address'.format(dhcp_relay_destination_ip))
-
-    vlan_name = 'Vlan{}'.format(vid)
-    vlan = db.cfgdb.get_entry('VLAN', vlan_name)
-    if len(vlan) == 0:
-        ctx.fail("{} doesn't exist".format(vlan_name))
-
-    dhcp_relay_dests = vlan.get('dhcp_servers', [])
-    if dhcp_relay_destination_ip in dhcp_relay_dests:
-        click.echo("{} is already a DHCP relay destination for {}".format(dhcp_relay_destination_ip, vlan_name))
-        return
-
-    dhcp_relay_dests.append(dhcp_relay_destination_ip)
-    vlan['dhcp_servers'] = dhcp_relay_dests
-    db.cfgdb.set_entry('VLAN', vlan_name, vlan)
-    click.echo("Added DHCP relay destination address {} to {}".format(dhcp_relay_destination_ip, vlan_name))
-    try:
-        click.echo("Restarting DHCP relay service...")
-        clicommon.run_command("systemctl stop dhcp_relay", display_cmd=False)
-        clicommon.run_command("systemctl reset-failed dhcp_relay", display_cmd=False)
-        clicommon.run_command("systemctl start dhcp_relay", display_cmd=False)
-    except SystemExit as e:
-        ctx.fail("Restart service dhcp_relay failed with error {}".format(e))
-
-@vlan_dhcp_relay.command('del')
-@click.argument('vid', metavar='<vid>', required=True, type=int)
-@click.argument('dhcp_relay_destination_ip', metavar='<dhcp_relay_destination_ip>', required=True)
-@clicommon.pass_db
-def del_vlan_dhcp_relay_destination(db, vid, dhcp_relay_destination_ip):
-    """ Remove a destination IP address from the VLAN's DHCP relay """
-
-    ctx = click.get_current_context()
-
-    if not clicommon.is_ipaddress(dhcp_relay_destination_ip):
-        ctx.fail('{} is invalid IP address'.format(dhcp_relay_destination_ip))
-
-    vlan_name = 'Vlan{}'.format(vid)
-    vlan = db.cfgdb.get_entry('VLAN', vlan_name)
-    if len(vlan) == 0:
-        ctx.fail("{} doesn't exist".format(vlan_name))
-
-    dhcp_relay_dests = vlan.get('dhcp_servers', [])
-    if not dhcp_relay_destination_ip in dhcp_relay_dests:
-        ctx.fail("{} is not a DHCP relay destination for {}".format(dhcp_relay_destination_ip, vlan_name))
-
-    dhcp_relay_dests.remove(dhcp_relay_destination_ip)
-    if len(dhcp_relay_dests) == 0:
-        del vlan['dhcp_servers']
-    else:
-        vlan['dhcp_servers'] = dhcp_relay_dests
-    db.cfgdb.set_entry('VLAN', vlan_name, vlan)
-    click.echo("Removed DHCP relay destination address {} from {}".format(dhcp_relay_destination_ip, vlan_name))
-    try:
-        click.echo("Restarting DHCP relay service...")
-        clicommon.run_command("systemctl stop dhcp_relay", display_cmd=False)
-        clicommon.run_command("systemctl reset-failed dhcp_relay", display_cmd=False)
-        clicommon.run_command("systemctl start dhcp_relay", display_cmd=False)
-    except SystemExit as e:
-        ctx.fail("Restart service dhcp_relay failed with error {}".format(e))

show/vlan.py

@@ -4,105 +4,141 @@
 
 import utilities_common.cli as clicommon
 
+
 @click.group(cls=clicommon.AliasedGroup)
 def vlan():
     """Show VLAN information"""
     pass
 
+
+def get_vlan_id(ctx, vlan):
+    vlan_prefix, vid = vlan.split('Vlan')
+    return vid
+
+
+def get_vlan_ip_address(ctx, vlan):
+    cfg, _ = ctx
+    _, vlan_ip_data, _ = cfg
+    ip_address = ""
+    for key in vlan_ip_data:
+        if not clicommon.is_ip_prefix_in_key(key):
+            continue
+        ifname, address = key
+        if vlan == ifname:
+            ip_address += "\n{}".format(address)
+
+    return ip_address
+
+
+def get_vlan_ports(ctx, vlan):
+    cfg, db = ctx
+    _, _, vlan_ports_data = cfg
+    vlan_ports = []
+    iface_alias_converter = clicommon.InterfaceAliasConverter(db)
+    # Here natsorting is important in relation to another
+    # column which prints port tagging mode.
+    # If we sort both in the same way using same keys
+    # we will result in right order in both columns.
+    # This should be fixed by cli code autogeneration tool
+    # and we won't need this specific approach with
+    # VlanBrief.COLUMNS anymore.
+    for key in natsorted(list(vlan_ports_data.keys())):
+        ports_key, ports_value = key
+        if vlan != ports_key:
+            continue
+
+        if clicommon.get_interface_naming_mode() == "alias":
+            ports_value = iface_alias_converter.name_to_alias(ports_value)
+
+        vlan_ports.append(ports_value)
+
+    return '\n'.join(vlan_ports)
+
+
+def get_vlan_ports_tagging(ctx, vlan):
+    cfg, db = ctx
+    _, _, vlan_ports_data = cfg
+    vlan_ports_tagging = []
+    # Here natsorting is important in relation to another
+    # column which prints vlan ports.
+    # If we sort both in the same way using same keys
+    # we will result in right order in both columns.
+    # This should be fixed by cli code autogeneration tool
+    # and we won't need this specific approach with
+    # VlanBrief.COLUMNS anymore.
+    for key in natsorted(list(vlan_ports_data.keys())):
+        ports_key, ports_value = key
+        if vlan != ports_key:
+            continue
+
+        tagging_value = vlan_ports_data[key]["tagging_mode"]
+        vlan_ports_tagging.append(tagging_value)
+
+    return '\n'.join(vlan_ports_tagging)
+
+
+def get_proxy_arp(ctx, vlan):
+    cfg, _ = ctx
+    _, vlan_ip_data, _ = cfg
+    proxy_arp = "disabled"
+    for key in vlan_ip_data:
+        if clicommon.is_ip_prefix_in_key(key):
+            continue
+        if vlan == key:
+            proxy_arp = vlan_ip_data[key].get("proxy_arp", "disabled")
+
+    return proxy_arp
+
+
+class VlanBrief:
+    """ This class is used as a namespace to
+    define columns for "show vlan brief" command.
+    The usage of this class is for external plugin
+    (in this case dhcp-relay) to append new columns
+    to this list.
+    """
+
+    COLUMNS = [
+        ("VLAN ID", get_vlan_id),
+        ("IP Address", get_vlan_ip_address),
+        ("Ports", get_vlan_ports),
+        ("Port Tagging", get_vlan_ports_tagging),
+        ("Proxy ARP", get_proxy_arp)
+    ]
+
+    @classmethod
+    def register_column(cls, column_name, callback):
+        """ Adds a new column to "vlan brief" output.
+        Expected to be used from plugins code to extend
+        this command with  additional VLAN fields. """
+
+        cls.COLUMNS.append((column_name, callback))
+
+
 @vlan.command()
 @click.option('--verbose', is_flag=True, help="Enable verbose output")
 @clicommon.pass_db
 def brief(db, verbose):
     """Show all bridge information"""
-    header = ['VLAN ID', 'IP Address', 'Ports', 'Port Tagging', 'DHCP Helper Address', 'Proxy ARP']
+    header = [colname for colname, getter in VlanBrief.COLUMNS]
     body = []
 
     # Fetching data from config db for VLAN, VLAN_INTERFACE and VLAN_MEMBER
-    vlan_dhcp_helper_data = db.cfgdb.get_table('VLAN')
+    vlan_data = db.cfgdb.get_table('VLAN')
     vlan_ip_data = db.cfgdb.get_table('VLAN_INTERFACE')
     vlan_ports_data = db.cfgdb.get_table('VLAN_MEMBER')
+    vlan_cfg = (vlan_data, vlan_ip_data, vlan_ports_data)
 
-    # Defining dictionaries for DHCP Helper address, Interface Gateway IP,
-    # VLAN ports and port tagging
-    vlan_dhcp_helper_dict = {}
-    vlan_ip_dict = {}
-    vlan_ports_dict = {}
-    vlan_tagging_dict = {}
-    vlan_proxy_arp_dict = {}
-
-    # Parsing DHCP Helpers info
-    for key in natsorted(list(vlan_dhcp_helper_data.keys())):
-        try:
-            if vlan_dhcp_helper_data[key]['dhcp_servers']:
-                vlan_dhcp_helper_dict[key.strip('Vlan')] = vlan_dhcp_helper_data[key]['dhcp_servers']
-        except KeyError:
-            vlan_dhcp_helper_dict[key.strip('Vlan')] = " "
-
-    # Parsing VLAN Gateway info
-    for key in vlan_ip_data:
-        if clicommon.is_ip_prefix_in_key(key):
-            interface_key = key[0].strip("Vlan")
-            interface_value = key[1]
-
-            if interface_key in vlan_ip_dict:
-                vlan_ip_dict[interface_key].append(interface_value)
-            else:
-                vlan_ip_dict[interface_key] = [interface_value]
-        else:
-            interface_key = key.strip("Vlan")
-            if 'proxy_arp' in vlan_ip_data[key]:
-                proxy_arp_status = vlan_ip_data[key]['proxy_arp'] 
-            else:
-                proxy_arp_status = "disabled"
-            
-            vlan_proxy_arp_dict[interface_key] = proxy_arp_status
-            
-            
+    for vlan in natsorted(vlan_data):
+        row = []
+        for column in VlanBrief.COLUMNS:
+            column_name, getter = column
+            row.append(getter((vlan_cfg, db), vlan))
+        body.append(row)
 
-    iface_alias_converter = clicommon.InterfaceAliasConverter(db)
-
-    # Parsing VLAN Ports info
-    for key in natsorted(list(vlan_ports_data.keys())):
-        ports_key = key[0].strip("Vlan")
-        ports_value = key[1]
-        ports_tagging = vlan_ports_data[key]['tagging_mode']
-        if ports_key in vlan_ports_dict:
-            if clicommon.get_interface_naming_mode() == "alias":
-                ports_value = iface_alias_converter.name_to_alias(ports_value)
-            vlan_ports_dict[ports_key].append(ports_value)
-        else:
-            if clicommon.get_interface_naming_mode() == "alias":
-                ports_value = iface_alias_converter.name_to_alias(ports_value)
-            vlan_ports_dict[ports_key] = [ports_value]
-        if ports_key in vlan_tagging_dict:
-            vlan_tagging_dict[ports_key].append(ports_tagging)
-        else:
-            vlan_tagging_dict[ports_key] = [ports_tagging]
-
-    # Printing the following dictionaries in tablular forms:
-    # vlan_dhcp_helper_dict={}, vlan_ip_dict = {}, vlan_ports_dict = {}
-    # vlan_tagging_dict = {}
-    for key in natsorted(list(vlan_dhcp_helper_dict.keys())):
-        if key not in vlan_ip_dict:
-            ip_address = ""
-        else:
-            ip_address = ','.replace(',', '\n').join(vlan_ip_dict[key])
-        if key not in vlan_ports_dict:
-            vlan_ports = ""
-        else:
-            vlan_ports = ','.replace(',', '\n').join((vlan_ports_dict[key]))
-        if key not in vlan_dhcp_helper_dict:
-            dhcp_helpers = ""
-        else:
-            dhcp_helpers = ','.replace(',', '\n').join(vlan_dhcp_helper_dict[key])
-        if key not in vlan_tagging_dict:
-            vlan_tagging = ""
-        else:
-            vlan_tagging = ','.replace(',', '\n').join((vlan_tagging_dict[key]))
-        vlan_proxy_arp = vlan_proxy_arp_dict.get(key, "disabled")
-        body.append([key, ip_address, vlan_ports, vlan_tagging, dhcp_helpers, vlan_proxy_arp])
     click.echo(tabulate(body, header, tablefmt="grid"))
 
+
 @vlan.command()
 @clicommon.pass_db
 def config(db):
@@ -141,3 +177,4 @@ def tablelize(keys, data):
 
     header = ['Name', 'VID', 'Member', 'Mode']
     click.echo(tabulate(tablelize(keys, data), header))
+