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

Build Docker images for various platforms in a matrix strategy #67

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

jsf9k
Copy link
Member

@jsf9k jsf9k commented Jul 25, 2021

🗣 Description

This pull request modifies the build.yml GitHub Actions workflow to build the Docker images for the various supported platforms in a matrix strategy. The idea for this change was motivated by a conversation between @mcdonnnj and me.

N.B.: This pull request is somewhat of a breaking change, as the matrix strategy breaks the previous single monolithic multi-platform build into a bundle of individual single-platform builds.

N.B.: I have done away with the PLATFORMS environment variable and explicitly listed the available platforms in the GitHub Actions workflow yml file. This is because I didn't see a way to easily convert a comma-delimited list of platforms stored in an environment variable into a YML list inside the workflow. If others are able to figure that out, and we think there is benefit in having the platforms stored in an environment variable, then I'm not opposed to changing it back.

💭 Motivation and context

This change makes it easier to temporarily drop support for a failing platform that we cannot fix, as we are currently experiencing with #64.

🧪 Testing

All pre-commit hooks and GitHub Actions pass.

✅ Checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.
  • All new and existing tests pass.

@jsf9k jsf9k added breaking change This issue or pull request involves changes to existing functionality improvement This issue or pull request will add or improve functionality, maintainability, or ease of use labels Jul 25, 2021
@jsf9k jsf9k self-assigned this Jul 25, 2021
This makes it easier to temporarily drop support for a failing platform
that we cannot fix, as we are currently experiencing  with
#64.

Co-authored-by: Nick M <50747025+mcdonnnj@users.noreply.github.com>
@jsf9k jsf9k force-pushed the improvement/build-platforms-in-matrix branch from a8a2e0d to e814cd6 Compare July 25, 2021 18:12
@jsf9k jsf9k marked this pull request as ready for review July 25, 2021 18:22
@jsf9k
Copy link
Member Author

jsf9k commented Jul 26, 2021

@mcdonnnj pointed out to me that this PR is incorrect as it stands. Each job in the matrix of platform jobs creates a platform-specific Docker image with a manifest supporting that one single platform, and whichever job finishes last wins. (The late bird gets the worm.)

What we actually want is to run the matrix of platform jobs to create local Docker images, then create a list of all the platform jobs that succeeded and use that to create an appropriate manifest. This is probably too much work for us to take on right now, especially since we can more easily resolve #64 by simply removing s390x from the list of platforms. The basic idea might make a good feature request issue for docker/build-push-action.

So my current thought is to close this PR unmerged, write up an issue for docker/build-push-action, and resolve #64 the simpler way described in the previous paragraph. What do you think @felddy and @mcdonnnj?

@jsf9k jsf9k added the blocked This issue or pull request is awaiting the outcome of another issue or pull request label Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked This issue or pull request is awaiting the outcome of another issue or pull request breaking change This issue or pull request involves changes to existing functionality improvement This issue or pull request will add or improve functionality, maintainability, or ease of use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant