Skip to content
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

Update ubi to configure kubedock and podman #179

Merged
merged 5 commits into from
Aug 8, 2024
Merged

Conversation

dkwon17
Copy link
Collaborator

@dkwon17 dkwon17 commented Jul 24, 2024

This PR adds podman/buildah and kubedock functionality to the ubi8 image.

To summarize the changes:

  1. In the dockerfile, download kubedock and create wrapper files around dnf and podman
  2. The dnf wrapper checks whether podman is installed. If it is installed, podman and kubedock are configured as it's done in the UDI8 image.

I had trouble building the images with podman, so the following instructions use docker, just like the contributing guide here

To test:

  1. Set env vars:
BASE_IMAGE=quay.io/<username>/<repo>:base-image
UDI=quay.io/<username/<repo>:udi
  1. Build and push the base image
cd base/ubi8
DOCKER_BUILDKIT=1 docker image build --progress=plain -t $BASE_IMAGE .
docker push $BASE_IMAGE
  1. Build and push the udi image
    First, replace line 4 of the udi dockerfile here so that the base image matches the base image built from step 2.

Then, run the following:

cd universal/ubi8
DOCKER_BUILDKIT=1 docker image build --progress=plain -t $UDI .
docker push $UDI
  1. Test by creating workspaces for the following cases

Case 1

The workspace starts with the base image <CHE_HOST>/#https://github.com/dkwon17/empty?image=<BASE_IMAGE>

Case 2

The workspace starts with the UDI image <CHE_HOST>/#https://github.com/dkwon17/quarkus-api-example/tree/per-workspace?image=<UDI>

Next, verify that the following command runs without any errors:

podman info

Run the Package and Build image devfile tasks to verify that podman image builds are also working.

Lastly, test kubedock by adding the KUBEDOCK_ENABLED=true env variable in the tooling container:

oc patch dw <workspace-name> --type='json' -p='[{"op": "add", "path": "/spec/template/components/0/container/env", "value": [{"name": "KUBEDOCK_ENABLED", "value": "true"}]}]' -n <namespace>

and by running the following when the workspace restarts:

