c8d: make the cache in classic builder work#46634
Merged
thaJeztah merged 5 commits intomoby:masterfrom Jan 17, 2024
Merged
Conversation
87d76f6 to
749958b
Compare
Member
Author
|
|
6b77b95 to
aa643b7
Compare
c752016 to
c459fd1
Compare
c459fd1 to
124c716
Compare
vvoland
reviewed
Oct 23, 2023
4dfa49a to
fcd91e9
Compare
laurazard
reviewed
Oct 23, 2023
laurazard
reviewed
Oct 23, 2023
This was referenced Oct 25, 2023
54 tasks
f2ce092 to
a520b8e
Compare
a520b8e to
c2ba48d
Compare
Contributor
|
FYI, the validate failure is caused by splitting one integration-cli test into two separate ones. |
Member
|
@vvoland oh! I saw that, but looks like I didn't comment; could you perhaps push a temporary commit similar to 7ed823e, so that we have the validate steps run? We can revert it after this is merged (we should have some option to allow skipping it in CI without having to make changes, but don't know what the best approach for that is with GitHub actions 😞) |
Contributor
|
Done (lol, me pushing a commit made a review request on behalf of @rumpl) |
ee77bfc to
9db487f
Compare
laurazard
approved these changes
Jan 17, 2024
Member
laurazard
left a comment
There was a problem hiding this comment.
LGTM 🎉 (yaaaaaay)
Left a couple of nitpicky comments/questions but there's probably reason for things to be that way/they're not anywhere big enough to be a blocker.
Comment on lines
+534
to
+536
| manifestDesc ocispec.Descriptor, | ||
| containerConfigDesc ocispec.Descriptor, | ||
| _ error, |
Member
There was a problem hiding this comment.
Nice! Reads better than just having the function return (ocispec.Descriptor, ocispec.Descriptor, error)
thaJeztah
reviewed
Jan 17, 2024
9db487f to
5ed41d6
Compare
In order for the cache in the classic builder to work we need to: - use the came comparison function as the graph drivers implementation - save the container config when commiting the image - use all images to search a 'FROM "scratch"' image - load all images if `cacheFrom` is empty Signed-off-by: Djordje Lukic <djordje.lukic@docker.com> Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Consider only images that were built `FROM scratch` as valid candidates for the `FROM scratch` + INSTRUCTION build step. The images are marked as `FROM scratch` based by the classic builder with a special label. It must be a new label instead of empty parent label, because empty label values are not persisted. Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Serialize ContainerConfig to content store and store its digest in label. Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
5ed41d6 to
bdc7d0c
Compare
Member
|
IT'S ALL GREEN (on Linux). IT'S ALL GREEN!! 🥳 🥳 🥳 🥳 
|
This was referenced Jan 17, 2024
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
- What I did
Makes the cache in classic builder work with the containerd image store
In order for the cache in the classic builder to work we need to:
cacheFromis empty- How I did it
- How to verify it
$ make DOCKER_GRAPHDRIVER=overlayfs TEST_FILTER=TestBuildAddCurrentDirWithoutCache TEST_INTEGRATION_USE_SNAPSHOTTER=1 TEST_IGNORE_CGROUP_CHECK=1 test-integration$ make DOCKER_GRAPHDRIVER=overlayfs TEST_FILTER=TestBuildAddRemoteFileWithAndWithoutCache TEST_INTEGRATION_USE_SNAPSHOTTER=1 TEST_IGNORE_CGROUP_CHECK=1 test-integration$ make DOCKER_GRAPHDRIVER=overlayfs TEST_FILTER=TestBuildCopyDirButNotFile TEST_INTEGRATION_USE_SNAPSHOTTER=1 TEST_IGNORE_CGROUP_CHECK=1 test-integrationAnd many others.
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)