llvm: Only compile for the relevant platform by HadrienPatte · Pull Request #343 · cilium/image-tools

HadrienPatte · 2025-06-24T12:15:00Z

Overall the same thing as #339 but for the llvm image

Currently the llvm image compiles clang for both the "native" arch and crosscompiles for the "aarch64" arch, to then only keep the binary matching the TARGETPLATFORM.

With this PR we only build one version of clang based on TARGETPLATFORM.

Additional changes:

Only copy cst test spec in the test stage where it's used to avoid having it in the final image
Updated the COMPILERS_IMAGE ref as the one used was a single arch adm64 one, the new one is a proper multi-platform manifest:

crane manifest quay.io/cilium/image-compilers:1732033829-330cbaf@sha256:5c54f614fb8ee7939492aa4b7d74b37922d98199f5993f6d957a1637ce30eb9e
{
  "schemaVersion": 2,
  "mediaType": "application/vnd.docker.distribution.manifest.list.v2+json",
  "manifests": [
    {
      "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
      "digest": "sha256:302fc3b1a13cd7ecf28178f52dd7cdb1757353e3f9468aab7f6200a7912ea999",
      "size": 504,
      "platform": {
        "architecture": "amd64",
        "os": "linux"
      }
    },
    {
      "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
      "digest": "sha256:7ce45a14c376d51b5fa901436f863770d8f60624e4e35b74ff34fd598f5a50e2",
      "size": 504,
      "platform": {
        "architecture": "arm64",
        "os": "linux"
      }
    }
  ]
}

Test of the new image:

$ docker build --platform linux/amd64,linux/arm64 -t llvm ./images/llvm
[...]

$ docker run --platform linux/arm64 llvm /bin/bash -c "uname -a; echo; clang --version; echo; llc --version; echo; llvm-objcopy --version"
Linux 8b00122d228c 6.10.14-linuxkit #1 SMP Tue Apr 15 16:00:54 UTC 2025 aarch64 aarch64 aarch64 GNU/Linux

clang version 19.1.7 (https://github.com/llvm/llvm-project.git cd708029e0b2869e80abe31ddb175f7c35361f90)
Target: unknown
Thread model: posix
InstalledDir: /usr/local/bin

LLVM (http://llvm.org/):
  LLVM version 19.1.7
  Optimized build.
  Default target:
  Host CPU: (unknown)

  Registered Targets:
    bpf   - BPF (host endian)
    bpfeb - BPF (big endian)
    bpfel - BPF (little endian)

llvm-objcopy, compatible with GNU objcopy
LLVM (http://llvm.org/):
  LLVM version 19.1.7
  Optimized build.

$ docker run --platform linux/amd64 llvm /bin/bash -c "uname -a; echo; clang --version; echo; llc --version; echo; llvm-objcopy --version"
Linux b11f4f4fd23b 6.10.14-linuxkit #1 SMP Tue Apr 15 16:00:54 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux

clang version 19.1.7 (https://github.com/llvm/llvm-project.git cd708029e0b2869e80abe31ddb175f7c35361f90)
Target: unknown
Thread model: posix
InstalledDir: /usr/local/bin

LLVM (http://llvm.org/):
  LLVM version 19.1.7
  Optimized build.
  Default target:
  Host CPU: westmere

  Registered Targets:
    bpf   - BPF (host endian)
    bpfeb - BPF (big endian)
    bpfel - BPF (little endian)

llvm-objcopy, compatible with GNU objcopy
LLVM (http://llvm.org/):
  LLVM version 19.1.7
  Optimized build.

joestringer

Curious were you able to test these changes with full multiplatform Cilium images on top?

EDIT: Ah helps if I read the PR description and not just the code. 👍

images/llvm/checkout-llvm.sh

images/llvm/build-llvm.sh

joestringer · 2025-07-07T21:25:26Z

@cilium/loader @ti-mo looking for your review on this.

Signed-off-by: Timo Beckers <timo@isovalent.com>

Signed-off-by: Hadrien Patte <hadrien.patte@datadoghq.com>

ti-mo · 2025-07-08T08:43:24Z

Gave this a rebase and removed copyright years from file headers.

With #41230, renovate will now handle updating those images, but since they haven't been updated in a while, there's some manual adjustments that are required before renovate can handle those. With cilium/image-tools#339 and cilium/image-tools#343, these two images no longer include their test files. This PR removes the test stage from the `runtime` image as it was just rerunning those tests and there are no `runtime` image tests. This PR also fixes the tests for the `builder` image. It turns out that those tests have been broken since #32767 because they haven't been running in CI for years. Those tests aren't running since we switched from the legacy docker engine to buildkit, see [details](https://docs.docker.com/build/building/multi-stage/#differences-between-legacy-builder-and-buildkit): > The legacy Docker Engine builder processes all stages of a Dockerfile leading up to the selected --target. It will build a stage even if the selected target doesn't depend on that stage. > > BuildKit only builds the stages that the target stage depends on. Future followups: * Does it make sense to have a test expect a specific version of `libprotoc` when this dependency is regularly automatically updated by renovate? With the current setup, this test will break everytime renovate updates `libprotoc`. I'd argue that the version string should be removed from the expected output so we only test that `protoc --version` runs sucessfully without expecting a given version string. * Consider either: * Update the CI to ensure those container structure tests are run on PRs * Remove container structure tests as after this PR, only the `builder` image will have [some](https://github.com/cilium/cilium/blob/a7de0143835a080750dbbde7285be37ab8599883/images/builder/test/spec.yaml) and all they test is that `protoc` is installed in the image Note: the release note for this PR is more focused on the user-visible change related to the update of the `bpftools` and `llvm` images. ```release images: Update `bpftools` and `llvm` images to reduce the size of the `cilium` image by 39MB (`amd64`) / 35MB (arm64) ``` Signed-off-by: Hadrien Patte <hadrien.patte@datadoghq.com>

With #41230, renovate will now handle updating those images, but since they haven't been updated in a while, there's some manual adjustments that are required before renovate can handle those. With cilium/image-tools#339 and cilium/image-tools#343, these two images no longer include their test files. This PR removes the test stage from the `runtime` image as it was just rerunning those tests and there are no `runtime` image tests. This PR also fixes the tests for the `builder` image. It turns out that those tests have been broken since #32767 because they haven't been running in CI for years. Those tests aren't running since we switched from the legacy docker engine to buildkit, see [details](https://docs.docker.com/build/building/multi-stage/#differences-between-legacy-builder-and-buildkit): > The legacy Docker Engine builder processes all stages of a Dockerfile leading up to the selected --target. It will build a stage even if the selected target doesn't depend on that stage. > > BuildKit only builds the stages that the target stage depends on. Future followups: * Does it make sense to have a test expect a specific version of `libprotoc` when this dependency is regularly automatically updated by renovate? With the current setup, this test will break everytime renovate updates `libprotoc`. I'd argue that the version string should be removed from the expected output so we only test that `protoc --version` runs successfully without expecting a given version string. * Consider either: * Update the CI to ensure those container structure tests are run on PRs * Remove container structure tests as after this PR, only the `builder` image will have [some](https://github.com/cilium/cilium/blob/a7de0143835a080750dbbde7285be37ab8599883/images/builder/test/spec.yaml) and all they test is that `protoc` is installed in the image Note: the release note for this PR is more focused on the user-visible change related to the update of the `bpftools` and `llvm` images. ```release images: Update `bpftools` and `llvm` images to reduce the size of the `cilium` image by 39MB (`amd64`) / 35MB (arm64) ``` Signed-off-by: Hadrien Patte <hadrien.patte@datadoghq.com>

With cilium#41230, renovate will now handle updating those images, but since they haven't been updated in a while, there's some manual adjustments that are required before renovate can handle those. With cilium/image-tools#339 and cilium/image-tools#343, these two images no longer include their test files. This PR removes the test stage from the `runtime` image as it was just rerunning those tests and there are no `runtime` image tests. This PR also fixes the tests for the `builder` image. It turns out that those tests have been broken since cilium#32767 because they haven't been running in CI for years. Those tests aren't running since we switched from the legacy docker engine to buildkit, see [details](https://docs.docker.com/build/building/multi-stage/#differences-between-legacy-builder-and-buildkit): > The legacy Docker Engine builder processes all stages of a Dockerfile leading up to the selected --target. It will build a stage even if the selected target doesn't depend on that stage. > > BuildKit only builds the stages that the target stage depends on. Future followups: * Does it make sense to have a test expect a specific version of `libprotoc` when this dependency is regularly automatically updated by renovate? With the current setup, this test will break everytime renovate updates `libprotoc`. I'd argue that the version string should be removed from the expected output so we only test that `protoc --version` runs successfully without expecting a given version string. * Consider either: * Update the CI to ensure those container structure tests are run on PRs * Remove container structure tests as after this PR, only the `builder` image will have [some](https://github.com/cilium/cilium/blob/a7de0143835a080750dbbde7285be37ab8599883/images/builder/test/spec.yaml) and all they test is that `protoc` is installed in the image Note: the release note for this PR is more focused on the user-visible change related to the update of the `bpftools` and `llvm` images. ```release images: Update `bpftools` and `llvm` images to reduce the size of the `cilium` image by 39MB (`amd64`) / 35MB (arm64) ``` Signed-off-by: Hadrien Patte <hadrien.patte@datadoghq.com>

With #41230, renovate will now handle updating those images, but since they haven't been updated in a while, there's some manual adjustments that are required before renovate can handle those. With cilium/image-tools#339 and cilium/image-tools#343, these two images no longer include their test files. This PR removes the test stage from the `runtime` image as it was just rerunning those tests and there are no `runtime` image tests. This PR also fixes the tests for the `builder` image. It turns out that those tests have been broken since #32767 because they haven't been running in CI for years. Those tests aren't running since we switched from the legacy docker engine to buildkit, see [details](https://docs.docker.com/build/building/multi-stage/#differences-between-legacy-builder-and-buildkit): > The legacy Docker Engine builder processes all stages of a Dockerfile leading up to the selected --target. It will build a stage even if the selected target doesn't depend on that stage. > > BuildKit only builds the stages that the target stage depends on. Future followups: * Does it make sense to have a test expect a specific version of `libprotoc` when this dependency is regularly automatically updated by renovate? With the current setup, this test will break everytime renovate updates `libprotoc`. I'd argue that the version string should be removed from the expected output so we only test that `protoc --version` runs successfully without expecting a given version string. * Consider either: * Update the CI to ensure those container structure tests are run on PRs * Remove container structure tests as after this PR, only the `builder` image will have [some](https://github.com/cilium/cilium/blob/a7de0143835a080750dbbde7285be37ab8599883/images/builder/test/spec.yaml) and all they test is that `protoc` is installed in the image Note: the release note for this PR is more focused on the user-visible change related to the update of the `bpftools` and `llvm` images. ```release images: Update `bpftools` and `llvm` images to reduce the size of the `cilium` image by 39MB (`amd64`) / 35MB (arm64) ``` Signed-off-by: Hadrien Patte <hadrien.patte@datadoghq.com>

With cilium#41230, renovate will now handle updating those images, but since they haven't been updated in a while, there's some manual adjustments that are required before renovate can handle those. With cilium/image-tools#339 and cilium/image-tools#343, these two images no longer include their test files. This PR removes the test stage from the `runtime` image as it was just rerunning those tests and there are no `runtime` image tests. This PR also fixes the tests for the `builder` image. It turns out that those tests have been broken since cilium#32767 because they haven't been running in CI for years. Those tests aren't running since we switched from the legacy docker engine to buildkit, see [details](https://docs.docker.com/build/building/multi-stage/#differences-between-legacy-builder-and-buildkit): > The legacy Docker Engine builder processes all stages of a Dockerfile leading up to the selected --target. It will build a stage even if the selected target doesn't depend on that stage. > > BuildKit only builds the stages that the target stage depends on. Future followups: * Does it make sense to have a test expect a specific version of `libprotoc` when this dependency is regularly automatically updated by renovate? With the current setup, this test will break everytime renovate updates `libprotoc`. I'd argue that the version string should be removed from the expected output so we only test that `protoc --version` runs successfully without expecting a given version string. * Consider either: * Update the CI to ensure those container structure tests are run on PRs * Remove container structure tests as after this PR, only the `builder` image will have [some](https://github.com/cilium/cilium/blob/a7de0143835a080750dbbde7285be37ab8599883/images/builder/test/spec.yaml) and all they test is that `protoc` is installed in the image Note: the release note for this PR is more focused on the user-visible change related to the update of the `bpftools` and `llvm` images. ```release images: Update `bpftools` and `llvm` images to reduce the size of the `cilium` image by 39MB (`amd64`) / 35MB (arm64) ``` Signed-off-by: Hadrien Patte <hadrien.patte@datadoghq.com>

HadrienPatte marked this pull request as ready for review June 24, 2025 15:30

HadrienPatte requested a review from a team as a code owner June 24, 2025 15:30

HadrienPatte requested a review from ti-mo June 24, 2025 15:30

HadrienPatte force-pushed the pr/HadrienPatte/llvm branch from a09ee02 to 7275e42 Compare July 2, 2025 13:09

HadrienPatte mentioned this pull request Jul 4, 2025

tester: Only compile for the relevant platform #353

Merged

joestringer approved these changes Jul 7, 2025

View reviewed changes

images/llvm/checkout-llvm.sh Outdated Show resolved Hide resolved

images/llvm/build-llvm.sh Outdated Show resolved Hide resolved

ti-mo and others added 2 commits July 8, 2025 10:39

Remove copyright years from file headers

53c1a86

Signed-off-by: Timo Beckers <timo@isovalent.com>

llvm: Only compile for the relevant platform

efc4c15

Signed-off-by: Hadrien Patte <hadrien.patte@datadoghq.com>

ti-mo force-pushed the pr/HadrienPatte/llvm branch from 7275e42 to efc4c15 Compare July 8, 2025 08:42

ti-mo requested review from a team as code owners July 8, 2025 08:42

ti-mo requested review from YutaroHayakawa and rolinh July 8, 2025 08:42

ti-mo approved these changes Jul 8, 2025

View reviewed changes

rolinh approved these changes Jul 8, 2025

View reviewed changes

joestringer approved these changes Jul 16, 2025

View reviewed changes

joestringer merged commit 9daceb2 into master Jul 16, 2025
21 of 23 checks passed

joestringer deleted the pr/HadrienPatte/llvm branch July 16, 2025 00:28

HadrienPatte mentioned this pull request Aug 23, 2025

images: Update bpftools and llvm images cilium/cilium#41339

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

llvm: Only compile for the relevant platform#343

llvm: Only compile for the relevant platform#343
joestringer merged 2 commits intomasterfrom
pr/HadrienPatte/llvm

HadrienPatte commented Jun 24, 2025

Uh oh!

joestringer left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

joestringer commented Jul 7, 2025

Uh oh!

ti-mo commented Jul 8, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

HadrienPatte commented Jun 24, 2025

Uh oh!

joestringer left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

joestringer commented Jul 7, 2025

Uh oh!

ti-mo commented Jul 8, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

joestringer left a comment •

edited

Loading