[minigraph] generate device under test minigraph file by maggiemsft · Pull Request #207 · sonic-net/sonic-mgmt

maggiemsft · 2017-07-05T22:48:49Z

Minigraph is generated based on topology name and type defined testbed.csv
Currently it only covers t0, t1 and t1-lag for 32 ports SONiC devices
Will work on 64 ports later.

… in testbed.csv

msftclas · 2017-07-05T22:48:53Z

@maggiemsft,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

lguohan · 2017-07-07T17:54:29Z

ansible/config_sonic_basedon_testbed.yml

+
+  - block:
+      - name: saved original minigraph file (if original file may don't exist, then ignore errors)
+        shell: mv /etc/sonic/minigraph.xml /etc/sonic/minigraph.xml.ori


ori [](start = 68, length = 3)

-> orig

Probably you're fixed another line. This is still 'ori'

pavel-shirshov

as commented

pavel-shirshov · 2017-07-07T20:45:42Z

ansible/config_sonic_basedon_testbed.yml

+# Template files for generating minigraph.xml are defined in template/topo directory
+#
+# To generate and deploy minigraph for SONiC switch matching the VM topology please use following command
+# ANSIBLE_SCP_IF_SSH=y ansible-playbook -i lab config_sonic_basedon_testbed.yml --vault-password-file=~/.password -l sonic_dut_name -e VM_base=VM0300 -e topo=t0 [-e deploy=true]


you can delete "ANSIBLE_SCP_IF_SSH=y" from here. It is not necessary.

pavel-shirshov · 2017-07-07T20:46:19Z

ansible/config_sonic_basedon_testbed.yml

+#
+# Parameters
+# -l str-msn2700-01          - the sonic_dut_name you are going to generate minigraph for
+# -e VM_base=VM0300          - the VM name which is used to as base to calculate VM name for this set


vm_base, not VM_base

Thanks, fixed

pavel-shirshov · 2017-07-07T20:46:39Z

ansible/config_sonic_basedon_testbed.yml

+# Parameters
+# -l str-msn2700-01          - the sonic_dut_name you are going to generate minigraph for
+# -e VM_base=VM0300          - the VM name which is used to as base to calculate VM name for this set
+# -e topo=t0                 - the name of removed topo


Please remove "removed" from here

pavel-shirshov · 2017-07-07T20:48:18Z

ansible/config_sonic_basedon_testbed.yml

+  gather_facts: yes
+  tasks:
+  - fail: msg="need to provide topology type and vm base like topo=t0 and vm_base=VM100"
+    when: topo and vm_base is not defined 


I think you need to check topo and vm_base in separate
I'd write it as
(topo is not defined) and (vm_base is not defined)

pavel-shirshov · 2017-07-07T20:48:29Z

ansible/config_sonic_basedon_testbed.yml

+    when: topo and vm_base is not defined 
+
+  - fail: msg="need hwsku, interface speed, netmask and interface prefix/postfix defined to generate configuration file"
+    when: hwsku and iface_speed and mgmt_subnet_mask_length and iface_prefix and iface_postfix is not defined


the same as previous comment

pavel-shirshov · 2017-07-07T20:55:08Z

ansible/library/testbed_vm_info.py

+        eos = {}
+        with open(self.topofile) as f:
+            vm_topology = yaml.load(f)
+            f.close()


'with' keyword will close the 'f' file descriptor for you. so it's better to remove f.close() from here

removed, thanks

pavel-shirshov · 2017-07-07T20:57:26Z

ansible/library/testbed_vm_info.py

+    def gather_veos_vm(self, eos_name, vm_index):
+        vmname = 'VM'+format(vm_index, '04d')
+        vmcmd = 'cat '+VM_INV_FILE+' | grep '+  vmname
+        p = subprocess.Popen(vmcmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)


probably it's more clean to open the inventory file as a text file and parse it with python?

changed. Thanks

pavel-shirshov

Please address comments I left.

pavel-shirshov · 2017-07-12T01:05:43Z

ansible/config_sonic_basedon_testbed.yml

+# Template files for generating minigraph.xml are defined in template/topo directory
+#
+# To generate and deploy minigraph for SONiC switch matching the VM topology please use following command
+# ansible-playbook -i lab config_sonic_basedon_testbed.yml --vault-password-file=~/.password -l sonic_dut_name -e VM_base=VM0300 -e topo=t0 [-e deploy=true]


VM_base -> vm_base

pavel-shirshov · 2017-07-12T01:08:30Z

ansible/config_sonic_basedon_testbed.yml

+
+  - block:
+      - name: saved original minigraph file (if original file may don't exist, then ignore errors)
+        shell: mv /etc/sonic/minigraph.xml /etc/sonic/minigraph.xml.ori


Probably you're fixed another line. This is still 'ori'

pavel-shirshov · 2017-07-12T01:11:48Z

ansible/group_vars/lab/lab.yml

 dns_servers: ['10.0.0.5', '10.0.0.6']
+
+# forced_mgmt_routes
+forced_mgmt_routes: ['10.0.0.100/31', '10.250.0.8', '10.255.0.0/28']


I'm not sure here: It should be '10.250.0.8' (without prefix), or '10.250.0.8/32'

Checked with current graph format, no /32

lguohan · 2017-07-12T07:30:17Z

ansible/lab

+[sonic_mlnx_40:vars]
+hwsku="ACS-MSN2700"
+iface_prefix="Ethernet"
+iface_postfix=""


should be suffix

lguohan · 2017-07-12T07:51:45Z

ansible/templates/topo/t1-lag.j2

+{% endif %}
+          <Prefix>FC00::{{ ('%0x' % (64+index*4 +1)) | upper }}/126</Prefix>
+        </IPInterface>
+{% endfor %}


this is very difficult to maintain. it is better to use thi file to generate the interface name. https://github.com/Azure/sonic-buildimage/blob/master/device/arista/x86_64-arista_7050_qx32/Arista-7050-QX32/port_config.ini

In buildimage repo, it builds a debian package named sonic-device-data package. you can install the package in the sonic-mgmt docker and get the actual interface name based on port_config.ini for each hwsku.

lguohan · 2017-07-12T07:53:34Z

ansible/templates/topo/t0.j2

+        <StartRouter>{{ inventory_hostname }}</StartRouter>
+        <StartPeer>FC00::{{ ('%0x' % (113 + index*4 )) | upper }}</StartPeer>
+        <EndRouter>ARISTA0{{ index+1 }}T1</EndRouter>
+        <EndPeer>FC00::{{ ('%0x' % (113 + index*4 +1)) | upper }}</EndPeer>


please be consistent on the whitespace usage

lguohan · 2017-07-12T07:55:21Z

ansible/lab

+[sonic_mlnx_40:vars]
+hwsku="ACS-MSN2700"
+iface_prefix="Ethernet"
+iface_postfix=""


remove iface_prefix and postfix, use port_config.ini in the buildimge repo to convert.

lguohan · 2017-07-12T08:00:52Z

ansible/config_sonic_basedon_testbed.yml

+        become: true
+        template: src=templates/topo/{{ template_name }}.j2
+                  dest=/etc/sonic/minigraph.xml
+


need to disable /etc/sonic/updategraph.conf to avoid downloading url from dhcp

lguohan · 2017-07-12T08:25:14Z

ansible/library/testbed_vm_info.py

+        vms = {}
+        with open(VM_INV_FILE) as f:
+            lines = f.readlines()
+        for  line in lines:


only one space here.

lguohan · 2017-07-12T08:26:58Z

👎

lguohan

use port_config.ini to get the actual port name

…feedback

lguohan · 2017-07-20T03:56:22Z

ansible/config_sonic_basedon_testbed.yml

+    register: p
+    connection: local
+
+  #### TODO: build the package into sonic-mgmt docker and remove this block of code ####


sonic-device-data is in sonic-mgmt docker now. you can remove this block of code now.

lguohan · 2017-07-20T03:56:54Z

ansible/config_sonic_basedon_testbed.yml

+  - fail: msg="need hwsku, interface speed, netmask and interface prefix/postfix defined to generate configuration file"
+    when: (hwsku is not defined) or (iface_speed is not defined) or (mgmt_subnet_mask_length is not defined)
+
+  - stat:


you can assume /usr/share/sonic/device is there in the container.

lguohan · 2017-07-20T03:57:36Z

ansible/config_sonic_basedon_testbed.yml

+      file: path=/tmp/buildimage state=absent
+      when: p.stat.isdir is not defined
+      connection: local
+    when: p.stat.isdir is not defined


remove this block as well.

lguohan · 2017-07-20T04:00:59Z

ansible/library/port_alias.py

+                    return
+                else:
+                    self.portmap['alias'] = True
+                    self.portmap['mapping'][mapping[0]] = mapping[2]


the portmap is index -> alias map. index is the row index in the port_config.ini. let's have the assumption that the ports in port_config.ini are from index 0 to maximum.

then if there is alias, the port_alias[index] = alias, else port_alias[index] = name.

lguohan · 2017-07-20T04:01:26Z

ansible/library/port_alias.py

+FILE_PATH = '/usr/share/sonic/device'
+PORTMAP_FILE = 'port_config.ini'
+
+class SonicPortmap():


SonicPortAliasMap

lguohan · 2017-07-20T04:01:55Z

ansible/library/port_alias.py

+            if 'Ethernet' in line:
+                mapping = line.split()
+                if len(mapping) < 3:
+                    self.portmap['alias'] = False


this line is not used, remove.

lguohan · 2017-07-20T04:05:28Z

ansible/templates/topo/dev_metadata.j2

+          <EnableFlowControl>true</EnableFlowControl>
+          <Index>1</Index>
+{% if port_alias['alias'] %}
+          <InterfaceName>{{ port_alias['mapping'][sonic_if_name] }}</InterfaceName>


port_alias['mapping'][sonic_if_name] -> port_alias[index]

i do not think you really need to first construct sonic_if_name, just use the index (0~31) for 32 port minigraph. you can assume the portname is ordered by the index in the port_config.ini

lguohan · 2017-07-20T04:11:15Z

ansible/config_sonic_basedon_testbed.yml

+      - name: save original updategraph configuration (if original file may don't exist, then ignore errors)
+        shell: mv /etc/sonic/updategraph.conf /etc/sonic/updategrph.conf.orig
+        become: true
+        ignore_errors: true


use lineinfile module to replace the enable=true line to enable=false line.

lguohan · 2017-07-20T04:11:34Z

ansible/config_sonic_basedon_testbed.yml

+                  dest=/etc/sonic/minigraph.xml
+        become: true
+
+      - name: disable automatic minigraph update if we are deploying new minigraph into SONiC


lguohan

as comments.

* generate match SONiC minigraph based on testbed name and type defined in testbed.csv * fixes based on feedback * using port_config to get interface name and alias and fixes based on feedback * use index to find the port name or alias

…ut multi-asic (sonic-net#207)  ### Description of PR  Summary: Fixes # (issue) - Split the original PR sonic-net#17130 into individual PRs for each test. - This is an enhancement to support VOQ Single DUT Multi-ASIC setup for T2 topo. ### Type of change  - [ ] Bug fix - [ ] Testbed and Framework(new/improvement) - [ ] New Test case - [ ] Skipped for non-supported platforms - [x] Test case improvement ### Back port request - [ ] 202012 - [ ] 202205 - [ ] 202305 - [ ] 202311 - [ ] 202405 - [x] 202411 ### Approach #### What is the motivation for this PR? - New testbed modification of VOQ Single DUT and multi-asic. #### How did you do it? - Skip internal neighbors for both VOQ single dut multi-asic as well as multi-dut multi-asic. #### How did you verify/test it? #### Any platform specific information? #### Supported testbed topology if it's a new test case? - T2 VOQ Single Dut Multi ASIC ### Documentation

…onic-net#7647) Includes below commits ``` sonic-swss 05f0438 2021-05-19 | Fix build error (sonic-net#1751) [Shi Su] 1682a8d 2021-05-12 | [Bulk mode] Improve log in bulk failures (sonic-net#1740) [Shi Su] 4509e88 2021-04-27 | [routeorch] Add support for blackhole routes (sonic-net#1723) [Shi Su] 642014d 2019-10-31 | [fix] Use the same storm detection condition for queue occupancy non-zero case as the zero case (sonic-net#1111) [Wenda Ni] sonic-utilities 97fc442 2021-05-10 | [config]: Use mod_entry when editing VLAN_INTERFACE (sonic-net#1602) [Lawrence Lee] f60d579 2021-04-15 | [techsupport] Update show ip interface command (sonic-net#1562) [Mahesh Maddikayala] 821d29d 2021-04-14 | Fix Multi-ASIC show specific resursive route by using common parsing function (sonic-net#1560) [gechiang] sonic-linux-kernel 61b331f 2021-05-06 | [dps200] Add dps200 PSU module driver (sonic-net#207) [schobtr] sonic-platform-daemons 5d2960f 2021-05-17 | [xcvrd] add support for logging mux_metrics events into state DB (sonic-net#185) [vdahiya12] a5c9207 2021-05-11 | [thermalctld] Enable stopping thermal manager (sonic-net#180) [Junchao-Mellanox] 9123f63 2021-05-10 | [muxcable] Remove Xcvrd Sleep (sonic-net#174) [Tamer Ahmed] ```

sonic-net#14862) #### Why I did it src/linkmgrd ``` * 35f7d1c - (HEAD -> 202012, origin/202012) [202012][active-standby][bsl] fix no mux probe issue sonic-net#201 (sonic-net#206) (2 weeks ago) [Jing Zhang] * be701fb - [202012] pick codeql fixes (sonic-net#207) (2 weeks ago) [Jing Zhang] * 0b42cec - Enable debug symbols (sonic-net#199) (3 weeks ago) [Longxiang Lyu] ```

generate match SONiC minigraph based on testbed name and type defined…

473ddce

… in testbed.csv

msftclas added the cla-not-required label Jul 5, 2017

maggiemsft requested review from lguohan, pavel-shirshov and sihuihan88 July 5, 2017 22:49

lguohan reviewed Jul 7, 2017

View reviewed changes

pavel-shirshov suggested changes Jul 7, 2017

View reviewed changes

fixes based on feedback

7eb5c51

stcheng added the Enhancement label Jul 11, 2017

pavel-shirshov approved these changes Jul 12, 2017

View reviewed changes

lguohan reviewed Jul 12, 2017

View reviewed changes

ansible/lab Outdated

[sonic_mlnx_40:vars]

hwsku="ACS-MSN2700"

iface_prefix="Ethernet"

iface_postfix=""

Copy link

Contributor

lguohan Jul 12, 2017

Choose a reason for hiding this comment

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

should be suffix

lguohan reviewed Jul 12, 2017

View reviewed changes

lguohan requested changes Jul 12, 2017

View reviewed changes

using port_config to get interface name and alias and fixes based on …

47a484e

…feedback

lguohan reviewed Jul 20, 2017

View reviewed changes

lguohan requested changes Jul 20, 2017

View reviewed changes

use index to find the port name or alias

b2ba867

lguohan approved these changes Jul 20, 2017

View reviewed changes

maggiemsft merged commit 03e9cdc into sonic-net:master Jul 20, 2017

maggiemsft deleted the masun/minigraph-gen branch July 28, 2017 03:01

Conversation

maggiemsft commented Jul 5, 2017

Uh oh!

msftclas commented Jul 5, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pavel-shirshov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pavel-shirshov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lguohan commented Jul 12, 2017

Uh oh!

lguohan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lguohan Jul 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lguohan Jul 20, 2017 •

edited

Loading

lguohan Jul 20, 2017 •

edited

Loading