-
Notifications
You must be signed in to change notification settings - Fork 217
feat(ci): Dockerイメージのpush先にGitHub Container Registryを追加 #1662
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
base: master
Are you sure you want to change the base?
Conversation
cannot use source images from a different registry than the target image: ghcr.io != docker.io
PR作成ありがとうございます!! ちょっとまだレビューできてないのですが、もしかしたらpush-by-digest=trueを指定すればdigestのみのpushができるかもと思ったのでちょっとコメントまで!! misskeyがそんな感じ…かも…?(自信なし) …とここまで書いてて思いましたが、ちょっとこのPRの主旨とは違うかもですね!! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
良い感じに思いました!!!
ちょっと出先なのですが、とりあえずコメントまで…!
情報ありがとうございます!
もっと簡単にできそうな他の方法を見つけました。Misskeyの方では、マニフェストリストの作成に
マニフェストリストを作るためのコマンドが2種類あることは認識していながら、違いがよくわかっていなかったのですが、buildxがマニフェスト機能をラップしていることから推測できる通りに、 マニフェストリストの作成と同時にpushすることを止めることはできなさそうなので、ローカルでマニフェストリストをそろえて、あとでまとめてpushするようなことはできませんが、その仕様で特に問題ないので使えそうです。 # Docker Hub (docker.io) -> ghcr.io(実際にやりたいのは逆ですが)
$ docker buildx imagetools create --progress plain --tag ghcr.io/aoirint/voicevox_engine:0.24.0-aoirint.5-hoge docker.io/aoirint/voicevox_engine@sha256:a7075904
ea6a152c3857b2ce8344414e1dc14e4d3ea733528e3c988d0fafdf0c docker.io/aoirint/voicevox_engine@sha256:681120c788c1560dbe48df4551a42eb21922d255941a0fe28704bb204711901a
#1 [internal] pushing ghcr.io/aoirint/voicevox_engine:0.24.0-aoirint.5-hoge
#1 0.000 copying sha256:681120c788c1560dbe48df4551a42eb21922d255941a0fe28704bb204711901a from docker.io/aoirint/voicevox_engine@sha256:681120c788c1560dbe48df4551a42eb21922d255941a0fe28704bb204711901a to ghcr.io/aoirint/voicevox_engine:0.24.0-aoirint.5-hoge
#1 0.000 copying sha256:a7075904ea6a152c3857b2ce8344414e1dc14e4d3ea733528e3c988d0fafdf0c from docker.io/aoirint/voicevox_engine@sha256:a7075904ea6a152c3857b2ce8344414e1dc14e4d3ea733528e3c988d0fafdf0c to ghcr.io/aoirint/voicevox_engine:0.24.0-aoirint.5-hoge
#1 2.899 pushing sha256:8daf3d9c3a731f7d10e5370fb9889ed17229c22a3bf723b3a7ad74ed13605e7c to ghcr.io/aoirint/voicevox_engine:0.24.0-aoirint.5-hoge
#1 DONE 3.2s
ついでに対応しようと思って試したくらいではありましたが、いまから追加変更するほどでもないかなと思うので、Issueを立てて別プルリクエストに分割しようと思います。 |
正式名称はDocker Hubだけれど、DOCKERHUB_USERNAMEやDOCKERHUB_TOKENが すでにdockerhub表記になっているので、合わせる
Issueを立てて、セルフアサインしました。 |
issue作成ありがとうございます!! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for pushing Docker images to GitHub Container Registry (ghcr.io) as part of the CI pipelines while retaining the option to push to Docker Hub depending on configuration. Key changes include:
- Updating the test workflow to pull images from ghcr.io and adding a login step for ghcr.io.
- Modifying the build workflow to generate Docker image names for both registries and conditionally enable Docker Hub pushes.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
.github/workflows/test-engine-container.yml | Updated IMAGE_NAME to target ghcr.io and added ghcr.io login step |
.github/workflows/build-engine-container.yml | Added inputs/outputs and steps to support conditional multi-registry builds |
Comments suppressed due to low confidence (1)
.github/workflows/build-engine-container.yml:48
- [nitpick] Consider unifying how Docker Hub configuration values are sourced (using vars versus outputs) and refactoring the inline bash block for clarity to enhance long-term maintainability.
{ if [[ -n "${{ vars.DOCKERHUB_USERNAME }}" && "${{ github.event.inputs.force_dockerhub_disabled }}" != 'true' ]]; then
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ほぼLGTMです!!
かなり色々考えてくださった素晴らしいPRだと感じました!!
あとinputs引数名とdockerhub_username用の変数を最定義する周りだけ・・・!
Co-authored-by: Hiroshiba <[email protected]>
Co-authored-by: Hiroshiba <[email protected]>
Co-authored-by: Hiroshiba <[email protected]>
Co-authored-by: Hiroshiba <[email protected]>
Co-authored-by: Hiroshiba <[email protected]>
…-io' into feature/docker-ghcr-io
…pty set for dockerhub_repository
提案通りの変更を入れたと思います。 CIが回ったらRe-request reviewしようと思います。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ほぼLGTMです!!
1箇所だけ念の為大丈夫そうか確認したく・・・!
: # Docker Hubの設定 | ||
{ | ||
if [[ -n "${{ vars.DOCKERHUB_USERNAME }}" && "${{ github.event.inputs.skip_dockerhub }}" != 'true' ]]; then | ||
echo "dockerhub_enabled=true" | ||
echo "dockerhub_repository=${{ vars.DOCKERHUB_USERNAME }}/voicevox_engine" | ||
else | ||
echo "dockerhub_enabled=false" | ||
fi | ||
} >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dockerhub_repositoryはどんな状況でも値が入る形にして、dockerhub_enabledはDOCKERHUB_USERNAME関係なくskip_dockerhubに従うのが見通し良さそうに思いました!!
: # Docker Hubの設定 | |
{ | |
if [[ -n "${{ vars.DOCKERHUB_USERNAME }}" && "${{ github.event.inputs.skip_dockerhub }}" != 'true' ]]; then | |
echo "dockerhub_enabled=true" | |
echo "dockerhub_repository=${{ vars.DOCKERHUB_USERNAME }}/voicevox_engine" | |
else | |
echo "dockerhub_enabled=false" | |
fi | |
} >> "$GITHUB_OUTPUT" | |
: # Docker Hubの設定 | |
{ | |
if [[ "${{ github.event.inputs.skip_dockerhub }}" != 'true' ]]; then | |
echo "dockerhub_enabled=true" | |
else | |
echo "dockerhub_enabled=false" | |
fi | |
echo "dockerhub_repository=${{ vars.DOCKERHUB_USERNAME }}/voicevox_engine" | |
} >> "$GITHUB_OUTPUT" |
スキップしてない場合はDOCKERHUB_USERNAME
が空白のときの挙動は空白として扱うのが自然である、という考え方です!
・・・/voicevox_engine
になっちゃうのはちょっと不自然ではあるかも・・・・・・。
あ! skip_dockerhub==false
なのにDOCKERHUB_USERNAME
が空白だった場合はエラーにする、とかがより良さそうかも?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dockerhub_repositoryはどんな状況でも値が入る形に
dockerhub_repository
については、自分としては現状の方がいい寄りのどちらでもいい、かなと思います。
これが設定されたりされなかったりするのは見通し悪い気はします。空文字列がnull扱いとして、/voicevox_engine
になるとDockerリポジトリ名として不正な値っぽいのが現状の方がいい寄りの理由ですが、結局dockerhub_enabled==false
なら使われないので、どちらでもよさそうです。
dockerhub_enabledはDOCKERHUB_USERNAME関係なくskip_dockerhubに従う
こちらについては、Docker Hubアカウントを紐づけていないフォークリポジトリで実行されたとき、DockerイメージビルドCIが失敗しないようにしたい、という意図があります( #1662 (comment) )。
skip_dockerhub
はデフォルトでfalseなので、
skip_dockerhub==false
なのにDOCKERHUB_USERNAME
が空白だった場合はエラーにする
にすると、初めてリポジトリをフォークしてCIを実行したとき、DockerイメージビルドCIは失敗するので、それを避けたいと思っています。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
これが設定されたりされなかったりするのは見通し悪い気はします。空文字列がnull扱いとして、
/voicevox_engine
になるとDockerリポジトリ名として不正な値っぽいのが現状の方がいい寄りの理由ですが、結局dockerhub_enabled==false
なら使われないので、どちらでもよさそうです。
あーなるほどです!
一番良いのは直和型にして、dockerhub_enabled==false
かつdockerhub_repository
に値が入るようなことを型的にあり得ないようにすることなんですが、まあbashだとそういうこともできずって感じですね。。。
これは経験則ですが、想定外の挙動を想定して別の値を代入しておくと、それはそれで将来の変更時に想定外の挙動を引き起こすこともあるので、今回の場合はあり得ないものをエラーにするのが良いかなと・・・!
初めてリポジトリをフォークしてCIを実行したとき、DockerイメージビルドCIは失敗するので、それを避けたい
あ、なるほどです! それもそうかもです。
まあでもかといってワークアラウンド的な実装をしてしまうのは、このPRがマージされた時点では一番良いかもですが、メンテナンス性を考えると最適解ではないこともあると思います!
デフォルトの引数で、dockerhubにpushしないようにするのはどうでしょう?
初めてフォークした人にも優しいし、pushできるかのチェックのときにONにすれば良さそう。
製品版ビルドのときにフラグ付け忘れちゃうことはありえますが、まあ経験上大丈夫かなと!!
その場合skip_dockerhub
をデフォルトtrueにしても良いですが、どっちかというと反転させてpush_dockerhub
をデフォルトtrueにしたほうが直感的だろうなーと思いました!
enable_dockerhub
にしちゃうのも良いかもです。
こうするとたぶんoutputs.dockerhub_enabled
が不要になってかなりスッキリする気がしますね!!!
まあ変更大変であれば、一旦skip_dockerhub
にしてTDOO
コメント書くのでもOKかもです。
でも結構綺麗になる気がするので、できればenable_dockerhub
にしちゃいたい気もします・・・・!!!
内容
CIのDockerイメージビルドにおいて、Dockerイメージのpush先にGitHub Container Registry (ghcr.io)を追加します。
これに伴って、 #1655 と #1661 を踏まえて、レートリミットによるCI実行回数の制限を緩和するため、以下の変更をします。
ghcr.io
にログインしてpullするように変更しましたまた、複数のDockerレジストリに対応することに伴って、DockerイメージビルドCIに以下の機能追加をします。
vars.DOCKERHUB_USERNAME
が設定されていない場合、ghcr.io
へのpushのみ行い、Docker Hubへのpushを行わない実装を見送った機能: 単一プラットフォームイメージのタグをDocker Hubにpushしないようにする
実装を試みましたが、複数プラットフォームに対応した単一のタグを表現する概念のマニフェストリストを作成するためには、事前に単一プラットフォームイメージのタグをレジストリにpushしておく必要があるようで、タグを作成しないのが難しいことがわかりました。
#1533 以前がそうだったように、buildxで複数プラットフォームに対応したマニフェストリストをビルドしたものを、いきなりpushすることはできているように見えるので、方法はあるかもしれないとは思っています。
地道な方法ですが、仮のタグを一回pushして、マニフェストリストをpushし終わったら消す、ということはできるかもしれません。途中で失敗したときに仮のタグが残らないようにする工夫ができると丁寧だと思います。新しく仮のタグを設けることもなく、単一プラットフォーム用のタグを削除するステップを追加するのでいいかもしれないです。
Docker CLIの範囲では、これ以上によさそうな実装方法がすぐ見つからなかったので、いったんこのプルリクエストではTODOコメントを残して実装を見送り、
ghcr.io
にもDocker Hubにも単一プラットフォームイメージのタグがpushされるようになっています。補足: Dockerリポジトリの作成について
自動的に作成され、権限が付与されるので、手動作成・設定は不要です。
ワークフローで、
を指定して、CIに
ghcr.io
のDockerリポジトリへの書き込み権限を付与しています。この状態でDockerイメージがpushされると、GitHubリポジトリと同名のDockerリポジトリが自動的に作成され、GitHubリポジトリに紐づけられ、CIにDockerリポジトリのAdmin権限が付与されます。
GitHubリポジトリの右下にDockerリポジトリへのリンクが増えるほか、Organizationのパッケージ一覧に項目が増えると思います。
作成後は、パッケージの設定で、Admin権限が付与されていることが確認できると思います。
パッケージの設定のスクリーンショット
関連 Issue