-
Notifications
You must be signed in to change notification settings - Fork 9
Swap fetcher image from openjdk to Corretto #808
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
Conversation
WalkthroughThe Dockerfile for the fetcher service was updated to use an Alpine-based Amazon Corretto OpenJDK 17 image instead of Debian-based OpenJDK. Package installation commands were switched from Changes
Possibly related PRs
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docker/fetcher/Dockerfile (1)
14-23: Alpine package installation: Migration toapkis correct. Consider slimming the image by removing unneeded tools (vim,bash) and dev deps. Confirm if Python libs and Bash are required at runtime.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docker/fetcher/Dockerfile(2 hunks)
🔇 Additional comments (1)
docker/fetcher/Dockerfile (1)
2-2: Update base image to Amazon Corretto Alpine: Solid upgrade to a maintained OpenJDK variant.
chewy-zlai
left a comment
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
## Summary Add support in the fetcher docker container to publish profiles to Google Cloud Profiler. Largely based on the [docs here](https://cloud.google.com/profiler/docs/profiling-java) We bake in the gcloud profiler so file in our docker image and if users opt-in via the env var (`ENABLE_GCLOUD_PROFILER`) we include in the JVM startup options. According to the [Google docs](https://cloud.google.com/profiler/docs/concepts-profiling), the overhead is low enough to keep running continuously in prod so we can start with an explicit enable / disable and fine-tune if needed. Had to test this out on a Gcloud VM and while doing so I noticed that my prior change to switch to Corretto (#808) doesn't work on Intel / AMD machines due to something messed up in the netty native lib so files. I switched back to a Debian based openjdk implementation and that seems to get things working. Profiles can be viewed on our canary - [Link](https://console.cloud.google.com/profiler;timespan=30m/chronon-fetcher;type=HEAP/inuse_space?inv=1&invt=AbyoEQ&project=canary-443022). They don't show anything super useful atm as I'm not running traffic thro' the system but we can see things are working..  ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Chores** - Updated the Docker image to use a Debian-based Java runtime for improved native library compatibility. - Adjusted package installation commands to use Debian's package manager. - Integrated Google Cloud Profiler Java agent into the container setup. - Modified startup script to support enabling the profiler via an environment variable and streamlined JVM options handling. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
While exploring setting up a GitHub action to publish our images to docker's registry and working through some auth issues, I noticed that the openjdk images are on the train to retirement (docker-library/openjdk#505 - seems definitely the case for 8 and 11, and the suggestion seems to be to switch to temurin in general). I have swapped us out to Amazon's corretto images as corretto is a fairly commonly used openjdk implementation and the images are available from Amazon's public ECR registries.
I was able to confirm I can build and publish using these images via GH actions (PR on platform incoming) and also test locally by pulling the image and starting up the containers:
Checklist
Summary by CodeRabbit