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

Allow Docker pushes on release branches #1568

Merged

Conversation

eread
Copy link
Contributor

@eread eread commented Nov 18, 2024

As outlined in #1432 (comment), the current workflow won't push to the Docker registry when a release is cut because releases now include a pull request.

This pull request increases the number of conditions under which a Docker image can be pushed.

Closes #1432.

@eread
Copy link
Contributor Author

eread commented Nov 18, 2024

@mre Could you review? Do you know if this will work as I've updated it here?

Otherwise, it might be possible in a separate step to include the new logic, and set an environment variable to true and then use that environment variable as the value of push.

WDYT?

push: ${{ github.event_name != 'pull_request' && github.actor != 'dependabot[bot]' }}
push: ${{ (github.event_name != 'pull_request' &&
github.actor != 'dependabot[bot]') ||
startsWith(github.ref_name, 'release-plz') }}
Copy link
Member

Choose a reason for hiding this comment

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

Looks good, but is the ref_name for release-plz documented somewhere? I was wondering if we should check for a tag name instead. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mre I've refactored this pull request. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mre Did you want me to ask another maintainer to review?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, sorry for dropping the ball on this. 😅

@eread eread force-pushed the eread/allow-docker-pushes-on-release-branches branch from 697800c to f2ae4a7 Compare November 18, 2024 22:55
@mre mre merged commit 685b653 into lycheeverse:master Dec 5, 2024
7 checks passed
@mre
Copy link
Member

mre commented Dec 5, 2024

Thanks for your contribution (and your patience), @eread. 🥳
Let's see if it works. 🤞

@eread eread deleted the eread/allow-docker-pushes-on-release-branches branch December 6, 2024 01:14
@eread
Copy link
Contributor Author

eread commented Dec 6, 2024

Thanks for your contribution (and your patience), @eread. 🥳 Let's see if it works. 🤞

Thanks for the merge @mre! I didn't see an easy way test it for sure. Hopefully a new release isn't too far away 😄

@eread
Copy link
Contributor Author

eread commented Dec 19, 2024

@mre @thomas-zahner We got a run of the new logic: https://github.com/lycheeverse/lychee/actions/runs/12389086296/job/34581417146

It looks like push still evaluated to false.

Also, the Docker command was run with --tag lycheeverse/lychee:pr-1554 --tag lycheeverse/lychee:sha-007d93a, rather than --tag lycheeverse/lychee:0.18.0. So still more to do there. 🤔

@eread
Copy link
Contributor Author

eread commented Dec 25, 2024

@mre @thomas-zahner We got a run of the new logic: https://github.com/lycheeverse/lychee/actions/runs/12389086296/job/34581417146

It looks like push still evaluated to false.

Also, the Docker command was run with --tag lycheeverse/lychee:pr-1554 --tag lycheeverse/lychee:sha-007d93a, rather than --tag lycheeverse/lychee:0.18.0. So still more to do there. 🤔

@trask I don't suppose you know how to resolve this?

@mre
Copy link
Member

mre commented Dec 25, 2024

The tags are because the pipeline didn't run on a release. The version tag is missing.

So either we're not listening to the right event or the logs belong to a different pipeline run.

@mre
Copy link
Member

mre commented Dec 25, 2024

Yeah, so we don't trigger a pull request on releases.

Typically this is done with

on:
  release:
    types: [published]

However, in our case we have a wait until the release binaries are created; otherwise the build would fail.

I don't know if there's a way to say that the workflow depends on another one. Alternatively, we could just have a "wait" step in the pipeline, which tries to curl the binary every 30 seconds or so until it succeeds.

@mre
Copy link
Member

mre commented Dec 25, 2024

We could trigger on release as shown above and use an action like https://github.com/marketplace/actions/wait-on to wait until the binaries were created.

@eread
Copy link
Contributor Author

eread commented Jan 6, 2025

Yeah, so we don't trigger a pull request on releases.

Typically this is done with

on:
  release:
    types: [published]

However, in our case we have a wait until the release binaries are created; otherwise the build would fail.

I don't know if there's a way to say that the workflow depends on another one. Alternatively, we could just have a "wait" step in the pipeline, which tries to curl the binary every 30 seconds or so until it succeeds.

@mre I must've misunderstood how these work: #1598. I guess the release is created when this is merged, and so the release isn't in the context of the pull request anymore.

@eread
Copy link
Contributor Author

eread commented Jan 6, 2025

@mre It looks like you can make jobs depend on each other with needs: https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#jobsjob_idneeds

@eread
Copy link
Contributor Author

eread commented Jan 6, 2025

We could trigger on release as shown above and use an action like https://github.com/marketplace/actions/wait-on to wait until the binaries were created.

@mre What about running when a tag is pushed: https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#onpushbranchestagsbranches-ignoretags-ignore?

@eread
Copy link
Contributor Author

eread commented Jan 6, 2025

We could trigger on release as shown above and use an action like https://github.com/marketplace/actions/wait-on to wait until the binaries were created.

@mre What about running when a tag is pushed: https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#onpushbranchestagsbranches-ignoretags-ignore?

@mre Or maybe we should remove this "escape early" configuration:

- name: Exit early if tag and not from release workflow
?

@mre
Copy link
Member

mre commented Jan 6, 2025

Hum, I'm not sure why that was there. I removed it.. Thanks for the idea. We can give it another try on the next release.

@mre
Copy link
Member

mre commented Jan 6, 2025

@mre It looks like you can make jobs depend on each other with needs: docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#jobsjob_idneeds

needs is just for expressing dependencies between jobs, not workflows. For that, you'd use workflows. In our case, we already do that. See here.

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.

Missing 0.15.x releases on docker hub
2 participants