-
Notifications
You must be signed in to change notification settings - Fork 20
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
Container workflow overhaul #198
Open
jhiemstrawisc
wants to merge
10
commits into
Reed-CompBio:master
Choose a base branch
from
jhiemstrawisc:container-workflow-overhaul
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Container workflow overhaul #198
jhiemstrawisc
wants to merge
10
commits into
Reed-CompBio:master
from
jhiemstrawisc:container-workflow-overhaul
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit splits the build + push action into its yaml definition and updates the build+push action we rely on from v1 to v6. We hadn't updated the action because there was some previous feature we either relied on, or a breaking change in a newer release we wanted to avoid. In any case, we can't remember what the feature was, so it's probably time to give the new stuff a go. I chose to split this into its own workflow definition because some of the next improvements I'd like to make involve updating the trigger logic. This could be done through the same `test-spras.yml`, but is cleaner/easier to read and understand if done as a standalone.
Previously we did an explicit `docker pull` for each of the containers we wanted to use as a cache base in the build tests. I don't _think_ this is explicilty necessary, as the v6 docker build+push action has a default cache-from type of "registry": https://docs.docker.com/reference/cli/docker/buildx/build/#cache-from In other words, each build step pulls what it needs to without the explicit management. Barring a need to pre-load containers for some speed optimization (I couldn't find any), it seems more prudent to let the action manage things.
Previously this test built every container every time someone pushed changes to a PR. While this was okay, it means that we ended up building containers even when the actual changes to the codebase had nothing to do with those containers. This commit attempts to create a separate build+remove templated action that uses `git diff` against an input path to determine if a container needs to be built. Workflow in development!
I realized the SPRAS container build wouldn't work with the previous action because it takes a different context not scoped to the container's Dockerfile directory. This commit (among a few other small cleanups) adds a `context` input to the new action so that I can pass the `./` value for the SPRAS container build.
The previous action trigger of `on: [push, pull_request]` looked like it was double dipping -- every time you pushed a new set of commits to a pull request, the workflow triggered twice. This instead runs the actions on new pull requests, or on synchronize/reopened events. The synchronize event occurs when new commits are received. Additionally, this still runs the test whenever someone pushes to master, which shouldn't be done...
Since this PR doesn't make changes to any of the docker-wrapper containers that might trigger individual container builds, I've set up a demonstration here. In particular, the reviewer should be able to see that the job for all pairs detected a change and rebuilt the container. Building steps for all other containers (except SPRAS) have been skipped. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR overhauls some of the GHA workflows designed to build spras's docker containers. In particular, it defines a new template in
.github/workflows/build-and-remove-template.yml
that tries to accomplish the following: