From ef8c7473b21837c3341dd94e5947d214abd97015 Mon Sep 17 00:00:00 2001
From: sanmalho <sandeep.malhotra@nokia.com>
Date: Wed, 23 Dec 2020 16:28:50 -0500
Subject: [PATCH 1/4] Support for gen-mg to work with multi-dut where VM's
 connect to only a single host; support for supervisor card.

gen-mg supported multi-dut, but in a dualtor topology, each VM is assumed to
be connected to all the DUT's. To support the VM to be connected to a single DUT:
 - In vm_topo_config, the fields like interface_indexes, intfs, ip_intf etc would be
   either empty or null. Add checks for these where they are being used in gen-mg

gen-mg also assumes that all interfaces connected to a PortChannel are sequential. To fix this:
 - In config_sonic_based_on_testbed.yml, changed interface_to_vms structure to be compatable to
   allow using with_subelements to get the intf_names

For 'supervisor' card, there are no BGP peers, interfaces etc. required.
 - A 'supervisor' card is identified by having 'type' field in the inventory with value 'supervisor'.
 - Add this check to skip generation of portions not valid for supervisor card
---
 ansible/config_sonic_basedon_testbed.yml |  9 ++++++---
 ansible/library/topo_facts.py            | 18 ++++++++++--------
 ansible/templates/minigraph_cpg.j2       |  6 ++++++
 ansible/templates/minigraph_device.j2    |  2 ++
 ansible/templates/minigraph_dpg.j2       | 20 ++++++++++++++++----
 ansible/templates/minigraph_png.j2       |  4 ++++
 ansible/templates/minigraph_template.j2  |  4 ++++
 7 files changed, 48 insertions(+), 15 deletions(-)

diff --git a/ansible/config_sonic_basedon_testbed.yml b/ansible/config_sonic_basedon_testbed.yml
index 497d83136e8..ad891465b96 100644
--- a/ansible/config_sonic_basedon_testbed.yml
+++ b/ansible/config_sonic_basedon_testbed.yml
@@ -134,7 +134,8 @@
 
   - name: find all interface indexes mapping connecting to VM
     set_fact:
-      interface_to_vms: "{{ interface_to_vms|default({}) | combine({ item.key: item.value['interface_indexes'][dut_index|int] }) }}"
+      interface_to_vms: "{{ interface_to_vms|default([]) + [ {'name': item.key, 'ports': item.value['interface_indexes'][dut_index|int] }] }}"
+    when: item.value|length
     with_dict: "{{ vm_topo_config['vm'] }}"
 
   - name: find all interface indexes connecting to VM
@@ -144,8 +145,10 @@
 
   - name: find all interface names
     set_fact:
-      intf_names: "{{ intf_names | default({}) | combine({item.key: port_alias[item.value[0]|int:item.value[-1]|int+1] }) }}"
-    with_dict: "{{ interface_to_vms }}"
+      intf_names: "{{ intf_names | default({}) | combine({item.0.name:  intf_names[item.0.name]|default([]) + [ port_alias[item.1]] }) }}"
+    with_subelements:
+      - "{{ interface_to_vms }}"
+      - "ports"
 
   - name: create minigraph file in ansible minigraph folder
     template: src=templates/minigraph_template.j2
diff --git a/ansible/library/topo_facts.py b/ansible/library/topo_facts.py
index 283ad8d7bba..5495b0a0907 100644
--- a/ansible/library/topo_facts.py
+++ b/ansible/library/topo_facts.py
@@ -153,15 +153,17 @@ def get_topo_config(self, topo_name):
                     ip = ipaddress.ip_address(ipstr.decode('utf8'))
                     for dut_index in range(0, dut_num):
                         if ip.version == 4:
-                            ipsubnet_str = vmconfig[vm]['peer_ipv4'][dut_index]+'/'+vmconfig[vm]['ipv4mask'][dut_index]
-                            ipsubnet = ipaddress.ip_interface(ipsubnet_str.decode('utf8'))
-                            if ip in ipsubnet.network:
-                                vmconfig[vm]['bgp_ipv4'][dut_index] = ipstr.upper()
+                            if vmconfig[vm]['peer_ipv4'][dut_index]:
+                                ipsubnet_str = vmconfig[vm]['peer_ipv4'][dut_index]+'/'+vmconfig[vm]['ipv4mask'][dut_index]
+                                ipsubnet = ipaddress.ip_interface(ipsubnet_str.decode('utf8'))
+                                if ip in ipsubnet.network:
+                                    vmconfig[vm]['bgp_ipv4'][dut_index] = ipstr.upper()
                         elif ip.version == 6:
-                            ipsubnet_str = vmconfig[vm]['peer_ipv6'][dut_index]+'/'+vmconfig[vm]['ipv6mask'][dut_index]
-                            ipsubnet = ipaddress.ip_interface(ipsubnet_str.decode('utf8'))
-                            if ip in ipsubnet.network:
-                                vmconfig[vm]['bgp_ipv6'][dut_index] = ipstr.upper()
+                            if vmconfig[vm]['peer_ipv6'][dut_index]:
+                                ipsubnet_str = vmconfig[vm]['peer_ipv6'][dut_index]+'/'+vmconfig[vm]['ipv6mask'][dut_index]
+                                ipsubnet = ipaddress.ip_interface(ipsubnet_str.decode('utf8'))
+                                if ip in ipsubnet.network:
+                                    vmconfig[vm]['bgp_ipv6'][dut_index] = ipstr.upper()
 
             vm_topo_config['vm'] = vmconfig
 
diff --git a/ansible/templates/minigraph_cpg.j2 b/ansible/templates/minigraph_cpg.j2
index 7a16d667869..8e556071873 100644
--- a/ansible/templates/minigraph_cpg.j2
+++ b/ansible/templates/minigraph_cpg.j2
@@ -29,17 +29,20 @@
 {% endfor %}
     </PeeringSessions>
     <Routers xmlns:a="http://schemas.datacontract.org/2004/07/Microsoft.Search.Autopilot.Evolution">
+{% if type is not defined or type != 'supervisor' %}
       <a:BGPRouterDeclaration>
         <a:ASN>{{ vm_topo_config['dut_asn'] }}</a:ASN>
         <a:Hostname>{{ inventory_hostname }}</a:Hostname>
         <a:Peers>
 {% for index in range(vms_number)  %}
+{% if vm_topo_config['vm'][vms[index]]['peer_ipv4'][dut_index|int] %}
           <BGPPeer>
             <Address>{{ vm_topo_config['vm'][vms[index]]['peer_ipv4'][dut_index|int] }}</Address>
             <RouteMapIn i:nil="true"/>
             <RouteMapOut i:nil="true"/>
             <Vrf i:nil="true"/>
           </BGPPeer>
+{% endif %}
 {% endfor %}
 {% if 'tor' in vm_topo_config['dut_type'] | lower %}
           <BGPPeer i:type="a:BGPPeerPassive">
@@ -65,12 +68,15 @@
         <a:RouteMaps/>
       </a:BGPRouterDeclaration>
 {% for index in range( vms_number) %}
+{% if vm_topo_config['vm'][vms[index]]['intfs'][dut_index|int]|length > 0 %}
       <a:BGPRouterDeclaration>
         <a:ASN>{{ vm_topo_config['vm'][vms[index]]['bgp_asn'] }}</a:ASN>
         <a:Hostname>{{ vms[index] }}</a:Hostname>
         <a:RouteMaps/>
       </a:BGPRouterDeclaration>
+{% endif %}
 {% endfor %}
+{% endif %}
     </Routers>
   </CpgDec>
 
diff --git a/ansible/templates/minigraph_device.j2 b/ansible/templates/minigraph_device.j2
index 8524e72de2b..c47756653d9 100644
--- a/ansible/templates/minigraph_device.j2
+++ b/ansible/templates/minigraph_device.j2
@@ -2,6 +2,7 @@
     <DeviceInfo>
       <AutoNegotiation>true</AutoNegotiation>
       <EthernetInterfaces xmlns:a="http://schemas.datacontract.org/2004/07/Microsoft.Search.Autopilot.Evolution">
+{% if type is not defined or type != 'supervisor' %}
 {% set num_of_intf = port_alias | length %}
 {% for index in range(num_of_intf) %}
         <a:EthernetInterface>
@@ -23,6 +24,7 @@
 {% endif %}
         </a:EthernetInterface>
 {% endfor %}
+{% endif %}
       </EthernetInterfaces>
       <FlowControl>true</FlowControl>
       <Height>0</Height>
diff --git a/ansible/templates/minigraph_dpg.j2 b/ansible/templates/minigraph_dpg.j2
index 3bcbaeb0014..d21b3eb155d 100644
--- a/ansible/templates/minigraph_dpg.j2
+++ b/ansible/templates/minigraph_dpg.j2
@@ -2,6 +2,7 @@
     <DeviceDataPlaneInfo>
       <IPSecTunnels/>
       <LoopbackIPInterfaces xmlns:a="http://schemas.datacontract.org/2004/07/Microsoft.Search.Autopilot.Evolution">
+{% if type is not defined or type != 'supervisor' %}
         <a:LoopbackIPInterface>
           <Name>HostIP</Name>
           <AttachTo>Loopback0</AttachTo>
@@ -39,6 +40,7 @@
         </a:LoopbackIPInterface>
       {%- endfor -%}
       {%- endif -%}
+{% endif %}
       </LoopbackIPInterfaces>
       <ManagementIPInterfaces xmlns:a="http://schemas.datacontract.org/2004/07/Microsoft.Search.Autopilot.Evolution">
         <a:ManagementIPInterface>
@@ -106,9 +108,10 @@
       </VlanInterfaces>
       <IPInterfaces>
 {% for index in range(vms_number) %}
+{% if vm_topo_config['vm'][vms[index]]['ip_intf'][dut_index|int] is not none %}
         <IPInterface>
           <Name i:nil="true"/>
-{% if 'port-channel' in vm_topo_config['vm'][vms[index]]['ip_intf']|lower %}
+{% if 'port-channel' in vm_topo_config['vm'][vms[index]]['ip_intf'][dut_index|int]|lower %}
           <AttachTo>PortChannel{{ ((index+1) |string).zfill(4) }}</AttachTo>
 {% else %}
           <AttachTo>{{ port_alias[vm_topo_config['vm'][vms[index]]['interface_indexes'][dut_index|int][0]] }}</AttachTo>
@@ -117,13 +120,14 @@
         </IPInterface>
         <IPInterface>
           <Name i:Name="true"/>
-{% if 'port-channel' in (vm_topo_config['vm'][vms[index]]['ip_intf']|lower) %}
+{% if 'port-channel' in vm_topo_config['vm'][vms[index]]['ip_intf'][dut_index|int]|lower %}
           <AttachTo>PortChannel{{ ((index+1) |string).zfill(4) }}</AttachTo>
 {% else %}
           <AttachTo>{{ port_alias[vm_topo_config['vm'][vms[index]]['interface_indexes'][dut_index|int][0]] }}</AttachTo>
 {% endif %}
           <Prefix>{{ vm_topo_config['vm'][vms[index]]['bgp_ipv6'][dut_index|int] }}/{{ vm_topo_config['vm'][vms[index]]['ipv6mask'][dut_index|int] }}</Prefix>
         </IPInterface>
+{% endif %}
 {% endfor %}
 {% if 'tor' in vm_topo_config['dut_type'] | lower %}
 {% for vlan, vlan_param in vlan_configs.items() %}
@@ -146,6 +150,7 @@
       </IPInterfaces>
       <DataAcls/>
       <AclInterfaces>
+{% if type is not defined or type != 'supervisor' %}
         <AclInterface>
           <InAcl>SNMP_ACL</InAcl>
           <AttachTo>SNMP</AttachTo>
