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

Speed up Docker build and fix entrypoint permissions #894

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+35 −22
Open

Speed up Docker build and fix entrypoint permissions #894

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ac6739c
Speed up Docker build and fix entrypoint permissions
PseudoResonance Jan 8, 2025
84c2bb5
Update Docker build for Vite
PseudoResonance Jan 8, 2025
c57a0fe
Include license in image
PseudoResonance Jan 8, 2025
0c59e3c
Shorten docker compose build and remove unnecessary filament/actions
PseudoResonance Jan 9, 2025
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
12 changes: 12 additions & 0 deletions .dockerignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,15 @@ storage/framework/cache/data/*
storage/framework/sessions/*
storage/framework/testing
storage/framework/views/*.php
Dockerfile
compose.yml
readme.md
.editorconfig
.env.example
.gitignore
.prettierrc.json
bounties.md
contributing.md
contributor_license_agreement.md
crowdin.yml
security.md
6 changes: 3 additions & 3 deletions .github/docker/entrypoint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,17 @@ ln -s /pelican-data/database/database.sqlite /var/www/html/database/

if ! grep -q "APP_KEY=" .env || grep -q "APP_KEY=$" .env; then
echo "Generating APP_KEY..."
php artisan key:generate --force
su -s /bin/ash -c "php artisan key:generate --force" www-data
else
echo "APP_KEY is already set."
fi

## make sure the db is set up
echo -e "Migrating Database"
php artisan migrate --force
su -s /bin/ash -c "php artisan migrate --force" www-data

echo -e "Optimizing Filament"
php artisan filament:optimize
su -s /bin/ash -c "php artisan filament:optimize" www-data

## start cronjobs for the queue
echo -e "Starting cron jobs."
Expand Down
38 changes: 19 additions & 19 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,41 +1,38 @@
# Pelican Production Dockerfile

FROM node:20-alpine AS yarn
#FROM --platform=$TARGETOS/$TARGETARCH node:20-alpine AS yarn

WORKDIR /build

COPY . ./

RUN yarn config set network-timeout 300000 \
&& yarn install --frozen-lockfile \
&& yarn run build:production

FROM php:8.3-fpm-alpine
# FROM --platform=$TARGETOS/$TARGETARCH php:8.3-fpm-alpine

COPY --from=composer:latest /usr/bin/composer /usr/local/bin/composer

WORKDIR /var/www/html

RUN touch .env

# Install dependencies
RUN apk update && apk add --no-cache \
libpng-dev libjpeg-turbo-dev freetype-dev libzip-dev icu-dev \
zip unzip curl \
caddy ca-certificates supervisor \
&& docker-php-ext-install bcmath gd intl zip opcache pcntl posix pdo_mysql

# Copy the Caddyfile to the container
COPY Caddyfile /etc/caddy/Caddyfile
# Install dependencies with Composer
COPY composer.json composer.lock ./
Copy link
Member

Choose a reason for hiding this comment

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

Confused why this is necessary when we do the copy below?

COPY app/helpers.php ./app/
Copy link
Member

Choose a reason for hiding this comment

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

This definitely confuses me even more. Definitely don't want to reference this file anywhere in Docker.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These 3 files are the bare minimum required to run composer install. The reason app/helpers.php is required as well, is because it's specified as an autoload file in composer.json, so composer install --optimize-autoloader looks for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also the reason behind splitting the install and build up like this is just optimizing for build caching. By only copying the dependency lists first, those first layers only need to rerun when the dependencies change. Then the entire codebase is copied as late as possible to minimize needlessly rerunning. GitHub actions can also be configured to utilize caching (although I didn't do that in this PR), which, with these changes, should help speed up the builds a bit.

Although this doesn't have as big of an impact anymore because of the Vite frontend rewrite. The majority of the build time is now taken up by yarn run build, but regardless, I tried to move it down as far as possible.


# Copy the application code to the container
COPY . .
RUN composer install --no-dev --optimize-autoloader

COPY --from=yarn /build/public/assets ./public/assets
# Install dependencies with Composer
COPY package.json yarn.lock ./
RUN apk add --no-cache yarn
RUN yarn config set network-timeout 300000 \
&& yarn install --frozen-lockfile

RUN touch .env
# Copy the application code to the container
COPY . .

RUN composer install --no-dev --optimize-autoloader
# Yarn build
RUN yarn run build

# Set file permissions
RUN chmod -R 755 storage bootstrap/cache \
Expand All @@ -44,12 +41,15 @@ RUN chmod -R 755 storage bootstrap/cache \
# Add scheduler to cron
RUN echo "* * * * * php /var/www/html/artisan schedule:run >> /dev/null 2>&1" | crontab -u www-data -

# Copy the Caddyfile to the container
COPY Caddyfile /etc/caddy/Caddyfile

## supervisord config and log dir
RUN cp .github/docker/supervisord.conf /etc/supervisord.conf && \
mkdir /var/log/supervisord/

HEALTHCHECK --interval=5m --timeout=10s --start-period=5s --retries=3 \
CMD curl -f http://localhost/up || exit 1
CMD curl -f http://localhost/up || exit 1

EXPOSE 80 443

Expand Down
2 changes: 2 additions & 0 deletions compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ x-common:
services:
panel:
image: ghcr.io/pelican-dev/panel:latest
build:
dockerfile: ./Dockerfile
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this assumed by default, does this change anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I just tested, and docker compose build or docker compose run --build will only build images with a build section specified. Although I realized build: ./ can serve the same purpose as this, if the Dockerfile has the default name.

restart: always
networks:
- default
Expand Down
1 change: 1 addition & 0 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"coderflex/filament-turnstile": "^2.2",
"dedoc/scramble": "^0.10.0",
"doctrine/dbal": "~3.6.0",
"filament/actions": "^3.2",
PseudoResonance marked this conversation as resolved.
Show resolved Hide resolved
"filament/filament": "^3.2",
"guzzlehttp/guzzle": "^7.8.1",
"laravel/framework": "^11.7",
Expand Down
Loading