A new build process, based on GitHub Actions #30283
Conversation
apparentlymart
added
the
1.1-backport
| // as it only tries to compile files that were included in the testdata | ||
| // (which is true for this test). | ||
| t.Skip("can't run without building a new provider executable") | ||
| } |
There was a problem hiding this comment.
Unless I'm misunderstanding something, these skipped e2e tests don't seem to run in CI at all any more. They seem to be skipped both in the new build-then-run actions and in the per-commit CircleCI runs.
If that's the case, is it feasible to fix that before merge?
There was a problem hiding this comment.
Ahh yes, you are right that the "normal" path which builds a custom binary sneakily updates this variable before it returns:
terraform/internal/command/e2etest/main_test.go
Lines 21 to 39 in 535da4e
I'll fix this up by adding a separate variable to track whether we're using an externally-provided executable or a locally-built one, and then change these conditions to all depend on that instead, as a more robust temporary solution to this quirk. Thanks!
For the moment this is just an experimental additional sidecar package build process, separate from the one we really use for releases, so that we can get some experience building in the GitHub Actions environment before hopefully eventually switching to using the artifacts from this process as the packages we'll release through the official release channels. It will react to any push to one of our release branches or to a release tag by building official-release-like .zip, .deb, and .rpm packages, along with Docker images, based on the content of the corresponding commit. For the moment this doesn't actually produce _shippable_ packages because in particular it doesn't know how to update our version/version.go file to hard-code the correct version number. Once Go 1.18 is release and we've upgraded to it we'll switch to using debug.ReadBuildInfo to determine our version number at runtime and so no longer need to directly update a source file for each release, but that functionality isn't yet available in our current Go 1.17 release.
This uses the decoupled build and run strategy to run the e2etests so that we can arrange to run the tests against the real release packages produced elsewhere in this workflow, rather than ones generated just in time by the test harness. The modifications to make-archive.sh here make it more consistent with its originally-intended purpose of producing a harness for testing "real" release executables. Our earlier compromise of making it include its own terraform executable came from a desire to use that script as part of manual cross-platform testing when we weren't yet set up to support automation of those tests as we're doing here. That does mean, however, that the terraform-e2etest package content must be combined with content from a terraform release package in order to produce a valid contest for running the tests. We use a single job to cross-compile the test harness for all of the supported platforms, because that build is relatively fast and so not worth the overhead of matrix build, but then use a matrix build to actually run the tests so that we can run them in a worker matching the target platform. We currently have access only to amd64 (x64) runners in GitHub Actions and so for the moment this process is limited only to the subset of our supported platforms which use that architecture.
This should eventually grow to be a step that actually verifies the validity of the docs source prior to publishing the artifact that a downstream publishing pipeline can consume, but for the moment it's really just a placeholder since we have no such validation step and no downstream pipeline consuming this artifact. The general idea here is that the artifacts from this workflow should be sufficient for all downstream release steps to occur without any direct access to the Terraform CLI repository, and so this is intended to eventually meet that ideal but as of this commit the website docs publishing step _does_ still depend on direct access to this repository.
We can use an extra matrix dimension to select which execution environment we'll use for each GOOS/GOARCH pair, and thus avoid duplicating the job definition for darwin just to set runs-on: macos-latest for it. This is not really an intended use of a matrix dimension because it's directly related to the existing "goos" one, rather than being an independent third dimension, but it doesn't matter in practice because we're using the "include" option to specify exact combinations, and thus we're not relying on the built-in functionality to generate all possible matrix combinations.
This workflow only generates artifacts and doesn't need to modify anything about the repository.
In our build workflow we'll treat Linux distribution packaging (currently .deb and .rpm packages) as a separate job, instead of embedding it into the "build" job, so that this step can happen concurrently with the other derived actions like the docker image build, and the e2etest runs.
apparentlymart
force-pushed
the
build-workflow-dev/matkins-initial
branch
from
7c04b0d to
2cde9e6
Compare
| # supported platforms. Even within that set, we can only run on | ||
| # architectures for which we have GitHub Actions runners available, | ||
| # which is currently only amd64 (x64). | ||
| # TODO: GitHub Actions does support _self-hosted_ arm and arm64 |
There was a problem hiding this comment.
There's also qemu. It is significantly slower as it's cpu instruction emulation. But for high-io/low-cpu tasks it works great.
For building arm64 docker image qemu would work great. It's just RUN apk add and COPY terraform
Just use this github action and then you can exec both amd64 and arm64 binaries: https://github.com/docker/setup-qemu-action.
Running all e2e tests with qemu will likely be slower, but should work ok... Maybe can keep tests amd64 and test arm64 binary? actions/setup-go doesn't allow specifying arch but it's not a problem if amd64 e2etests can tests arm64 binary.
There was a problem hiding this comment.
I can have a go and updating package-docker to build arm64 docker image and and e2etest-* to test arm64 binary using amd64 test harness if that sounds ok?
There was a problem hiding this comment.
Thanks for the tip!
I believe the packaging helper action we're using here already has some support for packaging across architectures -- I think I saw it work when I was first experimenting. However, I intentionally reduced it to just what our current release pipeline is capable of so that we wouldn't get potentially blocked by irrelevant failures in the meantime, given that we're currently using this only to get early feedback on build/e2etest problems.
I'd like to wait to add new architectures here until our release engineering team is ready to work with us on feeding this into the rest of the new pipeline, so that we can make sure whatever we do here is consistent with how other projects using the same pipeline will do it.
There was a problem hiding this comment.
Btw I had a look at https://github.com/hashicorp/actions-docker-build it already uses docker build buildx --platform $PLATFORM but $PLATFORM is only used internally not settable from action parameters. https://github.com/hashicorp/actions-docker-build/blob/v1/action.yml#L222-L224`
If arm64 binary is being built here? It will be easy to add the binary to arm64 docker image like this and will run well...
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
This introduces a new process, based on GitHub Actions, for producing the various release artifacts for Terraform CLI.
The ultimate goal here is for this workflow to build all of the artifacts needed to drive the Terraform CLI release process, so that the actual release process (maintained elsewhere) could consume these artifacts directly and no longer need to access the actual repository content, which in turn means that we can verify all of the artifacts before optionally launching the separate release process. The intended workflow then would be to first have this workflow build on the release branch, then once it's succeeded create a tag from the same commit to produce a releasable build. The branch build will have a synthetic version number including the commit ID, while the tag build would reflect the version number indicated in the tag.
However, for now my goals here are more modest: just to get a new process in place which will give us earlier warning if anything merged to a release branch or included in a tagged version has any build problems. No automated process is actually consuming the results of this for now, and I expect we'll iterate more on this workflow once we're ready to integrate this with a new release process. Which is to say: there are some known and documented limitations here, which we'll hopefully address in later commits, but which are not in scope for this PR.
The effect of merging this will be that any future merges to a release branch or to the
mainbranch will trigger a full build for all of our target platforms, including the construction of the various "wrapper package" types our official process produces (Deb packages, RPM packages, Docker Images). It will then run the end-to-end tests against thelinux_amd64,darwin_amd64, andwindows_amd64packages, which should increase our confidence that we built generally-working packages across those three platforms.Given that, I'd like to merge this to get that relatively-minor benefit for now and also to get some experience with the behavior of this workflow in advance of us integrating it with the rest of the release process. In particular, we'll be able to see how it behaves in our real workflow with release branches and tagging, whereas so far I've only been able to test it on this development branch.
Because for the moment we will not actually be releasing the packages built by this process (we'll continue to release the artifacts produced by the non-public release workflow as part of releasing), this doesn't technically assure us that the released packages would also pass the e2etests once built, but in practice the build process here is similar enough to the one the real release process uses that it should still serve as a good early warning signal.
The later work to integrate this with a new release process that actually consumes all of these artifacts will be in closer collaboration with the release engineering team and I expect it will result in at least some small changes to the details of how this works, so we can make sure this is producing artifacts in a suitable shape for the standard release process. However, for my part I did derive this from the build process templates provided from the release engineering team, and referred to the corresponding internal documentation, and so I believe it should already be broadly compatible with those assumptions and only require minor tweaks later.
Although this PR doesn't modify any "real" Terraform code, it does include some adjustments to the
internal/command/e2etestpackage to make it ready to support the separate build run approach used by this workflow. This separation between building the test harness and running it was an original intention of the e2etest design, to allow us to be sure that we're really testing the actual release executable and not a fresh build from source, but we previously didn't end up actually using it in that mode and so that capability has "bitrotted" a bit over the last few years.In particular, we introduced a few new tests that expect to be able to build new executables from source during the test run, which is counter to the original idea of a test suite we can run against pre-built executables. As a compromise for now I've just made the test harness skip over those tests when running in this mode, since we'll still get those run on Linux during the normal pull request check process. In a later commit I'd like to adjust this slightly so that the
make_archive.shscript also includes the needed supporting executables and thus we'll be able to make the separate test harness run the full test suite as originally intended.You can preview the effect of this workflow by viewing the Actions runs generated from this workflow on this development branch, though I believe you'll need to be a maintainer in this repository in order to be able to actually download the built artifacts.
Note that there's a quirk in the GitHub Actions UI whereby any artifact you download is always wrapped in a
.ziparchive container, even if the artifact itself is a.zipfile. That's kinda annoying for the various.zippackages we produce here, but is an expected part of how GitHub Actions works and doesn't really hurt for our final intended use because the GitHub Actions steps for consuming these artifacts downstream automatically unpacks the outer wrapping archive to reveal the real.zipfile inside. This extra level of wrapping is actually important because GitHub Actions's own packaging cannot preserve file permissions, and so the extra inner.zipfile we produced ourselves can preserve the executable permission on theterraformexecutable.