Open jiridanek opened 1 month ago
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please ask for approval from jiridanek. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
This PR is great, definitely will be good to have however
Great! In that case I'll go forward creating a Jira so this can be groomed. https://issues.redhat.com/browse/RHOAIENG-10350
the s2i image bring working dir: /opt/app-root/src
our images explicitly set this in the WORKDIR
instruction, so we're good in that respect
however we should thoroughly check these changes.
sure, that's why I created a jira for doing all the work in organized manner; the disk space savings are not all that significant, but I like how this removes much of the s2i magic, so that's what makes this seem worth doing to me
BTW, I was wondering what are the most important changes, so I prepared some basic script for the image comparison (I have a plan to improve it further and put it into our CI/GHA for our convenience). I tried to compare one of the base images you change here with what we have (note that your build is 2 weeks old, I took the quay image from today):
./ci/compare-images.sh quay.io/opendatahub/workbench-images@sha256:e92bf20e127e545bdf56887903dc72ad227082b8bc23f45ff4f0fc67e6430318 ghcr.io/jiridanek/notebooks/workbench-images:base-ubi9-python-3.9-jd_ubi_base_adedd4a943977ecdcb67bc6eb9eda572d10c3ddc
@jiridanek: The following tests failed, say /retest
to rerun all failed tests or /retest-required
to rerun all mandatory failed tests:
Test name | Commit | Details | Required | Rerun command |
---|---|---|---|---|
ci/prow/habana-notebooks-e2e-tests | adedd4a943977ecdcb67bc6eb9eda572d10c3ddc | link | true | /test habana-notebooks-e2e-tests |
ci/prow/images | adedd4a943977ecdcb67bc6eb9eda572d10c3ddc | link | true | /test images |
ci/prow/notebooks-ubi8-e2e-tests | adedd4a943977ecdcb67bc6eb9eda572d10c3ddc | link | true | /test notebooks-ubi8-e2e-tests |
ci/prow/runtime-rocm-tensorflow-ubi9-python-3-11-pr-image-mirror | adedd4a943977ecdcb67bc6eb9eda572d10c3ddc | link | true | /test runtime-rocm-tensorflow-ubi9-python-3-11-pr-image-mirror |
ci/prow/notebook-rocm-ubi9-python-3-11-pr-image-mirror | adedd4a943977ecdcb67bc6eb9eda572d10c3ddc | link | true | /test notebook-rocm-ubi9-python-3-11-pr-image-mirror |
ci/prow/notebook-rocm-jupyter-min-ubi9-python-3-11-pr-image-mirror | adedd4a943977ecdcb67bc6eb9eda572d10c3ddc | link | true | /test notebook-rocm-jupyter-min-ubi9-python-3-11-pr-image-mirror |
ci/prow/runtime-rocm-pytorch-ubi9-python-3-11-pr-image-mirror | adedd4a943977ecdcb67bc6eb9eda572d10c3ddc | link | true | /test runtime-rocm-pytorch-ubi9-python-3-11-pr-image-mirror |
ci/prow/codeserver-notebook-e2e-tests | adedd4a943977ecdcb67bc6eb9eda572d10c3ddc | link | true | /test codeserver-notebook-e2e-tests |
ci/prow/notebooks-ubi9-e2e-tests | adedd4a943977ecdcb67bc6eb9eda572d10c3ddc | link | true | /test notebooks-ubi9-e2e-tests |
ci/prow/rstudio-notebook-e2e-tests | adedd4a943977ecdcb67bc6eb9eda572d10c3ddc | link | true | /test rstudio-notebook-e2e-tests |
ci/prow/intel-notebooks-e2e-tests | adedd4a943977ecdcb67bc6eb9eda572d10c3ddc | link | true | /test intel-notebooks-e2e-tests |
ci/prow/runtimes-ubi9-e2e-tests | adedd4a943977ecdcb67bc6eb9eda572d10c3ddc | link | true | /test runtimes-ubi9-e2e-tests |
ci/prow/rocm-runtimes-ubi9-e2e-tests | adedd4a943977ecdcb67bc6eb9eda572d10c3ddc | link | true | /test rocm-runtimes-ubi9-e2e-tests |
ci/prow/rocm-notebooks-e2e-tests | adedd4a943977ecdcb67bc6eb9eda572d10c3ddc | link | true | /test rocm-notebooks-e2e-tests |
Full PR test history. Your PR dashboard.
https://issues.redhat.com/browse/RHOAIENG-10350
Description
The main benefit is size and cve exposure, as the python images come with packages we don't use; python and pip is enough for us.
Every image is made approx 300MB smaller by doing this!
Before:
After:
Additionally, using plain ubi makes things more explicit.
How Has This Been Tested?
I'll test this when making this change is planned into a sprint.
Merge criteria: