[PATCH 0/1] External fix for sporadic schroot race issue

[PATCH 0/1] External fix for sporadic schroot race issue

[Anton Mikanovich]

After some experimenting with locking inside and outside of schroot and
sbuild tools I've finally found the way how to made almost the same
locking in Isar only without much build speed influence.

The idea is to cover all separate schroot calls with the lock based on
what type of the call it is:
- Session begin and any commands in already present session will use
the lock in shared mode, so multiple executions possible.
- Session end which can remove config files and cause race scenario
will use the lock in exclusive mode, so it will wait for the time no
other schroot instances running.

Luckly we always use schroot with separate session create/end commands
even inside sbuild.
The only thing needed is to put a little script into the location
inserted to PATH.

This patch is just a copy of RFC was sent previously with no changes.
It was tested on our CI and now is ready to be merged.

Anton Mikanovich (1):
  meta: Protect schroot config management

 meta/classes/dpkg.bbclass   |  3 +++
 meta/classes/sbuild.bbclass |  6 ++++++
 scripts/schroot             | 43 +++++++++++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+)
 create mode 100755 scripts/schroot

-- 
2.34.1

-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/isar-users/20241220082959.3123651-1-amikan%40ilbers.de.

[PATCH 1/1] meta: Protect schroot config management

[Anton Mikanovich]

As schroot itself is not thread safe and can fail in case of reading
chroot/session config files when other instance removing those files,
we need to add external locking for race protection.
Run schroot through flock protected script provided via PATH by
default. Also protect with the same lock removing of configs.

Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
---
 meta/classes/dpkg.bbclass   |  3 +++
 meta/classes/sbuild.bbclass |  6 ++++++
 scripts/schroot             | 43 +++++++++++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+)
 create mode 100755 scripts/schroot

diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
index ef85890a..64404103 100644
--- a/meta/classes/dpkg.bbclass
+++ b/meta/classes/dpkg.bbclass
@@ -96,6 +96,9 @@ dpkg_runbuild() {
 
     export SBUILD_CONFIG="${SBUILD_CONFIG}"
 
+    # Provide locking filter for schroot
+    sbuild_add_env_filter "PATH"
+
     for envvar in http_proxy HTTP_PROXY https_proxy HTTPS_PROXY \
         ftp_proxy FTP_PROXY no_proxy NO_PROXY; do
         sbuild_add_env_filter "$envvar"
diff --git a/meta/classes/sbuild.bbclass b/meta/classes/sbuild.bbclass
index f68e8735..1ab72aad 100644
--- a/meta/classes/sbuild.bbclass
+++ b/meta/classes/sbuild.bbclass
@@ -14,6 +14,9 @@ SCHROOT_CONF_FILE ?= "${SCHROOT_CONF}/chroot.d/${SBUILD_CHROOT}"
 
 SBUILD_CONFIG="${WORKDIR}/sbuild.conf"
 
+# Lockfile available for all the users
+SCHROOT_LOCKFILE = "/tmp/schroot.lock"
+
 schroot_create_configs() {
     mkdir -p "${TMPDIR}/schroot-overlay"
     echo "Creating ${SCHROOT_CONF_FILE}"
@@ -54,6 +57,8 @@ EOSUDO
 }
 
 schroot_delete_configs() {
+    (flock -x 9
+    set -e
     sudo -s <<'EOSUDO'
         set -e
         if [ -d "${SBUILD_CONF_DIR}" ]; then
@@ -63,6 +68,7 @@ schroot_delete_configs() {
         echo "Removing ${SCHROOT_CONF_FILE}"
         rm -f "${SCHROOT_CONF_FILE}"
 EOSUDO
+    ) 9>"${SCHROOT_LOCKFILE}"
 }
 
 sbuild_add_env_filter() {
diff --git a/scripts/schroot b/scripts/schroot
new file mode 100755
index 00000000..f5320a6a
--- /dev/null
+++ b/scripts/schroot
@@ -0,0 +1,43 @@
+#!/bin/bash
+#
+# This software is a part of ISAR.
+# Copyright (C) 2024 ilbers GmbH
+#
+# SPDX-License-Identifier: MIT
+
+set -e
+
+# Save command line
+OPTS=("$@")
+
+# Analyze used flags
+while [ $# -gt 0 ]
+do
+    key="$1"
+
+    case $key in
+    -b|--begin-session)
+        BEGIN="1"
+        ;;
+    -r|--run-session)
+        RUN="1"
+        ;;
+    -e|--end-session)
+        END="1"
+        ;;
+    esac
+
+    shift
+done
+
+# Use exclusive lock for configs rm, shared for any other calls
+TYPE="-s"
+if [ "$END" == "1" ]; then
+    TYPE="-x"
+fi
+
+# A place for lock available for all the users
+LOCKDIR="/tmp"
+
+# Run schroot protected with lock
+flock $TYPE $LOCKDIR/schroot.lock /usr/bin/schroot "${OPTS[@]}"
-- 
2.34.1

-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/isar-users/20241220082959.3123651-2-amikan%40ilbers.de.

Re: [PATCH 0/1] External fix for sporadic schroot race issue

[Uladzimir Bely]

On Fri, 2024-12-20 at 10:29 +0200, Anton Mikanovich wrote:
> After some experimenting with locking inside and outside of schroot
> and
> sbuild tools I've finally found the way how to made almost the same
> locking in Isar only without much build speed influence.
> 
> The idea is to cover all separate schroot calls with the lock based
> on
> what type of the call it is:
> - Session begin and any commands in already present session will use
> the lock in shared mode, so multiple executions possible.
> - Session end which can remove config files and cause race scenario
> will use the lock in exclusive mode, so it will wait for the time no
> other schroot instances running.
> 
> Luckly we always use schroot with separate session create/end
> commands
> even inside sbuild.
> The only thing needed is to put a little script into the location
> inserted to PATH.
> 
> This patch is just a copy of RFC was sent previously with no changes.
> It was tested on our CI and now is ready to be merged.
> 
> Anton Mikanovich (1):
>   meta: Protect schroot config management
> 
>  meta/classes/dpkg.bbclass   |  3 +++
>  meta/classes/sbuild.bbclass |  6 ++++++
>  scripts/schroot             | 43
> +++++++++++++++++++++++++++++++++++++
>  3 files changed, 52 insertions(+)
>  create mode 100755 scripts/schroot
> 
> -- 
> 2.34.1
> 

Applied to next.

-- 
Best regards,
Uladzimir.



-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/isar-users/39e9170eeafbad5f43ab94ad2d2f98273ba32392.camel%40ilbers.de.