Replace cimg base image with ubuntu:latest#95
Conversation
delner
left a comment
delner
left a comment
There was a problem hiding this comment.
General Docker changes look good to me. Not sure how to answer the question about pip and --break-system-packages.
bric3
left a comment
bric3
left a comment
There was a problem hiding this comment.
I believe the built image is missing a few tools used in the scripts here and there in dd-trace-java.
Got a few questions / remarks though, that may be addressed later.
-
I noticed the versioning is based on year + month
dd-trace-java-docker-build/build
Line 212 in 3c26d22
Since this is like a major change shall we label this differently? That said this could affect other part of the build pipelines.
-
Circle CI image appear to use the
circleciuser, this requires commands to be run as sudo. Should we reproduce this behavior?
sarahchen6
force-pushed
the
sarahchen6/change-base-image
branch
from
3763033 to
bdee917
Compare
@bric3 What do you mean by labelling differently? From this PR, it seems like the intention is to label each image by when it was last pulled, so year/month seems to make sense 🤔 |
@bric3 Good point! Looking into this, it seems like it is good security practice for the Dockerfile to run as a non-root user (one ref). However, we can still install everything at the root without using the |
PerfectSlayer
left a comment
PerfectSlayer
left a comment
There was a problem hiding this comment.
Looking good.
Few comments about cleaning it up further:
- Is the
yq,dockeranddocker-composestill needed? I used to install them manually to override the outdated version from the CircleCI image but I don't know if they are used. - Similarly I don't know if autoforward is still used 🤷
- And I ask to @smola what
ubuntu17is for and if it is still relevant
Do you know how to test the image from your PR in the CI to test it?
Dockerfile
Outdated
| sudo cp -rf --remove-destination /etc/java-17-openjdk/* /usr/lib/jvm/ubuntu17/lib/ | ||
| sudo cp -f --remove-destination /etc/java-17-openjdk/jvm-amd64.cfg /usr/lib/jvm/ubuntu17/lib/ | ||
| apt-get update | ||
| apt-get install -y openjdk-17-jdk |
|
You might want to revisit the cron time too: |
sarahchen6
changed the title
bric3
left a comment
bric3
left a comment
There was a problem hiding this comment.
For the tags, it's just that the current image inherit cimg, while this PR changes it to ubuntu, I pondered on making the change of the base image more visible via tags.
sarahchen6
force-pushed
the
sarahchen6/change-base-image
branch
2 times, most recently
from
0144d54 to
a485235
Compare
|
Testing the image build with |
sarahchen6
force-pushed
the
sarahchen6/change-base-image
branch
from
f0d76bd to
727afce
Compare
|
🎉 |
4 participants
With our transition from CircleCI to Gitlab, we want to replace the
cimgbase image with the smaller, faster, saferubuntu:24.04image.