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.

Sign up for GitHub

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: impr dockerfile build speed; use entrypoint #195

Draft
wants to merge 1 commit into
base: stable
Choose a base branch
from
Draft

feat: impr dockerfile build speed; use entrypoint #195

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
30 changes: 18 additions & 12 deletions docker/core/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,29 +1,35 @@
# FIXME use alpine
Copy link
Member

Choose a reason for hiding this comment

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

Why would alpine be better?

Copy link
Member Author

Choose a reason for hiding this comment

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

not better but more "like a pro" (if you use lightweight containers is better, less footprint. Of course this is most important in systems we do not have, like kubernetes, but we want to be following good practices so it's good to mark this)

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good, then I would suggest making line 2 into FROM node:14.15.1-alpine

Copy link
Member Author

Choose a reason for hiding this comment

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

yes I didn't do it myself because we don't need that level of lightweight-ness yet, and I therefore didn't want to risk breaking image building et al ('larger' image means that we have more/different tools, e.g. we do not use apt-get but apk, and i didn't want to spend time on that)

FROM node:14

RUN mkdir -p /usr/app/src \
&& mkdir -p /usr/app/media \
&& mkdir -p /usr/app/scripts
&& mkdir -p /usr/app/scripts \
&& apt-get update \
&& apt-get install netcat -y
Copy link
Member

Choose a reason for hiding this comment

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

I believe @serge1peshcoff mentioned that we do not need this anymore, and we could also remove the wait script


RUN apt-get update
RUN apt-get install netcat -y
WORKDIR /usr/app/src

COPY ./docker/core/bootstrap.sh /usr/app/scripts/bootstrap.sh
COPY ./docker/core/wait.sh /usr/app/scripts/wait.sh
COPY . /usr/app/src
RUN mkdir /usr/app/src/state
COPY package.json /usr/app/src

RUN chown -R node:node /usr/app

WORKDIR /usr/app/src

USER node

ENV NPM_CONFIG_PREFIX=/home/node/.npm-global
ENV PATH="/home/node/.npm-global/bin:${PATH}"

RUN npm install -g --loglevel warn nodemon bunyan && npm cache clean --force
RUN npm install --loglevel warn
# FIXME no need for nodemon
Copy link
Member

Choose a reason for hiding this comment

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

I would keep nodemon for dev, but we can remove it in prod I guess

Copy link
Member Author

Choose a reason for hiding this comment

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

yes as mentioned above, there's the need for a discussion. Now I am trying to close as much as possible :D

RUN npm install -g --loglevel warn nodemon bunyan \
&& npm cache clean --force \
&& npm install --loglevel warn

COPY --chown=node:node ./docker/core/bootstrap.sh /usr/app/scripts/bootstrap.sh
COPY --chown=node:node ./docker/core/wait.sh /usr/app/scripts/wait.sh
COPY --chown=node:node . /usr/app/src
RUN mkdir /usr/app/src/state


CMD sh /usr/app/scripts/bootstrap.sh && nodemon -e "js,json" lib/run.js
ENTRYPOINT [ "/usr/app/scripts/bootstrap.sh" ]
CMD ["nodemon -e 'js,json' lib/run.js"]

EXPOSE 8084
4 changes: 3 additions & 1 deletion docker/core/bootstrap.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,6 @@ npm run db:create
echo "Migrating database..."
npm run db:migrate
echo "Seeding database..."
npm run db:seed
npm run db:seed

sh -c "${@}"
2 changes: 1 addition & 1 deletion docker/docker-compose.dev.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ services:
context: ./${PATH_CORE}/..
dockerfile: ./docker/core/Dockerfile
image: aegee/core:dev
command: sh -c "sh /usr/app/scripts/bootstrap.sh && nodemon -L -e 'js,json' lib/run.js | bunyan --color"
command: "nodemon -L -e 'js,json' lib/run.js | bunyan --color"
volumes:
- ./${PATH_CORE}/../config/:/usr/app/src/config
- ./${PATH_CORE}/../lib/:/usr/app/src/lib
Expand Down