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.

Sign up for GitHub

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

Jump to bottom

Revert frr service to systemd control and daemons to watchfrr control #3486

Closed
wants to merge 5 commits into from
Closed

Revert frr service to systemd control and daemons to watchfrr control #3486

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7bffe35
Merge pull request #11 from Azure/master
nikos-github Apr 24, 2019
bd77348
Merge pull request #12 from Azure/master
nikos-github Sep 19, 2019
39138cd
Revert frr service to systemd control and daemons to watchfrr control
Sep 19, 2019
143f8cf
Merge branch 'master' into frr-watchfrr
nikos-github Oct 16, 2019
82aeeeb
Fix services_start.sh after conflict resolution
Oct 16, 2019
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
5 changes: 3 additions & 2 deletions dockers/docker-fpm-frr/Dockerfile.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ RUN apt-get clean -y && \
apt-get autoremove -y && \
rm -rf /debs ~/.cache

COPY ["bgpcfgd", "start.sh", "/usr/bin/"]
COPY ["bgpcfgd", "start.sh", "services_start.sh", "/usr/bin/"]
COPY ["daemons", "/etc/frr/"]
COPY ["*.j2", "/usr/share/sonic/templates/"]
COPY ["supervisord.conf", "/etc/supervisor/conf.d/"]
COPY ["snmp.conf", "/etc/snmp/frr.conf"]
Expand All @@ -54,4 +55,4 @@ RUN chmod a+x /usr/bin/TSA && \
chmod a+x /usr/bin/TSB && \
chmod a+x /usr/bin/TSC

ENTRYPOINT ["/usr/bin/supervisord"]
ENTRYPOINT /usr/bin/services_start.sh && /usr/bin/supervisord
pavel-shirshov marked this conversation as resolved.
Show resolved Hide resolved
65 changes: 65 additions & 0 deletions dockers/docker-fpm-frr/daemons
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# This file tells the frr package which daemons to start.
#
# Sample configurations for these daemons can be found in
# /usr/share/doc/frr/examples/.
#
# ATTENTION:
#
# When activation a daemon at the first time, a config file, even if it is
# empty, has to be present *and* be owned by the user and group "frr", else
# the daemon will not be started by /etc/init.d/frr. The permissions should
# be u=rw,g=r,o=.
# When using "vtysh" such a config file is also needed. It should be owned by
# group "frrvty" and set to ug=rw,o= though. Check /etc/pam.d/frr, too.
nikos-github marked this conversation as resolved.
Show resolved Hide resolved
#
# The watchfrr and zebra daemons are always started.
#
bgpd=yes
ospfd=no
ospf6d=no
ripd=no
ripngd=no
isisd=no
pimd=no
ldpd=no
nhrpd=no
eigrpd=no
babeld=no
sharpd=no
pbrd=no
bfdd=no
fabricd=no
pavel-shirshov marked this conversation as resolved.
Show resolved Hide resolved

#
# If this option is set the /etc/init.d/frr script automatically loads
# the config via "vtysh -b" when the servers are started.
# Check /etc/pam.d/frr if you intend to use "vtysh"!
#
vtysh_enable=yes
zebra_options=" -A 127.0.0.1 -s 90000000 -M fpm -M snmp"
bgpd_options=" -A 127.0.0.1 -M snmp"
ospfd_options=" -A 127.0.0.1"
ospf6d_options=" -A ::1"
ripd_options=" -A 127.0.0.1"
ripngd_options=" -A ::1"
isisd_options=" -A 127.0.0.1"
pimd_options=" -A 127.0.0.1"
ldpd_options=" -A 127.0.0.1"
nhrpd_options=" -A 127.0.0.1"
eigrpd_options=" -A 127.0.0.1"
babeld_options=" -A 127.0.0.1"
sharpd_options=" -A 127.0.0.1"
pbrd_options=" -A 127.0.0.1"
staticd_options="-A 127.0.0.1"
bfdd_options=" -A 127.0.0.1"
fabricd_options="-A 127.0.0.1"

# The list of daemons to watch is automatically generated by the init script.
#watchfrr_options=""

# for debugging purposes, you can specify a "wrap" command to start instead
# of starting the daemon directly, e.g. to use valgrind on ospfd:
# ospfd_wrap="/usr/bin/valgrind"
# or you can use "all_wrap" for all daemons, e.g. to use perf record:
# all_wrap="/usr/bin/perf record --call-graph -"
# the normal daemon command is added to this at the end.
36 changes: 36 additions & 0 deletions dockers/docker-fpm-frr/services_start.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#!/usr/bin/env bash

mkdir -p /etc/frr

CONFIG_TYPE=`sonic-cfggen -d -v 'DEVICE_METADATA["localhost"]["docker_routing_config_mode"]'`

if [ "$CONFIG_TYPE" == "separated" ]; then
sonic-cfggen -d -y /etc/sonic/deployment_id_asn_map.yml -t /usr/share/sonic/templates/bgpd.conf.j2 > /etc/frr/bgpd.conf
sonic-cfggen -d -t /usr/share/sonic/templates/zebra.conf.j2 > /etc/frr/zebra.conf
sonic-cfggen -d -t /usr/share/sonic/templates/staticd.conf.j2 > /etc/frr/staticd.conf
echo "no service integrated-vtysh-config" > /etc/frr/vtysh.conf
rm -f /etc/frr/frr.conf
elif [ "$CONFIG_TYPE" == "unified" ]; then
sonic-cfggen -d -y /etc/sonic/deployment_id_asn_map.yml -t /usr/share/sonic/templates/frr.conf.j2 >/etc/frr/frr.conf
echo "service integrated-vtysh-config" > /etc/frr/vtysh.conf
rm -f /etc/frr/bgpd.conf /etc/frr/zebra.conf /etc/frr/staticd.conf
elif [ "$CONFIG_TYPE" == "split" ] || [ -z "$CONFIG_TYPE" ]; then
echo "service integrated-vtysh-config" > /etc/frr/vtysh.conf
rm -f /etc/frr/bgpd.conf /etc/frr/zebra.conf /etc/frr/staticd.conf
fi

sonic-cfggen -d -t /usr/share/sonic/templates/isolate.j2 > /usr/sbin/bgp-isolate
chown root:root /usr/sbin/bgp-isolate
chmod 0755 /usr/sbin/bgp-isolate

sonic-cfggen -d -t /usr/share/sonic/templates/unisolate.j2 > /usr/sbin/bgp-unisolate
chown root:root /usr/sbin/bgp-unisolate
chmod 0755 /usr/sbin/bgp-unisolate

mkdir -p /var/sonic
echo "# Config files managed by sonic-config-engine" > /var/sonic/config_status

rm -f /var/run/rsyslogd.pid

service rsyslog start
service frr start
Copy link
Collaborator

Choose a reason for hiding this comment

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

we already have supervised to start frr process, it is good to unified all process management inside the docker. We have unified way to manage process restart, docker restart, process crash messages. I do not really understand why we need to change that.

Copy link
Collaborator Author

@nikos-github nikos-github Oct 16, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I don't really understand either why we had to change from systemd in the first place. There was no functional requirement in the first place to change to supervisord. I don't see the goodness in unifying all process management inside the docker under supervisord especially since we ended up with multiple problems out of it. The routing application should be standalone and free of any such dependancies. It also doesn't need this unification. These changes were agreed in the August meeting we had and I sent meeting minutes. I also fail to understand why we are holding this PR as hostage when it fixes 5 blocking critical issues for a month now.

41 changes: 0 additions & 41 deletions dockers/docker-fpm-frr/start.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,45 +1,4 @@
#!/usr/bin/env bash

mkdir -p /etc/frr

CONFIG_TYPE=`sonic-cfggen -d -v 'DEVICE_METADATA["localhost"]["docker_routing_config_mode"]'`

if [ -z "$CONFIG_TYPE" ] || [ "$CONFIG_TYPE" == "separated" ]; then
sonic-cfggen -d -y /etc/sonic/deployment_id_asn_map.yml -t /usr/share/sonic/templates/bgpd.conf.j2 > /etc/frr/bgpd.conf
sonic-cfggen -d -t /usr/share/sonic/templates/zebra.conf.j2 > /etc/frr/zebra.conf
sonic-cfggen -d -t /usr/share/sonic/templates/staticd.conf.j2 > /etc/frr/staticd.conf
echo "no service integrated-vtysh-config" > /etc/frr/vtysh.conf
rm -f /etc/frr/frr.conf
elif [ "$CONFIG_TYPE" == "unified" ]; then
sonic-cfggen -d -y /etc/sonic/deployment_id_asn_map.yml -t /usr/share/sonic/templates/frr.conf.j2 >/etc/frr/frr.conf
echo "service integrated-vtysh-config" > /etc/frr/vtysh.conf
rm -f /etc/frr/bgpd.conf /etc/frr/zebra.conf /etc/frr/staticd.conf
fi

sonic-cfggen -d -t /usr/share/sonic/templates/isolate.j2 > /usr/sbin/bgp-isolate
chown root:root /usr/sbin/bgp-isolate
chmod 0755 /usr/sbin/bgp-isolate

sonic-cfggen -d -t /usr/share/sonic/templates/unisolate.j2 > /usr/sbin/bgp-unisolate
chown root:root /usr/sbin/bgp-unisolate
chmod 0755 /usr/sbin/bgp-unisolate

mkdir -p /var/sonic
echo "# Config files managed by sonic-config-engine" > /var/sonic/config_status

rm -f /var/run/rsyslogd.pid

supervisorctl start rsyslogd

supervisorctl start bgpcfgd

# Start Quagga processes
supervisorctl start zebra
supervisorctl start staticd
supervisorctl start bgpd

if [ "$CONFIG_TYPE" == "unified" ]; then
supervisorctl start vtysh_b
fi

supervisorctl start fpmsyncd
48 changes: 1 addition & 47 deletions dockers/docker-fpm-frr/supervisord.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,55 +21,9 @@ startsecs=0
stdout_logfile=syslog
stderr_logfile=syslog

[program:rsyslogd]
command=/usr/sbin/rsyslogd -n
priority=3
autostart=false
autorestart=false
startsecs=0
stdout_logfile=syslog
stderr_logfile=syslog

[program:zebra]
command=/usr/lib/frr/zebra -A 127.0.0.1 -s 90000000 -M fpm -M snmp
priority=4
autostart=false
autorestart=false
startsecs=0
stdout_logfile=syslog
stderr_logfile=syslog

[program:staticd]
command=/usr/lib/frr/staticd -A 127.0.0.1
priority=4
autostart=false
autorestart=false
startsecs=0
stdout_logfile=syslog
stderr_logfile=syslog

[program:bgpd]
command=/usr/lib/frr/bgpd -A 127.0.0.1 -M snmp
priority=5
stopsignal=KILL
autostart=false
autorestart=false
startsecs=0
stdout_logfile=syslog
stderr_logfile=syslog

[program:vtysh_b]
command=/usr/bin/vtysh -b
pavel-shirshov marked this conversation as resolved.
Show resolved Hide resolved
priority=6
autostart=false
autorestart=false
startsecs=0
stdout_logfile=syslog
stderr_logfile=syslog

[program:fpmsyncd]
command=fpmsyncd
priority=6
priority=3
autostart=false
autorestart=false
startsecs=0
Expand Down
1 change: 1 addition & 0 deletions files/build_templates/sonic_debian_extension.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,7 @@ sudo chmod 755 $FILESYSTEM_ROOT/usr/bin/mlnx-fw-upgrade.sh
sudo mkdir $FILESYSTEM_ROOT/etc/sonic/frr
sudo touch $FILESYSTEM_ROOT/etc/sonic/frr/frr.conf
sudo touch $FILESYSTEM_ROOT/etc/sonic/frr/vtysh.conf
sudo cp dockers/docker-fpm-frr/daemons $FILESYSTEM_ROOT/etc/sonic/frr/
sudo chown -R $FRR_USER_UID:$FRR_USER_GID $FILESYSTEM_ROOT/etc/sonic/frr
sudo chmod -R 640 $FILESYSTEM_ROOT/etc/sonic/frr/
sudo chmod 750 $FILESYSTEM_ROOT/etc/sonic/frr
Expand Down
4 changes: 2 additions & 2 deletions rules/docker-fpm-frr.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ DOCKER_FPM_FRR_DBG = $(DOCKER_FPM_FRR_STEM)-$(DBG_IMAGE_MARK).gz

$(DOCKER_FPM_FRR)_PATH = $(DOCKERS_PATH)/$(DOCKER_FPM_FRR_STEM)

$(DOCKER_FPM_FRR)_DEPENDS += $(FRR) $(FRR_SNMP) $(SWSS) $(LIBYANG)
$(DOCKER_FPM_FRR)_DEPENDS += $(FRR) $(FRR_SNMP) $(FRR_PYTHONTOOLS) $(SWSS) $(LIBYANG)
nikos-github marked this conversation as resolved.
Show resolved Hide resolved
$(DOCKER_FPM_FRR)_DBG_DEPENDS = $($(DOCKER_CONFIG_ENGINE_STRETCH)_DBG_DEPENDS)
$(DOCKER_FPM_FRR)_DBG_DEPENDS += $(SWSS_DBG) $(LIBSWSSCOMMON_DBG) \
$(FRR_DBG) $(FRR_SNMP_DBG) $(LIBYANG_DBG)
$(FRR_DBG) $(FRR_SNMP_DBG) $(FRR_PYTHONTOOLS) $(LIBYANG_DBG)

$(DOCKER_FPM_FRR)_DBG_IMAGE_PACKAGES = $($(DOCKER_CONFIG_ENGINE_STRETCH)_DBG_IMAGE_PACKAGES)

Expand Down