diff mbox series

tools/build-docs-container: make concurrent-safe

Message ID 20251009-iid-file-v1-1-d9ca8563c88c@cherry.de
State New
Headers show
Series tools/build-docs-container: make concurrent-safe | expand

Commit Message

Quentin Schulz Oct. 9, 2025, 8:59 a.m. UTC
From: Quentin Schulz <quentin.schulz@cherry.de>

We aren't that interested in tags actually, the only thing it brings is
a belief that we are going to run exactly what we just built. The issue
is that this is incorrect.

Indeed, one could simply run the script in parallel for the same image.
Script runtime A will build the image A and tag it X, Script runtime B
will build the image B and tag it X, Script runtime B will run the tag X
(image B), Script runtime A will run the tag X (image B). One way to fix
this could be to introduce random numbers in the tag so that it's always
unique, but we would be flooding the system with useless tags.

Instead, we can use the sha of the generated image and run that sha
directly. If it's the same across rebuilds, it'll stay the same. If it's
different, the sha will be different and thus we are safe from
concurrent use.

The only downside is that we cannot infer from the image sha the
underlying distro we're testing.

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 documentation/tools/build-docs-container | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)


---
base-commit: 09c7800333b17b21e50d2a089a3ae1b123697243
change-id: 20251009-iid-file-f276784e0425

Best regards,

Comments

Quentin Schulz Oct. 9, 2025, 9:29 a.m. UTC | #1
On 10/9/25 10:59 AM, Quentin Schulz wrote:
> From: Quentin Schulz <quentin.schulz@cherry.de>
> 
> We aren't that interested in tags actually, the only thing it brings is
> a belief that we are going to run exactly what we just built. The issue
> is that this is incorrect.
> 
> Indeed, one could simply run the script in parallel for the same image.
> Script runtime A will build the image A and tag it X, Script runtime B
> will build the image B and tag it X, Script runtime B will run the tag X
> (image B), Script runtime A will run the tag X (image B). One way to fix
> this could be to introduce random numbers in the tag so that it's always
> unique, but we would be flooding the system with useless tags.
> 
> Instead, we can use the sha of the generated image and run that sha
> directly. If it's the same across rebuilds, it'll stay the same. If it's
> different, the sha will be different and thus we are safe from
> concurrent use.
> 
> The only downside is that we cannot infer from the image sha the
> underlying distro we're testing.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
> ---
>   documentation/tools/build-docs-container | 17 +++++++++++------
>   1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/documentation/tools/build-docs-container b/documentation/tools/build-docs-container
> index 70e05f295..f3ef21304 100755
> --- a/documentation/tools/build-docs-container
> +++ b/documentation/tools/build-docs-container
> @@ -64,10 +64,6 @@ main ()
>   
>     OCI=$(which "$CONTAINERCMD")
>   
> -  # docker build doesn't accept 2 colons, so "sanitize" the name
> -  local sanitized_dockername
> -  sanitized_dockername=$(echo "$image" | tr ':.' '-')
> -
>     local version
>     version=$(echo "$image" | awk -F: '{print $NF}')
>   
> @@ -139,8 +135,14 @@ main ()
>         ;;
>     esac
>   
> +  local image_sha
> +  local image_id_file

Shouldn't be local if we want the trap to work (otherwise reports 
unbound variable).

Also, this actually isn't concurrent safe as we would be building in the 
same build (_build) directory. We could fix that with:

diff --git a/documentation/Makefile b/documentation/Makefile
index bade78fe8..94d19e350 100644
--- a/documentation/Makefile
+++ b/documentation/Makefile
@@ -11,7 +11,7 @@ SOURCEDIR      = .
  VALEDOCS       ?= $(SOURCEDIR)
  SPHINXLINTDOCS ?= $(SOURCEDIR)
  IMAGEDIRS      = */svg
-BUILDDIR       = _build
+BUILDDIR       ?= _build
  DESTDIR        = final
  SVG2PNG        = rsvg-convert
  SVG2PDF        = rsvg-convert
diff --git a/documentation/tools/build-docs-container 
b/documentation/tools/build-docs-container
index f3ef21304..3bc66970f 100755
--- a/documentation/tools/build-docs-container
+++ b/documentation/tools/build-docs-container
@@ -60,6 +60,7 @@ main ()
    fi

    local image="$1"
