Skip to content

[libcxx] Install runner last when building CI containers #148072

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

Conversation

boomanaiden154
Copy link
Contributor

This patch changes when we install the GHA runner in the CI containers. Instead
of having it in the base image, we install it last. This will enable a follow up
patch that will do some setup enabling building the full container image with an
existing base image, thus enabling updating the GHA runner without modifying the
important bits.

Created using spr 1.3.4
@boomanaiden154 boomanaiden154 requested a review from a team as a code owner July 10, 2025 22:54
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jul 10, 2025
@boomanaiden154
Copy link
Contributor Author

This is a prereq of #148073.

@llvmbot
Copy link
Member

llvmbot commented Jul 10, 2025

@llvm/pr-subscribers-libcxx

Author: Aiden Grossman (boomanaiden154)

Changes

This patch changes when we install the GHA runner in the CI containers. Instead
of having it in the base image, we install it last. This will enable a follow up
patch that will do some setup enabling building the full container image with an
existing base image, thus enabling updating the GHA runner without modifying the
important bits.


Full diff: https://github.com/llvm/llvm-project/pull/148072.diff

2 Files Affected:

  • (modified) libcxx/utils/ci/Dockerfile (+15-2)
  • (modified) libcxx/utils/ci/docker-compose.yml (+2-1)
diff --git a/libcxx/utils/ci/Dockerfile b/libcxx/utils/ci/Dockerfile
index 0a1985b02807b..316e9c7490991 100644
--- a/libcxx/utils/ci/Dockerfile
+++ b/libcxx/utils/ci/Dockerfile
@@ -312,5 +312,18 @@ CMD /opt/android/container-setup.sh && buildkite-agent start
 #
 FROM builder-base AS actions-builder
 
-WORKDIR /home/runner
-USER runner
+ARG GITHUB_RUNNER_VERSION
+
+RUN useradd gha -u 1001 -m -s /bin/bash
+RUN adduser gha sudo
+RUN echo '%sudo ALL=(ALL) NOPASSWD:ALL' >> /etc/sudoers
+WORKDIR /home/gha
+USER gha
+
+ENV RUNNER_MANUALLY_TRAP_SIG=1
+ENV ACTIONS_RUNNER_PRINT_LOG_TO_STDOUT=1
+RUN mkdir actions-runner && \
+    cd actions-runner && \
+    curl -O -L https://github.com/actions/runner/releases/download/v$GITHUB_RUNNER_VERSION/actions-runner-linux-x64-$GITHUB_RUNNER_VERSION.tar.gz && \
+    tar xzf ./actions-runner-linux-x64-$GITHUB_RUNNER_VERSION.tar.gz && \
+    rm ./actions-runner-linux-x64-$GITHUB_RUNNER_VERSION.tar.gz
diff --git a/libcxx/utils/ci/docker-compose.yml b/libcxx/utils/ci/docker-compose.yml
index 2189a41555c2f..20536bc32fa65 100644
--- a/libcxx/utils/ci/docker-compose.yml
+++ b/libcxx/utils/ci/docker-compose.yml
@@ -10,7 +10,8 @@ services:
       dockerfile: Dockerfile
       target: actions-builder
       args:
-        BASE_IMAGE: ghcr.io/actions/actions-runner:2.326.0
+        BASE_IMAGE: ubuntu:jammy
+        GITHUB_RUNNER_VERSION: "2.326.0"
         <<: *compiler_versions
 
   android-buildkite-builder:

@boomanaiden154 boomanaiden154 requested review from ldionne and EricWF July 10, 2025 22:54
Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does ./libcxx/utils/ci/run-buildbot-container still work after this change? LGTM assuming it does.

Also, we'll need to test this change before we switch to this image: that can be done by building this image, associating it to the -next GH runner group inside llvm-zorg, and then creating a PR that changes our workflows to target the -next runners. Kinda complicated, but that's the simplest we have until we can specify an image directly inside the GH workflow file.

@boomanaiden154
Copy link
Contributor Author

Does ./libcxx/utils/ci/run-buildbot-container still work after this change? LGTM assuming it does.

Yeah, this doesn't change the functionality of that script at all.

Also, we'll need to test this change before we switch to this image: that can be done by building this image, associating it to the -next GH runner group inside llvm-zorg, and then creating a PR that changes our workflows to target the -next runners.

Yep. I'll look at getting that done at least once probably once #148073 (a review there would be helpful) lands so everything is set up to upgrade the runner binary without the rest of the container.

Kinda complicated, but that's the simplest we have until we can specify an image directly inside the GH workflow file.

We're meeting with someone today to discuss kubernetes-sigs/apiserver-network-proxy#748, so we'll see what we can do.

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM assuming this doesn't break libcxx/utils/ci/run-buildbot-container.

@boomanaiden154
Copy link
Contributor Author

LGTM assuming this doesn't break libcxx/utils/ci/run-buildbot-container.

That will continue to work. It's the same container image with the same contents, just with a swapped build order.

Although we might want to swap the script to use the base image after #148073 because running locally doesn't require the actions runner binary, but that's a minor optimization and it will work fine either way.

Created using spr 1.3.6
@boomanaiden154
Copy link
Contributor Author

@ldionne I'm thinking land this and #148073 despite the failing CI so that when apt.llvm.org starts working again and the container gets fixed. Does that sound good to you?

boomanaiden154 added a commit to boomanaiden154/llvm-project that referenced this pull request Jul 25, 2025
This patch changes when we install the GHA runner in the CI containers. Instead
of having it in the base image, we install it last. This will enable a follow up
patch that will do some setup enabling building the full container image with an
existing base image, thus enabling updating the GHA runner without modifying the
important bits.

Pull Request: llvm#148072
@boomanaiden154 boomanaiden154 merged commit 6107e3a into main Jul 28, 2025
74 of 76 checks passed
@boomanaiden154 boomanaiden154 deleted the users/boomanaiden154/libcxx-install-runner-last-when-building-ci-containers branch July 28, 2025 19:32
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Jul 28, 2025
This patch changes when we install the GHA runner in the CI containers. Instead
of having it in the base image, we install it last. This will enable a follow up
patch that will do some setup enabling building the full container image with an
existing base image, thus enabling updating the GHA runner without modifying the
important bits.

Reviewers: EricWF, ldionne

Reviewed By: ldionne

Pull Request: llvm/llvm-project#148072
TIFitis pushed a commit that referenced this pull request Jul 28, 2025
This patch changes when we install the GHA runner in the CI containers. Instead
of having it in the base image, we install it last. This will enable a follow up
patch that will do some setup enabling building the full container image with an
existing base image, thus enabling updating the GHA runner without modifying the
important bits.

Reviewers: EricWF, ldionne

Reviewed By: ldionne

Pull Request: #148072
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants