-
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
[dhcp_relay]: Check whether device requires DHCP relay upon docker start #371
Conversation
dockers/docker-dhcp-relay/config.sh
Outdated
DEVICE_ROLE=`sonic-cfggen -m /etc/sonic/minigraph.xml -v "minigraph_devices['$MINIGRAPH_HOSTNAME']['type']"` | ||
|
||
if [ $DEVICE_ROLE != "ToRRouter" ]; then | ||
echo "Device does not require DHCP relay. docker-dhcp-relay exiting..." |
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.
if the script exits here, will the docker exits?
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 think if you return a nonzero value, it will.
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.
Taoyu is correct. I have tested the behavior, and exiting with a nonzero value will cause the docker container to exit, whereas exiting with a return value of 0 will cause the container to keep running.
dockers/docker-dhcp-relay/config.sh
Outdated
sonic-cfggen -m /etc/sonic/minigraph.xml -y /etc/sonic/dhcp_relay.yml -t /usr/share/sonic/templates/isc-dhcp-relay.j2 > /etc/default/isc-dhcp-relay | ||
MINIGRAPH_HOSTNAME=`sonic-cfggen -m /etc/sonic/minigraph.xml -v "minigraph_hostname"` | ||
DEVICE_ROLE=`sonic-cfggen -m /etc/sonic/minigraph.xml -v "minigraph_devices['$MINIGRAPH_HOSTNAME']['type']"` | ||
|
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.
you can directly use -v "minigraph_devices[minigraph_hostname]['type']", don't need to call it twice.
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.
Good to know. Simplified in 0fdd3c8. Thanks!
should we do this in dhcprelay service unit file? if we detect it is not tor, we do not even start the container. |
It would be cleaner to have this logic outside the container, but systemd provides a limited set of conditionals to work with in the unit files, none of which I'm aware of allow us to directly check the device type from the minigraph. Do you have an idea in mind? |
How about doing it in /usr/bin/dhcp_relay.sh instead of in the service definition file? You can check minigraph there and if the switch is not ToR you just do nothing and tell systemd you succeeded to start the service. |
Thanks for the idea, but after speaking with Guohan and Shuotian, we decided the service shouldn't even be started if not needed. This also applies to teamd, and may apply to other services in the future. I will soon submit another solution that disables the services on first boot. |
If you are going to disable the service on first boot, please make sure your disabling happens after update_graph service so we don't mistakenly disable it based on a default graph. |
Thanks for the heads-up! :) |
b9ca7e2
to
42ae02b
Compare
Abandoning in favor of #427 |
sairedis * 248a095 2018-11-07 | Add best candidate search for acl table (sonic-net#371) [Kamil Cudnik] * d1e26c3 2018-11-07 | Ignore order when compare QOS MAP list entries (sonic-net#372) [Kamil Cudnik] * e8df347 2018-11-05 | Add best candidate search for router interface using tunnel term tabl… (sonic-net#370) [Kamil Cudnik] * 8ae173b 2018-11-01 | Refactor saidump graph generator (sonic-net#367) [Kamil Cudnik] common * 2a37c5c 2018-11-07 | Add system warm-reboot check in WarmRestart class (sonic-net#246) (HEAD, origin/master, origin/HEAD) [zhenggen-xu] * fb082b5 2018-11-06 | Fix ProducerStateTable::clear() to clear StateHash properly (sonic-net#244) [Taoyu Li] * 0ee636b 2018-11-05 | Remove local googletest submodule and link with system gtest (sonic-net#245) [Qi Luo] * b56ffba 2018-10-31 | Add NEIGH_RESTORE_TABLE in stateDB (sonic-net#243) [zhenggen-xu] * b58c69d 2018-10-28 | swss-common: add op 'create' case to avoid flush immediately (sonic-net#219) [Dong Zhang] swss * afdcf34 2018-11-12 | Support neighsyncd system warmreboot. (sonic-net#661) (HEAD, origin/master, origin/HEAD) [zhenggen-xu] * f380685 2018-11-09 | Routing-stack warm-reboot feature. (sonic-net#602) [Rodny Molina] * 9fbcb60 2018-11-09 | Add warm-reboot mode for teammgrd (sonic-net#678) [pavel-shirshov] * 8c60787 2018-11-09 | Don't add loopback ip2me route again if already configured (sonic-net#656) [Jipan Yang] * 6eb1613 2018-11-08 | [test]: Add interface IPv6 add/remove test case (sonic-net#677) [Shuotian Cheng] * 5de5054 2018-11-07 | [vrfmgrd]: Support VNET setting (sonic-net#657) [Marian Pritsak] * f666011 2018-11-06 | [teammgrd]: Add retry logic when enslaving member port into team (sonic-net#669) [Shuotian Cheng] * 36e304d 2018-11-05 | [vstest]: print output when runcmd returns error (sonic-net#672) [lguohan] * aede5d4 2018-11-05 | [test]: Fix clean up wrong interface IP (sonic-net#673) [Shuotian Cheng] * aeceaca 2018-11-02 | [portmgrd]: portmgrd shall be responsible for all ports update (sonic-net#668) [Shuotian Cheng] * 5796e54 2018-11-02 | Orchagent warm restart data restore requires three iterations (sonic-net#670) [Jipan Yang] * 15a2299 2018-11-01 | [vlanmgrd]: Do not bring up VLAN members by default (sonic-net#667) [Shuotian Cheng] * 44a4460 2018-11-01 | [test]: Clean up LAGs after finishing the test (sonic-net#666) [Shuotian Cheng] Signed-off-by: Guohan Lu <gulv@microsoft.com>
sairedis * 248a095 2018-11-07 | Add best candidate search for acl table (#371) [Kamil Cudnik] * d1e26c3 2018-11-07 | Ignore order when compare QOS MAP list entries (#372) [Kamil Cudnik] * e8df347 2018-11-05 | Add best candidate search for router interface using tunnel term tabl… (#370) [Kamil Cudnik] * 8ae173b 2018-11-01 | Refactor saidump graph generator (#367) [Kamil Cudnik] common * 2a37c5c 2018-11-07 | Add system warm-reboot check in WarmRestart class (#246) (HEAD, origin/master, origin/HEAD) [zhenggen-xu] * fb082b5 2018-11-06 | Fix ProducerStateTable::clear() to clear StateHash properly (#244) [Taoyu Li] * 0ee636b 2018-11-05 | Remove local googletest submodule and link with system gtest (#245) [Qi Luo] * b56ffba 2018-10-31 | Add NEIGH_RESTORE_TABLE in stateDB (#243) [zhenggen-xu] * b58c69d 2018-10-28 | swss-common: add op 'create' case to avoid flush immediately (#219) [Dong Zhang] swss * afdcf34 2018-11-12 | Support neighsyncd system warmreboot. (#661) (HEAD, origin/master, origin/HEAD) [zhenggen-xu] * f380685 2018-11-09 | Routing-stack warm-reboot feature. (#602) [Rodny Molina] * 9fbcb60 2018-11-09 | Add warm-reboot mode for teammgrd (#678) [pavel-shirshov] * 8c60787 2018-11-09 | Don't add loopback ip2me route again if already configured (#656) [Jipan Yang] * 6eb1613 2018-11-08 | [test]: Add interface IPv6 add/remove test case (#677) [Shuotian Cheng] * 5de5054 2018-11-07 | [vrfmgrd]: Support VNET setting (#657) [Marian Pritsak] * f666011 2018-11-06 | [teammgrd]: Add retry logic when enslaving member port into team (#669) [Shuotian Cheng] * 36e304d 2018-11-05 | [vstest]: print output when runcmd returns error (#672) [lguohan] * aede5d4 2018-11-05 | [test]: Fix clean up wrong interface IP (#673) [Shuotian Cheng] * aeceaca 2018-11-02 | [portmgrd]: portmgrd shall be responsible for all ports update (#668) [Shuotian Cheng] * 5796e54 2018-11-02 | Orchagent warm restart data restore requires three iterations (#670) [Jipan Yang] * 15a2299 2018-11-01 | [vlanmgrd]: Do not bring up VLAN members by default (#667) [Shuotian Cheng] * 44a4460 2018-11-01 | [test]: Clean up LAGs after finishing the test (#666) [Shuotian Cheng] Signed-off-by: Guohan Lu <gulv@microsoft.com>
…t#358)" (sonic-net#371) This reverts commit 8764902.
…onic-net#358)" (sonic-net#371)" (sonic-net#388) This reverts commit 8fc09d0.
… automatically (#15133) src/sonic-platform-common * ff72811 - (HEAD -> 202205, origin/202205) Fix issue '<' not supported between instances of 'NoneType' and 'int' (#371) (5 hours ago) [Junchao-Mellanox] * f2a419d - Render Media lane and Media assignment options info from Application Code (#368) (8 hours ago) [rajann] * d8bad10 - Retrieve FW version using CDB command for CMIS transceivers + handle single bank FW versioning (#372) (8 hours ago) [mihirpat1]
…D automatically (#15366) src/sonic-platform-daemons * 18815c7 - (HEAD -> 202205, origin/202205) Revert "[ycabled] refactor code for onboarding async client changes;refactor (#355)" (3 minutes ago) [Ying Xie] * 5324554 - Revert "add async notification support in active-active topo; refactor code for ycable tasks for change events (#327)" (3 minutes ago) [Ying Xie] * cbbe2b5 - Revert "[ycabled] fix bug for `show mux status` delayed response (#364)" (3 minutes ago) [Ying Xie] * 9746709 - Revert "[dualtor] Fix command `show mux status` (#371)" (3 minutes ago) [Ying Xie] * 551ab3c - Revert "[ycabled] correct the wrong function call for 'config hwmode state' (#372)" (3 minutes ago) [Ying Xie]
…D automatically (#15749) src/sonic-platform-daemons * 112656c - (HEAD -> 202205, origin/202205) [ycabled][active-active] no initialize Async Client, when no active-active cable type; fix names for all ycabled threads (#373) (4 minutes ago) [vdahiya12] * e325d5a - Revert "Revert "[ycabled] correct the wrong function call for 'config hwmode state' (#372)"" (4 minutes ago) [Ying Xie] * ddabca1 - Revert "Revert "[dualtor] Fix command `show mux status` (#371)"" (4 minutes ago) [Ying Xie] * 28918da - Revert "Revert "[ycabled] fix bug for `show mux status` delayed response (#364)"" (4 minutes ago) [Ying Xie] * a849de9 - Revert "Revert "add async notification support in active-active topo; refactor code for ycable tasks for change events (#327)"" (4 minutes ago) [Ying Xie] * cf1e73a - Revert "Revert "[ycabled] refactor code for onboarding async client changes;refactor (#355)"" (4 minutes ago) [Ying Xie]
…tomatically (#18433) #### Why I did it src/sonic-linux-kernel ``` * 59e5ea1 - (HEAD -> 202311, origin/202311) Disable small sector erase size for UBIFS on flash (#382) (4 hours ago) [Mridul Bajpai] * ed95e5d - [202311] Dynamic write timeout support for optoe driver (#371) (8 hours ago) [mihirpat1] ``` #### How I did it #### How to verify it #### Description for the changelog
[code sync] Merge code from sonic-net/sonic-buildimage:202205 to 202205
If not, log a message and exit docker. Other conditions can be added in the future as needed.
With OneImage, all dockers must be deployed, but DHCP relay may not be needed. It's cleaner to have docker exit than to continue running and serving no purpose. Log message is helpful to understand why.
Signed-off-by: Joe LeVeque jolevequ@microsoft.com