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

Jump to bottom

Herokuからfly.ioへ移行 #518

Merged
merged 14 commits into from
Nov 25, 2022
Merged

Herokuからfly.ioへ移行 #518

merged 14 commits into from
Nov 25, 2022
+289 −42

Conversation

momocus
Copy link
Owner

@momocus momocus commented Nov 16, 2022

Close #500

  • fly.ioへデプロイするのに必要なDockerfileなどを追加した。
    • これまであった開発用Dockerfileは名前を変えた。
  • Github Actionsでmasterへのpush時に自動Deployが走るようにした。

Note
本来ならば開発用のDockerfileと本番用のDockerfileを分ける必要はないはず。統合したい。

@momocus momocus requested a review from yonta November 16, 2022 03:03
@yonta
Copy link
Collaborator

yonta commented Nov 16, 2022

本来ならば開発用のDockerfileと本番用のDockerfileを分ける必要はないはず。統合したい。

Dockerfile内部で条件分岐するってこと？
開発環境と本番環境ではnpmやgemの必要なパッケージが違うから、違うDockerfileが必要な気がしているんだけど。
それはそれで分岐するためにdocker-compose 引数ほげほげみたいなコマンド呼び出しが必要になって、結局管理するためにMakefileを導入しようとかになるんじゃないかな。

@yonta
Copy link
Collaborator

yonta commented Nov 16, 2022

あああああ、heredoc対応してないDockerfileが増やされる～～～～うわーーーん

yonta
yonta requested changes Nov 16, 2022
View reviewed changes
Copy link
Collaborator

@yonta yonta left a comment

Choose a reason for hiding this comment

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

すごい！fly.ioで動くのが見えてきた！
変更依頼です。

fly.Dockerfile Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
fly.Dockerfile Outdated Show resolved Hide resolved
fly.Dockerfile Show resolved Hide resolved
@yonta
Copy link
Collaborator

yonta commented Nov 16, 2022

本来ならば開発用のDockerfileと本番用のDockerfileを分ける必要はないはず。統合したい。

PRの中身みたけど、やっぱり統合は厳しそう。
fly.io用のdockerイメージは特殊なイメージを使っている。
また、本番用と違ってイメージを最小化するためにマルチステージビルドを使っている。
これは変わりにビルド時間が長いや、必要なコマンドなどのツールがイメージに含まれないなど、開発に使いづらい要素があるよ。

yonta
yonta reviewed Nov 16, 2022
View reviewed changes
fly.Dockerfile Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Nov 19, 2022 •
edited

Codecov Report

Base: 94.94% // Head: 94.94% // No change to project coverage 👍

Coverage data is based on head (16d3815) compared to base (995106b).
Patch has no changes to coverable lines.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master     #518   +/-   ##
=======================================
  Coverage   94.94%   94.94%           
=======================================
  Files          16       16           
  Lines         297      297           
=======================================
  Hits          282      282           
  Misses         15       15

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@momocus momocus force-pushed the switch-to-fly.io branch 6 times, most recently from f79e91a to 30446fa Compare November 22, 2022 12:37
@momocus
Copy link
Owner Author

momocus commented Nov 22, 2022

c8032c2 のあとmasterにrebaseしたら、Github Actionsによるcheckでこけていた。これを解消するため、以下を行った。

  • prettierとrubocop で検出されたエラー修正(f2df937)
  • hadolintで検出されたエラー修正 (2fd154b, 58ad230)
  • hadolintが Dockerfile: withBinaryFile: does not exist (No such file or directory)になっていたので、dev.Dockerfile, fly.Dockerfile の両方にhadolintがかかるようcheck.ymlを修正(8ae3604)

note:
ローカルでのフォーマットチェックに使うrun-all-checks.shでは、Dockerfileとつく名前のファイルを検索してhadolintかけるようになっていたので、Dockerfile: withBinaryFile: does not exist (No such file or directory)は起こらなかった。

ref:
hadolint-actions の複数ファイル実行についてのIssue How do I run multiple files

@momocus momocus requested a review from yonta November 22, 2022 13:16
yonta
yonta requested changes Nov 24, 2022
View reviewed changes
Copy link
Collaborator

@yonta yonta left a comment

Choose a reason for hiding this comment

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

議論に追記しました。
もう少し修正を。

.github/workflows/check.yml Outdated Show resolved Hide resolved
@momocus momocus requested a review from yonta November 24, 2022 17:20
@momocus
開発用Dockerfile名を変更
123b52f 
flyにdeployするのに専用のDockerfileを作るため、
開発用のDockerfile名を変えた。
@momocus
fly launch コマンドによるファイル生成
5c4af8d 
自動生成されたファイルに対しする変更箇所を分かりやすくするため、
この時点でコミットする。
@momocus
Fix: ビルド時間短縮のため、.dockerignoreを修正
326ba6d 
`fly deploy`によるビルドでソースのスキャンか転送かが行われるらしく、
ビルドに入らないフォルダ・ファイルは.dockerignoreに書いておくと時短ができる。
.dockerignoreの中身は.gitignoreを参考にした。
@momocus
Fix: fly.Dockerfile
767b32c 
flyで自動生成されたDockerfileの、yarn installが失敗していたため
dev.Dockerfileを参考にして修正した。
@momocus
Fix: server起動前にassets:precompileする
9fe0894 
build taskでassets:precompileすると、credentialsがなくてfailする。
server起動前にassets:precompileすることで解決。

https://fly.io/docs/rails/getting-started/existing/#access-to-environment-variables-at-build-time
@momocus
Update: readmeのdeployについて更新
03bac25 
デプロイ先は人によりけりなので、開発者用のメモとしてwikiに移動した。
https://github.com/momocus/sakazuki/wiki/Deployment
@momocus
Update: masterへのpush時に自動でflyへデプロイする
9561c9c 
https://fly.io/docs/app-guides/continuous-deployment-with-github-actions/
@momocus
Fix: linterによる修正
0aa8718 
prettierとrubocopによるエラー箇所をそれぞれ修正
@momocus
Fix: fly.Dockerfile
85dbae4 
- ファイルのパーミッション修正
- SHELLオプション -o pipefailを設定(hadlint DL4006)
- nodeのバージョン修正
- 不要なパッケージを削除
- パッケージのバージョンを指定
- GemfileとGemfile.lockのコピーをわかりやすくした
@momocus
Fix: hadolintのignore
1180f8a 
- DL3048
  - LABELにアンダースコアが使われているため警告が出る。
  - fly側がこのLABELを何に使っているか不明なため、修正せずにhadolint ignoreする。
- DL3008
  - apt-get install するパッケージにバージョン名がないと警告が出る。
  - BUILD_PACKAGESのように環境変数を使うとhadolintで解析できず、バージョンを指定していても警告が出る。
  - 後述の理由により、修正せずにhadolint ignoreする。
- DL3025
  - CMDの引数がJSON arrayではないため、警告が出る。
  - 後述の理由により、修正せずにhadolint ignoreする。

`fly lanch`で生成されるDockerfileにはARGが多用されており、
`fly deploy`の引数やfly.tomlでオーバーライドできるようになっている。
今のところオーバーライドは使っていないが、元のDockerfileから大きく変えると不都合が出るかもしれないのでそのままにしておく。
もしかしたらdevとprodで同じDockerfileを使い回すなど、将来使えるかもしれない。

ref: https://fly.io/docs/reference/configuration/#specify-docker-build-arguments
@momocus
Fix: Dockerfile名の変更をGithub Actionに反映
9d66d0d 
- hadolint-actionは複数ファイルを同時にかけられないので、冗長だがstepを一つ増やした。
@momocus
Fix: dockerignoreをホワイトリスト方式にした
e62f2bf
momocus and others added 2 commits November 25, 2022 16:59
@momocus @yonta
Update .github/workflows/check.yml
4d9f32b 
不要なコメントを削除

Co-authored-by: SAITOU Keita <keita44.f4@gmail.com>
@momocus
Update: dev.Dockerfileのnodeバージョンを上げた
16d3815
@momocus
Copy link
Owner Author

momocus commented Nov 25, 2022

@yonta #527 解消のため、masterにrebaseしてforce-pushしました

yonta
yonta approved these changes Nov 25, 2022
View reviewed changes
Copy link
Collaborator

@yonta yonta left a comment

Choose a reason for hiding this comment

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

Great JOB!!!!!!! 👏

@yonta yonta merged commit add9de2 into master Nov 25, 2022
@yonta
Copy link
Collaborator

yonta commented Nov 27, 2022

移行先 https://sakazuki.fly.dev/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yonta yonta yonta approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

herokuからの移行
2 participants
@momocus @yonta