-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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 improvements #1200
Build improvements #1200
Conversation
aaa14ea
to
51db528
Compare
51db528
to
0e72e76
Compare
0e72e76
to
8b50a77
Compare
44e48cf
to
c8fb446
Compare
c8fb446
to
2d14172
Compare
da359e4
to
1539c6a
Compare
3245ea3
to
8a09d0a
Compare
I have a more generic question for the analysis doc. When you separate build from test, how are you going to know that you are testing the right image when it comes to downloading it and testing? |
It's configured explicitly in the CircleCI config at the moment, and for any given revision it can be determined based on git tree hashes. You can read E.g. given
This explicitly precludes that image was built and published before tests run, which was already required previously, but not explicitly. |
a9e8c18
to
937bc39
Compare
@marccarre I've updated the proposal doc, and rebased this - could you please take another look? |
937bc39
to
20bdaf8
Compare
20bdaf8
to
c2e123e
Compare
37e523a
to
cb7c7b1
Compare
runs-on: ubuntu-18.04 | ||
steps: | ||
- uses: actions/checkout@v1 | ||
- uses: docker://weaveworks/eksctl-build:19ff80e010d2fdb7be3c44fc6491e13395a682ad |
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.
Nit: do you know if there is a way to factorise docker://weaveworks/eksctl-build:19ff80e010d2fdb7be3c44fc6491e13395a682ad
?
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 sure. For now make -f Makefile.docker update-build-image-manifest
handles updating it here and in .circleci/config.yml
.
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.
Some of the fields in github configs accept shell syntax (like foo-$(echo 123)
), but that didn't work here.
GO_VERSION: 1.12.6 | ||
GOCACHE: /home/circleci/.cache/go-build/ | ||
GOPATH: /home/circleci/go | ||
BUILD_IMAGE: weaveworks/eksctl-build:19ff80e010d2fdb7be3c44fc6491e13395a682ad |
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.
Nit: do you know if there is a way to factorise BUILD_IMAGE: weaveworks/eksctl-build:19ff80e010d2fdb7be3c44fc6491e13395a682ad
in order to have it defined at a higher-level in the YAML file?
This looks reasonable to me. The only way is forward. And we can always refine later. |
- remove `.dockerignore` file - isolate context to speed-up `docker build` - tag build image and intermediate container automatically - use more cosistent lower-case variable variable names - make `git diff` error hint fixes - split base build image - includes Docker CLI - use it in Circle CI
- there is no need to build the image in CI as a primary job - pulling the image is as quick as restoring cache - we can control what is being cached - we don't have to rely on how CircleCI cache works, so our jobs are more portable - cleanup - remove outdated Skaffold config - cleanup Dockerfile, cache all of the modules, use GOPROXY - upgrade CircleCI jobs' resource class - start trial of GitHub Actions in parallel to CircleCI - start with two workflows - one is for running tests & building the binaries in pre-existing image - one for updating images - split Docker-related targets from the main Makefile, avoiding having to have Go installed in CI job that builds images - extract common variables into separate `Makefile.common` - update modules that are used in the build - fix file license header filename
cb7c7b1
to
c146cdd
Compare
Description
Basic summary:
See proposal document within for more details.
Checklist
make build
)make test
)