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

Allow pulling partial layer chains from an image #2795

Merged
merged 1 commit into from
May 5, 2022

Conversation

tonistiigi
Copy link
Member

Adds a new WithLayerLimit option to llb.Image
only pulls specified number of layers instead of
full image.

This can be used in combination with DiffOp/MergeOp
to pull any subset of layers from an image in any order.

Signed-off-by: Tonis Tiigi tonistiigi@gmail.com

@tonistiigi tonistiigi requested a review from sipsma April 10, 2022 21:51
@@ -116,6 +116,11 @@ func Image(ref string, opts ...ImageOption) State {
attrs[pb.AttrImageRecordType] = info.RecordType
}

if ll := info.layerLimit; ll != nil {
attrs[pb.AttrImageLayerLimit] = strconv.FormatInt(int64(*ll), 10)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably worth setting this as a uint or just enforcing that it's not negative.

Copy link
Member Author

Choose a reason for hiding this comment

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

In attrs it is a string. I don't want to set WithLayerLimit to uint as then it will require casting on every usage. I've added validation on the server-side to make sure the value is positive.

@@ -5728,6 +5729,115 @@ func testBuildInfoNoExport(t *testing.T, sb integration.Sandbox) {
require.Equal(t, exbi.Sources[0].Ref, "docker.io/library/busybox:latest")
}

func testPullWithLayerLimit(t *testing.T, sb integration.Sandbox) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Worth adding a quick test in here that setting the limit to 0 doesn't break anything, also some of the other corner cases like that an error is returned when limit is greater than layers in the image.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added testcase

dt, err := json.Marshal(struct {
Digest digest.Digest
OS string
Arch string
Variant string `json:",omitempty"`
Limit *int `json:",omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

a nil Limit will be encoded as a json null value, so does that mean that anyone upgrading to a Buildkit with this feature will get cache misses on already pulled images due to the fact that the cache key changed here? Not exactly the end of the world if so, just making sure that's alright.

Copy link
Member Author

Choose a reason for hiding this comment

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

"omitempty" in here takes care of that. Key only changes if the limit is set.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh cool, didn't know that

Adds a new `WithLayerLimit` option to `llb.Image`
only pulls specified number of layers instead of
full image.

This can be used in combination with DiffOp/MergeOp
to pull any subset of layers from an image in any order.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
@tonistiigi tonistiigi merged commit e071c35 into moby:master May 5, 2022
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.

None yet

2 participants