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

Add Dockerfile for comps-base image #1127

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

eero-t
Copy link
Contributor

@eero-t eero-t commented Jan 8, 2025

Description

Add Docker file for a base image that can be used by GenAIExamples applications.

With a shared base image, CI can do test builds much faster, they take much less space locally and in registry, and images sharing the same base download much faster.

With this, almost all of the 20 application Dockerfiles, e.g. this one:
https://github.com/opea-project/GenAIExamples/blob/main/ChatQnA/Dockerfile

Can be reduced just to:

# Copyright (C) 2025 Intel Corporation
# SPDX-License-Identifier: Apache-2.0

ARG BASE_TAG=latest
FROM opea/comps-base:$BASE_TAG

COPY ./chatqna.py $HOME/chatqna.py
ENTRYPOINT ["python", "chatqna.py"]

(For releases, docker build command can be given e.g. --build-arg=BASE_TAG=1.2 option.)

Issues

This is first part in fixing image sizes and how much time and disk space is used for them. There are multiple tickets about the image sizes.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Dependencies

n/a

Tests

Tested manually that the added Dockerfile, and ChatQnA & DocSum Dockerfiles relying on it build, and that the resulting ChatQnA & DocSum applications work fine.

Next steps

Somebody (else) needs to:

  • Add checking & building of this image to CI
  • Do nightly builds of it and upload them both to CI and DockerHub registries for applications
  • Do similar simplification / optimization for rest of the Dockerfiles (ones in this repo, and UI ones in GenAIExamples)

@eero-t
Copy link
Contributor Author

eero-t commented Jan 8, 2025

With current opea/chatqna Dockerfile, the image size is 828MB.

The image size resulting from this new base image + the ChatQnA Dockerfile example, is 436MB.

=> image size halves, as it drops installation of redundant package (Mesa 3D libs, Git and their deps, e.g. Perl).

Every additional application image will take only fraction of that, because base layer already exists. Docker / crictl still will list the combines size though (docker lists uncompressed one, crictl compressed one).

@eero-t
Copy link
Contributor Author

eero-t commented Jan 8, 2025

=> image size halves, as it drops installation of redundant package (Mesa 3D libs, Git and their deps, e.g. Perl).

Earlier opea-project/GenAIExamples#1031 PR showed that CI tests for all apps pass even without these extra packages. That's why this new base image does not include them.

@eero-t
Copy link
Contributor Author

eero-t commented Jan 8, 2025

Several Dockerfiles in "GenAIExamples" include also following which I've dropped from the base:

SHELL ["/bin/bash", "-o", "pipefail", "-c"]
...
RUN echo 'ulimit -S -n 999999' >> ~/.bashrc

Because hadolint does not complain about former missing, and because all these containers run Python directly instead of Bash. But having them there should not harm either, as they take no space, unlike the unused packages.

Copy link
Collaborator

@ashahba ashahba left a comment

Choose a reason for hiding this comment

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

Thanks @eero-t for this PR.
My feedback is only a minor one.

Dockerfile Show resolved Hide resolved
@poussa
Copy link

poussa commented Jan 9, 2025

This is good and should be merged ASAP. Only then examples can start using it.

@eero-t
Copy link
Contributor Author

eero-t commented Jan 9, 2025

Rebased to main.

@eero-t
Copy link
Contributor Author

eero-t commented Jan 9, 2025

This is good and should be merged ASAP. Only then examples can start using it.

Next steps would be:

  • adding this to nightly build images,
  • pushing the new image to registry used by CI (in this and GenAIExamples repo), and
  • pushing that image also to DockerHub OPEA project.

However, I have no clue how to achieve that, so unfortunately somebody else needs to do that.

Only after that, CI can pass for opea-project/GenAIExamples#1369 so that it can also be merged.

@fidencio
Copy link

fidencio commented Jan 9, 2025

While merged is block, @eero-t, what about squashing everything into the same commit? Just a suggestion, not a requirement.

@eero-t
Copy link
Contributor Author

eero-t commented Jan 9, 2025

While merged is block, @eero-t, what about squashing everything into the same commit? Just a suggestion, not a requirement.

Done.

Copy link
Collaborator

@ashahba ashahba left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link

@fidencio fidencio left a comment

Choose a reason for hiding this comment

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

lgtm, thanks @eero-t!

Copy link
Collaborator

@chensuyue chensuyue left a comment

Choose a reason for hiding this comment

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

This design LGTM, just need more time to update CICD.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants