-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Smoke test binaries for EL7/glibc-2.17 compatiblity #17706
Conversation
bc86b86
to
b2721a7
Compare
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.
LGTM! Thanks @dekimsey!
Verified in https://github.com/hashicorp/nomad-enterprise/actions/runs/5359393093
and in this PR in https://github.com/hashicorp/nomad/actions/runs/5359921287/jobs/9724253179?pr=17706
I'll merge this in once CI is done.
b2721a7
to
63915a6
Compare
63915a6
to
3f7e5fb
Compare
Looks like everything is clean, so one last push to drop the on.push override used for testing. |
3f7e5fb
to
0822c83
Compare
Looks like this failure https://github.com/hashicorp/nomad/actions/runs/5360327078/jobs/9725065223?pr=17706 was introduced by #17705. And that UI test is a known flake.) Don't worry about that for this PR. Let's get this merged in and I'll fix the other one. |
Fixed that command test in #17715 |
This adds a quick smoke test of our binaries to verify we haven't exceeeded the maximum GLIBC (2.17) version during linking which would break our ability to execute on EL7 machines.
0822c83
to
0cb728e
Compare
@gulducat does this PR need to get updated in order to work with the changes we made for permissions/secrets/ENT runners? |
@tgross doesn't look to me like it needs anything extra since there are no secrets being used 👍 |
Summary
This adds a quick smoke test of our binaries to verify we haven't exceeded the maximum glibc (2.17) version supported on older OSes (EL7). Which would break our ability to execute on older OSes.
Why
This is related to our recent efforts to upgrade our GH build environment. When CGO enabled builds are built on Ubuntu-22 they include symbols to glibc versions (2.32+) that exceed our desired minimum OS version (EL7). To mitigate we are instead building on Ubuntu-20. However, Ubuntu-20 itself also includes a newer glibc version. So we need to make sure we check the binaries to prevent us from unintentionally dropping support by adding explicit smoke testing of the binary.
For clarity, here's a breakdown of distribution and glibc versions to provide a bit more context to why we should test on EL7:
Notes
This only affects CGO_ENABLED=1 binaries (like nomad).
This smoke test is only verifying
amd64
binaries because RH doesn't have anarm64
container image for EL7 even though it ships the OS (this is fixed as of ubi8). However, no version releases anarm
image.Related
(cross-liking for HC reference)