fix: fix a race condition preventing Docker metrics from being collected correctly (#116)

* fix: start monitor in `init` cgroup to avoid race condition

* refactor: move cgroup manipulation to the top

* fix: disable `containerd` image store on cgroup v1

Author: masontikhonov

run.sh

@@ -1,8 +1,38 @@
 #!/bin/bash
 
+echo "Entering $0 at $(date) "
+
+# It is required to keep this block at the top of the script!
+# All processes started by this script must be moved to `./init` group on cgroup v2.
+if [ ! -f /sys/fs/cgroup/cgroup.controllers ]; then
+  echo "Using cgroup v1"
+else
+  echo "Using cgroup v2"
+  CURRENT_CGROUP=$(cat /proc/self/cgroup | sed 's/0:://')
+  CURRENT_CGROUP_PATH="/sys/fs/cgroup/${CURRENT_CGROUP}"
+  echo "Current cgroup: ${CURRENT_CGROUP}"
+
+  # Move the processes from the current group to the `./init` group,
+  # otherwise the current group will become of type "domain threaded",
+  # and it will not be possible to enable required controllers for DinD group.
+  # Ref: https://github.com/moby/moby/blob/38805f20f9bcc5e87869d6c79d432b166e1c88b4/hack/dind#L28-L38
+  echo "Creating init cgroup ${CURRENT_CGROUP_PATH}/init"
+  mkdir -p ${CURRENT_CGROUP_PATH}/init
+  echo "Moving existing processes from ${CURRENT_CGROUP_PATH} to ${CURRENT_CGROUP_PATH}/init"
+  xargs -rn1 < ${CURRENT_CGROUP_PATH}/cgroup.procs > ${CURRENT_CGROUP_PATH}/init/cgroup.procs || :
+  echo "Done moving existing processes from ${CURRENT_CGROUP_PATH} to ${CURRENT_CGROUP_PATH}/init"
+
+  # Set `memory.oom.group=0` to disable killing all processes in cgroup at once on OOM.
+  # if all processes are killed at once, the system will not be able to detect this event;
+  # instead, we expect separate pipeline steps to be killed if total consumptions exceed limits.
+  MEMORY_OOM_GROUP="${CURRENT_CGROUP_PATH}/memory.oom.group"
+  echo "Ensuring memory.oom.group is set to 0 to disable killing all processes in cgroup at once on OOM"
+  echo "0" > "${MEMORY_OOM_GROUP}"
+  echo "Current memory.oom.group value: $(cat "${MEMORY_OOM_GROUP}")"
+fi
+
 DIR=$(dirname $0)
 
-echo "Entering $0 at $(date) "
 DOCKERD_DATA_ROOT=${DOCKERD_DATA_ROOT:-/var/lib/docker}
 DIND_VOLUME_STAT_DIR=${DIND_VOLUME_STAT_DIR:-${DOCKERD_DATA_ROOT}/dind-volume}
 DIND_VOLUME_CREATED_TS_FILE=${DIND_VOLUME_STAT_DIR}/created
@@ -133,10 +163,6 @@ if [[ -n "${USE_DIND_IMAGES_LIB}" && "${USE_DIND_IMAGES_LIB}" != "false" ]]; the
 fi
 echo "DOCKERD_PARAMS = ${DOCKERD_PARAMS}"
 
-# Starting monitor
-${DIR}/monitor/start.sh  <&- &
-MONITOR_PID=$!
-
 ### start docker with retry
 DOCKERD_PID_FILE=/var/run/docker.pid
 DOCKERD_PID_MAXWAIT=${DOCKERD_PID_MAXWAIT:-20}
@@ -188,29 +214,8 @@ do
 
   echo "Starting dockerd"
   if [ ! -f /sys/fs/cgroup/cgroup.controllers ]; then
-    echo "Using cgroup v1"
-    dockerd ${DOCKERD_PARAMS} <&- &
+    dockerd --feature containerd-snapshotter=false ${DOCKERD_PARAMS} <&- &
   else
-    echo "Using cgroup v2"
-    CURRENT_CGROUP=$(cat /proc/self/cgroup | sed 's/0:://')
-    CURRENT_CGROUP_PATH="/sys/fs/cgroup/${CURRENT_CGROUP}"
-    echo "Current cgroup: ${CURRENT_CGROUP}"
-
-    # Move the processes from the current group to the `./init` group,
-    # otherwise the current group will become of type "domain threaded",
-    # and it will not be possible to enable required controllers for DinD group.
-    # Ref: https://github.com/moby/moby/blob/38805f20f9bcc5e87869d6c79d432b166e1c88b4/hack/dind#L28-L38
-    mkdir -p ${CURRENT_CGROUP_PATH}/init
-    xargs -rn1 < ${CURRENT_CGROUP_PATH}/cgroup.procs > ${CURRENT_CGROUP_PATH}/init/cgroup.procs || :
-
-    # Set `memory.oom.group=0` to disable killing all processes in cgroup at once on OOM.
-    # if all processes are killed at once, the system will not be able to detect this event;
-    # instead, we expect separate pipeline steps to be killed if total consumptions exceed limits.
-    MEMORY_OOM_GROUP="${CURRENT_CGROUP_PATH}/memory.oom.group"
-    echo "Ensuring memory.oom.group is set to 0 to disable killing all processes in cgroup at once on OOM"
-    echo "0" > "${MEMORY_OOM_GROUP}"
-    echo "Current memory.oom.group value: $(cat "${MEMORY_OOM_GROUP}")"
-
     # Explicitly set --cgroup-parent to prevent DinD containers escaping the pod cgroup on cgroup v2.
     dockerd --feature containerd-snapshotter=false --cgroup-parent "${CURRENT_CGROUP}/codefresh-dind" ${DOCKERD_PARAMS} <&- &
   fi
@@ -246,6 +251,10 @@ do
   break
 done
 
+# Starting monitor
+${DIR}/monitor/start.sh  <&- &
+MONITOR_PID=$!
+
 # Starting cleaner agent
 if [[ -z "${DISABLE_CLEANER_AGENT}" && -z "${SIGTERM}" ]]; then
   ${DIR}/cleaner/cleaner-agent.sh  <&- &

service.yaml

@@ -1 +1 @@
-version: 3.0.10
+version: 3.0.11