chore(ci): build ADP against glibc once again - #2102
Conversation
|
Regression Detector (Agent Data Plane)Optimization Goals:
|
…matches the target glibc version for the given build target/platform
tobz
added
the
type/chore
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8920ee32bc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Ran a manual verification to check the min glibc version: amd64 arm64 |
jszwedko
left a comment
jszwedko
left a comment
There was a problem hiding this comment.
One comment about the floor for aarch64, otherwise 👍
| @@ -0,0 +1,61 @@ | |||
| #!/usr/bin/env sh | |||
There was a problem hiding this comment.
Big ➕ to this check. I remember we often accidentally broke glibc compatibility on Vector.
| export CC_aarch64_unknown_linux_gnu="${_ctng}-gcc" | ||
| export CXX_aarch64_unknown_linux_gnu="${_ctng}-g++" | ||
| export AR_aarch64_unknown_linux_gnu="${_ctng}-ar" | ||
| TARGET_MAX_GLIBC="2.23" |
There was a problem hiding this comment.
Is this the right glibc max? Looking at the Agent docker image, at least, the Agent seems to only depend on glibc 2.17. Also, ADP seems to only use 2.18:
root@594c77fe9a2e:/# objdump -T /usr/local/bin/agent-data-plane | grep GLIBC | sed 's/.*GLIBC_\([.0-9]*\).*/\1/g' | sort -Vu
2.17
2.18
There was a problem hiding this comment.
This is the right one I believe, yes.
Based on our supported platforms matrix for ARMv8 (aka ARM64), the minimum glibc version jumps up to 2.23.
There was a problem hiding this comment.
Tested with the aarch64 deb package too: the Agent seems to only depend on 2.17.
There was a problem hiding this comment.
As far as the 2.18 thing... you have to separate out weak symbols. The "floor" is the highest glibc version detected for non-weak symbols, which are the symbols that, when missing, will cause the process to panic.
Weak symbols are basically the spiritual equivalent of Option<T>.
There was a problem hiding this comment.
For posterity: we opted to set the floor at 2.17 for both AMD64 and ARM64 since they're actually building that way currently. We'll opt to raise the ARM64 side back up to 2.23 after chatting with folks internally: the supported platforms aspect is correct, but we might actually want to expand it further back if possible.
## Summary The DNS-avoidance workaround added in #2041 (and followed up in #2060) was needed because the previous musl build of ADP failed to bootstrap when \`/etc/resolv.conf\` had no nameservers — Hickory could not load system DNS config even though the Core Agent IPC endpoint is a literal loopback address that requires no resolution. Now that ADP builds against glibc again (#2102), that limitation is gone. This removes the workaround (`endpoint_requires_dns_resolution`, `without_dns_resolution`, and related scaffolding) while keeping the `adp-ipc-no-dns` integration test as a regression guard. ## Test plan - `adp-ipc-no-dns` integration test confirms ADP bootstraps correctly with an empty `/etc/resolv.conf` - Unit tests that covered the removed code (`endpoint_requires_dns_resolution`, `connector_builds_without_dns_resolution`) are removed along with the code they covered 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: jesse.szwedko <jesse.szwedko@datadoghq.com>
## Summary The DNS-avoidance workaround added in #2041 (and followed up in #2060) was needed because the previous musl build of ADP failed to bootstrap when \`/etc/resolv.conf\` had no nameservers — Hickory could not load system DNS config even though the Core Agent IPC endpoint is a literal loopback address that requires no resolution. Now that ADP builds against glibc again (#2102), that limitation is gone. This removes the workaround (`endpoint_requires_dns_resolution`, `without_dns_resolution`, and related scaffolding) while keeping the `adp-ipc-no-dns` integration test as a regression guard. ## Test plan - `adp-ipc-no-dns` integration test confirms ADP bootstraps correctly with an empty `/etc/resolv.conf` - Unit tests that covered the removed code (`endpoint_requires_dns_resolution`, `connector_builds_without_dns_resolution`) are removed along with the code they covered 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: jesse.szwedko <jesse.szwedko@datadoghq.com> 8c7f7a8
## Summary This PR removes Hickory DNS in favor of glibc/GAI. After cutting back to glibc over MUSL in #2102, we want to follow-up by also removing Hickory DNS such that we revert back completely to relying on glibc for DNS resolution. This ensures that any glibc/NSS black magic that is configured on a given system works properly when ADP is used the same as it would when operations are happening in the Datadog Agent directly. We've kept our own resolver implementation but simply forward to `hyper`'s `GaiResolver` under the hood. This leaves the necessary scaffolding in place to, in the future, change how we spawn the underlying tasks that run the actual resolve operations. ## Change Type - [ ] Bug fix - [ ] New feature - [x] Non-functional (chore, refactoring, docs) - [ ] Performance ## How did you test this PR? Existing unit and integration tests. ## References DADP-2 Closes #2115 Co-authored-by: toby.lawrence <toby.lawrence@datadoghq.com>
## Summary This PR removes Hickory DNS in favor of glibc/GAI. After cutting back to glibc over MUSL in #2102, we want to follow-up by also removing Hickory DNS such that we revert back completely to relying on glibc for DNS resolution. This ensures that any glibc/NSS black magic that is configured on a given system works properly when ADP is used the same as it would when operations are happening in the Datadog Agent directly. We've kept our own resolver implementation but simply forward to `hyper`'s `GaiResolver` under the hood. This leaves the necessary scaffolding in place to, in the future, change how we spawn the underlying tasks that run the actual resolve operations. ## Change Type - [ ] Bug fix - [ ] New feature - [x] Non-functional (chore, refactoring, docs) - [ ] Performance ## How did you test this PR? Existing unit and integration tests. ## References DADP-2 Closes #2115 Co-authored-by: toby.lawrence <toby.lawrence@datadoghq.com> c35e0a9
2 participants
Summary
This PR switches ADP to build against glibc instead of MUSL.
A long time ago, we made the decision to link against MUSL, instead of glibc, as our "libc" library. We did this because cleanly and repeatably building against an old enough version of glibc (in keeping with the Datadog Agent's oldest supported operating system versions) was very difficult and fragile. Linking against MUSL, and statically linking in general, let us control our own destiny by not having to worry about using an insanely old build toolchain.
However, as we've started working ADP into the Datadog Agent releases, small issues have come up around our lack of glibc usage.... things like servers that do all of their DNS resolution through glibc NSS/SSSD magic, which totally bypasses normal DNS resolution semantics relied upon by MUSL or other userland DNS library implementations. Similarly, other concerns around the lack of dynamic linking at all have come up over time, namely over what it might mean to try and integrate non-Rust code into ADP.
Rather than continue to step on more landmines around glibc incompatibilities, or a lack of dynamic linking, we've taken another look at once again building ADP against glibc, and the right version of glibc... and things look better these days, so we're switching back.
This PR, in a nutshell, does the following:
build-cicontainer image to be based off ofdatadog-agent-buildimages/linux, the container image used by the actual DDA CI for building the Agent on LinuxThere's still some follow-up cleanup we'll need to do, but in general, this is mostly solved by using the container image that Agent uses and just setting the right settings to tell Cargo to invoke things with the crosstool-NG toolchain so that the right glibc version gets used.
Change Type
How did you test this PR?
Ensured that the existing CI build jobs (AMD64/ARM64, FIPS vs non-FIPS) still built correctly and that they built against glibc, and the right version of glibc.
References
DADP-2