$ podman run hello-world
!... Hello Podman World ...!

         .--"--.           
       / -     - \         
      / (O)   (O) \        
   ~~~| -=(,Y,)=- |         
    .---. /`  \   |~~      
 ~/  o  o \~~~~.----. ~~   
  | =(X)= |~  / (O (O) \   
   ~~~~~~~  ~| =(Y_)=-  |   
  ~~~~    ~~~|   U      |~~ 

Project:   https://github.com/containers/podman
Website:   https://podman.io
Desktop:   https://podman-desktop.io
Documents: https://docs.podman.io
YouTube:   https://youtube.com/@Podman
X/Twitter: @Podman_io
Mastodon:  @[email protected]

@dkwon17 dkwon17 marked this pull request as ready for review July 25, 2024 21:39
@AObuchow
Copy link
Contributor

@dkwon17 Looks good to me so far, will continue testing & reviewing on Monday 👍

Copy link
Collaborator

@svor svor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested by following steps from the description on dogfooding instance.
Everything works as expected except kubedock verification (second screenshot):

screenshot-che-dogfooding apps che-dev x6e0 p1 openshiftapps com-2024 07 29-18_14_31

screenshot-che-dogfooding apps che-dev x6e0 p1 openshiftapps com-2024 07 29-18_24_07

Copy link
Contributor

@AObuchow AObuchow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested following the PR instructions & everything seems to work as expected :) Left some comments.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we remove the podman wrapper from the universal image now that it's moved to the base image?

universal/ubi8/Dockerfile Show resolved Hide resolved
base/ubi8/dnf-wrapper.sh Outdated Show resolved Hide resolved
@AObuchow
Copy link
Contributor

Everything works as expected except kubedock verification (second screenshot)

@svor did your workspace restart after patching in KUBEDOCK_ENABLED? i.e. oc patch dw <workspace-name> --type='json' -p='[{"op": "add", "path": "/spec/template/components/0/container/env", "value": [{"name": "KUBEDOCK_ENABLED", "value": "true"}]}]' -n <namespace>

In my testing, I got export KUBEDOCK_ENABLED && ./entrypoint.sh to work in enabling kubedock from the UDI.

@svor
Copy link
Collaborator

svor commented Jul 29, 2024

Yes, I've restarted my workspace. I'll do another try tomorrow, maybe I've missed something 🤔

Copy link
Collaborator

@svor svor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as expected
screenshot-nimbusweb me-2024 07 30-15_12_00

@svor
Copy link
Collaborator

svor commented Jul 30, 2024

Maybe it makes sense to install podman into the base image
@dkwon17 @AObuchow @ibuziuk wdyt?

@dkwon17
Copy link
Collaborator Author

dkwon17 commented Jul 30, 2024

@svor that would make things simpler, but the downside is that it would increase the size of the base image from ~150MB to ~280MB

I prefer the simpler way and have podman in the base image

@AObuchow
Copy link
Contributor

I also prefer the simpler way to have podman in the base image, but an increase of the base image from ~150MB to ~280MB is roughly a 187% increase in size.

I don't know if we should assume that users (consumers of the UBI) will always want to be able to build containers. Kubedock is only ~30mb in size, so the majority of the increase in image size is caused by installing podman. IMO the current approach makes sense for a base image.

@svor
Copy link
Collaborator

svor commented Jul 30, 2024

also it could be confused a bit that we have ENVs like PODMAN_WRAPPER_PATH and PODMAN_ORIGINAL_PATH in base image but no podman

@AObuchow
Copy link
Contributor

also it could be confused a bit that we have ENVs like PODMAN_WRAPPER_PATH and PODMAN_ORIGINAL_PATH in base image but no podman

That's a good point.

I don't think we have an official page for universal developer (and base) image documentation in the Che Docs. When looking at the docs, I only see mentions of the UDI.

The change made by this PR might warrant adding a Che Docs page on the UBI & UDI, where we could mention that installing podman on the UBI will automatically install kubedock? There we could also mention the existence of the podman and dnf wrapper potentially, to clarify any confusion.

@dkwon17
Copy link
Collaborator Author

dkwon17 commented Jul 31, 2024

Thank you for the feedback @AObuchow @svor

I'm leaning towards the simple idea of adding podman into the base image to avoid adding the dnf wrapper logic which could be fragile, and to avoid possible side effects for images that build on top of it

@AObuchow
Copy link
Contributor

I'm leaning towards the simple idea of adding podman into the base image to avoid adding the dnf wrapper logic which could be fragile, and to avoid possible side effects for images that build on top of it

I'm +1 for this. If consumers of the UBI complain about it's size, we always could go with the current approach of using a wrapper script for dnf.

@dkwon17 dkwon17 marked this pull request as draft August 2, 2024 18:25
@dkwon17 dkwon17 marked this pull request as ready for review August 4, 2024 01:35
Signed-off-by: David Kwon <[email protected]>
@dkwon17
Copy link
Collaborator Author

dkwon17 commented Aug 5, 2024

I've updated this PR to include podman into the base image, and removed the podman wrapper from the UDI.

Could I please get another look at this PR @svor @AObuchow ?

Copy link
Collaborator

@svor svor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dkwon17 am I right that each image that extends base image should have source kubedock_setup in entrypoint.json to enable kubedock? If it's true, maybe we could add this information into README?

@openshift-ci openshift-ci bot added the lgtm label Aug 5, 2024
@dkwon17
Copy link
Collaborator Author

dkwon17 commented Aug 6, 2024

@svor Yes, that's correct, I will update the readme too for more details

@openshift-ci openshift-ci bot removed the lgtm label Aug 6, 2024
@dkwon17
Copy link
Collaborator Author

dkwon17 commented Aug 6, 2024

I've updated the README

@openshift-ci openshift-ci bot added the lgtm label Aug 7, 2024
Copy link

openshift-ci bot commented Aug 7, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: AObuchow, dkwon17, ibuziuk, svor

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@dkwon17 dkwon17 merged commit 888d46a into main Aug 8, 2024
2 checks passed
@dkwon17 dkwon17 deleted the update-ubi-kubedock branch August 8, 2024 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants