Skip to content

Conversation

kzys
Copy link
Contributor

@kzys kzys commented Nov 15, 2019

Issue #, if available:

#313

Description of changes:

As described in #313, firecracker-containerd-integ-test is copying the entire source directory to the container in the very early stage, which makes incremental build slower. This PR changes the strategy to

  • Build binaries without Docker
  • Copy the binaries to a Docker image later
  • Only use go.mod and go.sum to download dependencies inside a container

That will make incremental build much faster.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@kzys
Copy link
Contributor Author

kzys commented Nov 15, 2019

I'm going to move "runtime" tests as well, hopefully in this PR.

@kzys kzys force-pushed the tiny-integ-test-image branch 2 times, most recently from 1248603 to d7990de Compare November 15, 2019 23:43
Copy link
Contributor

@IRCody IRCody left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you planning to keep all the commits or squash some?

@kzys
Copy link
Contributor Author

kzys commented Nov 18, 2019

27c842a should be a separate commit. I'm fine about squashing other three into one.

@IRCody
Copy link
Contributor

IRCody commented Nov 18, 2019

That SGTM.

kzys added 2 commits November 18, 2019 11:30
It doesn't have to be.

Signed-off-by: Kazuyoshi Kato <[email protected]>
Building firecracker-containerd-integ-test takes time and
we cannot utilize build cache much since we copy the entire
source tree to the container image in the very beginning of
the build process.

This change introduces firecracker-containerd-integ-test-tiny
which doesn't build binaries such as firecracker-containerd
(they are built without Docker) but able to run "go test".

Signed-off-by: Kazuyoshi Kato <[email protected]>
@kzys kzys force-pushed the tiny-integ-test-image branch from 27c842a to 8f58ad2 Compare November 18, 2019 19:41
Copy link
Contributor

@sipsma sipsma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks! I was planning on getting to #313 once I was freed up, which I should be soon now that the drive mount rate-limiter PR is out, so I'm happy to finish the rest of that issue up after this PR is merged

@kzys kzys merged commit a922bc4 into firecracker-microvm:master Nov 18, 2019
@kzys kzys deleted the tiny-integ-test-image branch November 18, 2019 21:37
fangn2 pushed a commit to fangn2/firecracker-containerd that referenced this pull request Mar 23, 2023
Add "lint" target to check Amazon's license header
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants