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: [Dockerfile] ビルド時にキャッシュが効くよう構成変更 #8

Merged
merged 1 commit into from Feb 11, 2022

Conversation

kounoike
Copy link
Contributor

変更の種類

  • 不具合の修正
  • 新しい機能の追加
  • 改善・リファクタリング(機能は追加されないが、動作やコードを改善する破壊的でない変更)

チェックリスト:

  • 開発資料データベース設計 は読みましたか?
    • もともと自分用のメモですが、このプロジェクトの開発方針や設計などが記載されています。開発時の参考にしてください。
  • Git のコミットメッセージは開発資料に記載のフォーマットになっていますか?(重要)
    • このプロジェクト上のコミットメッセージは、基本的に全てこのフォーマットに従って記述されています(文字数は問いません)。
    • 正しいフォーマットになっていない場合、force push で必ずコミットメッセージを変更してから送信してください。
  • このプロジェクトのコーディング規約に従ったコードになっていますか?
    • コーディング規約は開発資料に記載されています。
    • そもそもコーディング規約と言えるほど大層なものではありませんが、一度目を通しておいてください。
  • プルリクエストは master ブランチに送信されていますか?
    • release ブランチはリリースした時にしか更新されません。必ず master ブランチに送信してください。

※コーディング規約にDockerfileについて記載がないためチェックをつけていません。.editorconfigは守れてるはずです。

説明

ちょっとDockerfileの書き方解説のような部分もあるので長くなります。変更点は大きく以下の3点です。

  1. マルチステージビルドの導入
  2. 構成順序の変更
  3. クライアントのビルドを追加

元の Dockerfile では ADD ./ /code が最初の方にあるため、ソースコード全体のどこかが変更されただけでそれ以降のレイヤーキャッシュが無効になっていました。また、ダウンロード&展開のためにaria2やxzなどサーバの実行に必要のないツールがインストールされていました。Docker イメージはADD とか RUN のコマンドごとにレイヤーが構成されるため、あとで削除しても全体のイメージサイズは削減できません。
そこで、1. のマルチステージビルドを導入し、ダウンロード&展開は別ステージのイメージで行い、成果物だけを最終的な実行イメージにコピーする形にしました。3. で追加したクライアントのビルドも、node.jsなどを別ステージのイメージでインストール(というかベースイメージから取ってきてますけど)しているので、最終的な実行イメージには.htmlや.jsなどの必要なもの以外含まれません。ちなみに、マルチステージビルドになっていると、buildx をインストールすることで並列ビルドをすることもできます(各ステージを並列にビルドしていく)。

また、2. の構成順序の変更というのは、docker build するとき、Dockerfile の上から順に見ていって、以前ビルドしたことがあるコマンドだけの場合(COPY/ADDの場合はコピーされるファイルに変更がない場合)、以前ビルドした際のレイヤーを使いまわします(レイヤーキャッシュ)。基本的には変更されにくいものを上に、変更されやすいものを下に書いていくと良いです。
例えば、ダウンロードではAMDのライブラリはそれほど更新されないですが、サードパーティライブラリはリリースごとには更新されます。なので、AMDのライブラリが先に処理するように変更しました。
また、yarn コマンドによる node_modules の作成や pipenv sync などでインストールされる依存ライブラリが変更になることはアプリケーションの更新に比べて頻度が低いはずです。そこで、依存パッケージ管理に使われるファイル(Pipfile, Pipfile.lock, package.json, yarn.lock)だけをCOPYし、依存パッケージのインストールを行い、その後あらためてアプリケーション全体のコピーを行っています。

動機とコンテキスト

既存の Dockerfile ではビルド時のレイヤーキャッシュが効かないような書き方になっており、コードの変更がある度に docker build で各種ダウンロードなどが走って非効率的であったため、改善した。
また、client ディレクトリ以下が事前に yarn build してあることが前提になっており、更新忘れが発生する可能性があったので、docker 内でクライアントもビルドするようにした。

順序の変更、マルチステージビルドの導入。併せてclientのビルド忘れが無いようにclientもDockerfile内でビルド
@kounoike
Copy link
Contributor Author

kounoike commented Feb 11, 2022

明確に書くのを忘れてましたが、公開したとして、ユーザがダウンロードすることになる最終イメージのサイズも小さくなってるはずです。

@tsukumijima
Copy link
Owner

tsukumijima commented Feb 11, 2022

プルリクエストをありがとうございます。素晴らしいですね……(すごい)
Docker にマルチステージビルドという機能があることすら知らなかったので、すべてが目から鱗でした。

動作自体は問題なさそうですが、一つだけ。
クライアントを Docker 内でビルドするように変更されていますが、ビルドした dist はリリース時や実装上きりのいいタイミングで更新し、Git に含めています。これは Node.js / npm / yarn などの開発者向けのツールをインストールしなくても、Python さえ入っていれば使えるようにしたいためです。
このため、Docker 上でビルドしてしまうと Git の内容と齟齬が出てしまいます。また開発時は docker-compose.yaml で /code/client 自体をローカルのフォルダにマウントしている(=Docker 上でビルドしたところで使われない)ことも考慮すると、クライアントのビルドを Docker 上で行う必要はないと思うのですが、どうでしょうか?

@kounoike
Copy link
Contributor Author

Docker 上でビルドしてしまうと Git の内容と齟齬が出てしまいます。

Docker イメージに含まれるものと Git の dist 以下に差異が出てしまう場合はありますが、それは dist の更新を忘れたなどの人為的なミスによるものです。人はどうしてもそういう「うっかり」をやってしまうので、それを避けるような仕組みにしていくことが重要です。
また、その場合は src 以下のファイルと Git の dist 以下が対応していないことになります。これは好ましくない状態なので、せめて Docker イメージの中は src 以下のファイルと対応する成果物が入るようにしようと考えて、client のビルドを Dockerfile の中に追加しました。

冗談交じりの極端な例を出すと、「src の中はまともに見せてるけど、実は dist を手で書き換えて coinhive を仕込んでる」みたいなことが無いという証明にもなりますね、というのもあります。

また開発時は

開発のことを中途半端に考えると変な Docker イメージになりかねません。この Dockerfile は最終的に Docker Hub なり GitHub Container Registry なりにイメージを上げてユーザが使えるようにするものと考えていました。

開発目的で特化するならむしろ client を yarn dev で配信できるようにするとかそういう工夫が必要になると思います。

ちなみに上記は「私ならこう考える」という説明であって、最終的な判断はもちろんお任せします&必要なら修正もしますので、遠慮なくおっしゃってくださいね。

@kounoike
Copy link
Contributor Author

ちなみにこの構造(本来 Git に含めないのが正しい運用だけど、諸々の都合で含めちゃっている)は、TS-Live! の方でも Wasm ファイルのビルドで同じことをやっているので、同じことを色々考えてます。

Wasm のビルドはこちらの client のビルドと比べて時間がかかるので、TS-Live! では GitHub Actions でリリース時だけビルドし直すようにしました。そして、私もうっかりものなので、そこそこの頻度で Debug ビルドの .wasm を Git に含めちゃっているのですが、その場合でもリリースされて GHCR に上がるイメージだけは Release ビルドになるようにしています。

@tsukumijima
Copy link
Owner

tsukumijima commented Feb 11, 2022

Docker イメージに含まれるものと Git の dist 以下に差異が出てしまう場合はありますが、それは dist の更新を忘れたなどの人為的なミスによるものです。人はどうしてもそういう「うっかり」をやってしまうので、それを避けるような仕組みにしていくことが重要です。

確かにそうですね。もっとも、ユーザー数は非 Docker + Windows の方が多そうなので、忘れることは基本ない(もし忘れたとしてもすぐに追加する)とは思いますが。
ひとまずこのままマージしますね。もしビルド時間など何かしらの支障があればオミットする方向で。

開発のことを中途半端に考えると変な Docker イメージになりかねません。この Dockerfile は最終的に Docker Hub なり GitHub Container Registry なりにイメージを上げてユーザが使えるようにするものと考えていました。

了解です。まだ安定していないので、もし Docker Hub などに上げるとしても当分先になりそうですが…

開発目的で特化するならむしろ client を yarn dev で配信できるようにするとかそういう工夫が必要になると思います。

client を yarn dev で配信できるようにというと…? Docker 内で yarn dev できるようにということですか?
サーバーと違って Docker 内でクライアントの開発をするのはあまり旨味がないかなーと思いますし、そもそも私が非 Docker の Windows 機で開発しているので、今のところそうするつもりはないですね(Windows の方が諸々つまづくポイントが多いので、Windows 向けに書いて Linux 向けに移植するほうが手間が少ない…)。

@tsukumijima tsukumijima merged commit 261891b into tsukumijima:master Feb 11, 2022
@kounoike
Copy link
Contributor Author

それを言ったら今のdocker-compose.ymlの立場が無い気がw
まあ、私も開発の際はホストに必要なもの揃えてやるスタイルの方が好みですね・・・

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