Skip to content

Fix istio proxy build after moving Envoy to hermteic toolchain#6726

Merged
istio-testing merged 13 commits into
istio:masterfrom
krinkinmu:fix-envoy-build
Jan 8, 2026
Merged

Fix istio proxy build after moving Envoy to hermteic toolchain#6726
istio-testing merged 13 commits into
istio:masterfrom
krinkinmu:fix-envoy-build

Conversation

@krinkinmu
Copy link
Copy Markdown
Contributor

@krinkinmu krinkinmu commented Dec 5, 2025

What this PR does / why we need it:

It makes changes to the Istio proxy Bazel configs to migrate to using Envoy's hermetic LLVM toolchain for builds. The "hermetic" here means that as part of the build process Bazel will download the LLVM toolchain, including compiler, linker and libraries, and will use them to build.

One thing worth calling out specifically, is that Envoy will also download a sysroot that packages glibc vers 2.28, which is, I think, the same version of glibc that we are currently using, so we still should be able to run the resulting Envoy binary outside of the container environment on old OSes.

NOTE: llvm toolchain itself is not statically built, unfortunately, and not all required libraries come with the toolchain. One library specifically that I noticed might cause problems is libncurses. Clang in the hermetic llvm toolchain is built against libncurses version 5, while some major distributions (i.e., Ubuntu) moved towards version 6.

It does not matter when building inside a container - we use a pretty old base image that should have the required version of the libncurses library, however building things outside the container may require jumping through a few hoops to install the right libraries if the hermetic toolchain is used.

I don't think there is a sensible way to workaround this issue in the hermetic toolchain itself (I wish LLVM folks just released a static version of the toolchain, but alas they don't do that). So IMO the best path forward would be to just allow easy switching between hermetic toolchain and host toolchain, which this PR does not implement yet. However, if we ok in principle with this approach, I can probably add support for that.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

Signed-off-by: Mikhail Krinkin <mkrinkin@microsoft.com>
@istio-testing
Copy link
Copy Markdown
Collaborator

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@istio-testing istio-testing added the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Dec 5, 2025
@istio-policy-bot
Copy link
Copy Markdown

😊 Welcome @krinkinmu! This is either your first contribution to the Istio proxy repo, or it's been
a while since you've been here.

You can learn more about the Istio working groups, Code of Conduct, and contribution guidelines
by referring to Contributing to Istio.

Thanks for contributing!

Courtesy of your friendly welcome wagon.

@istio-testing istio-testing added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 5, 2025
@krinkinmu
Copy link
Copy Markdown
Contributor Author

/test all

Copy link
Copy Markdown
Member

@howardjohn howardjohn left a comment

Choose a reason for hiding this comment

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

Does this mean you can finally just run bazel build and it works??? amazing!!

@krinkinmu
Copy link
Copy Markdown
Contributor Author

Does this mean you can finally just run bazel build and it works??? amazing!!

Almost, but there are corner cases. E.g., llvm tools themselves are not built statically, so they still depend on some external libraries, but at the very least installing clang should not be necessary.

And we still need to figure out if we want to go this way or rely on a local toolchain.

@krinkinmu krinkinmu mentioned this pull request Dec 5, 2025
Comment thread .bazelrc Outdated
Signed-off-by: Mikhail Krinkin <mkrinkin@microsoft.com>
@zirain
Copy link
Copy Markdown
Member

zirain commented Dec 9, 2025

/test all

@zirain
Copy link
Copy Markdown
Member

zirain commented Dec 12, 2025

@krinkinmu can you update your PR base on envoyproxy/envoy#42572?

@krinkinmu
Copy link
Copy Markdown
Contributor Author

@zirain will do that later today

Signed-off-by: Mikhail Krinkin <mkrinkin@microsoft.com>
@krinkinmu
Copy link
Copy Markdown
Contributor Author

/test all

@krinkinmu
Copy link
Copy Markdown
Contributor Author

Interesting... It works locally, but not on CI. I need to dig a bit more.

Signed-off-by: Mikhail Krinkin <mkrinkin@microsoft.com>
@krinkinmu
Copy link
Copy Markdown
Contributor Author

/test all

@krinkinmu
Copy link
Copy Markdown
Contributor Author

At least it's building now...

@krinkinmu
Copy link
Copy Markdown
Contributor Author

@zirain it looks like it builds. I'd need to double check what it's actually building and that the version of GLIBC it needs is indeed 2.28, but on the surface it looks ok.

@krinkinmu
Copy link
Copy Markdown
Contributor Author

Aha... I missed something in the configuration apparently - it didn't really use the hermetic sysroot and instead used host GLIBC, which is ok when building insider a container, but not what we actually wanted.

Signed-off-by: Mikhail Krinkin <mkrinkin@microsoft.com>
@krinkinmu
Copy link
Copy Markdown
Contributor Author

/test all

@krinkinmu
Copy link
Copy Markdown
Contributor Author

I tested it locally and it builds using the right sysroot with glibc 2.28

Signed-off-by: Mikhail Krinkin <mkrinkin@microsoft.com>
@krinkinmu
Copy link
Copy Markdown
Contributor Author

/test all

Signed-off-by: Mikhail Krinkin <mkrinkin@microsoft.com>
@krinkinmu
Copy link
Copy Markdown
Contributor Author

/test all

@krinkinmu krinkinmu requested a review from zirain December 15, 2025 17:43
@krinkinmu
Copy link
Copy Markdown
Contributor Author

Regarding libncurses problem - we discussed in Slack a bit and it seems to be preferable to just ignore it for now. The issue only affects those who build proxy outside of the container and for folks advanced enough to go that path it shouldn't be too difficult to go through a few additional hoops to install libncurses5 if it's not available.

Eventually we will move to a newer version of clang and newer versions shouldn't have that problem anymore, so eventually we wouldn't need to worry about it at all.

@krinkinmu
Copy link
Copy Markdown
Contributor Author

/test release-test-arm64

3 similar comments
@krinkinmu
Copy link
Copy Markdown
Contributor Author

/test release-test-arm64

@fjglira
Copy link
Copy Markdown

fjglira commented Jan 7, 2026

/test release-test-arm64

@keithmattix
Copy link
Copy Markdown
Contributor

/test release-test-arm64

Signed-off-by: Mikhail Krinkin <mkrinkin@microsoft.com>
@krinkinmu
Copy link
Copy Markdown
Contributor Author

/test release-test-arm64

Signed-off-by: Mikhail Krinkin <mkrinkin@microsoft.com>
@krinkinmu
Copy link
Copy Markdown
Contributor Author

/test release-test-arm64

Signed-off-by: Mikhail Krinkin <mkrinkin@microsoft.com>
@krinkinmu
Copy link
Copy Markdown
Contributor Author

/test release-test-arm64

Signed-off-by: Mikhail Krinkin <mkrinkin@microsoft.com>
@krinkinmu
Copy link
Copy Markdown
Contributor Author

/test release-test-arm64

Signed-off-by: Mikhail Krinkin <mkrinkin@microsoft.com>
@krinkinmu
Copy link
Copy Markdown
Contributor Author

/test all

@krinkinmu krinkinmu marked this pull request as ready for review January 7, 2026 23:00
@krinkinmu krinkinmu requested a review from a team as a code owner January 7, 2026 23:00
@istio-testing istio-testing removed the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Jan 7, 2026
@krinkinmu
Copy link
Copy Markdown
Contributor Author

/test release-test

@zirain
Copy link
Copy Markdown
Member

zirain commented Jan 8, 2026

/retest

Copy link
Copy Markdown
Member

@zirain zirain left a comment

Choose a reason for hiding this comment

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

Thanks for taking this forward.

@zirain
Copy link
Copy Markdown
Member

zirain commented Jan 8, 2026

Regarding libncurses problem - we discussed in Slack a bit and it seems to be preferable to just ignore it for now. The issue only affects those who build proxy outside of the container and for folks advanced enough to go that path it shouldn't be too difficult to go through a few additional hoops to install libncurses5 if it's not available.

Eventually we will move to a newer version of clang and newer versions shouldn't have that problem anymore, so eventually we wouldn't need to worry about it at all.

this's fine for most of the cases.

it would be better if you can add a hint(use clang-19+ when you're building proxy outside of devcontainer) in README.

@dhawton
Copy link
Copy Markdown
Member

dhawton commented Jan 8, 2026

/retest

@istio-testing istio-testing merged commit 175fddf into istio:master Jan 8, 2026
7 checks passed
krinkinmu added a commit to krinkinmu/community that referenced this pull request Jan 28, 2026
I'm an Envoy maintainer and have made contributions in proxy repository
and telemetry related changes in other repos:

* istio/proxy#6726 - fix proxy build after Envoy
  move to hermetic toolchain (including debugging the OOM issue in our
  CI that broke tests)
* istio/proxy#6793 (plus there are a few in
  flight PRs)
* designed telemetry changes for ambient multi-network (see
  https://docs.google.com/document/d/1gyDsViXTHlozTbHFuggNmYFrL6P3om4ZvTMdy_-Ul74/edit?usp=sharing)

There are a few other contributions in Istio that I made (particularly
in ambient multi-cluster), but they are not directly related to
telemetry.

Signed-off-by: Mikhail Krinkin <mkrinkin@microsoft.com>
istio-testing pushed a commit to istio/community that referenced this pull request Jan 30, 2026
* Add krinkinmu as a telemetry maintainer

I'm an Envoy maintainer and have made contributions in proxy repository
and telemetry related changes in other repos:

* istio/proxy#6726 - fix proxy build after Envoy
  move to hermetic toolchain (including debugging the OOM issue in our
  CI that broke tests)
* istio/proxy#6793 (plus there are a few in
  flight PRs)
* designed telemetry changes for ambient multi-network (see
  https://docs.google.com/document/d/1gyDsViXTHlozTbHFuggNmYFrL6P3om4ZvTMdy_-Ul74/edit?usp=sharing)

There are a few other contributions in Istio that I made (particularly
in ambient multi-cluster), but they are not directly related to
telemetry.

Signed-off-by: Mikhail Krinkin <mkrinkin@microsoft.com>

* Fix test

Signed-off-by: Mikhail Krinkin <mkrinkin@microsoft.com>

---------

Signed-off-by: Mikhail Krinkin <mkrinkin@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants