Message ID | 20251009-iid-file-v1-1-d9ca8563c88c@cherry.de |
---|---|
State | New |
Headers | show |
Series | tools/build-docs-container: make concurrent-safe | expand |
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
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
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
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 --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" \ "$@" }