Speculative fixes for docker image building hanging #1149
Merged
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
It's root on most systems. Seems unnecessary to do this. If it needs to be written to, it should be done as root.
In particular we want the root user in the docker container to be buildkite-agent:docker on the host, so use id -u and getent group to determine these dynamically.
triarius
changed the title
triarius
commented
Jul 3, 2023
| --userns=host \ | ||
| --rm \ | ||
| "tonistiigi/binfmt:${QEMU_BINFMT_TAG}" \ | ||
| --install all |
There was a problem hiding this comment.
Now, after this is done, we restart the docker daemon.
triarius
changed the title
triarius
force-pushed
the
pdp-1276-fix-first-build-of-dockerfilepostgres-on-bkbk-is-failing-and
branch
5 times, most recently
from
9eb9f8d
to
2ab8737
Compare
|
We've dogfooded this version for a bit, and we have a way forward. Let's merge it into v6 and tag v6.0.0-beta2 |
triarius
commented
Jul 13, 2023
| docker buildx build -f tests/Dockerfile --progress=plain -t buildkite-postgres:latest tests: | ||
| exit-status: 0 | ||
| timeout: 30000 | ||
|
|
There was a problem hiding this comment.
This tests building a problematic container image
There was a problem hiding this comment.
Worth adding this as a comment in the file?
DrJosh9000
approved these changes
Jul 17, 2023
| docker buildx build -f tests/Dockerfile --progress=plain -t buildkite-postgres:latest tests: | ||
| exit-status: 0 | ||
| timeout: 30000 | ||
|
|
There was a problem hiding this comment.
Worth adding this as a comment in the file?
|
@DrJosh9000 I'm going to beef up the tests based on what I've learnt since making this PR. Watch this space for another PR where I'll explain it better. |
triarius
deleted the
pdp-1276-fix-first-build-of-dockerfilepostgres-on-bkbk-is-failing-and
branch
Merged
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.
A lot of the PR is cleanup, but there are some speculative fixes for the failure of building of a docker image that was revealed during dogfooding.
I've added the building of that Docker image to the tests, and it clearly passes, but there may be some very rare flake involved.
The changes that may affect this are:
Installing multiarch qemu (via a docker container) has been moved from after the docker daemon has been configured to before, then all trace of the image that was used to do this is removed.
The files (
/etc/subuid,/etc/subgid) needed for docker usernamespaceing are generated on first boot as I noticed that they seem to be modified dynamically after we copy them in. They should be dynamic as the values of thebuildkite-agentuser's UID and thedockergroup's GID are dynamic. It was a coincidence that the hard-coded values did not change between builds of the AMI before.