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

WIP: Add GHCR build option & update build to include ARM64 #57

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

adamus1red
Copy link
Contributor

No description provided.

@adamus1red adamus1red marked this pull request as draft June 4, 2024 21:02
Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

I don't have any experience with GHCR, but I think I've either spotted something important or found an opportunity to learn something important. 😁

.github/workflows/docker-publish.yml Outdated Show resolved Hide resolved
.github/workflows/docker-publish.yml Outdated Show resolved Hide resolved
@adamus1red
Copy link
Contributor Author

adamus1red commented Jun 5, 2024

Based on the build data from https://github.com/sopel-irc/docker-sopel/actions/runs/9383410081

Tags for non-default python (i.e. 3.11)

"tags": [
      "ghcr.io/sopel-irc/sopel:8-py3.11",
      "ghcr.io/sopel-irc/sopel:8.0-py3.11",
      "ghcr.io/sopel-irc/sopel:8.0.0-py3.11"
    ]

Tags for default python 3.12

"tags": [
      "ghcr.io/sopel-irc/sopel:8-py3.12",
      "ghcr.io/sopel-irc/sopel:8.0-py3.12",
      "ghcr.io/sopel-irc/sopel:8.0.0-py3.12",
      "ghcr.io/sopel-irc/sopel:8-py3",
      "ghcr.io/sopel-irc/sopel:8.0-py3",
      "ghcr.io/sopel-irc/sopel:8.0.0-py3",
      "ghcr.io/sopel-irc/sopel:8",
      "ghcr.io/sopel-irc/sopel:8.0",
      "ghcr.io/sopel-irc/sopel:8.0.0",
      "ghcr.io/sopel-irc/sopel:latest"
    ],

I believe the hub tags are not showing as the secret value is not exposed to contributer builds. On my own the tags for ghcr and docker hub are shown as being created.

@adamus1red adamus1red marked this pull request as ready for review June 5, 2024 11:54
@adamus1red adamus1red requested a review from dgw June 5, 2024 20:58
@dgw
Copy link
Member

dgw commented Jun 5, 2024

I believe the hub tags are not showing as the secret value is not exposed to contributer builds.

Most likely, yes. I don't know why @HumorBaby implemented the image name as a secret, but I'd guess that it's because non-encrypted variables didn't exist until 2023. We could just… change the Docker Hub image name to a variable.

The little I know about GHCR tells me that we are meant to use the sopel-irc namespace, because the workflow runs in that organization and you can only push images within your own namespace, so it might not make sense to use a variable there.

Unless something a little more complicated would make sense? Could have three variables:

  • DOCKER_HUB_NAMESPACE
  • GHCR_NAMESPACE
  • CONTAINER_NAME

Yielding Docker tags like $DOCKER_HUB_NAMESPACE/$CONTAINER_NAME and GHCR tags like $GHCR_NAMESPACE/$CONTAINER_NAME. But that sounds overly complicated to me. Is there actually any reason for the image names not to be a static part of the YAML, other than the fact that they now appear in multiple places? (One can't DRY out the multiple references to a secret/variable, but in the unlikely event we need to change image names the var makes it very slightly easier to update.)

@adamus1red
Copy link
Contributor Author

adamus1red commented Jun 5, 2024

Is there actually any reason for the image names not to be a static part of the YAML, other than the fact that they now appear in multiple places? (One can't DRY out the multiple references to a secret/variable, but in the unlikely event we need to change image names the var makes it very slightly easier to update.)

It breaks builds on forks if it isn't configurable or using a variable. Since it'll try to push to sopel-irc namespace without being permissioned for it.

@dgw
Copy link
Member

dgw commented Jun 5, 2024

That's fair. So the GHCR build should keep using github.repository_owner, and the Docker Hub builds should keep using a variable if people creating their own forks is a supported use case.

Would it help if I set a new DOCKER_HUB_IMAGE_NAME variable for you to use in this updated workflow? There's no reason for the name to be secret; the resulting image is public, after all.

@adamus1red adamus1red changed the title Add GHCR build option & update build to include ARM64 WIP: Add GHCR build option & update build to include ARM64 Jun 5, 2024
@adamus1red adamus1red marked this pull request as draft June 5, 2024 23:19
@dgw dgw removed their request for review June 5, 2024 23:52
@adamus1red adamus1red self-assigned this Jun 5, 2024
@adamus1red adamus1red added enhancement New feature or request labels Jun 5, 2024
@adamus1red adamus1red marked this pull request as ready for review June 5, 2024 23:54
@HumorBaby
Copy link
Contributor

I don't know why @HumorBaby implemented the image name as a secret, but I'd guess that it's becaus…

*_Ahem_*, have a gander at the blame/history for the workflow 😜 to find the true culprit: @adamus1red !!

@adamus1red
Copy link
Contributor Author

I don't know why @HumorBaby implemented the image name as a secret, but I'd guess that it's becaus…

*_Ahem_*, have a gander at the blame/history for the workflow 😜 to find the true culprit: @adamus1red !!

Probably when I complained about my build failing every time I did a push and getting an email about it

Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

While reiterating that I am not "the maintainer" of this Docker stuff, I haven't seen much out of @HumorBaby and am curious about his take on this.

I'm definitely in favor of adding ARM64 support, and pushing images to GHCR is always nice as an alternative to the Docker Inc. repository… but I don't think this should just up and remove the "nightly" builds without prior discussion.

@@ -33,12 +33,13 @@ jobs:
# Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it
- uses: actions/checkout@v2

-
-
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
-
-

name: Set up QEMU
uses: docker/setup-qemu-action@v3

-
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
-
-

Comment on lines 94 to 96
name: Build Nightly SOPEL for ${{ matrix.python_images }}
name: Build SOPEL for ${{ matrix.python_images }}
uses: docker/build-push-action@v2
if: github.event_name == 'schedule' && matrix.python_images == env.default_python
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't smell right. A "nightly" build pushed to Docker Hub should be able to coexist with both GHCR and ARM64 added into the mix. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be fixed in latest version

@adamus1red adamus1red force-pushed the adamus1red-ci branch 2 times, most recently from 626c1a1 to c458265 Compare June 13, 2024 00:08
@adamus1red adamus1red force-pushed the adamus1red-ci branch 4 times, most recently from 259d339 to 17221bf Compare June 13, 2024 09:36
Use docker/metadata-action to generate tags and labels
Add second docker/login-action to support GHCR
Use docker/setup-qemu-action for multi-arch build
Update docker/serup-buildx-action to v3
Fix nightly to only be built for 3.12 and fix image tags
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Idea] Push to GHCR in addition to Docker Hub [FR] Add multi-arch builds
3 participants