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

Use Ubuntu base image for main container #1476

Merged

Conversation

felixvanoost
Copy link
Contributor

@felixvanoost felixvanoost commented Mar 18, 2023

Replaces the base Docker image for yuzutech/kroki (eclipse-temurin:11.0.17_8-jre-alpine) with the Ubuntu flavour of the latest JRE 11 image (eclipse-temurin:11.0.18_10-jre-jammy). This variant has both arm64 and amd64 variants available, which should resolve one of the last remaining roadblocks to adding native ARM support to Kroki.

The use of a Ubuntu base image may have some other secondary advantages as well, e.g. more widespread support during development/debugging.

Notes:

  • I still need to quantify the final size difference between the Ubuntu and Alpine images.
  • The latest version of graphviz available using apt (2.42.2) is behind the latest version available using apk (3.00.0), so there may be some unexpected consequences from this change.

@felixvanoost
Copy link
Contributor Author

@ggrossetie I'm getting the following error related to canvas when trying to run Vega with PNG or PDF files. This also happens locally on my machine:

response: Error 500: Error: CanvasRenderer is missing a valid canvas or context

What's puzzling me is that kroki-builder-vega seems to be working fine; it passes all the dpkg tests. This leads me to believe that it's something related to how canvas is being brought into the main image. I noticed that it's currently being copied separately:

COPY --from=kroki-builder-vega /app/app.bin /usr/bin/vega
COPY --from=kroki-builder-vega /app/canvas.node /usr/bin/canvas.node

Do you know why this was done? I'm not really familiar with Node.js but it seems like the the Vega app.bin shouldn't need any external dependencies?

@ggrossetie
Copy link
Member

Do you know why this was done? I'm not really familiar with Node.js but it seems like the the Vega app.bin shouldn't need any external dependencies?

As far as I remember, canvas is an external dependency. I don't recall exactly why but https://github.com/vercel/pkg cannot include it inside the vega binary, that's why we need it.

If you get an error Error: CanvasRenderer is missing a valid canvas or context, please install the canvas package (or install vega-cli).

From https://vega.github.io/vega-lite/usage/compile.html

@felixvanoost felixvanoost changed the title Use ubuntu base image for main container Use Ubuntu base image for main container Mar 21, 2023
@felixvanoost
Copy link
Contributor Author

felixvanoost commented Mar 21, 2023

@ggrossetie I found a way to include the canvas package within the final binary executable generated by adding it as a pkg asset (see docs). This avoids the manual copy step and seems to fix the issue I was having. All the tests are passing locally on my machine now.

@ggrossetie
Copy link
Member

@felixvanoost Nice! Let's see how it goes on the CI.

@felixvanoost
Copy link
Contributor Author

This is ready to merge from my side. If approved, I'll use this as the starting point for creating an arm64 version of the main image in a separate PR.

@ggrossetie
Copy link
Member

Thanks, I will take a closer look this week-end.

@ggrossetie ggrossetie merged commit 488cfc4 into yuzutech:main Mar 25, 2023
1 check passed
@ggrossetie
Copy link
Member

LGTM, thanks again for your contribution 👌🏻

@felixvanoost felixvanoost deleted the use-ubuntu-base-image-for-main-container branch May 8, 2023 23:13
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.

None yet

2 participants