From 42ae02b66503a1eb10bcea59f574e3829cf6ef45 Mon Sep 17 00:00:00 2001 From: Taoyu Li Date: Wed, 22 Mar 2017 13:04:48 -0700 Subject: [PATCH] [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. --- files/build_templates/bgp.service.j2 | 3 ++- files/build_templates/database.service.j2 | 3 ++- files/build_templates/dhcp_relay.service.j2 | 3 ++- files/build_templates/docker_image_ctl.j2 | 12 ++++++++---- files/build_templates/lldp.service.j2 | 3 ++- files/build_templates/pmon.service.j2 | 3 ++- files/build_templates/snmp.service.j2 | 3 ++- files/build_templates/swss.service.j2 | 6 +++--- files/build_templates/teamd.service.j2 | 3 ++- 9 files changed, 25 insertions(+), 14 deletions(-) diff --git a/files/build_templates/bgp.service.j2 b/files/build_templates/bgp.service.j2 index 7fa3c0eb11..c7ba8b8a5f 100644 --- 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 index fd4f34435e..c353653e45 100644 --- 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 index 95d66c605f..c0e993eec0 100644 --- 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 index be62b43515..10679765eb 100644 --- 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 index f66e9d682a..1ddda15a49 100644 --- 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 index d50f5be628..9f1a029824 100644 --- 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 index 3744c58689..493d6bd8fc 100644 --- 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 index 3ec8161fa0..7466e8c145 100644 --- 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 index 49ecb640d8..e291a39b38 100644 --- 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]