@@ -169,20 +174,27 @@
 {% if enable_data_plane_acl|default('true')|bool %}
         <AclInterface>
           <AttachTo>
+{%- set acl_intfs = [] -%}
 {%- for index in range(vms_number) %}
 {% if 'port-channel' in vm_topo_config['vm'][vms[index]]['ip_intf'][dut_index|int]|lower %}
-PortChannel{{ ((index+1) |string).zfill(4) }}{% if not loop.last %};{% endif %}
+{% set a_intf = 'PortChannel' + ((index+1) |string).zfill(4) %}
+{{- acl_intfs.append(a_intf) -}}
 {% endif %}
 {% endfor %}
 {% for index in range(vms_number) %}
 {% if 'port-channel' not in vm_topo_config['vm'][vms[index]]['ip_intf']|lower %}
-{{ port_alias[vm_topo_config['vm'][vms[index]]['interface_indexes'][dut_index|int][0]] }}{% if not loop.last %};{% endif %}
+{% if vm_topo_config['vm'][vms[index]]['intfs'][dut_index|int]|length %}
+{% set a_intf = port_alias[vm_topo_config['vm'][vms[index]]['interface_indexes'][dut_index|int][0]] %}
+{{- acl_intfs.append(a_intf) -}}
+{% endif %}
 {% endif %}
 {% endfor -%}
+{{- acl_intfs|join(',') -}}
           </AttachTo>
           <InAcl>DataAcl</InAcl>
           <Type>DataPlane</Type>
         </AclInterface>
+{% endif %}
 {% endif %}
       </AclInterfaces>
       <DownstreamSummaries/>
diff --git a/ansible/templates/minigraph_png.j2 b/ansible/templates/minigraph_png.j2
index 6050258641d..451638b50ad 100644
--- a/ansible/templates/minigraph_png.j2
+++ b/ansible/templates/minigraph_png.j2
@@ -113,10 +113,13 @@
 {% endif %}
 {% if VM_topo %}
 {% for dev in neighbor_eosvm_mgmt %}
+{% if vm_topo_config['vm'][dev]['intfs'][dut_index|int]|length %}
 {% if 'T1' in dev %}
 {% set dev_type = 'LeafRouter' %}
 {% elif 'T2' in dev %}
 {% set dev_type = 'SpineRouter' %}
+{% elif 'T3' in dev %}
+{% set dev_type = 'CoreRouter' %}
 {% elif 'T0' in dev %}
 {% set dev_type = 'ToRRouter' %}
 {% else %}
@@ -129,6 +132,7 @@
          </ManagementAddress>
          <HwSku>Arista-VM</HwSku>
       </Device>
+{% endif %}
 {% endfor %}
 {% endif %}
     </Devices>
diff --git a/ansible/templates/minigraph_template.j2 b/ansible/templates/minigraph_template.j2
index cfb81b12c17..5894b9b5816 100644
--- a/ansible/templates/minigraph_template.j2
+++ b/ansible/templates/minigraph_template.j2
@@ -2,10 +2,14 @@
 {% set vms=vm_topo_config['vm'].keys() | sort %}
 {% set vms_number = vms | length %}
 {% if 'loopback' in vm_topo_config['DUT'] %}
+{% if type is not defined or type != 'supervisor' %}
 {% set lp_ipv4 = vm_topo_config['DUT']['loopback']['ipv4'][dut_index|int] %}
 {% set lp_ipv4_addr = lp_ipv4.split('/')[0] %}
+{% endif %}
+{% if type is not defined or type != 'supervisor' %}
 {% set lp_ipv6 = vm_topo_config['DUT']['loopback']['ipv6'][dut_index|int] %}
 {% set lp_ipv6_addr = lp_ipv6.split('/')[0] %}
+{% endif %}
 {% else %}
 {% set lp_ipv4 = '10.1.0.32/32' %}
 {% set lp_ipv4_addr = '10.1.0.32' %}

