-
Notifications
You must be signed in to change notification settings - Fork 68
Add ubi-base based on ubi10 #237
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Oleksii Kurinnyi <[email protected]>
Signed-off-by: Oleksii Kurinnyi <[email protected]>
Signed-off-by: Oleksii Kurinnyi <[email protected]>
Signed-off-by: Oleksii Kurinnyi <[email protected]>
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: akurinnoy 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 |
Signed-off-by: Oleksii Kurinnyi <[email protected]>
5fafed5
to
022fc16
Compare
Signed-off-by: Oleksii Kurinnyi <[email protected]>
Pull Request UBI 10 images published ✨ Base Image: quay.io/devfile/base-developer-image:ubi10-pr-237 |
Pull Request images published ✨ |
@@ -0,0 +1,55 @@ | |||
#!/bin/bash |
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.
There appears to be duplication between ubi9/
and ubi10/
. Would it make sense to factor out shared scripts (e.g. kubedock_setup.sh
, podman-wrapper.sh
) into a common location to reduce duplication and ease maintenance?
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.
I kept this duplication on purpose, so that after switching to ubi10, we can delete the ubi9/
directory.
@dkwon17 Is this approach correct?
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.
Yes, eventually I'd imagine we will deprecate ubi9 sometime after fully adopting ubi10, just like we did for ubi8
README.md
Outdated
| `wget` |`wget` |`wget` | | ||
| `zip` |`zip` |`zip` | | ||
| `zsh` |`NOT AVAILABLE (fedora only)` |`NOT AVAILABLE (fedora only)` | | ||
| **TOTAL SIZE** | **903MB** (341MB compressed) | **TODO** | |
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.
Will this be updated with final values or any additional notes before merging the PR?
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.
updated
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.
As for me this info is redundant in README.md:
- It requires manual updates whenever we install or remove something, which is easy to forget.
- The value may differ across architectures.
- If anyone needs this detail, they can always find it on quay.io, where the image is published
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.
@dkwon17 can we remove the image size info?
echo user:10000:65536 > /etc/subgid | ||
|
||
# Adjust storage.conf to enable Fuse storage. | ||
RUN sed -i -e 's|^#mount_program|mount_program|g' -e '/additionalimage.*/a "/var/lib/shared",' /usr/share/containers/storage.conf |
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.
In the base/ubi9
image, I noticed that the storage.conf
file path /etc/containers/storage.conf
Is this location supposed to be different for UBI9 and UBI10?
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.
In ubi10
there is no /etc/containers/storage.conf
so used the default distribution-provided configuration file, which is /usr/share/containers/storage.conf
. Now I understand that the correct approach would have been to copy the default storage.conf
to /etc/containers/
.
I also noticed that the entrypoint.sh
creates a new config that takes precedence over the system-wide ones:
developer-images/base/ubi10/entrypoint.sh
Lines 14 to 22 in 699a882
# Configure container builds to use vfs or fuse-overlayfs | |
if [ ! -d "${HOME}/.config/containers" ]; then | |
mkdir -p ${HOME}/.config/containers | |
if [ -c "/dev/fuse" ] && [ -f "/usr/bin/fuse-overlayfs" ]; then | |
(echo '[storage]';echo 'driver = "overlay"';echo '[storage.options.overlay]';echo 'mount_program = "/usr/bin/fuse-overlayfs"') > ${HOME}/.config/containers/storage.conf | |
else | |
(echo '[storage]';echo 'driver = "vfs"') > "${HOME}"/.config/containers/storage.conf | |
fi | |
fi |
It looks like this modifications to storage.conf
in the Dockerfile are dead code.
base/ubi10/Dockerfile
Outdated
|
||
# Add kubedock | ||
# See release page for details https://github.com/joyrex2001/kubedock/releases/tag/0.18.1 | ||
ENV KUBEDOCK_VERSION 0.18.1 |
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.
Could we also incorporate the changes from #235 as well?
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.
I'm preparing a fixup for this.
Thank you @akurinnoy , What I've successfully tested so far:
|
Pull Request UBI 10 images published ✨ Base Image: quay.io/devfile/base-developer-image:ubi10-pr-237 |
Pull Request images published ✨ |
Signed-off-by: Oleksii Kurinnyi <[email protected]>
Signed-off-by: Oleksii Kurinnyi <[email protected]>
699a882
to
fab1905
Compare
Pull Request UBI 10 images published ✨ Base Image: quay.io/devfile/base-developer-image:ubi10-pr-237 |
Pull Request images published ✨ |
I tested the PR using devfile, and it seems to be working
Installed Tools
|
This PR introduces new
ubi-base
image based onubi10
. It adds GH workflows for production builds and for PR validations. BuildingUDI
based onubi10
is disabled so far.This PR also enhances GH workflows to make them fork-friendly:
One can set repository variable
REGISTRY
to their custom registry (e.g.quay.io/yourusername
). Otherwise, the fallback registry (quay.io/devfile
) will be used.How to test
Create a DevWorkspace, it should start successfully:
Devfile
link: https://gist.github.com/akurinnoy/afccc6da6105a5c87911acb648bb88a9/raw/c83a1952c939ca51bd847d70a1f32158554f50e4/ubi10-base
DevWorkspace
Open Che Code and check if all the development tools are included, see https://github.com/devfile/developer-images/blob/86af50296b6c4187376feea5e22a9e993fbb00e5/README.md#included-development-tools
Check (thanks @dkwon17 ):
podman build -t hello https://github.com/containers/PodmanHello.git
)podman run quay.io/dkwon17/test:1234
)How to get image size