- 
                Notifications
    
You must be signed in to change notification settings  - Fork 38
 
Switched Docker runtime image to jlink #371
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
Details: * bumped JDK to the latest 21 in the builder image * switched the runtime base image to chainguard-base * updated git version to 2.50.1-r1 * used jlink to create smaller image without not needed JDK modules, man pages and so on
| 
           PS. on my machine this made the image size ~360MiB.  | 
    
| 
           buildkite test this  | 
    
          
 Hey @pioorg thanks for coming back! I expected the main benefit of this change would've been the image size, but 360MB is larger than the current size (which I believe is sitting somewhere around 240-250MB). Or is the main aim to use most up-to-date versions (git, jdk), and while doing that also minimise the impact of the bump on the overall image size?  | 
    
          
 Hi @lorenabalan The updates of the JDK version is a kind of by-product, which IMHO should happen anyway, because   | 
    
| 
           buildkite test this  | 
    
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.
Hi @lorenabalan I currently see over 500MiB on my machine:
Ooh okay, that's not what I see in the registry. Locally I get a different number too but I think the size in Docker desktop is a bit misleading, so I've been looking more at what docker inspect returns.
docker pull docker.elastic.co/integrations/crawler:latest 
docker inspect docker.elastic.co/integrations/crawler:latest --format='{{.Size}}' | awk '{printf "%.2f MB\n", $1/1024/1024}'
I checked that the new wolfi image is indeed smaller
make build-docker-wolfi
docker inspect crawler-ci-wolfi --format='{{.Size}}' | awk '{printf "%.2f MB\n", $1/1024/1024}'
159.01 MB on this branch vs 242.98 MB on main.
Approving based on the above, cc @mattnowzari for visibility. 😊
| 
           On my Mac this yields 343.32 MB vs 602.47 MB 🤷  | 
    
          💔 Failed to create backport PR(s)The backport operation could not be completed due to the following error: The backport PRs will be merged automatically after passing CI. To backport manually run:  | 
    
Details: * bumped JDK to the latest 21 in the builder image * switched the runtime base image to chainguard-base * updated git version to 2.50.1-r1 * used `jlink `to create smaller image without not needed JDK modules, man pages and so on ### Closes #370 ### Checklists <!--You can remove unrelated items from checklists below and/or add new items that may help during the review.--> #### Pre-Review Checklist - [x] This PR does NOT contain credentials of any kind, such as API keys or username/passwords (double check `crawler.yml.example` and `elasticsearch.yml.example`) - [ ] This PR has a meaningful title - [ ] This PR links to all relevant GitHub issues that it fixes or partially addresses - If there is no GitHub issue, please create it. Each PR should have a link to an issue - [ ] this PR has a thorough description - [ ] Covered the changes with automated tests - [x] Tested the changes locally - [ ] Added a label for each target release version (example: `v0.1.0`) - [ ] Considered corresponding documentation changes - [ ] Contributed any configuration settings changes to the configuration reference - [ ] Ran `make notice` if any dependencies have been added #### Changes Requiring Extra Attention <!--Please call out any changes that require special attention from the reviewers and/or increase the risk to availability or security of the system after deployment. Remove the ones that don't apply.--> This PR **has to be well tested before merging**, to ensure all necessary modules are present in the customised JDK image. Until proper CI and tests are completed, please treat it as a work in progress. - [ ] Security-related changes (encryption, TLS, SSRF, etc) - [ ] New external service dependencies added. ### Related Pull Requests <!--List any relevant PRs here or remove the section if this is a standalone PR. * https://github.com/elastic/.../pull/123--> ### Release Note <!--If you think this enhancement/fix should be included in the release notes, please write a concise user-facing description of the change here. You should also label the PR with `release_note` so the release notes author(s) can easily look it up.--> (cherry picked from commit 6e66f73)
          💚 All backports created successfully
 Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation  | 
    
# Backport This will backport the following commits from `main` to `0.4`: - [Switched Docker runtime image to jlink (#371)](#371) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) Co-authored-by: Piotr Przybył <23506256+pioorg@users.noreply.github.com>
Details: * bumped JDK to the latest 21 in the builder image * switched the runtime base image to chainguard-base * updated git version to 2.50.1-r1 * used `jlink `to create smaller image without not needed JDK modules, man pages and so on ### Closes #370 ### Checklists <!--You can remove unrelated items from checklists below and/or add new items that may help during the review.--> #### Pre-Review Checklist - [x] This PR does NOT contain credentials of any kind, such as API keys or username/passwords (double check `crawler.yml.example` and `elasticsearch.yml.example`) - [ ] This PR has a meaningful title - [ ] This PR links to all relevant GitHub issues that it fixes or partially addresses - If there is no GitHub issue, please create it. Each PR should have a link to an issue - [ ] this PR has a thorough description - [ ] Covered the changes with automated tests - [x] Tested the changes locally - [ ] Added a label for each target release version (example: `v0.1.0`) - [ ] Considered corresponding documentation changes - [ ] Contributed any configuration settings changes to the configuration reference - [ ] Ran `make notice` if any dependencies have been added #### Changes Requiring Extra Attention <!--Please call out any changes that require special attention from the reviewers and/or increase the risk to availability or security of the system after deployment. Remove the ones that don't apply.--> This PR **has to be well tested before merging**, to ensure all necessary modules are present in the customised JDK image. Until proper CI and tests are completed, please treat it as a work in progress. - [ ] Security-related changes (encryption, TLS, SSRF, etc) - [ ] New external service dependencies added. ### Related Pull Requests <!--List any relevant PRs here or remove the section if this is a standalone PR. * https://github.com/elastic/.../pull/123--> ### Release Note <!--If you think this enhancement/fix should be included in the release notes, please write a concise user-facing description of the change here. You should also label the PR with `release_note` so the release notes author(s) can easily look it up.--> (cherry picked from commit 6e66f73)
          💚 All backports created successfully
 Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation  | 
    
# Backport This will backport the following commits from `main` to `0.3`: - [Switched Docker runtime image to jlink (#371)](#371) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) Co-authored-by: Piotr Przybył <23506256+pioorg@users.noreply.github.com>
Details:
jlinkto create smaller image without not needed JDK modules, man pages and so onCloses #370
Checklists
Pre-Review Checklist
crawler.yml.exampleandelasticsearch.yml.example)v0.1.0)make noticeif any dependencies have been addedChanges Requiring Extra Attention
This PR has to be well tested before merging, to ensure all necessary modules are present in the customised JDK image. Until proper CI and tests are completed, please treat it as a work in progress.
Related Pull Requests
Release Note