Skip to content
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

remove minlink initial value while fallback is define #4090

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

remove minlink initial value while fallback is define #4090

wants to merge 2 commits into from

Conversation

twtseng-tim
Copy link

@twtseng-tim twtseng-tim commented Jan 31, 2020

- What I did
remove minlink initial value while fallback is define [#2053]
- How I did it

- How to verify it
add <Fallback>true</Fallback> in minigraph.xml where defined Portchannel
Do: config load_minigraph -y

- Description for the changelog

before this change I got following error message:

Running command: /usr/local/bin/sonic-cfggen -H -m -j /etc/sonic/init_cfg.json--write-to-db
Traceback (most recent call last):
  File "/usr/local/bin/sonic-cfggen", line 280, in <module>
    main()
  File "/usr/local/bin/sonic-cfggen", line 212, in main
    deep_update(data, parse_xml(minigraph, platform))
  File "/usr/local/lib/python2.7/dist-packages/minigraph.py", line 562, in parse_xml
    (intfs, lo_intfs, mgmt_intf, vlans, vlan_members, pcs, pc_members, acls, vni) = parse_dpg(child, hostname)
  File "/usr/local/lib/python2.7/dist-packages/minigraph.py", line 206, in parse_dpg
    pcs[pcintfname] = {'members': pcmbr_list, 'fallback': pcintf.find(str(QName(ns, "Fallback"))).text, 'min_links': str(int(math.ceil(len() * 0.75)))}
TypeError: len() takes exactly one argument (0 given)

after this change it can load normally:

~$ sudo config load_minigraph -y
Running command: systemctl stop dhcp_relay
Running command: systemctl stop swss
Running command: systemctl stop snmp
Warning: Stopping snmp.service, but it can still be activated by:
  snmp.timer
Running command: systemctl stop lldp
Running command: systemctl stop pmon
Running command: systemctl stop bgp
Running command: systemctl stop teamd
Running command: systemctl stop hostcfgd
Running command: /usr/local/bin/sonic-cfggen -H -m -j /etc/sonic/init_cfg.json --write-to-db
Running command: pfcwd start_default
Running command: config qos reload
Buffer definition template not found at /usr/share/sonic/device/x86_64-accton_as7816_64x-r0/Accton-AS7816-64X/buffers.json.j2

Running command: systemctl restart hostname-config
Running command: systemctl restart interfaces-config
Running command: systemctl restart ntp-config
Running command: systemctl restart rsyslog-config
Running command: systemctl restart swss
Running command: systemctl restart bgp
Running command: systemctl restart teamd
Running command: systemctl restart pmon
Running command: systemctl restart lldp
Running command: systemctl restart snmp
Running command: systemctl restart dhcp_relay
Running command: systemctl restart hostcfgd
Please note setting loaded from minigraph will be lost after system reboot. To preserve setting, run `config save`.
**- A picture of a cute animal (not mandatory but encouraged)**

chenkelly
chenkelly previously approved these changes Feb 7, 2020
@twtseng-tim
Copy link
Author

Hi, @hzheng5 @stcheng , should i do any modify for pass this pr.

@twtseng-tim
Copy link
Author

Hi, @hzheng5 @stcheng , Is there any suggestion for this pr?

@@ -221,7 +221,7 @@ def parse_dpg(dpg, hname):
intfs_inpc.append(pcmbr_list[i])
pc_members[(pcintfname, pcmbr_list[i])] = {'NULL': 'NULL'}
if pcintf.find(str(QName(ns, "Fallback"))) != None:
pcs[pcintfname] = {'members': pcmbr_list, 'fallback': pcintf.find(str(QName(ns, "Fallback"))).text, 'min_links': str(int(math.ceil(len() * 0.75)))}
pcs[pcintfname] = {'members': pcmbr_list, 'fallback': pcintf.find(str(QName(ns, "Fallback"))).text}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should put pcmbr_list as len parameter here.

Copy link
Author

@twtseng-tim twtseng-tim May 19, 2020

Choose a reason for hiding this comment

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

I see here #2053 (comment) decided to remove this parameter
fallback support multi-member now?

@pavel-shirshov
Copy link
Contributor

sonic-net/sonic-swss#951

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants