Skip to content
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

Dockerfile.clang: Don't pin clang patch version #1530

Merged
merged 1 commit into from
Sep 29, 2023
Merged

Conversation

michi-covalent
Copy link
Contributor

@michi-covalent michi-covalent commented Sep 29, 2023

I boldly assert that it's sufficient to specify the clang version by
using clang-15 package. This way we can pick up the latest security
release automatically. Otherwise this Dockerfile keeps breaking when
there is a new security release and the old package gets deleted.

Ref: https://github.com/cilium/tetragon/actions/runs/6355442579/job/17263533977#step:6:239
Ref: https://packages.ubuntu.com/jammy/clang-15

Signed-off-by: Michi Mutsuzaki [email protected]

@michi-covalent michi-covalent added the release-note/ci This PR makes changes to the CI. label Sep 29, 2023
@michi-covalent michi-covalent marked this pull request as ready for review September 29, 2023 18:31
@michi-covalent michi-covalent requested a review from a team as a code owner September 29, 2023 18:31
@michi-covalent
Copy link
Contributor Author

@chancez
Copy link
Contributor

chancez commented Sep 29, 2023

This is probably because the base image is ubuntu:22.04 and a recent update went from 22.04.2 to 22.04.3 is my guess. Is there a reason the apt install is specifying specific versions? You're basically getting the latest of the package anyways, and have to use the correct version of the package for the base image's Ubuntu patch release version anyways.

@michi-covalent
Copy link
Contributor Author

yeah not sure why we are pinning the version. @jrfastab might know 🤔

@willfindlay
Copy link
Contributor

Our BPF programs are very sensitive to compiler version. That's why we're trying to pin it.

@michi-covalent michi-covalent force-pushed the pr/michi/clang branch 2 times, most recently from 6e5d69c to 81e7a56 Compare September 29, 2023 18:51
@michi-covalent
Copy link
Contributor Author

Our BPF programs are very sensitive to compiler version. That's why we're trying to pin it.

i see, but what can we do if the package keeps disappearing 💭

@willfindlay
Copy link
Contributor

@michi-covalent I'm not super familiar with apt but if there's a way to pin it to 1.15 without caring about patch version that would be ideal

@michi-covalent
Copy link
Contributor Author

yeah it's already pinned, they have different packages for claing, like clang-15, so it won't randomly get bumped up to clang-16 for example 🥰 let me update this PR.

I boldly assert that it's sufficient to specify the clang version by
using clang-15 package. This way we can pick up the latest security
release automatically. Otherwise this Dockerfile keeps breaking when
there is a new security release and the old package gets deleted.

Ref: https://github.com/cilium/tetragon/actions/runs/6355442579/job/17263533977#step:6:239
Ref: https://packages.ubuntu.com/jammy/clang-15

Signed-off-by: Michi Mutsuzaki <[email protected]>
@willfindlay
Copy link
Contributor

ERROR: failed to solve: docker.io/library/golang:1.21.1@sha256:19600fdcae402165dcdab18cb9649540bde6be7274dedb5d205b2f84029fe909: failed to copy: httpReadSeeker: failed open: unexpected status code https://registry-1.docker.io/v2/library/golang/manifests/sha256:13a7dc7b46068caeaf811cc8af56d394a89c97d9b194550f8be5a4de05ca5f99: 429 Too Many Requests - Server message: toomanyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: https://www.docker.com/increase-rate-limit

:jarno-eclipse:

@michi-covalent michi-covalent changed the title Bump up clang version Dockerfile.clang: Don't pin clang patch version Sep 29, 2023
@michi-covalent
Copy link
Contributor Author

:jarno-eclipse:

yeah dockerhub rate limits actions runners that are not hosted by github. opened #1532 to make things slightly better.

@jrfastab
Copy link
Contributor

ok but wish we were using clang-16

@jrfastab
Copy link
Contributor

I'm ok to try this

Copy link
Contributor

@jrfastab jrfastab left a comment

Choose a reason for hiding this comment

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

ACK but if it gives us trouble we can revert.

@michi-covalent michi-covalent merged commit 8a42df9 into main Sep 29, 2023
33 checks passed
@michi-covalent michi-covalent deleted the pr/michi/clang branch September 29, 2023 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/ci This PR makes changes to the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants