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 7fa3c0eb1150..c7ba8b8a5f52 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 fd4f34435e6a..c353653e4562 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 95d66c605f85..c0e993eec065 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 be62b4351556..10679765eb3a 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 f66e9d682af6..1ddda15a49c4 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 d50f5be628de..9f1a0298240c 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 3744c586896f..493d6bd8fcfd 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 3ec8161fa00a..7466e8c14577 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 49ecb640d895..e291a39b387d 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]