getslash / backslash

Centralized test reporting and monitoring service
Other
16 stars 12 forks source link

Add a docker build action for github #647

Closed klimburg closed 2 years ago

klimburg commented 3 years ago

Additionally:

Fixes #646

klimburg commented 3 years ago

@vmalloc Let me know what you think. This uses github releases as the primary mechanism for publishing but is also enabled for PRs as I wanted to be able to easily deploy images built to my testing environment.

klimburg commented 3 years ago

Not surprisingly this failed because the job attempts to post to ghcr.io for the GetSlash org which isnt allowed

#46 ERROR: denied: installation not allowed to Create organization package
------
 > pushing ghcr.io/getslash/backslash:pr-647 with docker:
------
error: denied: installation not allowed to Create organization package
Error: buildx failed with: error: denied: installation not allowed to Create organization package

But it does successfully work in my org where that is enabled action run container image

vmalloc commented 3 years ago

@klimburg I think the publishing to ghcr.io should only be from develop and master, no?

klimburg commented 2 years ago

@klimburg I think the publishing to ghcr.io should only be from develop and master, no?

Do you mean triggering with regards to the on: pull_request option. So

on:
  pull_request:
    branches:
    - '*'

becomes

on:
  pull_request:
    branches:
    - 'develop'
    - 'master

which I think is quite reasonable. This would still trigger on most PRs (in your repo) since the typical target would be develop. I made it all because I have other branches I plan on using in my fork so I can deploy features into prod before you take them upstream. So I might have a PR that targets ps-develop. That said I can still accomplish that by removing the pull_request triggers entirely and just use a release marked as a prerelease e.g. 2.18.0a1 in order to trigger a build that would publish a container I could then deploy to one of my environments.

There is another option that maybe you meant which would be to disable the push field in the Build and Publish action for all PRs. This would still result in testing the container could be built which definitely has value.

push: ${{ github.event.release != null}} would only then push on releases which seems like maybe it meets the intent of only publishing containers on release (which would typically be from develop or master but could still be something else if desired by the repo owner).

      - name: Build and push Docker image
        uses: docker/build-push-action@a66e35b9cbcf4ad0ea91ffcaf7bbad63ad9e0229
        with:
          context: .
          file: docker/Dockerfile
          push: ${{ github.event.release != null}}  # true
          tags: ${{ steps.meta.outputs.tags }}
          labels: ${{ steps.meta.outputs.labels }}
vmalloc commented 2 years ago

I meant that no release needs to be published by default on PRs and side branches... It would spam GH releases unnecessarily. Is your intent having a release made for each PR?

klimburg commented 2 years ago

Ok then push: ${{ github.event.release != null}} would do whats intended and not publish to ghcr.io for any pull requests and the only way to publish a container to ghcr.io would be to use the github release mechanism manually. This seems like a good idea.

klimburg commented 2 years ago

Ok so disabling push on PR and enabling the cache were both successful. I did realize that because the CI.yml only triggers on push it ran in my fork but not here. Not sure if we should change that so it also runs on pull_request, up to you. Let me know if there are any other changes you wish to see on this PR otherwise, I'm happy to squash and change this from a draft pr to a real one.

vmalloc commented 2 years ago

@klimburg for some reason GH still requires Travis. I deleted the configuration in develop - can you rebase on top of it to see if it is resolved?

klimburg commented 2 years ago

@klimburg for some reason GH still requires Travis. I deleted the configuration in develop - can you rebase on top of it to see if it is resolved?

Will do

klimburg commented 2 years ago

Ok that didnt do it. Let me try closing this PR and opening another one

klimburg commented 2 years ago

I am guessing you need to update the branch protection rules in the repo settings

vmalloc commented 2 years ago

@klimburg indeed. Fixed it now. Thanks!