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

[build_template]: combine the init config write into one block #1513

Merged
merged 1 commit into from
Mar 20, 2018

Conversation

sihuihan88
Copy link
Contributor

Signed-off-by: Sihui Han sihan@microsoft.com

- What I did
Combine the init config write into one block. Otherwise it will result in wrong init config format when multiple build options are enabled.
- How I did it

- How to verify it
Test on DUT
- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

Signed-off-by: Sihui Han <sihan@microsoft.com>
{% if enable_pfcwd_on_start == "y" %}
sudo bash -c "echo '{ \"DEVICE_METADATA\": { \"localhost\": { \"default_pfcwd_status\": \"enable\" } } }' >> $FILESYSTEM_ROOT/etc/sonic/init_cfg.json"
{% endif %}
sudo bash -c "echo '{ \"DEVICE_METADATA\": { \"localhost\": { \"default_bgp_status\": {% if shutdown_bgp_on_start == "y" %}\"down\"{% else %}\"up\"{% endif %}, \"default_pfcwd_status\": {% if enable_pfcwd_on_start == "y" %}\"enable\"{% else %}\"disable\"{% endif %} } } }' >> $FILESYSTEM_ROOT/etc/sonic/init_cfg.json"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we continue appending (>>) here, or should we simply write a new file (>)?

@taoyl-ms: Do you have an opinion on this?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would vote for '>'. But as it now, '>>' didn't change existing behavior, @jleveque if @taoyl-ms cannot response in time, do you mind check-in as-is and improve later?

Copy link
Contributor

@taoyl-ms taoyl-ms Mar 20, 2018

Choose a reason for hiding this comment

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

It should not matter as this file should not exist upon this moment. Personally, I prefer ">>" because if anything goes wrong, it will expose the problem by creating an illegal file, instead of hiding it by overwritting.

Copy link
Contributor

@taoyl-ms taoyl-ms Mar 20, 2018

Choose a reason for hiding this comment

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

Like, for this particular bug, if we were using '>' instead of '>>', we won't see an illegal json, but BGP startup setting will simply be missing, which will be even more difficult to identify and to debug.

@yxieca yxieca merged commit 6d592d8 into sonic-net:master Mar 20, 2018
@sihuihan88 sihuihan88 deleted the dev/sihan/init branch March 20, 2018 17:52
Sabareesh-Kumar-Anandan pushed a commit to Sabareesh-Kumar-Anandan/sonic-buildimage that referenced this pull request Dec 20, 2020
[buffermgmt] more build error fixes when compiling for armhf (32-bit) (sonic-net#1559)
Sflow fix to avoid NULL in field. (sonic-net#1531)
[fgnhgorch] Fg Nhg link handling (sonic-net#1537)
[dpb]: make sure port is in admin down state before remove port. (sonic-net#1513)
[FPMSYNCD/FDBSYNCD] EVPN Type-5 route removing prefix-len for host route
and removing junk character present in the mac (sonic-net#1553)
Added support for EVPN L3 VXLAN as described in the PR sonic-net/SONiC#437 (sonic-net#1267)

Signed-off-by: Sabareesh Kumar Anandan <sanandan@marvell.com>
jleveque added a commit that referenced this pull request Dec 24, 2020
* src/sonic-swss c7ee75f...cadf28f (24):
  > Revert "Add support for headroom pool watermark (#1453)"
  > [VxlanOrch] pytest for EVPN VXLAN (#1318)
  > [restore_neighbors] python3 support for restore_neighbors.py (#1542)
  > [buffermgmt] more build error fixes when compiling for armhf (32-bit) (#1559)
  > Sflow fix to avoid NULL in field. (#1531)
  > [fgnhgorch] Fg Nhg link handling (#1537)
  > [dpb]: make sure port is in admin down state before remove port. (#1513)
  > [FPMSYNCD/FDBSYNCD] EVPN Type-5 route removing prefix-len for host route and removing junk character present in the mac (#1553)
  > Added support for EVPN L3 VXLAN as described in the PR sonic-net/SONiC#437 (#1267)
  > [crm]: Typecast to unit64_t to avoid divide by 0 during overflow (#1550)
  > [vxlanmgr] Fix build error when compiling for armhf (32-bit) (#1552)
  > [Dynamic buffer calc]  Support dynamic buffer calculation (#1338)
  > [dvs] Clean-up dvs_database and dvs_common (#1541)
  > [VxlanMgr] changes for EVPN VXLAN (#1266)
  > Statistics support for Tx and Rx counters of different frame sizes (#1536)
  > [orchagent/phy]: Add firmware info propagation (#1540)
  > [vxlanorch] Use PRI instead of %l to avoid warnings in 32-bit arch (#1539)
  > [FDBSYNCD] Added support for EVPN as described in the PR sonic-net/SONiC#437 (#1276)
  > [everflow] Add retry mechanism for mirror sessions and policers (#1486)
  > Enable ACL table type  mirror_v6 for Innovium Platform (#1527)
  > [fgnhgorch] Change format specifier %lu to %zu for size_t (#1529)
  > [dvs] Fix issue where concurrent netns operations cause test setup to fail (#1535)
  > Add support for headroom pool watermark (#1453)
  > Change gAsicInstance to type string with max length limit (#1526)
theasianpianist pushed a commit to theasianpianist/sonic-buildimage that referenced this pull request Feb 5, 2022
…ic-net#1513)

Bring down the port before remove. 

During DPB SET request to bring down the port may get replaced DEL. Make sure to bring down the port before remove.
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.

4 participants