From a96f9ecee9eef00559beefa77fc32e4aaa111a1e Mon Sep 17 00:00:00 2001 From: abdosi <58047199+abdosi@users.noreply.github.com> Date: Mon, 11 May 2020 11:05:44 -0700 Subject: [PATCH] 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 * 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 * Address Review Comments --- dockers/docker-lldp-sv2/Dockerfile.j2 | 5 ++-- dockers/docker-lldp-sv2/docker-lldp-init.sh | 5 ++++ .../{supervisord.conf => supervisord.conf.j2} | 4 ++++ files/build_templates/lldp.service.j2 | 1 + src/sonic-config-engine/minigraph.py | 24 ++++++++++++------- .../tests/test_multinpu_cfggen.py | 5 ++-- 6 files changed, 32 insertions(+), 12 deletions(-) create mode 100755 dockers/docker-lldp-sv2/docker-lldp-init.sh rename dockers/docker-lldp-sv2/{supervisord.conf => supervisord.conf.j2} (87%) create mode 120000 files/build_templates/lldp.service.j2 diff --git a/dockers/docker-lldp-sv2/Dockerfile.j2 b/dockers/docker-lldp-sv2/Dockerfile.j2 index 6a720514ef9b..af2b0373c373 100644 --- a/dockers/docker-lldp-sv2/Dockerfile.j2 +++ b/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"] diff --git a/dockers/docker-lldp-sv2/docker-lldp-init.sh b/dockers/docker-lldp-sv2/docker-lldp-init.sh new file mode 100755 index 000000000000..ae507e8f506a --- /dev/null +++ b/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 diff --git a/dockers/docker-lldp-sv2/supervisord.conf b/dockers/docker-lldp-sv2/supervisord.conf.j2 similarity index 87% rename from dockers/docker-lldp-sv2/supervisord.conf rename to dockers/docker-lldp-sv2/supervisord.conf.j2 index 73ff52f4420e..beae3aa9425e 100644 --- a/dockers/docker-lldp-sv2/supervisord.conf +++ b/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 diff --git a/files/build_templates/lldp.service.j2 b/files/build_templates/lldp.service.j2 new file mode 120000 index 000000000000..1adb318b9154 --- /dev/null +++ b/files/build_templates/lldp.service.j2 @@ -0,0 +1 @@ +per_namespace/lldp.service.j2 \ No newline at end of file diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index 45397215d003..fb462dada594 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -248,14 +248,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 @@ -291,7 +301,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 @@ -804,10 +813,8 @@ 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': { @@ -815,7 +822,7 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None): 'deployment_id': deployment_id, 'region': region, 'docker_routing_config_mode': docker_routing_config_mode, - 'hostname': name, + 'hostname': hostname, 'hwsku': hwsku, 'type': current_device['type'] } @@ -825,6 +832,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 diff --git a/src/sonic-config-engine/tests/test_multinpu_cfggen.py b/src/sonic-config-engine/tests/test_multinpu_cfggen.py index 6facae0451db..4aefbb78fc61 100644 --- a/src/sonic-config-engine/tests/test_multinpu_cfggen.py +++ b/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')