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.

Sign up for GitHub

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
Merged

Allow pulling partial layer chains from an image #2795

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 120 additions & 0 deletions client/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ func TestIntegration(t *testing.T) {
testUncompressedRegistryCacheImportExport,
testStargzLazyRegistryCacheImportExport,
testCallInfo,
testPullWithLayerLimit,
)
tests = append(tests, diffOpTestCases()...)
integration.Run(t, tests, mirrors)
Expand Down Expand Up @@ -5752,6 +5753,125 @@ 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

integration.SkipIfDockerd(t, sb, "direct push")
requiresLinux(t)
c, err := New(sb.Context(), sb.Address())
require.NoError(t, err)
defer c.Close()

st := llb.Scratch().
File(llb.Mkfile("/first", 0644, []byte("first"))).
File(llb.Mkfile("/second", 0644, []byte("second"))).
File(llb.Mkfile("/third", 0644, []byte("third")))

def, err := st.Marshal(sb.Context())
require.NoError(t, err)

registry, err := sb.NewRegistry()
if errors.Is(err, integration.ErrRequirements) {
t.Skip(err.Error())
}
require.NoError(t, err)

target := registry + "/buildkit/testlayers:latest"

_, err = c.Solve(sb.Context(), def, SolveOpt{
Exports: []ExportEntry{
{
Type: ExporterImage,
Attrs: map[string]string{
"name": target,
"push": "true",
},
},
},
}, nil)
require.NoError(t, err)

// pull 2 first layers
st = llb.Image(target, llb.WithLayerLimit(2)).
File(llb.Mkfile("/forth", 0644, []byte("forth")))

def, err = st.Marshal(sb.Context())
require.NoError(t, err)

destDir, err := os.MkdirTemp("", "buildkit")
require.NoError(t, err)
defer os.RemoveAll(destDir)

_, err = c.Solve(sb.Context(), def, SolveOpt{
Exports: []ExportEntry{{
Type: ExporterLocal,
OutputDir: destDir,
}},
}, nil)
require.NoError(t, err)

dt, err := os.ReadFile(filepath.Join(destDir, "first"))
require.NoError(t, err)
require.Equal(t, string(dt), "first")

dt, err = os.ReadFile(filepath.Join(destDir, "second"))
require.NoError(t, err)
require.Equal(t, string(dt), "second")

_, err = os.ReadFile(filepath.Join(destDir, "third"))
require.Error(t, err)
require.True(t, errors.Is(err, os.ErrNotExist))

dt, err = os.ReadFile(filepath.Join(destDir, "forth"))
require.NoError(t, err)
require.Equal(t, string(dt), "forth")

// pull 3rd layer only
st = llb.Diff(
llb.Image(target, llb.WithLayerLimit(2)),
llb.Image(target)).
File(llb.Mkfile("/forth", 0644, []byte("forth")))

def, err = st.Marshal(sb.Context())
require.NoError(t, err)

destDir, err = os.MkdirTemp("", "buildkit")
require.NoError(t, err)
defer os.RemoveAll(destDir)

_, err = c.Solve(sb.Context(), def, SolveOpt{
Exports: []ExportEntry{{
Type: ExporterLocal,
OutputDir: destDir,
}},
}, nil)
require.NoError(t, err)

_, err = os.ReadFile(filepath.Join(destDir, "first"))
require.Error(t, err)
require.True(t, errors.Is(err, os.ErrNotExist))

_, err = os.ReadFile(filepath.Join(destDir, "second"))
require.Error(t, err)
require.True(t, errors.Is(err, os.ErrNotExist))

dt, err = os.ReadFile(filepath.Join(destDir, "third"))
require.NoError(t, err)
require.Equal(t, string(dt), "third")

dt, err = os.ReadFile(filepath.Join(destDir, "forth"))
require.NoError(t, err)
require.Equal(t, string(dt), "forth")

// zero limit errors cleanly
st = llb.Image(target, llb.WithLayerLimit(0))

def, err = st.Marshal(sb.Context())
require.NoError(t, err)

_, err = c.Solve(sb.Context(), def, SolveOpt{}, nil)
require.Error(t, err)
require.Contains(t, err.Error(), "invalid layer limit")
}

func testCallInfo(t *testing.T, sb integration.Sandbox) {
c, err := New(sb.Context(), sb.Address())
require.NoError(t, err)
Expand Down
6 changes: 6 additions & 0 deletions client/llb/resolver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ func ResolveDigest(v bool) ImageOption {
})
}

func WithLayerLimit(l int) ImageOption {
return imageOptionFunc(func(ii *ImageInfo) {
ii.layerLimit = &l
})
}

// ImageMetaResolver can resolve image config metadata from a reference
type ImageMetaResolver interface {
ResolveImageConfig(ctx context.Context, ref string, opt ResolveImageConfigOpt) (digest.Digest, []byte, error)
Expand Down
6 changes: 6 additions & 0 deletions client/llb/source.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

addCap(&info.Constraints, pb.CapSourceImageLayerLimit)
}

src := NewSource("docker-image://"+ref, attrs, info.Constraints) // controversial
if err != nil {
src.err = err
Expand Down Expand Up @@ -204,6 +209,7 @@ type ImageInfo struct {
metaResolver ImageMetaResolver
resolveDigest bool
resolveMode ResolveMode
layerLimit *int
RecordType string
}

Expand Down
1 change: 1 addition & 0 deletions solver/pb/attr.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const AttrImageResolveModeDefault = "default"
const AttrImageResolveModeForcePull = "pull"
const AttrImageResolveModePreferLocal = "local"
const AttrImageRecordType = "image.recordtype"
const AttrImageLayerLimit = "image.layerlimit"

const AttrLocalDiffer = "local.differ"
const AttrLocalDifferNone = "none"
Expand Down
12 changes: 10 additions & 2 deletions solver/pb/caps.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ var Caps apicaps.CapList
// considered immutable. After a capability is marked stable it should not be disabled.

const (
CapSourceImage apicaps.CapID = "source.image"
CapSourceImageResolveMode apicaps.CapID = "source.image.resolvemode"
CapSourceImage apicaps.CapID = "source.image"
CapSourceImageResolveMode apicaps.CapID = "source.image.resolvemode"
CapSourceImageLayerLimit apicaps.CapID = "source.image.layerlimit"

CapSourceLocal apicaps.CapID = "source.local"
CapSourceLocalUnique apicaps.CapID = "source.local.unique"
CapSourceLocalSessionID apicaps.CapID = "source.local.sessionid"
Expand Down Expand Up @@ -86,6 +88,12 @@ func init() {
Status: apicaps.CapStatusExperimental,
})

Caps.Init(apicaps.Cap{
ID: CapSourceImageLayerLimit,
Enabled: true,
Status: apicaps.CapStatusExperimental,
})

Caps.Init(apicaps.Cap{
ID: CapSourceLocal,
Enabled: true,
Expand Down
40 changes: 32 additions & 8 deletions source/containerimage/pull.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,17 +152,19 @@ type puller struct {
*pull.Puller
}

func mainManifestKey(ctx context.Context, desc ocispecs.Descriptor, platform ocispecs.Platform) (digest.Digest, error) {
func mainManifestKey(ctx context.Context, desc ocispecs.Descriptor, platform ocispecs.Platform, layerLimit *int) (digest.Digest, error) {
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

}{
Digest: desc.Digest,
OS: platform.OS,
Arch: platform.Architecture,
Variant: platform.Variant,
Limit: layerLimit,
})
if err != nil {
return "", err
Expand All @@ -178,7 +180,7 @@ func (p *puller) CacheKey(ctx context.Context, g session.Group, index int) (cach
progressFactory := progress.FromContext(ctx)

_, err = p.g.Do(ctx, "", func(ctx context.Context) (_ interface{}, err error) {
if p.cacheKeyErr != nil || p.cacheKeyDone == true {
if p.cacheKeyErr != nil || p.cacheKeyDone {
return nil, p.cacheKeyErr
}
defer func() {
Expand All @@ -203,6 +205,13 @@ func (p *puller) CacheKey(ctx context.Context, g session.Group, index int) (cach
return nil, err
}

if ll := p.id.LayerLimit; ll != nil {
if *ll > len(p.manifest.Descriptors) {
return nil, errors.Errorf("layer limit %d is greater than the number of layers in the image %d", *ll, len(p.manifest.Descriptors))
}
p.manifest.Descriptors = p.manifest.Descriptors[:*ll]
}

if len(p.manifest.Descriptors) > 0 {
progressController := &controller.Controller{
WriterFactory: progressFactory,
Expand Down Expand Up @@ -233,7 +242,7 @@ func (p *puller) CacheKey(ctx context.Context, g session.Group, index int) (cach
}

desc := p.manifest.MainManifestDesc
k, err := mainManifestKey(ctx, desc, p.Platform)
k, err := mainManifestKey(ctx, desc, p.Platform, p.id.LayerLimit)
if err != nil {
return nil, err
}
Expand All @@ -243,7 +252,11 @@ func (p *puller) CacheKey(ctx context.Context, g session.Group, index int) (cach
if err != nil {
return nil, err
}
p.configKey = cacheKeyFromConfig(dt).String()
ck, err := cacheKeyFromConfig(dt, p.id.LayerLimit)
if err != nil {
return nil, err
}
p.configKey = ck.String()
p.cacheKeyDone = true
return nil, nil
})
Expand Down Expand Up @@ -336,16 +349,27 @@ func (p *puller) Snapshot(ctx context.Context, g session.Group) (ir cache.Immuta

// cacheKeyFromConfig returns a stable digest from image config. If image config
// is a known oci image we will use chainID of layers.
func cacheKeyFromConfig(dt []byte) digest.Digest {
func cacheKeyFromConfig(dt []byte, layerLimit *int) (digest.Digest, error) {
var img ocispecs.Image
err := json.Unmarshal(dt, &img)
if err != nil {
return digest.FromBytes(dt)
if layerLimit != nil {
return "", errors.Wrap(err, "failed to parse image config")
}
return digest.FromBytes(dt), nil // digest of config
}
if layerLimit != nil {
l := *layerLimit
if len(img.RootFS.DiffIDs) < l {
return "", errors.Errorf("image has %d layers, limit is %d", len(img.RootFS.DiffIDs), l)
}
img.RootFS.DiffIDs = img.RootFS.DiffIDs[:l]
}
if img.RootFS.Type != "layers" || len(img.RootFS.DiffIDs) == 0 {
return ""
return "", nil
}
return identity.ChainID(img.RootFS.DiffIDs)

return identity.ChainID(img.RootFS.DiffIDs), nil
}

func oneOffProgress(ctx context.Context, id string) func(err error) error {
Expand Down
10 changes: 10 additions & 0 deletions source/identifier.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,15 @@ func FromLLB(op *pb.Op_Source, platform *pb.Platform) (Identifier, error) {
return nil, err
}
id.RecordType = rt
case pb.AttrImageLayerLimit:
l, err := strconv.Atoi(v)
if err != nil {
return nil, errors.Wrapf(err, "invalid layer limit %s", v)
}
if l <= 0 {
return nil, errors.Errorf("invalid layer limit %s", v)
}
id.LayerLimit = &l
}
}
}
Expand Down Expand Up @@ -190,6 +199,7 @@ type ImageIdentifier struct {
Platform *ocispecs.Platform
ResolveMode ResolveMode
RecordType client.UsageRecordType
LayerLimit *int
}

func NewImageIdentifier(str string) (*ImageIdentifier, error) {
Expand Down