From e33501cfac95517f6a94a2b43fa2d2cdedfc56f5 Mon Sep 17 00:00:00 2001
From: sanmalho <sandeep.malhotra@nokia.com>
Date: Wed, 23 Dec 2020 17:23:26 -0500
Subject: [PATCH 2/4] Minor fix

---
 ansible/templates/minigraph_dpg.j2 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ansible/templates/minigraph_dpg.j2 b/ansible/templates/minigraph_dpg.j2
index d21b3eb155d..9b6d1db4741 100644
--- a/ansible/templates/minigraph_dpg.j2
+++ b/ansible/templates/minigraph_dpg.j2
@@ -189,7 +189,7 @@
 {% endif %}
 {% endif %}
 {% endfor -%}
-{{- acl_intfs|join(',') -}}
+{{- acl_intfs|join(';') -}}
           </AttachTo>
           <InAcl>DataAcl</InAcl>
           <Type>DataPlane</Type>

From b66519961244185ed704460c8db2c46f3502b675 Mon Sep 17 00:00:00 2001
From: sanmalho <sandeep.malhotra@nokia.com>
Date: Tue, 19 Jan 2021 10:10:41 -0500
Subject: [PATCH 3/4] Added comments to topo_facts as per review

---
 ansible/library/topo_facts.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ansible/library/topo_facts.py b/ansible/library/topo_facts.py
index 5495b0a0907..c6f2dfd86fc 100644
--- a/ansible/library/topo_facts.py
+++ b/ansible/library/topo_facts.py
@@ -153,12 +153,14 @@ def get_topo_config(self, topo_name):
                     ip = ipaddress.ip_address(ipstr.decode('utf8'))
                     for dut_index in range(0, dut_num):
                         if ip.version == 4:
+                            # Each VM might not be connected to all the DUT's, so check if this VM is a peer to DUT at dut_index
                             if vmconfig[vm]['peer_ipv4'][dut_index]:
                                 ipsubnet_str = vmconfig[vm]['peer_ipv4'][dut_index]+'/'+vmconfig[vm]['ipv4mask'][dut_index]
                                 ipsubnet = ipaddress.ip_interface(ipsubnet_str.decode('utf8'))
                                 if ip in ipsubnet.network:
                                     vmconfig[vm]['bgp_ipv4'][dut_index] = ipstr.upper()
                         elif ip.version == 6:
+                            # Each VM might not be connected to all the DUT's, so check if this VM is a peer to DUT at dut_index
                             if vmconfig[vm]['peer_ipv6'][dut_index]:
                                 ipsubnet_str = vmconfig[vm]['peer_ipv6'][dut_index]+'/'+vmconfig[vm]['ipv6mask'][dut_index]
                                 ipsubnet = ipaddress.ip_interface(ipsubnet_str.decode('utf8'))

From 933c5cc420b81bc03402171570f56886f0fc6de2 Mon Sep 17 00:00:00 2001
From: sanmalho <sandeep.malhotra@nokia.com>
Date: Fri, 22 Jan 2021 10:46:41 -0500
Subject: [PATCH 4/4] Remove a check that is not requied in
 config_sonic_basedon_testbed.yml based on review comment

---
 ansible/config_sonic_basedon_testbed.yml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/ansible/config_sonic_basedon_testbed.yml b/ansible/config_sonic_basedon_testbed.yml
index ad891465b96..cf9bc37836b 100644
--- a/ansible/config_sonic_basedon_testbed.yml
+++ b/ansible/config_sonic_basedon_testbed.yml
@@ -135,7 +135,6 @@
   - name: find all interface indexes mapping connecting to VM
     set_fact:
       interface_to_vms: "{{ interface_to_vms|default([]) + [ {'name': item.key, 'ports': item.value['interface_indexes'][dut_index|int] }] }}"
-    when: item.value|length
     with_dict: "{{ vm_topo_config['vm'] }}"
 
   - name: find all interface indexes connecting to VM