Skip to content

Allow pulling partial layer chains from an image - #2795

Merged
tonistiigi merged 1 commit into
moby:masterfrom
tonistiigi:layerlimit
May 5, 2022
Merged

Allow pulling partial layer chains from an image#2795
tonistiigi merged 1 commit into
moby:masterfrom
tonistiigi:layerlimit

Conversation

@tonistiigi

Copy link
Copy Markdown
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
Comment thread client/llb/source.go
}

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

Copy link
Copy Markdown
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
Copy Markdown
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.

Comment thread client/client_test.go
require.Equal(t, exbi.Sources[0].Ref, "docker.io/library/busybox:latest")
}

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

Copy link
Copy Markdown
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
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added testcase

OS string
Arch string
Variant string `json:",omitempty"`
Limit *int `json:",omitempty"`

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

2 participants