[oneimage]: Fix race condition in systemd container services (#421)

When Type=simple, systemd will consider the service activated immediately after specified in ExecStart process is started. If there is downstream service depending on the state prepared in ExecStart, there will be race condition. For example, issue #390. In this case, database.service calls database.sh, which calls docker run or docker start -a to start database container. However, systemd considers database.service successfully started at the time database.sh begins, not after docker run finishes. As database.service is consider started, bgp.service can be started. The redis database, which bgp service depends on, might or might not have been started at this time point. To fix this issue (and still keeping the functionality to monitor docker status with systemd), we split the ExecStart process into an ExecStartPre part and an ExecStart part. docker run is splitted into docker run -d then docker attach , while docker start -a is splitted into docker start and then docker attach. In this way, we make sure the downstream services are blocked until container is successfully started.
sonic-net · Mar 22, 2017 · 42ae02b · 42ae02b
1 parent bd77f9a
commit 42ae02b
Show file tree

Hide file tree

Showing 9 changed files with 25 additions and 14 deletions.
diff --git a/files/build_templates/bgp.service.j2 b/files/build_templates/bgp.service.j2
@@ -5,7 +5,8 @@ After=database.service
 
 [Service]
 User={{ sonicadmin_user }}
-ExecStart=/usr/bin/{{docker_container_name}}.sh start
+ExecStartPre=/usr/bin/{{docker_container_name}}.sh start
+ExecStart=/usr/bin/{{docker_container_name}}.sh attach
 ExecStop=/usr/bin/{{docker_container_name}}.sh stop
 
 [Install]

diff --git a/files/build_templates/database.service.j2 b/files/build_templates/database.service.j2
@@ -5,7 +5,8 @@ After=docker.service
 
 [Service]
 User={{ sonicadmin_user }}
-ExecStart=/usr/bin/{{docker_container_name}}.sh start
+ExecStartPre=/usr/bin/{{docker_container_name}}.sh start
+ExecStart=/usr/bin/{{docker_container_name}}.sh attach
 ExecStop=/usr/bin/{{docker_container_name}}.sh stop
 
 [Install]

diff --git a/files/build_templates/dhcp_relay.service.j2 b/files/build_templates/dhcp_relay.service.j2
@@ -5,7 +5,8 @@ After=interfaces-config.service
 
 [Service]
 User={{ sonicadmin_user }}
-ExecStart=/usr/bin/{{ docker_container_name }}.sh start
+ExecStartPre=/usr/bin/{{ docker_container_name }}.sh start
+ExecStart=/usr/bin/{{ docker_container_name }}.sh attach
 ExecStop=/usr/bin/{{ docker_container_name }}.sh stop
 
 [Install]

diff --git a/files/build_templates/docker_image_ctl.j2 b/files/build_templates/docker_image_ctl.j2
@@ -7,25 +7,29 @@ HWSKU=`sonic-cfggen -m /etc/sonic/minigraph.xml -v minigraph_hwsku`
 start() {
     docker inspect --type container {{docker_container_name}} &>/dev/null
     if [ "$?" -eq "0" ]; then
-        docker start -a {{docker_container_name}}
+        docker start {{docker_container_name}}
     else
-        docker run {{docker_image_run_opt}} \
+        docker run -d {{docker_image_run_opt}} \
             -v /usr/share/sonic/device/$PLATFORM:/usr/share/sonic/platform:ro \
             -v /usr/share/sonic/device/$PLATFORM/$HWSKU:/usr/share/sonic/hwsku:ro \
             --name={{docker_container_name}} {{docker_image_name}}
     fi
 }
 
+attach() {
+    docker attach --no-stdin {{docker_container_name}}
+}
+
 stop() {
     docker stop {{docker_container_name}}
 }
 
 case "$1" in
-    start|stop)
+    start|stop|attach)
         $1
         ;;
     *)
-        echo "Usage: $0 {start|stop}"
+        echo "Usage: $0 {start|stop|attach}"
         exit 1
         ;;
 esac

diff --git a/files/build_templates/lldp.service.j2 b/files/build_templates/lldp.service.j2
@@ -5,7 +5,8 @@ After=database.service
 
 [Service]
 User={{ sonicadmin_user }}
-ExecStart=/usr/bin/{{docker_container_name}}.sh start
+ExecStartPre=/usr/bin/{{docker_container_name}}.sh start
+ExecStart=/usr/bin/{{docker_container_name}}.sh attach
 ExecStop=/usr/bin/{{docker_container_name}}.sh stop
 
 [Install]

diff --git a/files/build_templates/pmon.service.j2 b/files/build_templates/pmon.service.j2
@@ -5,7 +5,8 @@ After=database.service
 
 [Service]
 User={{ sonicadmin_user }}
-ExecStart=/usr/bin/{{docker_container_name}}.sh start
+ExecStartPre=/usr/bin/{{docker_container_name}}.sh start
+ExecStart=/usr/bin/{{docker_container_name}}.sh attach
 ExecStop=/usr/bin/{{docker_container_name}}.sh stop
 
 [Install]

diff --git a/files/build_templates/snmp.service.j2 b/files/build_templates/snmp.service.j2
@@ -5,7 +5,8 @@ After=database.service
 
 [Service]
 User={{ sonicadmin_user }}
-ExecStart=/usr/bin/{{docker_container_name}}.sh start
+ExecStartPre=/usr/bin/{{docker_container_name}}.sh start
+ExecStart=/usr/bin/{{docker_container_name}}.sh attach
 ExecStop=/usr/bin/{{docker_container_name}}.sh stop
 
 [Install]

diff --git a/files/build_templates/swss.service.j2 b/files/build_templates/swss.service.j2
@@ -20,9 +20,9 @@ ExecStartPre=-/etc/init.d/xpnet.sh stop
 ExecStartPre=/etc/init.d/xpnet.sh start
 {% endif %}
 
-# systemd allows only one parent process within service,
-# so we spawn both dockers from single bash parent
-ExecStart=/bin/bash -c "/usr/bin/{{docker_container_name}}.sh start & /usr/bin/syncd.sh start & wait -n 0"
+ExecStartPre=/usr/bin/{{docker_container_name}}.sh start 
+ExecStartPre=/usr/bin/syncd.sh start
+ExecStart=/usr/bin/{{docker_container_name}}.sh attach
 
 ExecStop=/usr/bin/{{docker_container_name}}.sh stop
 ExecStopPost=/usr/bin/syncd.sh stop

diff --git a/files/build_templates/teamd.service.j2 b/files/build_templates/teamd.service.j2
@@ -5,7 +5,8 @@ After=database.service
 
 [Service]
 User={{ sonicadmin_user }}
-ExecStart=/usr/bin/{{docker_container_name}}.sh start
+ExecStartPre=/usr/bin/{{docker_container_name}}.sh start
+ExecStart=/usr/bin/{{docker_container_name}}.sh attach
 ExecStop=/usr/bin/{{docker_container_name}}.sh stop
 
 [Install]