+  local orig_image=$image
    shift

    OCI=$(which "$CONTAINERCMD")
@@ -160,6 +161,7 @@ main ()
      --volume="$DOCS_DIR:/docs:rw"
      --workdir=/docs
      --security-opt label=disable
+    --env BUILDDIR="_build-$orig_image-$image_sha"
    )

    if [ "$OCI" = "docker" ]; then

This would help a bit more but we would probably still have an issue 
with concurrent runs of $(SOURCEDIR)/set_versions.py which edits the 
source tree (generates poky.yaml and switchers.js).

I can send a v2 with the local image_id_file fixed, an additional patch 
to build in separate build directories and reword the commit logs so 
they don't seem to be guaranteeing concurrent safety.

It should however help make sure building two yocto-docs repos 
separately at the same time is now safe.

Cheers,
Quentin
Antonin Godard Oct. 9, 2025, 10:01 a.m. UTC | #2
On Thu Oct 9, 2025 at 10:59 AM CEST, Quentin Schulz wrote:
[...]
> The only downside is that we cannot infer from the image sha the
> underlying distro we're testing.

Maybe we could retag the image after it is built and include the image name +
the sha in the tag? With `$OCI tag source dest`, then `$OCI rmi source`?

Antonin
Antonin Godard Oct. 9, 2025, 10:04 a.m. UTC | #3
On Thu Oct 9, 2025 at 11:29 AM CEST, Quentin Schulz via lists.yoctoproject.org wrote:
> On 10/9/25 10:59 AM, Quentin Schulz wrote:
>> From: Quentin Schulz <quentin.schulz@cherry.de>
>> 
>> We aren't that interested in tags actually, the only thing it brings is
>> a belief that we are going to run exactly what we just built. The issue
>> is that this is incorrect.
>> 
>> Indeed, one could simply run the script in parallel for the same image.
>> Script runtime A will build the image A and tag it X, Script runtime B
>> will build the image B and tag it X, Script runtime B will run the tag X
>> (image B), Script runtime A will run the tag X (image B). One way to fix
>> this could be to introduce random numbers in the tag so that it's always
>> unique, but we would be flooding the system with useless tags.
>> 
>> Instead, we can use the sha of the generated image and run that sha
>> directly. If it's the same across rebuilds, it'll stay the same. If it's
>> different, the sha will be different and thus we are safe from
>> concurrent use.
>> 
>> The only downside is that we cannot infer from the image sha the
>> underlying distro we're testing.
>> 
>> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
>> ---
>>   documentation/tools/build-docs-container | 17 +++++++++++------
>>   1 file changed, 11 insertions(+), 6 deletions(-)
>> 
>> diff --git a/documentation/tools/build-docs-container b/documentation/tools/build-docs-container
>> index 70e05f295..f3ef21304 100755
>> --- a/documentation/tools/build-docs-container
>> +++ b/documentation/tools/build-docs-container
>> @@ -64,10 +64,6 @@ main ()
>>   
>>     OCI=$(which "$CONTAINERCMD")
>>   
>> -  # docker build doesn't accept 2 colons, so "sanitize" the name
>> -  local sanitized_dockername
>> -  sanitized_dockername=$(echo "$image" | tr ':.' '-')
>> -
>>     local version
>>     version=$(echo "$image" | awk -F: '{print $NF}')
>>   
>> @@ -139,8 +135,14 @@ main ()
>>         ;;
>>     esac
>>   
>> +  local image_sha
>> +  local image_id_file
>
> Shouldn't be local if we want the trap to work (otherwise reports 
> unbound variable).
>
> Also, this actually isn't concurrent safe as we would be building in the 
> same build (_build) directory. We could fix that with:
>
> diff --git a/documentation/Makefile b/documentation/Makefile
> index bade78fe8..94d19e350 100644
> --- a/documentation/Makefile
> +++ b/documentation/Makefile
> @@ -11,7 +11,7 @@ SOURCEDIR      = .
>   VALEDOCS       ?= $(SOURCEDIR)
>   SPHINXLINTDOCS ?= $(SOURCEDIR)
>   IMAGEDIRS      = */svg
> -BUILDDIR       = _build
> +BUILDDIR       ?= _build
>   DESTDIR        = final
>   SVG2PNG        = rsvg-convert
>   SVG2PDF        = rsvg-convert
> diff --git a/documentation/tools/build-docs-container 
> b/documentation/tools/build-docs-container
> index f3ef21304..3bc66970f 100755
> --- a/documentation/tools/build-docs-container
> +++ b/documentation/tools/build-docs-container
> @@ -60,6 +60,7 @@ main ()
>     fi
>
>     local image="$1"
> +  local orig_image=$image
>     shift
>
>     OCI=$(which "$CONTAINERCMD")
> @@ -160,6 +161,7 @@ main ()
>       --volume="$DOCS_DIR:/docs:rw"
>       --workdir=/docs
>       --security-opt label=disable
> +    --env BUILDDIR="_build-$orig_image-$image_sha"
>     )
>
>     if [ "$OCI" = "docker" ]; then
>
> This would help a bit more but we would probably still have an issue 
> with concurrent runs of $(SOURCEDIR)/set_versions.py which edits the 
> source tree (generates poky.yaml and switchers.js).
>
> I can send a v2 with the local image_id_file fixed, an additional patch 
> to build in separate build directories and reword the commit logs so 
> they don't seem to be guaranteeing concurrent safety.
>
> It should however help make sure building two yocto-docs repos 
> separately at the same time is now safe.

