docker library: build when all base packages are ready #656
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1. Save space Rather than building parts of the manifest on the assumption that all architectures will catch up, wait for all of them. When the are all there, build them all in one go, and clean them all up at the end. The *-reference.txt file contains the name of the builders that are ready. Its used as a counter so the only thing that matters is the number of unique lines. Removed use of podman. There where probably some incompatible things here. 2. be retriggerable If any part of the build fails, clean the images up. Any manual retrigger on the tarbuildnum for a ubi/non-ubi will retrigger the building of the manifest, and test. This saves us rebuilding the packages. So this includes docker-library-build.sh. Failure of an architecture cleans them all. 3. tests now x86_64 only The docker-library-test.sh is adjusted because of the manifest name. The less obvious change is because the image is a manifest, running the test suite on this will only run on the native amd64 architecture. There where timeouts on non-amd64 sometimes still which where pretty much always false positives. 4. simplified docker-library-manifest With the manifest already done in the build step, this is simply pushing that manifest and all the tags around it. A re-arrangements means we push the tags (latest/earliest...) first (with image). Then the image with its main tag (like :10.5). Then we build the debug images. As there is now an easy implementation for this so its a one run process. Cleanup is aslo part of this script. The reference.txt files are removed. buildah rmi --prune --force will remove all untagged images, including any that had running containers on them during a build process. The rest of the cleanup is the same (without an if condition around it). The is probably scope for cleaning this up some more.
Hit a buildah quirk/bug so disabling debug image build until better solution presents itself.
fauust
approved these changes
Jan 20, 2025
There was a problem hiding this comment.
Looks good. There are some minor improvement/fix.
I have the feeling that maintenance would be eased by creating a bash library on the model of install/upgrade tests.
It's complex to correctly review it without testing, so, I suggest that we iterate on DEV for that.
|
|
||
| # ensure unique entries for each arch | ||
| echo "$buildername" >> "$reffile" | ||
| sort -u "$reffile" -o "$reffile" |
| entries=$(wc -l < "$reffile") | ||
| if [ "$entries" -lt ${#arches[@]} ]; then | ||
| echo "Only $entries architectures so far" | ||
| exit |
There was a problem hiding this comment.
specify exit, either 0 or >0. This helps understanding if it's "normal" or "error".
Not mandatory but since we are in bash, you could use integer comparison:
if (($entries < ${#arches[@]})); then
...
fi| else | ||
| build "${builderarch}" | ||
| fi | ||
| trap cleanup ERR |
| @@ -87,49 +113,81 @@ annotate() { | |||
| done | |||
There was a problem hiding this comment.
annotate function does not seem to be used.
| image=mariadb-${tarbuildnum}-${builderarch}${ubi} | ||
| if ! buildah manifest exists "$image"; then | ||
| echo "No manifest we can't push" | ||
| exit |
There was a problem hiding this comment.
- echo "No manifest we can't push"
+ echo "No manifest, we can't push"
- exit
+ exit 0/1^^ see previous comment about exit codes.
| if [[ "$buildername" = *-rhel-9-rpm-autobake ]]; then | ||
| image=${image}-ubi | ||
| fi | ||
|
|
||
| if ! buildah manifest exists "$image"; then | ||
| echo "No manifest we can't test" | ||
| exit |
There was a problem hiding this comment.
~Same comment as previous one.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
debug info containers disabled for now. Scripts test locally ok when passed same BB arguments.
Rather than building parts of the manifest on the assumption that all architectures will catch up, wait for all of them.
When the are all there, build them all in one go, and clean them all up at the end.
The *-reference.txt file contains the name of the builders that are ready. Its used as a counter so the only thing that matters is the number of unique lines.
Removed use of podman. There where probably some incompatible things here.
If any part of the build fails, clean the images up. Any manual retrigger on the tarbuildnum for a ubi/non-ubi will retrigger the building of the manifest, and test. This saves us rebuilding the packages.
So this includes docker-library-build.sh. Failure of an architecture cleans them all.
The docker-library-test.sh is adjusted because of the manifest name.
The less obvious change is because the image is a manifest, running the test suite on this will only run on the native amd64 architecture.
There where timeouts on non-amd64 sometimes still which where pretty much always false positives.
With the manifest already done in the build step,
this is simply pushing that manifest and all the
tags around it.
A re-arrangements means we push the tags (latest/earliest...) first (with image). Then the image with its main tag (like :10.5).
Then we build the debug images. As there is now an easy implementation for this so its a one run process.
Cleanup is also part of this script. The reference.txt files are removed.
buildah rmi --prune --force will remove all untagged images, including any that had running containers on them during a build process.
The rest of the cleanup is the same (without an if condition around it). The is probably scope for cleaning this up some more.