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

fix: update Dockerfile with python and .gyp to support install and run of Glee project in container #1208

Merged
merged 17 commits into from
Mar 29, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
FROM node:16-alpine
FROM node:20-alpine

ARG ASYNCAPI_CLI_VERSION=0.59.1
# docker build -t cli:20 . # to build image
derberg marked this conversation as resolved.
Show resolved Hide resolved
# docker compose --file ./test/manual/docker-compose.yaml up # to launch container with running studio, note you need to add a random asyncapi.yaml to ./output folder

# Create a non-root user
RUN addgroup -S myuser && adduser -S myuser -G myuser
Expand All @@ -14,11 +15,12 @@ ENV PUPPETEER_SKIP_CHROMIUM_DOWNLOAD true
# Since 0.14.0 release of html-template chromium is needed for pdf generation.
# More custom packages for specific template should not be added to this dockerfile. Instead, we should come up with some extensibility solution.
RUN apk --update add git chromium && \
apk add --no-cache --virtual .gyp python3 make g++ && \
rm -rf /var/lib/apt/lists/* && \
rm /var/cache/apk/*

# Installing latest released npm package
RUN npm install --ignore-scripts -g @asyncapi/cli@"$ASYNCAPI_CLI_VERSION"
RUN npm install --ignore-scripts -g @asyncapi/cli

Copy link
Member

Choose a reason for hiding this comment

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

@eelcofolkertsma you added ASYNCAPI_CLI_VERSION back in line 4 but it is still missing here - basically not used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, fixed that one too
If you use Dockerfile without change you get latest version of cli. If you add a version-value you get image for that version of cli
Tested OK in Github codespaces (default version=1.7.3, forced version=1.7.2) with docker build -t cli . and docker run cli --version

# Switch to the non-root user
USER myuser
Expand Down
16 changes: 16 additions & 0 deletions test/manual/docker-compose.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
version: '3'
Copy link
Member

Choose a reason for hiding this comment

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

why do you think it is needed to keep in the repo if in the end it is not used in any automated tests? what is the value of this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems current image is not tested at all, not automated nor manual. Example: the ARG for cli version that I removed as dysfunctional. I guess that behavior has been around for many months.
Than a manual test is better than nothing.
I lack skill to develop an automated test for the image, but fine with me to drop the manual test resource and raise an issue to validate the docker image in some automated way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On the ARG... I have pulled image from DockerHub and inspected version of asyncapi in resulting container. The ARG is ignored in building of image! Current version is 1 something, not 0.59 as requested per ARG. And this is good.
I can see value to manually install an older (trusted) version of CLI, but for the image you would expect latest and greatest version. Docker Hub has the older versions as well as in docker pull asyncapi/cli:1.6.14

Copy link
Member

Choose a reason for hiding this comment

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

oh, you're right, we missed it and did not add a simple docker build testing - lemme fix that, we have some generic workflows that I can pull in

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

ok, so automated tests are in place now

next

yes, we know the arg is ignored (overwritten) in release: https://github.com/asyncapi/cli/blob/master/.github/workflows/release-docker.yml#L42 so we always want to release a latest

the arg in docker file was added for a reason -> see the story in #675

does it explain all?
maybe instead of removing ARG ASYNCAPI_CLI_VERSION=0.59.1 we should add a comment for future, linking to the other PR I pointed out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You predict my thoughts. The ARG goes back in with comment that it is for manual image create only, github workflow overrides. #675 has lots of insider communication, perhaps a bit too much ;-)


services:
studio:
image: cli:20
ports:
- "8081:3210"
volumes:
- ${PWD}/output:/app
networks:
- default
user: root
command: start studio

networks:
default: {}
Loading