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 publishing Docker image to GitHub #1132

Merged
merged 1 commit into from
May 21, 2021

Conversation

markuspoerschke
Copy link
Contributor

The workflow definition was copied from https://docs.github.com/en/actions/guides/publishing-docker-images#publishing-images-to-github-packages.

No changes were needed to publish the docker image to GitHub's container registry.

You can find a successfull run here: https://github.com/markuspoerschke/danger-js/runs/2240530067?check_suite_focus=true (since you can test it only once you released a new version...)

@markuspoerschke
Copy link
Contributor Author

Fixes #923

@f-meloni
Copy link
Member

f-meloni commented Mar 31, 2021

Is this action using the org's package registry or is it deploying the package directly on the danger-js repo's package?
We moved the various docker images all under the Danger org's package registry (e.g., https://github.com/orgs/danger/packages/container/package/danger-swift), that offers a better handling of the Docker Packages (You can for example make them public and don't require authentication to use them so that you can directly import the docker package from the action using docker://), and should be the suggested one.

Comment on lines 17 to 18
registry: docker.pkg.github.com
repository: danger/danger-js/danger-danger-js
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be

Suggested change
registry: docker.pkg.github.com
repository: danger/danger-js/danger-danger-js
registry: ghcr.io/danger
repository: danger-js

But I'm not familiar with this action

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this action using the org's package registry or is it deploying the package directly on the danger-js repo's package?

It is currently using the repo's package. I will change it to the organizations namespace.

I will keep the host name as mentioned in the official documentation. ghcr.io is an alias only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just double checked. There is a difference in the hostnames: https://docs.github.com/en/packages/guides/migrating-to-github-container-registry-for-docker-images

It is recommended to use ghcr.io to use the GitHub Container Registry. So I will change this as well.

@markuspoerschke markuspoerschke force-pushed the fix-docker-image-build branch 2 times, most recently from a2fccba to bdbe974 Compare March 31, 2021 21:05
@markuspoerschke markuspoerschke marked this pull request as draft March 31, 2021 21:21
@markuspoerschke
Copy link
Contributor Author

markuspoerschke commented Mar 31, 2021

I checked the official documentation and there is another exmaple how to use ghcr.io. To me it looked clear to copy the initial example.

I will check

The above changes did not work in my fork. I will test there before opening this PR again.

@markuspoerschke
Copy link
Contributor Author

Docker Image Build Process at Danger Systems

Problem

The Danger JS is currently not publishing an up to date Docker Image. To stay compliant with the other Danger projects, the build process for Docker Images should be according to the other projects.

Objective

Danger JS should provide an up to date Docker Image. This analysis should defined

  • to where the Docker Image is published

  • how the build process looks like

Related Danger Projects

The following Danger projects are considered in this analysis:

Analysis

Published to Build process Remarks
JS docker.pkg.github.com/danger/danger-js/danger-danger-js GitHub Action Build Process is broken
Swift ghcr.io/danger/danger-swift GitHub Action
Ruby hub.docker.com/r/dangersystems/danger-rb Docker Hub Only latest development build, no tags
Kotlin ghcr.io/danger/danger-kotlin GitHub Action
Python ? ? I could not find any published Docker Image

Conclusion

The implementations for Swift and Kotlin are providing a working Docker Image. They are using the recommended GitHub Container Registry for Docker Images.

Danger JS should use the same mechanism to build and publish like Kotlin and Swift.

The Build script for Docker Images can be copied from Kotlin. It was most recently edited by @f-meloni who might be able to help setting up the correct secrets.

@markuspoerschke markuspoerschke marked this pull request as ready for review April 10, 2021 13:59
@orta
Copy link
Member

orta commented Apr 10, 2021

Seems reasonable to me, doesn't look like it changes my deploy flow at all 👍🏻

Copy link
Member

@f-meloni f-meloni left a comment

Choose a reason for hiding this comment

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

Looks good to me.
@orta if you want I can add my secrets here as well, like I did on the other danger versions.

@orta
Copy link
Member

orta commented Apr 10, 2021

Sure, do it 👍🏻

@shyim
Copy link
Contributor

shyim commented May 20, 2021

What's missing here? I can help when needed :)

@f-meloni
Copy link
Member

f-meloni commented May 21, 2021

Sure, do it 👍🏻

I've added the secrets, so we should be fine

@orta
Copy link
Member

orta commented May 21, 2021

OK, then we'll see on the next release 👍🏻

@orta orta merged commit 4d435a3 into danger:main May 21, 2021
@markuspoerschke markuspoerschke deleted the fix-docker-image-build branch May 22, 2021 10:04
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.

4 participants