Move to CentOS 7; begin adding profiling deps#1660
Conversation
| - "Prepare Code": | ||
| filters: | ||
| branches: | ||
| ignore: levi/profiling-rs |
There was a problem hiding this comment.
I think we can now remove this (though it was a good idea to gain some CI processing time)
There was a problem hiding this comment.
Huh, that part was removed before I ever merged that PR. Guess the branch/merge/squash order meant this one kept it or something.
| dockerfile: Dockerfile_profiling | ||
| args: | ||
| baseImage: datadog/dd-trace-ci:php-8.0_alpine-3.16 | ||
| # baseImage: localhost:5000/datadog/dd-trace-ci:alpine-3.16 |
There was a problem hiding this comment.
todo for myself: clean this up (this was to point at local registry as things were not working with remote)
| @@ -0,0 +1,116 @@ | |||
| FROM centos:7 | |||
There was a problem hiding this comment.
Yay, old, but not so old (and still maintained version) 👍
Co-authored-by: r1viollet <[email protected]>
2489ff1 to
f06e92e
Compare
Codecov Report
@@ Coverage Diff @@
## master #1660 +/- ##
=======================================
Coverage 79.04% 79.04%
=======================================
Files 125 125
Lines 12349 12349
=======================================
Hits 9761 9761
Misses 2588 2588 📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
5de3ea8 to
15c9902
Compare
morrisonlevi
changed the title
| @@ -5,73 +5,75 @@ services: | |||
| image: 'datadog/dd-trace-ci:centos-6' | |||
There was a problem hiding this comment.
Is this github or is this file still in a centos/6 dir instead of centos/7 ? Shouldn't we just clean this up fully here, only centos 7?
There was a problem hiding this comment.
I like the confidence :-)
Levi mentioned he wanted to keep both alive for now (and he usually really likes to nuke things), should we consider removing everything now ?
There was a problem hiding this comment.
Just chiming in to confirm that I think nuking it just now is a bit premature. The more we nuke the harder it will be to restore in case we need to ^_^
Although I suppose it's easily copyable from the php-5 branch anyway?
bwoebi
left a comment
bwoebi
left a comment
There was a problem hiding this comment.
Generally looks good to me.
4 participants
Description
This is a continuation of #1648 which was unintentionally closed earlier today. Squashed and rebased.
This drops CentOS 6 from CI but doesn't quite nuke everything related to it in case we need to revert. CentOS 7 is now the oldest OS we support. This does update the CentOS 6 images to use the latest released versions of PHP. I built them locally but haven't pushed them up yet.
This also adds profiling dependencies like protoc-c, libclang, clang, and more. The image size is a bit larger but hopefully not too much slower to be unusable in CI. It needs a lot of memory and time to build LLVM/Clang 14 (which matches the Rust version we use, 1.60), and needs at least clang 12 to parse PHP 8.1 headers which is why we can't just use the llvm toolset in the OS repos. This PR isn't using these profiling dependencies in CI just quite yet, though I tried building locally and it worked. So future PRs may adjust it but hopefully not too much.
Readiness checklist
Tests added for this feature/bug.Eh, kinda.Reviewer checklist