-
-
Notifications
You must be signed in to change notification settings - Fork 22
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
Modify action to build and consume container #124
Conversation
I've added the secrets at this point. After first build, we'll need to make the container public, and then associate it with this repository; I'll go ahead and do that when released. |
To speed things up further, we can build and release the actual container on release publication, and then have the action itself consume the image instead of the Dockerfile. Additionally, we can force the release process to use the latest set of changes by defining a local GHA that uses the Dockerfile instead. I've proven this works at https://github.com/weierophinney/test-local-action/, where I observed that the build happens relative to the checkout directory, and not to where the action is defined. Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
0518d61
to
98bf124
Compare
MAJOR="${PREFIX}:$(echo ${TAG} | cut -d. -f1)" | ||
MINOR="${MAJOR}.$(echo ${TAG} | cut -d. -f2)" | ||
PATCH="${PREFIX}:${TAG}" | ||
echo "::set-output name=tags::${MAJOR}%0A${MINOR}%0A${PATCH}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GHA replaces %0A
with \n
, making these line-delimited tags. This creates a list that looks like:
ghcr.io/laminas/automatic-releases:1
ghcr.io/laminas/automatic-releases:1.11
ghcr.io/laminas/automatic-releases:1.11.0
which is then passed as the variable "tags" to the next step.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably not the best idea to rely on undocumented behavior. Seems like an implementation detail that may behave differently in the future.
However, they have documented to pass as json, if you need something like this:
name: build
on: push
jobs:
job1:
runs-on: ubuntu-latest
outputs:
matrix: ${{ steps.set-matrix.outputs.matrix }}
steps:
- id: set-matrix
run: echo "::set-output name=matrix::{\"include\":[{\"project\":\"foo\",\"config\":\"Debug\"},{\"project\":\"bar\",\"config\":\"Release\"}]}"
job2:
needs: job1
runs-on: ubuntu-latest
strategy:
matrix: ${{fromJSON(needs.job1.outputs.matrix)}}
steps:
- run: build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@glensc This is actually the approach recommended by the GHA team, and I discovered it here: https://git.luolix.topmunity/t/set-output-truncates-multiline-strings/16852/3 (they are indicating that they transform ANSI escape sequences emitted in outputs).
A later note in that thread indicates that it's undocumented, but that they will be updating the docs to reflect this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've done some thinking on this, and decided to go with the JSON approach; I'm now doing:
with:
tags: ${{ join(fromJSON(steps.tags.outputs.tags), "\n") }}
(as they "tags" value is expected to be multiline).
This should address any concerns about ANSI escape sequences, and ensure we are forwards-compatible with any changes GHA makes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update: using "," as the separator, as CSV is supported for that value, and the "\n"
value was causing issues with the GHA expressions.
runs: | ||
using: 'docker' | ||
image: '../../../Dockerfile' | ||
args: | ||
- ${{ inputs.command-name }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I discovered is that while the image is in a directory relative to this one, the actual build happens relative to $GITHUB_WORKSPACE
, so everything works correctly. By doing this, we get the benefits of whatever changes we've made for this release... when creating the release.
@@ -15,6 +15,6 @@ inputs: | |||
|
|||
runs: | |||
using: 'docker' | |||
image: 'Dockerfile' | |||
image: 'docker://ghcr.io/laminas/automatic-releases:1' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the magic: the action now uses an image on ghcr.io, which means only a pull operation is required, and no build operations!
@@ -17,7 +17,7 @@ jobs: | |||
uses: "actions/checkout@v2" | |||
|
|||
- name: "Release" | |||
uses: "./" | |||
uses: "./.github/actions/automatic-releases/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And this part ensures we use that local action, which builds from the Dockerfile directly instead of using the image.
To address concerns with using ANSI escape sequences to create multiline outputs, this patch does the following: - Emits a JSON array instead. - Creates a single job, combining steps from each previous job; this prevents the need to use `needs`, making it possible to test the solution locally. Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
e4641b2
to
8af0b90
Compare
GHA does not like newlines in strings used as part of the expression syntax. Since docker/build-push-action allows CSV, comma separated will accomplish the same goals. Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
release-container: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- name: Compile tag list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could use https://github.com/crazy-max/ghaction-docker-meta here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True - but this is fast and doesn't require any additional overhead as it's using tooling already in the environment.
Thanks for the link, though - I'd not seen that one!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides minor improvement suggestions, LGTM 👍
PATCH="${PREFIX}:${TAG}" | ||
echo "::set-output name=tags::[\"${MAJOR}\",\"${MINOR}\",\"${PATCH}\"]" | ||
- name: Checkout | ||
uses: actions/checkout@v2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not super-important, but some spacing (blank lines around blocks) would go a long way with improving readability
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
uses: docker/login-action@v1 | ||
with: | ||
registry: ghcr.io | ||
username: ${{ secrets.CONTAINER_USERNAME }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These secrets should be prefixed, to make it clear they are local to our build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done; used AUTOMATIC_RELEASES_
as the prefix.
file: ./Dockerfile | ||
platforms: linux/amd64 | ||
push: true | ||
tags: ${{ join(fromJSON(steps.tags.outputs.tags), ",") }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we push each image as 3 tags? Ok for me, hard to read/follow if not versed in github actions (requires comments around it)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes - actual release version, minor release version, major release version.
Rationale is that if we change something in a new minor release that causes issues for somebody, they can pin to the previous minor. Alternately, they can pin to a specific "known good" release. (There are existing actions for creating tags that do the same; I chose to do it using standard *nix tooling, as it introduces zero additional overhead.)
Prefixed with `AUTOMATIC_RELEASES_` Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
- name: Compile tag list | ||
id: tags | ||
run: | | ||
TAG=${GITHUB_REF/refs\/tags\//} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TAG=${GITHUB_REF/refs\/tags\//} | |
TAG=${GITHUB_REF#refs/tags/} |
I'm sure I've already suggested this elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
To speed things up further, we can build and release the actual container on release publication, and then have the action itself consume the image instead of the Dockerfile.
Additionally, we can force the release process to use the latest set of changes by defining a local GHA that uses the Dockerfile instead.
I've proven this works at https://github.com/weierophinney/test-local-action/, where I observed that the build happens relative to the checkout directory, and not to where the action is defined.
This will require defining two secrets in this repository:
Once setup, this should automatically build and push tags for the docker image, and the action will use the image to do its work.