-
Notifications
You must be signed in to change notification settings - Fork 12.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rehome libcxx-builder docker image & attempt gentler termination. #71604
Conversation
There are three changes present in this PR. 1. Use github packages for libcxx-builder rather than dockerhub. The ldionne/libcxx-builder image will now be hosted at ghcr.io/libcxx/libcxx-builder. This has the benefit of allowing members of the github org to push new versions. In the future I hope to add github actions to rebuild the image as needed. 2. Add docker-compose file The compose file allows to to specify the package repository, so that users can simply write 'docker compose build' and 'docker compose push'. It also gives us a centralized place to manage version arguments which change frequently. 3. Use non-shell CMD form. This may help the google libcxx builders disconnect more gracefully as the shell form of CMD may eat the shutdown signal. I'm hoping this corrects inaccurate agent counts from the buildkite API, since when the VM's terminate, they do so without signaling it to buildkite, which hangs around waiting for them to reconnect. It's likely more changes will be needed though.
@llvm/pr-subscribers-libcxx Author: Eric (EricWF) ChangesThere are three changes present in this PR.
The ldionne/libcxx-builder image will now be hosted at ghcr.io/libcxx/libcxx-builder. This has the benefit of allowing members of the github org to push new versions. In the future I hope to add github actions to rebuild the image as needed.
The compose file allows to to specify the package repository, so that users can simply write 'docker compose build' and 'docker compose push'. It also gives us a centralized place to manage version arguments which change frequently.
This may help the google libcxx builders disconnect more gracefully as the shell form of CMD may eat the shutdown signal. I'm hoping this corrects inaccurate agent counts from the buildkite API, since when the VM's terminate, they do so without signaling it to buildkite, which hangs around waiting for them to reconnect. It's likely more changes will be needed though. Full diff: https://github.com/llvm/llvm-project/pull/71604.diff 4 Files Affected:
diff --git a/libcxx/utils/ci/Dockerfile b/libcxx/utils/ci/Dockerfile
index 962de84e852ce59..53267fd1121e079 100644
--- a/libcxx/utils/ci/Dockerfile
+++ b/libcxx/utils/ci/Dockerfile
@@ -22,12 +22,11 @@
# If you're only looking to run the Docker image locally for debugging a
# build bot, see the `run-buildbot-container` script located in this directory.
#
-# A pre-built version of this image is maintained on DockerHub as ldionne/libcxx-builder.
-# To update the image, rebuild it and push it to ldionne/libcxx-builder (which
-# will obviously only work if you have permission to do so).
+# A pre-built version of this image is maintained on Github under the libc++ organization, as ghcr.io/libcxx/libcxx-builder.
+# To update the image, rebuild it and push it to github (all members of the libc++ organization should be able to do this).
#
-# $ docker build -t ldionne/libcxx-builder libcxx/utils/ci
-# $ docker push ldionne/libcxx-builder
+# $ docker compose build
+# $ docker compose push
#
FROM ubuntu:jammy
@@ -66,8 +65,8 @@ RUN locale-gen
# LLVM 15, we still need to have Clang 12 in this Docker image because the LLVM
# 14 release branch CI uses it. The tip-of-trunk CI will never use Clang 12,
# though.
-# LLVM POST-BRANCH bump version
-ENV LLVM_HEAD_VERSION=18
+ARG LLVM_HEAD_VERSION
+ENV LLVM_HEAD_VERSION=${LLVM_HEAD_VERSION}
RUN apt-get update && apt-get install -y lsb-release wget software-properties-common
RUN wget https://apt.llvm.org/llvm.sh -O /tmp/llvm.sh
RUN bash /tmp/llvm.sh $(($LLVM_HEAD_VERSION - 3)) # for CI transitions
@@ -97,8 +96,9 @@ RUN apt-get update && apt-get install -y llvm-$(($LLVM_HEAD_VERSION - 2))-dev ll
libomp5-$LLVM_HEAD_VERSION
# Install the most recent GCC, like clang install the previous version as a transition.
-ENV GCC_LATEST_VERSION=13
RUN add-apt-repository ppa:ubuntu-toolchain-r/test
+ARG GCC_LATEST_VERSION # populated in the docker-compose file
+ENV GCC_LATEST_VERSION=${GCC_LATEST_VERSION}
RUN apt-get update && apt install -y gcc-$((GCC_LATEST_VERSION - 1)) g++-$((GCC_LATEST_VERSION - 1))
RUN apt-get update && apt install -y gcc-$GCC_LATEST_VERSION g++-$GCC_LATEST_VERSION
@@ -131,4 +131,4 @@ ENV PATH="${PATH}:/home/libcxx-builder/.buildkite-agent/bin"
RUN echo "tags=\"queue=libcxx-builders,arch=$(uname -m),os=linux\"" >> "/home/libcxx-builder/.buildkite-agent/buildkite-agent.cfg"
# By default, start the Buildkite agent (this requires a token).
-CMD buildkite-agent start
+CMD ["buildkite-agent", "start"]
diff --git a/libcxx/utils/ci/docker-compose.yml b/libcxx/utils/ci/docker-compose.yml
new file mode 100644
index 000000000000000..7f03d8f3af0f293
--- /dev/null
+++ b/libcxx/utils/ci/docker-compose.yml
@@ -0,0 +1,11 @@
+version: '3.7'
+services:
+ libcxx-builder:
+ image: ghcr.io/libcxx/libcxx-builder
+ build:
+ context: .
+ dockerfile: Dockerfile
+ args:
+ GCC_LATEST_VERSION: 13
+ # LLVM POST-BRANCH bump version
+ LLVM_HEAD_VERSION: 18
diff --git a/libcxx/utils/ci/run-buildbot-container b/libcxx/utils/ci/run-buildbot-container
index 07fb2706beb20e3..7c00b88097f17b8 100755
--- a/libcxx/utils/ci/run-buildbot-container
+++ b/libcxx/utils/ci/run-buildbot-container
@@ -26,6 +26,6 @@ if [[ ! -d "${MONOREPO_ROOT}/libcxx/utils/ci" ]]; then
echo "Was unable to find the root of the LLVM monorepo; are you running from within the monorepo?"
exit 1
fi
-docker pull ldionne/libcxx-builder
-docker run -it --volume "${MONOREPO_ROOT}:/llvm" --workdir "/llvm" --cap-add=SYS_PTRACE ldionne/libcxx-builder \
+docker pull ghcr.io/libcxx/libcxx-builder
+docker run -it --volume "${MONOREPO_ROOT}:/llvm" --workdir "/llvm" --cap-add=SYS_PTRACE ghcr.io/libcxx/libcxx-builder \
bash -c 'git config --global --add safe.directory /llvm ; exec bash'
diff --git a/libcxx/utils/ci/vendor/android/Dockerfile b/libcxx/utils/ci/vendor/android/Dockerfile
index 0acfff8e031dc81..4d35334f89bb220 100644
--- a/libcxx/utils/ci/vendor/android/Dockerfile
+++ b/libcxx/utils/ci/vendor/android/Dockerfile
@@ -7,13 +7,13 @@
#===----------------------------------------------------------------------===##
#
-# This Dockerfile extends ldionne/libcxx-builder with Android support, including
+# This Dockerfile extends ghcr.io/libcxx/libcxx-builder with Android support, including
# Android Clang and sysroot, Android platform-tools, and the Docker client.
#
# $ docker build -t libcxx-builder-android libcxx/utils/ci/vendor/android
#
-FROM ldionne/libcxx-builder
+FROM ghcr.io/libcxx/libcxx-builder
# Switch back to the root user to install things into /opt and /usr.
USER root
|
FYI I cancelled the pre-commit on this change, since it doesn't affect anything the pre-commit tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this. LGTM modulo some comments. Please wait for @ldionne to approve too.
# | ||
# $ docker build -t ldionne/libcxx-builder libcxx/utils/ci | ||
# $ docker push ldionne/libcxx-builder | ||
# $ docker compose build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is "docker-compose" and "docker compose"
On Ubuntu 22.04 (the lastest LTS version) "docker compose" is not available.
This link contains more information.
docker-archive/compose-cli#1404 (comment)
Can you add a comment that "docker-compose" can be used when "docker compose" is not available?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know that it can be, I can't test. And I think we want to require modern enough versions of docker to use modern features.
You can get docker compose on Ubuntu using the official install scripts. [1] It's how I get docker on debian. And while using additional package repos is normally smelly, it seems to be the norm for docker.
curl -fsSL https://get.docker.com -o get-docker.sh
sudo sh get-docker.sh
Co-authored-by: Mark de Wever <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this! Are the permissions on ghcr.io/libcxx set up already so that we can start building the image and pushing it?
If you're a member of the github.com/libcxx org, you should be able to It's probably worth having someone else test this to confirm it works. |
version: '3.7' | ||
services: | ||
libcxx-builder: | ||
image: ghcr.io/libcxx/libcxx-builder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you create a libcxx organization for this? Is there any reason not to use the llvm org?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been squatting on the org for years now after noticing it was unclaimed. I recently started using it to test and prototype self hosted github actions. It's a lot easier to iterate the much smaller org.
Github permissions still seem rather course grained, which might be an issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I really like the idea of using the github container registry for this, but I'm going to recommend you use the llvm org to host the packages for a few reasons:
- In the past the LLVM Foundation has discouraged sub-projects from creating their own spaces on GitHub separate from the main project. Having a libcxx org for testing and prototype doesn't seem like a problem to me, but I think hosting official project images there may cross the line.
- Bandwidth charges: GitHub has recently extended it's free period for container storage and bandwidth, so this isn't an issue today, but in the future if they start charging, you will likely need to request a budget from the LLVM Foundation for this which would require moving the containers to the llvm org. Also, you aren't charged for bandwidth when downloads come from GitHub Actions, but this only applies to containers in your current org from what I can tell.
I understand that it's easier to manage permissions using the libcxx org, but it sounds like there be specific fine-grained permissions just for the container registry, so I can look into setting that up for you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't disagree with anything you have said.
The libc++ org is better than the status quo of ldionne's personal dockerhub, but LLVM's org is the most appropriate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tstellar One thing we're also trying to avoid is being too dependent on heavy processes or bottlenecks for making modifications of this kind. Right now with how permissions are set up in the LLVM org, we're not empowered to make these kinds of changes IIUC. With our own organization, we are.
I am not disagreeing that LLVM is the right place to do all of it (it totally is), but in the balance is also our ability to actually get stuff working, maintain them and solve our problems without having to wait for someone with enough admin privileges to set things up for us (even though you all have been amazingly responsive and helpful whenever we needed help). My request for admin-ship in the LLVM org was related to this amongst other things.
I'm going to go ahead and merge this, and we'll move it to LLVM once the permission issues are cleared up. Merging this unblocks me on other work. |
…vm#71604) There are three changes present in this PR. 1. Use github packages for libcxx-builder rather than dockerhub. The ldionne/libcxx-builder image will now be hosted at ghcr.io/libcxx/libcxx-builder. This has the benefit of allowing members of the github org to push new versions. In the future I hope to add github actions to rebuild the image as needed. 2. Add docker-compose file The compose file allows to to specify the package repository, so that users can simply write 'docker compose build' and 'docker compose push'. It also gives us a centralized place to manage version arguments which change frequently. 3. Use non-shell CMD form. This may help the google libcxx builders disconnect more gracefully as the shell form of CMD may eat the shutdown signal. I'm hoping this corrects inaccurate agent counts from the buildkite API, since when the VM's terminate, they do so without signaling it to buildkite, which hangs around waiting for them to reconnect. It's likely more changes will be needed though. --------- Co-authored-by: Mark de Wever <[email protected]>
There are three changes present in this PR.
The ldionne/libcxx-builder image will now be hosted at ghcr.io/libcxx/libcxx-builder. This has the benefit of allowing members of the github org to push new versions.
In the future I hope to add github actions to rebuild the image as needed.
The compose file allows to to specify the package repository, so that users can simply write 'docker compose build' and 'docker compose push'.
It also gives us a centralized place to manage version arguments which change frequently.
This may help the google libcxx builders disconnect more gracefully as the shell form of CMD may eat the shutdown signal. I'm hoping this corrects inaccurate agent counts from the buildkite API, since when the VM's terminate, they do so without signaling it to buildkite, which hangs around waiting for them to reconnect. It's likely more changes will be needed though.