Changes for LLDP docker to support multi-npu platforms (#4530)

* Changes for LLDP for Multi NPU Platoforms:-
a) Enable LLDP for Host namespace for Management Port
b) Make sure Management IP is avaliable in per asic namespace
   needed for LLDP Chassis configuration
c) Make sure chassis mac-address is correct in per asic namespace
d) Do not run lldp on eth0 of per asic namespace and avoid chassis
   configuration for same
e) Use Linux hostname instead from Device Metadata for lldp chassis
   configuration since in multi-npu platforms device metadata hostname
   will be differnt

Signed-off-by: Abhishek Dosi <[email protected]>

* Address Review Comment with following changes:
a) Use Device Metadata hostname even in per namespace conatiner.
   updated minigraph parsing for same to have hostname as system
   hostname and add new key for asic name

b) Minigraph changes to have MGMT_INTERFACE Key in per asic/namespace
   config also as needed for LLDP for setting chassis management IP.

Signed-off-by: Abhishek Dosi <[email protected]>

* Address Review Comments

Author: abdosi

dockers/docker-lldp-sv2/Dockerfile.j2

@@ -35,12 +35,13 @@ RUN apt-get purge -y python-pip && \
            /python-wheels          \
            ~/.cache
 
+COPY ["docker-lldp-init.sh", "/usr/bin/"]
 COPY ["start.sh", "/usr/bin/"]
-COPY ["supervisord.conf", "/etc/supervisor/conf.d/"]
+COPY ["supervisord.conf.j2", "/usr/share/sonic/templates/"]
 COPY ["lldpd.conf.j2", "/usr/share/sonic/templates/"]
 COPY ["lldpd", "/etc/default/"]
 COPY ["lldpmgrd", "/usr/bin/"]
 COPY ["files/supervisor-proc-exit-listener", "/usr/bin"]
 COPY ["critical_processes", "/etc/supervisor"]
 
-ENTRYPOINT ["/usr/bin/supervisord"]
+ENTRYPOINT ["/usr/bin/docker-lldp-init.sh"]

dockers/docker-lldp-sv2/docker-lldp-init.sh

@@ -0,0 +1,5 @@
+#!/usr/bin/env bash
+#Generate supervisord.conf based on device metadata
+mkdir -p /etc/supervisor/conf.d/
+sonic-cfggen -d -t /usr/share/sonic/templates/supervisord.conf.j2 > /etc/supervisor/conf.d/supervisord.conf
+exec /usr/bin/supervisord

dockers/docker-lldp-sv2/supervisord.conf renamed to dockers/docker-lldp-sv2/supervisord.conf.j2

@@ -31,7 +31,11 @@ stderr_logfile=syslog
 # - `-dd` means to stay in foreground, log warnings to console
 # - `-ddd` means to stay in foreground, log warnings and info to console
 # - `-dddd` means to stay in foreground, log all to console
+{% if DEVICE_METADATA['localhost']['sub_role'] is defined and DEVICE_METADATA['localhost']['sub_role']|length %}
+command=/usr/sbin/lldpd -d -I Ethernet* -C Ethernet*
+{% else %}
 command=/usr/sbin/lldpd -d -I Ethernet*,eth0 -C eth0
+{% endif %}
 priority=3
 autostart=false
 autorestart=false

files/build_templates/lldp.service.j2

@@ -0,0 +1 @@
+per_namespace/lldp.service.j2

src/sonic-config-engine/minigraph.py

@@ -252,14 +252,24 @@ def parse_asic_png(png, asic_name, hostname):
     return (neighbors, devices, port_speeds)
 
 def parse_dpg(dpg, hname):
+    aclintfs = None
+    mgmtintfs = None
     for child in dpg:
-        """In Multi-NPU platforms the acl intfs are defined only for the host not for individual asic.
+        """ 
+            In Multi-NPU platforms the acl intfs are defined only for the host not for individual asic.
             There is just one aclintf node in the minigraph
