-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[sonic-cfggen]:move get_machine_info function to lib #489
Conversation
Signed-off-by: Sihui Han <sihan@microsoft.com>
Machine info is not minigraph-related, so it feels a bit out of place in minigraph.py. Is there a need to call this function that can't be accomplished by calling sonic-cfggen? |
Signed-off-by: Sihui Han <sihan@microsoft.com>
Machine info is actually used to parse the minigraph. The reason we move it here is that teamshow and maybe other tools in future will have to use this function to parse the minigraph. It's better to reference a python library rather than calling a script (sonic-cfggen) directly. So we decide to move it here after discussion. #Resolved |
In reply to: 292682895 [](ancestors = 292682895) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As comments
agree that get_platform_info can be a library, but it is different from minigraph, so can we have a separate library file for it? maybe sonic_platform.py? |
I think we need to use the machine info during parsing minigraph. Platform info determines the directory of port_config.ini which will be used to find the port_alias_map during parsing the minigraph. Without correct platform info, parsing minigraph will fail. Also as the API of parse_xml shows, it takes platform as input. It also makes sense that we separate it from minigraph. I will create a new library file called sonic_machine.py will provides all the machine related info in the same folder. |
What about the file port_config.ini? Currently the parsing logic is inside the minigraph.py file although it is also not necessary for parsing the xml file. Do we want to treat the port_config.ini file similar to machine.conf file? Do we want to keep the port_config.ini parsing logics still inside the minigraph.py file or into a separate file or into the sonic_platform.py library? |
parse port_config.ini can also be moved to sonic_platform.py |
parse_port_config relies updates port_alias_map which is a global variable in minigraph.py during its process. Moving it out of minigraph.py might require major code change. The current motivation is to unblock teamshow. I will move the get_platform_info and get_machine_info to sonic_platform.py first. We can create an issue and refactor the parse_port_config part later. |
agree to refactor port_config.ini later. |
Signed-off-by: Sihui Han <sihan@microsoft.com>
Updated as comments. |
I think it makes sense to move port_config.ini reading logic out from minigraph.py and remove that global variable as well. I'm fine either doing it in this PR or I could do it later. |
@@ -15,7 +15,7 @@ def get_test_suite(): | |||
author='Taoyu Li', | |||
author_email='taoyl@microsoft.com', | |||
url='https://github.com/Azure/sonic-buildimage', | |||
py_modules=['minigraph', 'openconfig_acl'], | |||
py_modules=['minigraph', 'openconfig_acl', 'sonic_platform'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'minigraph', 'openconfig_acl', 'sonic_platform' [](start = 18, length = 47)
How about just one module? @taoyl-ms any comment? #Closed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm for splitting into multiple modules based on functionality. Actually I've always been feeling not comfortable to have port config logic in minigraph but have not had chance to refactor it.
In reply to @qiluo:
In reply to: 292685706 [](ancestors = 292685706,292682895) |
retest please |
@sihuihan88 could you add new issues mentioned in this pull request? |
swss: * e34104e 2018-04-13 | [pfcwd]: support BIG_RED_SWITCH mode (#467) (HEAD, origin/201803) [sihuihan88] * 1f857d5 2018-04-25 | [buffermgr]: remove the item from consumer queue if invalid (#489) [sihuihan88] utilities: * 0b9bb2b 2018-04-26 | Stop services before pushing new config during "load_minigraph" (#247) (HEAD, origin/201803) [Prince Sunny] * dc119c9 2018-04-18 | [show logging] For following, change 'tail -f' to 'tail -F' in order to retry in the case log is rotated (#240) [Joe LeVeque] * 08da428 2018-04-16 | [pfcwd]: add cli to enable/disable BIG_RED_SWITCH mode (#237) [sihuihan88] Signed-off-by: Guohan Lu <gulv@microsoft.com>
…2755) sonic-sairedis: e8cb879 Make object list deterministic when iterating (#438) 5486f97 Ignore ACL_COUNTER bytes and packets during comparison logic (#443) b138ff9 Notify OA about exception and process only restart query events (#437) 0974a43 Set MTU value on created tap device for virtual switch (#436) e2f50e8 Increase eth buffer size to 16k for virtual switch (#435) 79fb388 [bfn] Ged rid of ld_preload. Link against libsai only (#429) 2e47b78 Remove MAC alignment WA for Mellanox platforms. (#430) e1354fe Add pre match to comparison logic and unittests (#423) 18a5ebb Drop FDB notifications if they contain invalid OIDs (#428) sonic-swss-common: 8af58ad sonic-swss-common: Add vxlan macros to schema (#269) 76837bf Make class Select support batch read from selectables (#270) 4cf643e Add multiple fields hdel support (#267) a710529 Update PFC_WD table name in CONFIG_DB (#266) 3c452c1 Update README.md (#268) sonic-swss: e329dbd Survive pfc watchdog storm action across warm-reboot (#794) sonic-utilities: 6ee0aea (HEAD, origin/master, origin/HEAD) [config]: Change the order of interface commands (#504) 5ae30d2 [show vlan brief] Support 'alias' interface naming mode (#497) bafebf9 Update neighbor advertiser (#498) fa90083 [clear/main.py]: clear ndp command. (#450) 65f69e4 [show interface neighbor expected] Support 'alias' interface naming mode (#495) aae39e7 updated show ipv6 interface for alias mode (#493) 170fed9 [warm-reboot] initialize warm reboot state table before warm rebooting (#492) 06cd99f Allow config shutdown and startup operations on valid PortChannel interface names (#474) 98cdebb [show ip interface] Add support for 'alias' interface naming mode (#486) 5f1de81 [show] Add serial numbers/uptime/hwinfo to 'show version' output (#488) e78a866 [route_check] Move scripts under scripts/ directory, add to setup.py (#489) d347527 Change PFC watchdog CONFIG_DB table name from PFC_WD_TABLE to PFC_WD (#475) Signed-off-by: Wenda Ni <wenni@microsoft.com>
[installer]: Suppress tar xz warning about time stamp in the future, if date is not correctly set (sonic-net#1562) \[sonic-platform-common\] Update submodule (sonic-net#1563) \- Includes the following commits: \- \[bcmshell.py\] Match extra whitespace before prompt in regex (#3) \- add support for qsfp28 eeprom (#2) \[baseimage\]: bring down eth0 before restart networking (sonic-net#1555) cfggen generates new eth0 configuration. Need to first clean existing configuration on eth0 before bring up new configuration on eth0. Thus, we need to first bring down eth0 before putting new configuration into /etc/network/ interfaces \[mellanox\]: Update MLNX SAI pointer (sonic-net#1557) \[minigraph.py\] Add support to parse tacacs server information (sonic-net#1549) \* \[minigraph.py\] Add support to parse tacacs server information \[router advertiser\] Only start radvd process if device role is 'ToRRouter' (sonic-net#1569) \[submodules\]: update sonic-swss (sonic-net#1570) \[submodules\]: update sonic-utilities (sonic-net#1571) \[cfggen\]: ignore acl when its type is not defined (sonic-net#1568) \[installer\]: Umount before delete partition (sonic-net#1575) Use eth0 interface only to generate lldpd SystemId (sonic-net#1577) Allow one Service ACL to bind to multiple services (sonic-net#1576) \* \[caclmgrd\] Also ignore IP protocol if found in rule; we will only use our predefined protocols \[snmp\]: Bind snmpd to all ip addresses (sonic-net#1587) \[device\] Update Arista driver submodule (sonic-net#1585) Watchdog timeout increased \[devices\]: Fix type for qos.json in 7060 and S6100 (sonic-net#1582) \[minigraph\]: ignore minigraph ports which are not in port_config.ini (sonic-net#1593) \[minigraph\] Fix parser on PNG DeviceInterfaceLink Bandwidth (sonic-net#1592) \* \[minigraph\] Fix parser on PNG DeviceInterfaceLink Bandwidth \[Broadcom SAI\] upgrade Broadcom SAI to version 3.1.3.4-10 (sonic-net#1591) \* \[Broadcom SAI\] upgrade Broadcom SAI to version 3.1.3.4-9 Includes configuration files for following devices: \- Quanta 1X1B-32X \- Dell Z9264F \- Inventec D7054Q28B and D7032Q28B \* \[bcm sai\] upgrade sai version to 3.1.3.4-10 include configuration change to 7060 T0. 50G support for Arista 7060 (sonic-net#1580) \* 50G SKU for Arista 7060 Marvell's updates for SONiC 201803 over SAI v1.2 (sonic-net#1588) \[Mellanox\] Add support for a new platform LS-SN2700 \[devices\]: Merge ingress service pools of lossless and lossy traffic for TD2 (sonic-net#1578) \[sonic-utilities\] add pfcstat and queuestat tool (sonic-net#1606) Add support for S6100 switchport LEDs (sonic-net#1610) \[ip-in-ip\]: Fix config template to apply correct platform depended values (sonic-net#1619) \[platform-common\]: Update sonic-platform-common submodule (sonic-net#1620) \[sfputilbase\]: Add logic to parse the title of port_config.ini file \[sonic-cfggen\] Be case insensitive to hostname in minigraph (sonic-net#1614) \[bugfix\]: pass correct port name to led_control.py in ledd \[cfggen\]: Fix build by fixing pyangbind version (sonic-net#1633) \[swss\]: update sonic-swss submodule \* ea34b92 2018-04-24 | Fix tables handling race condition in buffermgr (sonic-net#484) (HEAD -> 201803, origin/201803) \[Andriy Moroz\] \* 53831be 2018-04-19 | \[pfcwd\]: create PFCWD acl instead of L3 ACL (sonic-net#479) \[sihuihan88\] \[radvd\] Ensure at least one interface is specified in radvd.conf before starting radvd (sonic-net#1636) \[updategraph\]: Keep updategraph service active after start (sonic-net#1651) \[docker-lldpd\]: Various fixes (sonic-net#1650) \* We don't need configure anything until we have interfaces created \* Don't run lldpcli for a port, until a port is up and running \* Remove lldpd socket before starting lldpd \* Fix sample files for lldpd configuration \* Another attempt to make the test working \* Quick fix for lldpd paused after start bug \[submodules\]: update swss and utilities modules swss: \* e34104e 2018-04-13 | \[pfcwd\]: support BIG_RED_SWITCH mode (sonic-net#467) (HEAD, origin/201803) \[sihuihan88\] \* 1f857d5 2018-04-25 | \[buffermgr\]: remove the item from consumer queue if invalid (sonic-net#489) \[sihuihan88\] utilities: \* 0b9bb2b 2018-04-26 | Stop services before pushing new config during "load_minigraph" (sonic-net#247) (HEAD, origin/201803) \[Prince Sunny\] \* dc119c9 2018-04-18 | \[show logging\] For following, change 'tail -f' to 'tail -F' in order to retry in the case log is rotated (sonic-net#240) \[Joe LeVeque\] \* 08da428 2018-04-16 | \[pfcwd\]: add cli to enable/disable BIG_RED_SWITCH mode (sonic-net#237) \[sihuihan88\] \[snmp\]: Fix a race between snmpd-config-updater and snmpd (sonic-net#1628) There is a small window in which snmpd might not have registered a callback for SIGHUP and which will result in its death if snmpd-config-updater send this signal meant for a config reload. \[snmpd\]: Fix typo in is_platform_arista (sonic-net#1634) \[mellanox\]: Update SAI version to 1.11.4 and SDK to 4.2.7303 (sonic-net#1655) \[docker-dhcp\]: Fix the sonic build issue (sonic-net#1659) Install the built version of isc-dhcp-client in docker-dhcp-relay \[swss\]: update swss 118b3f0 2018-05-01 | Populate existing interface cache, bring down before configDone \[zebra.conf\] Fix template issue with multiple lo addresses (sonic-net#1662) \* \[zebra.conf\] Fix template issue with multiple lo addresses \* Add unitest for Loopback1 \[swss\]: Change the hash seed to 0 for ToR and 10 for Leaf routers (sonic-net#1667) Due to some ASIC platform limitations, the hash seed range is from 0 to 15. Thus the switch.json.j2 template is updated so that ToRRouter is using hash seed 0 and LeafRouter is using hash seed 10. \[snmp\]: Stop spamming logs with statfs permission denied log message (sonic-net#1668) \[broadcom\]: update broadcom SAI to 3.1.3.4-11 (sonic-net#1670) Provide better ECMP load-balancing via hash seed \[sonic-cfggen\]: fix bgpd and zebra template for sonic-cfggen test I took the original patch (bebb7a0) into 201803 branch need to also adapt the patch since we do not have commit (d423841) in 201803 branch. \[swss\]: update sonic-swss module \[201803 d57f9a1\] \[lua\]: use not to check whether the field exists (sonic-net#492) \[device\] Update arista driver submodule (sonic-net#1674) \[submodule\]: Update submodule sonic-snmpagent (sonic-net#1642) sonic-cfggen supports hwsku parameter (sonic-net#1631) \*Note\*: tuned test data during cherry-pick \[device\] Add PSU utility for platform ly1200 of MiTAC (sonic-net#1673) \[platform/broadcom/sonic-platform-modules-mitac\] Install acpi package for daemon and adjust i2c sequence \[mellanox\]: Update MLNX SAI pointer (sonic-net#1684) 40G profile for Arista 7060 (sonic-net#1677) Update buffers config for Mellanox 27xx devices (sonic-net#1649) \* Update buffers config for Mellanox 27xx devices \* Remove buffers template test for msn27xx \[submodule\]: Update submodule sonic-snmpagent: Improve mib fundamental classes (sonic-net#1689) \[sonic-utilities\]: update sonic utilities submodule \* 951633b 2018-05-04 | \[generate_dump\]: fix a saidump file copy bug (sonic-net#248) (HEAD, origin/201803) \[Kebo Liu\] \* 69baff7 2018-05-03 | \[acl_loader\]: Missing one colon (sonic-net#252) \[Shuotian Cheng\] \* 557248d 2018-05-02 | \[acl-loader\]: Add --table_name option to update full operation (sonic-net#249) \[Shuotian Cheng\] \* a8aadee 2018-04-30 | \[acl-loader\]: Change the header from Rule ID to Table (sonic-net#250) \[Shuotian Cheng\] \[swss\]: update sonic-swss \* b57c376 2018-05-10 | \[teamsyncd\]: Add team_ifindex2ifname return value check (sonic-net#500) (HEAD, origin/201803) \[Shuotian Cheng\] \* 236843f 2018-05-07 | Fix Crm Acl used counter update (sonic-net#496) \[Nadiya\] \[swss\]: update sonic-swss c374357 2018-04-23 | Fix ZeroBufferProfile parameters (sonic-net#485) (HEAD -> 201803) \[Andriy Moroz\] \[platform\]: Fixed Cavium platform modules build. (sonic-net#1694) \[submodule\]: Update submodule sonic-snmpagent: Improve mib fundamental classes: retry after reinit_data() throws (sonic-net#1700) Merge branch 'github-1803' Conflicts: dockers/docker-router-advertiser/start.sh platform/broadcom/sai.mk platform/mellanox/mlnx-sai.mk src/sonic-config-engine/sonic-cfggen src/sonic-config-engine/tests/sample_output/ports.json src/sonic-config-engine/tests/test_cfggen.py src/sonic-platform-daemons src/sonic-snmpagent src/sonic-swss src/sonic-utilities \[baseimage\]: Disable DAD for eth0 explicitly (sonic-net#1701) \[quagga\]: update quagga submodule (sonic-net#1698) \* \[quagga\]: update quagga submodule 0bc6bd6 2018-05-11 | ignore nexthop attribute when NLRI is present (#18) (HEAD, origin/debian/0.99.24.1, origin/HEAD) \[lguohan\] \* add vs bgp test Fix the build error Revert "\[sonic-cfggen\]: fix bgpd and zebra template for sonic-cfggen test" This reverts commit b29d835. Fix the build issue for sonic-cfggen test Merge branch 'github-1803' --5/12 Conflicts: src/sonic-quagga src/sonic-snmpagent RB=1312391 G=lnos-reviewers R=pchaudha,pmao,rmolina,zxu A=
…t#489) * [buffermgr]: remove the item from consumer queue if invalid Signed-off-by: Sihui Han <sihan@microsoft.com> * change error to task_invalid_entry Signed-off-by: Sihui Han <sihan@microsoft.com>
…t#489) * [buffermgr]: remove the item from consumer queue if invalid Signed-off-by: Sihui Han <sihan@microsoft.com> * change error to task_invalid_entry Signed-off-by: Sihui Han <sihan@microsoft.com>
* fixed issue of permenent block on redisBufferRead() due to unprotected access from different threads syncd main thread was doing g_redisClient->hget without acquiring g_mutex, thereby causing either syncd main thread or notification thread to block indefinitely on redisBufferRead(). * Delete .DS_Store . * resolved merge conflict * Revert "resolved merge conflict" This reverts commit f0094914cb93249482ee79fc5d4db402aecf79d6. * Revert "Merge remote-tracking branch 'upstream/master'" This reverts commit 550aca906c4f0ccca0d03a95c96152e9a37abae1, reversing changes made to 676c0b58c4452478161d14a04140ed663127e91d. * Revert "Revert "Merge remote-tracking branch 'upstream/master'"" This reverts commit 03fc996e8c530505a3ad22b23e75ef2c77ebc668. * Revert "Revert "resolved merge conflict"" This reverts commit e840a6b0f4d2df1679d654796f4e303f73d98abe. * Update syncd.cpp * Update syncd.cpp * Update syncd.cpp * Update syncd.cpp
Signed-off-by: Sihui Han sihan@microsoft.com