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

[dhcp_relay] Only attempt to start 'isc-dhcp-relay' group if it is not empty #1713

Merged
merged 1 commit into from
May 16, 2018
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 12 additions & 9 deletions dockers/docker-dhcp-relay/start.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,16 @@ rm -f /var/run/rsyslogd.pid
# Start rsyslog
supervisorctl start rsyslogd

# Wait for all interfaces to come up and be assigned IPv4 addresses before
# starting the DHCP relay agent(s). If an interface the relay should listen
# on is down, the relay agent will not start. If an interface the relay should
# listen on is up but does not have an IP address assigned when the relay
# agent starts, it will not listen or send on that interface for the lifetime
# of the process.
/usr/bin/wait_for_intf.sh
# If our supervisor config has entries in the "isc-dhcp-relay" group...
if [ $(supervisorctl status | grep -c "^isc-dhcp-relay:") -gt 0 ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

if supervisorctl status | grep -q "^isc-dhcp-relay:"; then do the same, but faster

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm... "grep -q" actually seems slower for me...

root@sonic:/# time supervisorctl status | grep -c "^isc-dhcp-relay:"
1

real    0m0.443s
user    0m0.348s
sys     0m0.088s
root@sonic:/# time supervisorctl status | grep -q "^isc-dhcp-relay:"

real    0m0.470s
user    0m0.384s
sys     0m0.068s

Copy link
Contributor

Choose a reason for hiding this comment

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

:) when you write [ you run an external command in the bash script

$ which [
/usr/bin/[

So bash must to run subprocess to calculate the boolean for your if
Executing the subprocess is not so fast.
Faster would be use [[ which is internal bash instruction.
Or just use error code from grep directly as I wrote in the first comment.

# Wait for all interfaces to come up and be assigned IPv4 addresses before
# starting the DHCP relay agent(s). If an interface the relay should listen
# on is down, the relay agent will not start. If an interface the relay
# should listen on is up but does not have an IP address assigned when the
# relay agent starts, it will not listen or send on that interface for the
# lifetime of the process.
/usr/bin/wait_for_intf.sh

# Start the DHCP relay agent(s)
supervisorctl start isc-dhcp-relay:*
# Start all DHCP relay agent(s)
supervisorctl start isc-dhcp-relay:*
fi