-             Get the aclintfs node first.
+            Get the aclintfs node first.
         """
-        if child.find(str(QName(ns, "AclInterfaces"))) is not None:
+        if aclintfs is None and child.find(str(QName(ns, "AclInterfaces"))) is not None:
             aclintfs = child.find(str(QName(ns, "AclInterfaces")))
-
+        """
+            In Multi-NPU platforms the mgmt intfs are defined only for the host not for individual asic
+            There is just one mgmtintf node in the minigraph
+            Get the mgmtintfs node first. We need mgmt intf to get mgmt ip in per asic dockers.
+        """
+        if mgmtintfs is None and child.find(str(QName(ns, "ManagementIPInterfaces"))) is not None:
+            mgmtintfs = child.find(str(QName(ns, "ManagementIPInterfaces")))
+        
         hostname = child.find(str(QName(ns, "Hostname")))
         if hostname.text.lower() != hname.lower():
             continue
@@ -295,7 +305,6 @@ def parse_dpg(dpg, hname):
                 mvrf_en_flag = mv.find(str(QName(ns, "mgmtVrfEnabled"))).text
                 mvrf["vrf_global"] = {"mgmtVrfEnabled": mvrf_en_flag}
 
-        mgmtintfs = child.find(str(QName(ns, "ManagementIPInterfaces")))
         mgmt_intf = {}
         for mgmtintf in mgmtintfs.findall(str(QName(ns1, "ManagementIPInterface"))):
             intfname = mgmtintf.find(str(QName(ns, "AttachTo"))).text
@@ -807,18 +816,16 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None):
 
     if asic_name is None:
         current_device = [devices[key] for key in devices if key.lower() == hostname.lower()][0]
-        name = hostname
     else:
         current_device = [devices[key] for key in devices if key.lower() == asic_name.lower()][0]
-        name = asic_name
 
     results = {}
     results['DEVICE_METADATA'] = {'localhost': {
         'bgp_asn': bgp_asn,
         'deployment_id': deployment_id,
         'region': region,
         'docker_routing_config_mode': docker_routing_config_mode,
-        'hostname': name,
+        'hostname': hostname,
         'hwsku': hwsku,
         'type': current_device['type']
         }
@@ -828,6 +835,7 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None):
     if sub_role is not None:
         current_device['sub_role'] = sub_role
         results['DEVICE_METADATA']['localhost']['sub_role'] =  sub_role
+        results['DEVICE_METADATA']['localhost']['asic_name'] =  asic_name
     results['BGP_NEIGHBOR'] = bgp_sessions
     results['BGP_MONITORS'] = bgp_monitors
     results['BGP_PEER_RANGE'] = bgp_peers_with_range

src/sonic-config-engine/tests/test_multinpu_cfggen.py

@@ -116,7 +116,7 @@ def test_mgmt_port(self):
         self.assertDictEqual(output, {'eth0': {'alias': 'eth0', 'admin_status': 'up'}})
         for asic in range(NUM_ASIC):
             output = json.loads(self.run_script_for_asic(argument, asic, self.port_config[asic]))
-            self.assertDictEqual(output, {})
+            self.assertDictEqual(output, {'eth0': {'alias': 'eth0', 'admin_status': 'up'}})
 
     def test_frontend_asic_portchannels(self):
         argument = "-m {} -p {} -n asic0 --var-json \"PORTCHANNEL\"".format(self.sample_graph, self.port_config[0])
@@ -213,7 +213,8 @@ def test_device_asic_metadata(self):
         for asic in range(NUM_ASIC):
             output = json.loads(self.run_script_for_asic(argument, asic,self.port_config[asic]))
             asic_name  = "asic{}".format(asic)
-            self.assertEqual(output['localhost']['hostname'], asic_name)
+            self.assertEqual(output['localhost']['hostname'], 'multi_npu_platform_01')
+            self.assertEqual(output['localhost']['asic_name'], asic_name)
             self.assertEqual(output['localhost']['type'], 'Asic')
             if asic == 0 or asic == 1:
                 self.assertEqual(output['localhost']['sub_role'], 'FrontEnd')