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

feat: build docker images in CI #231

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

Conversation

nperez0111
Copy link

@nperez0111 nperez0111 commented Jun 30, 2024

Hi, really like the project. I wanted to have docker images that were already pre-built to self-host. So, I got the project building in GitHub Container Registry & built automatically in CI.

Definitely some optimizations could be made to the Docker images to get this to work better though like:

  1. We could leverage Docker multi-stage builds to reduce final image size
  2. We could pick a better base image (seems like the current base image is bundling python? That doesn't seem neccessary & I could build it without it. I didn't include it in the PR because I wasn't totally sure if this was needed for a feature that I did not know about)
  3. I found the build times to be extremely slow. I think it is because I'm doing a multiple architecture build but I actually checked and saw that yarn installs were a good chunk of the time. Could have a cache between them or honestly just switch from yarn since it is known to be slow at resolving. (I already added frozen-lockfiles which was a significant speedup)

One thing that I had to do though, we not modify the config file (since that is better handled by a volume) at build time to include the build time arg. So what I did was make env vars be a fallback for values that are actually specified in the config (so updating the config should always override an env var declarations)

You can see the docker images (and pull them if you like!) here:

Comment on lines +3 to +4
ARG NEXT_PUBLIC_WS_URL=ws://127.0.0.1:3001
ARG NEXT_PUBLIC_API_URL=http://127.0.0.1:3001/api
Copy link
Author

Choose a reason for hiding this comment

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

args can have defaults

ENV NEXT_PUBLIC_WS_URL=${NEXT_PUBLIC_WS_URL}
ENV NEXT_PUBLIC_API_URL=${NEXT_PUBLIC_API_URL}

WORKDIR /home/perplexica

COPY ui /home/perplexica/

RUN yarn install
RUN yarn install --frozen-lockfile
Copy link
Author

Choose a reason for hiding this comment

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

speed up

@@ -1,21 +1,19 @@
FROM nikolaik/python-nodejs:python3.12-nodejs20-bullseye

ARG SEARXNG_API_URL
ENV SEARXNG_API_URL=${SEARXNG_API_URL}
Copy link
Author

Choose a reason for hiding this comment

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

have the arg show up in the env

@@ -21,6 +21,7 @@ services:
- 3001:3001
volumes:
- backend-dbstore:/home/perplexica/data
- ./config.toml:/home/perplexica/config.toml
Copy link
Author

Choose a reason for hiding this comment

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

bind the config file into the container

@@ -7,5 +7,5 @@ OPENAI = "" # OpenAI API key - sk-1234567890abcdef1234567890abcdef
GROQ = "" # Groq API key - gsk_1234567890abcdef1234567890abcdef

[API_ENDPOINTS]
SEARXNG = "http://localhost:32768" # SearxNG API URL
SEARXNG = "" # SearxNG API URL - http://localhost:32768
Copy link
Author

Choose a reason for hiding this comment

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

No need to specify it anymore, as the env var will be used if not specified

@@ -37,7 +37,8 @@ export const getOpenaiApiKey = () => loadConfig().API_KEYS.OPENAI;

export const getGroqApiKey = () => loadConfig().API_KEYS.GROQ;

export const getSearxngApiEndpoint = () => loadConfig().API_ENDPOINTS.SEARXNG;
export const getSearxngApiEndpoint = () =>
loadConfig().API_ENDPOINTS.SEARXNG || process.env['SEARXNG_API_URL'];
Copy link
Author

Choose a reason for hiding this comment

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

Fallback to the env var

id: build-and-push-app
with:
context: .
platforms: linux/amd64,linux/arm64
Copy link
Author

Choose a reason for hiding this comment

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

Builds for multiple arches

Comment on lines +121 to +138
- name: Sign the backend images with GitHub OIDC Token
env:
DIGEST: ${{ steps.build-and-push-backend.outputs.digest }}
TAGS: ${{ steps.meta-backend.outputs.tags }}
run: |
images=""
for tag in ${TAGS}; do
images+="${tag}@${DIGEST} "
done
cosign sign --yes ${images}

- name: Attest app
uses: actions/attest-build-provenance@v1
id: attest-app
with:
subject-name: ghcr.io/${{ env.IMAGE_NAME }}-app
subject-digest: ${{ steps.build-and-push-app.outputs.digest }}
push-to-registry: true
Copy link
Author

Choose a reason for hiding this comment

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

image signing & attestation are best practices for docker images

@akramer-zibra
Copy link

akramer-zibra commented Jun 30, 2024

@ItzCrazyKns This looks like a professional enhancement to this lovely project. +1 for this

Copy link

@huyn3195 huyn3195 left a comment

Choose a reason for hiding this comment

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

LGTM

@ItzCrazyKns
Copy link
Owner

Hey @nperez0111, thanks for the PR! All of the changes looks good but I cannot merge the CI pipeline for the docker image building as if the config.toml file doesn't gets configured correctly, the page won't load and it would throw an error, some of the fields are still not present in the config menu to be configured from the frontend so that might cause an issue, we can implement the CI part later when these things are implemented.

@mictadlo
Copy link

mictadlo commented Aug 2, 2024

Hi,
I found here how to get searXNG running with Portainer. Now I would like to get Perplexica inside Portainer but is there a way to provide API keyes, searchXNG , Ollama URLs, port via env rather than config.toml file.

Thank you for consideration.

@nperez0111
Copy link
Author

Yea I think in general the maintainer is looking at how to store and configure these settings. I appreciate that it can be difficult since there are so many options to configure with: env vars, runtime values, config file, even the db could store settings.

I made some modifications to read from env vars since that is quite common in docker. But I'll let the maintainer cook on what he actually wants to build, support and maintain.

@rqi14
Copy link

rqi14 commented Aug 5, 2024

I've got a version of mine work.
https://github.com/rqi14/Perplexica-docker

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