Skip to content

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

Open
wants to merge 27 commits into
base: master
Choose a base branch
from

Conversation

aoirint
Copy link
Member

@aoirint aoirint commented Apr 26, 2025

内容

CIのDockerイメージビルドにおいて、Dockerイメージのpush先にGitHub Container Registry (ghcr.io)を追加します。

これに伴って、 #1655#1661 を踏まえて、レートリミットによるCI実行回数の制限を緩和するため、以下の変更をします。

  • Dockerイメージのビルドキャッシュの格納先・取得元を ghcr.io に変更
  • Dockerイメージのテストで使用するイメージの取得元を ghcr.io に変更
    • これまでDockerイメージのテストでは、レートリミットの厳しい認証されていないユーザーとしてDocker HubからDockerイメージをpullしていたことに気づきましたが、ghcr.ioにログインしてpullするように変更しました

また、複数のDockerレジストリに対応することに伴って、DockerイメージビルドCIに以下の機能追加をします。

  • vars.DOCKERHUB_USERNAMEが設定されていない場合、ghcr.ioへのpushのみ行い、Docker Hubへのpushを行わない

実装を見送った機能: 単一プラットフォームイメージのタグをDocker Hubにpushしないようにする

露出する中間docker imageも減らせる

実装を試みましたが、複数プラットフォームに対応した単一のタグを表現する概念のマニフェストリストを作成するためには、事前に単一プラットフォームイメージのタグをレジストリにpushしておく必要があるようで、タグを作成しないのが難しいことがわかりました。

#1533 以前がそうだったように、buildxで複数プラットフォームに対応したマニフェストリストをビルドしたものを、いきなりpushすることはできているように見えるので、方法はあるかもしれないとは思っています。

地道な方法ですが、仮のタグを一回pushして、マニフェストリストをpushし終わったら消す、ということはできるかもしれません。途中で失敗したときに仮のタグが残らないようにする工夫ができると丁寧だと思います。新しく仮のタグを設けることもなく、単一プラットフォーム用のタグを削除するステップを追加するのでいいかもしれないです。

Docker CLIの範囲では、これ以上によさそうな実装方法がすぐ見つからなかったので、いったんこのプルリクエストではTODOコメントを残して実装を見送り、ghcr.ioにもDocker Hubにも単一プラットフォームイメージのタグがpushされるようになっています。

補足: Dockerリポジトリの作成について

自動的に作成され、権限が付与されるので、手動作成・設定は不要です。

ワークフローで、

permissions:
  packages: write

を指定して、CIにghcr.ioのDockerリポジトリへの書き込み権限を付与しています。

この状態でDockerイメージがpushされると、GitHubリポジトリと同名のDockerリポジトリが自動的に作成され、GitHubリポジトリに紐づけられ、CIにDockerリポジトリのAdmin権限が付与されます。

GitHubリポジトリの右下にDockerリポジトリへのリンクが増えるほか、Organizationのパッケージ一覧に項目が増えると思います。

作成後は、パッケージの設定で、Admin権限が付与されていることが確認できると思います。

パッケージの設定のスクリーンショット

image

関連 Issue

@aoirint aoirint marked this pull request as ready for review April 26, 2025 16:44
@aoirint aoirint requested a review from a team as a code owner April 26, 2025 16:44
@aoirint aoirint requested review from Hiroshiba and removed request for a team April 26, 2025 16:44
@aoirint
Copy link
Member Author

aoirint commented Apr 26, 2025

手元のリポジトリでビルドが通ったので、Ready for Reviewにしました。

@Hiroshiba
Copy link
Member

PR作成ありがとうございます!!

ちょっとまだレビューできてないのですが、もしかしたらpush-by-digest=trueを指定すればdigestのみのpushができるかもと思ったのでちょっとコメントまで!!
(望んでるものなのかわからないのですが、この辺りはaoirintさんのが詳しそうなので…🙇)

misskeyがそんな感じ…かも…?(自信なし)
https://github.com/misskey-dev/misskey/blob/develop/.github/workflows/docker.yml

…とここまで書いてて思いましたが、ちょっとこのPRの主旨とは違うかもですね!!
とりあえず一旦コメントまで!!(できそうだったらissue使ってまとめてみても良いかも?)

Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

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

良い感じに思いました!!!
ちょっと出先なのですが、とりあえずコメントまで…!

@aoirint
Copy link
Member Author

aoirint commented Apr 27, 2025

もしかしたらpush-by-digest=trueを指定すればdigestのみのpushができるかも

情報ありがとうございます!

docker/build-push-actionで、単一プラットフォームイメージのビルドのときは、タグを空欄にして、push-by-digest=trueを指定してみて、latestが上書きされず、タグなしイメージがpushされることを確認できれば、そのアプローチでできるかもしれません。

もっと簡単にできそうな他の方法を見つけました。Misskeyの方では、マニフェストリストの作成にdocker manifest createではなく、docker buildx imagetools createを使っていたので、このコマンドを使って手元で試してみたところ、欲しかった機能の、異なるレジストリ間でイメージをコピーする機能がありそうなことがわかりました。

docker manifest createの方だと、作成したマニフェストリストをdocker manifest pushしたときにcannot use source images from a different registry than the target image: ghcr.io != docker.ioのようなエラーが発生してpushできなかったのであきらめていました。

マニフェストリストを作るためのコマンドが2種類あることは認識していながら、違いがよくわかっていなかったのですが、buildxがマニフェスト機能をラップしていることから推測できる通りに、buildxの方が高級なAPIだということがわかりました。

マニフェストリストの作成と同時に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

ちょっとこのPRの主旨とは違うかもですね!!

ついでに対応しようと思って試したくらいではありましたが、いまから追加変更するほどでもないかなと思うので、Issueを立てて別プルリクエストに分割しようと思います。

@aoirint
Copy link
Member Author

aoirint commented Apr 27, 2025

Issueを立てて別プルリクエストに分割しようと思います。

Issueを立てて、セルフアサインしました。

@aoirint aoirint requested a review from Hiroshiba April 27, 2025 05:15
@Hiroshiba
Copy link
Member

issue作成ありがとうございます!!
まずはこのプルリクエストをレビューします…!

@Hiroshiba Hiroshiba requested a review from Copilot April 30, 2025 16:29
Copy link
Contributor

@Copilot Copilot AI left a 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

Copy link
Member

@Hiroshiba Hiroshiba left a 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用の変数を最定義する周りだけ・・・!

@aoirint
Copy link
Member Author

aoirint commented May 3, 2025

提案通りの変更を入れたと思います。

CIが回ったらRe-request reviewしようと思います。

@aoirint aoirint requested a review from Hiroshiba May 3, 2025 04:42
Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

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

ほぼLGTMです!!

1箇所だけ念の為大丈夫そうか確認したく・・・!

Comment on lines +46 to +54
: # 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"
Copy link
Member

@Hiroshiba Hiroshiba May 3, 2025

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に従うのが見通し良さそうに思いました!!

Suggested change
: # 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が空白だった場合はエラーにする、とかがより良さそうかも?

Copy link
Member Author

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は失敗するので、それを避けたいと思っています。

Copy link
Member

@Hiroshiba Hiroshiba May 3, 2025

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にしちゃいたい気もします・・・・!!!

@aoirint aoirint changed the title feat(ci): Dockerイメージのpush先にGitHub Container Registry (ghcr.io)を追加 feat(ci): Dockerイメージのpush先にGitHub Container Registryを追加 May 3, 2025
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.

DockerイメージのGitHub Container Registry (ghcr.io)への格納提案
2 participants