Agreed, it still a bit better than what we currently have, I think we should do
that for now.

Thanks,
Antonin
Quentin Schulz Oct. 9, 2025, 10:07 a.m. UTC | #4
On 10/9/25 12:01 PM, Antonin Godard wrote:
> On Thu Oct 9, 2025 at 10:59 AM CEST, Quentin Schulz wrote:
> [...]
>> The only downside is that we cannot infer from the image sha the
>> underlying distro we're testing.
> 
> Maybe we could retag the image after it is built and include the image name +
> the sha in the tag? With `$OCI tag source dest`, then `$OCI rmi source`?
> 

You would get one tag per run of the script, polluting your image tags.. 
I'm not sure it's worth it tbh.

We do get the output of podman build though which does print the FROM 
layer, c.f.:

tools/build-docs-container leap:15.6 html
Emulate Docker CLI using podman. Create /etc/containers/nodocker to 
quiet msg.
STEP 1/12: FROM docker.io/opensuse/leap:15.6

We can also add an echo somewhere to display what we're currently 
testing for example.

Quentin
diff mbox series

Patch

diff --git a/documentation/tools/build-docs-container b/documentation/tools/build-docs-container
index 70e05f295..f3ef21304 100755
--- a/documentation/tools/build-docs-container
+++ b/documentation/tools/build-docs-container
@@ -64,10 +64,6 @@  main ()
 
   OCI=$(which "$CONTAINERCMD")
 
-  # docker build doesn't accept 2 colons, so "sanitize" the name
-  local sanitized_dockername
-  sanitized_dockername=$(echo "$image" | tr ':.' '-')
-
   local version
   version=$(echo "$image" | awk -F: '{print $NF}')
 
@@ -139,8 +135,14 @@  main ()
       ;;
   esac
 
+  local image_sha
+  local image_id_file
+  image_id_file=$(mktemp)
+  # Don't clutter tmpfs on fails
+  trap 'rm -f "$image_id_file"' EXIT
+
   $OCI build \
-    --tag "yocto-docs-$sanitized_dockername:latest" \
+    --iidfile "$image_id_file" \
     --build-arg ARG_FROM="docker.io/$image" \
     --build-arg DOCS="$docs" \
     --build-arg DOCS_PDF="$docs_pdf" \
@@ -148,6 +150,9 @@  main ()
     --file "$SCRIPT_DIR/$containerfile" \
     "$SH_DIR/"
 
+  image_sha="$(< "$image_id_file")"
+  rm "$image_id_file"
+
   local -a args_run=(
     --rm
     --interactive
@@ -171,7 +176,7 @@  main ()
 
   $OCI run \
     "${args_run[@]}" \
-    "yocto-docs-$sanitized_dockername" \
+    "$image_sha" \
     "$@"
 }