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

workflow: don't push Docker container on PR #2984

Merged
merged 1 commit into from
Sep 15, 2023

Conversation

blocktrron
Copy link
Member

The docker container on PR action is broken in case the PR is not opened within the Gluon project.

Remove the build on PR so the pipeline does not fail for third party PRs.

The docker container on PR action is broken in case the PR is not opened
within the Gluon project.

Remove the build on PR so the pipeline does not fail for third party
PRs.

Signed-off-by: David Bauer <[email protected]>
@neocturne
Copy link
Member

Hmm, optimally, we'd build the container (so the build is checked for PRs), but don't attempt to push it. Not sure if there is any simple way to achieve that.

@herbetom
Copy link
Contributor

Something like the following? (sorry, github doesn't allow to do this as a suggestion)

      - name: Build Docker image
        if: github.event_name == 'pull_request'
        uses: docker/build-push-action@f2a1d5e99d037542a71f64918e516c093c6f3fc4
        with:
          context: ./contrib/docker
          push: false
          tags: ${{ steps.meta.outputs.tags }}
          labels: ${{ steps.meta.outputs.labels }}
      - name: Build and push Docker image
        if: github.event_name != 'pull_request'
        uses: docker/build-push-action@f2a1d5e99d037542a71f64918e516c093c6f3fc4
        with:
          context: ./contrib/docker
          push: true
          tags: ${{ steps.meta.outputs.tags }}
          labels: ${{ steps.meta.outputs.labels }}

Hoewer, this is only one potential half of what's required. In a fork the action would still fail because the destination of the push is still hardcoded.

@neocturne
Copy link
Member

Something like the following? (sorry, github doesn't allow to do this as a suggestion)

      - name: Build Docker image
        if: github.event_name == 'pull_request'
        uses: docker/build-push-action@f2a1d5e99d037542a71f64918e516c093c6f3fc4
        with:
          context: ./contrib/docker
          push: false
          tags: ${{ steps.meta.outputs.tags }}
          labels: ${{ steps.meta.outputs.labels }}
      - name: Build and push Docker image
        if: github.event_name != 'pull_request'
        uses: docker/build-push-action@f2a1d5e99d037542a71f64918e516c093c6f3fc4
        with:
          context: ./contrib/docker
          push: true
          tags: ${{ steps.meta.outputs.tags }}
          labels: ${{ steps.meta.outputs.labels }}

Hoewer, this is only one potential half of what's required. In a fork the action would still fail because the destination of the push is still hardcoded.

Hmm, maybe it would even be possible to set the value of push depending on the condition, rather than duplicating the whole section?

@blocktrron
Copy link
Member Author

I'd remove it for now so the CI won't complain. Can someone create a PR with the suggested changes so we can verify they work as expected?

@blocktrron blocktrron merged commit 9577d2f into freifunk-gluon:master Sep 15, 2023
10 checks passed
herbetom pushed a commit to herbetom/gluon that referenced this pull request Mar 17, 2024
The docker container on PR action is broken in case the PR is not opened
within the Gluon project.

Remove the build on PR so the pipeline does not fail for third party
PRs.

Signed-off-by: David Bauer <[email protected]>
(cherry picked from commit 9577d2f)
Signed-off-by: Tom Herbers <[email protected]>
hafu pushed a commit to Freifunk-Potsdam/gluon that referenced this pull request Jun 2, 2024
The docker container on PR action is broken in case the PR is not opened
within the Gluon project.

Remove the build on PR so the pipeline does not fail for third party
PRs.

Signed-off-by: David Bauer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants