RHOAIENG-10350 feat(Dockerfiles): switch from s2i python images to plain ubi/c9s bases #641
Conversation
jiridanek
changed the title
base/ubi8-python-3.8/Dockerfile
Outdated
| @@ -1,4 +1,21 @@ | |||
| FROM registry.access.redhat.com/ubi8/python-38:latest | |||
| FROM registry.access.redhat.com/ubi8/ubi:latest | |||
There was a problem hiding this comment.
Can we use ubi-minimal instead?
There was a problem hiding this comment.
That would either mean living with microdnf, or installing regular dnf first thing, at which point the benefits or the ubi-minimal base are pretty much lost, me thinks.
In any case, I'd want to do such large changes in multiple steps, across multiple PRs.
So far, I don't even have the modest change in this PR groomed. It's necessary to move slowly and cautiously, and to build consensus.
The disk savings are small, 300mb across three base images.
There was a problem hiding this comment.
/hold
@jiridanek can you share there is JIRA or github issue linked for this work.
This PR is great, definitely will be good to have however
This work has significant impact on the overall product, without changing the architecture , this change would disrupt some flow i believe.
the s2i image bring working dir: /opt/app-root/src
i don't think ubi:latest, has that working dir,
this might work only because, the PVC gets mounted at the path,
As mount would create the path.
however we should thoroughly check these changes.
Great! In that case I'll go forward creating a Jira so this can be groomed. https://issues.redhat.com/browse/RHOAIENG-10350 |
jiridanek
changed the title
our images explicitly set this in the
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):
|
|
+1 for this update, I like this. 🙂 As Harshad mentioned, we should thoroughly test everything to ensure there are no issues with the PVCs. Otherwise, it /lgtm |
yeah; was just thinking whether nginx gets properly installed after this change; I did not think of it before https://issues.redhat.com/browse/RHOAIENG-10350 is opened to properly schedule the work |
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. Additionally, using plain ubi makes things more explicit.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@jiridanek: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
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: