From f771f0cc55bc9497d35fc07504d16ec4b7f5f458 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Min=C3=A1=C5=99?= Date: Fri, 5 Aug 2016 17:12:05 +0200 Subject: [PATCH] Make import image more efficient MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fetch manifest directly by tag without resolving it first. This also works around satellite not setting Content-Type on `GET /v2//manifests/` Signed-off-by: Michal Minář --- pkg/image/importer/client_test.go | 7 ++++++- pkg/image/importer/importer.go | 29 ++++++++++++++++++----------- pkg/image/importer/importer_test.go | 17 ++++++++++------- 3 files changed, 34 insertions(+), 19 deletions(-) diff --git a/pkg/image/importer/client_test.go b/pkg/image/importer/client_test.go index 5c73e6a1c0bc..e473cfe6389f 100644 --- a/pkg/image/importer/client_test.go +++ b/pkg/image/importer/client_test.go @@ -37,7 +37,7 @@ func (r *mockRetriever) Repository(ctx gocontext.Context, registry *url.URL, rep } type mockRepository struct { - repoErr, getErr, getTagErr, tagErr, untagErr, allTagErr, err error + repoErr, getErr, getByTagErr, getTagErr, tagErr, untagErr, allTagErr, err error blobs *mockBlobStore @@ -59,6 +59,11 @@ func (r *mockRepository) Exists(ctx context.Context, dgst digest.Digest) (bool, return false, r.getErr } func (r *mockRepository) Get(ctx context.Context, dgst digest.Digest, options ...distribution.ManifestServiceOption) (distribution.Manifest, error) { + for _, option := range options { + if _, ok := option.(distribution.WithTagOption); ok { + return r.manifest, r.getByTagErr + } + } return r.manifest, r.getErr } func (r *mockRepository) Delete(ctx context.Context, dgst digest.Digest) error { diff --git a/pkg/image/importer/importer.go b/pkg/image/importer/importer.go index aab1a78d799c..a5a0f6b3da13 100644 --- a/pkg/image/importer/importer.go +++ b/pkg/image/importer/importer.go @@ -484,17 +484,24 @@ func (isi *ImageStreamImporter) importRepositoryFromDocker(ctx gocontext.Context continue } limiter.Accept() - desc, err := repo.Tags(ctx).Get(ctx, importTag.Name) - if err != nil { - glog.V(5).Infof("unable to get tag %q for repository %#v: %#v", importTag.Name, repository, err) - importTag.Err = formatRepositoryError(repository, importTag.Name, "", err) - continue - } - manifest, err := s.Get(ctx, desc.Digest) + + manifest, err := s.Get(ctx, "", distribution.WithTag(importTag.Name)) if err != nil { - glog.V(5).Infof("unable to access digest %q for tag %q for repository %#v: %#v", desc.Digest, importTag.Name, repository, err) - importTag.Err = formatRepositoryError(repository, importTag.Name, "", err) - continue + glog.V(5).Infof("unable to get manifest by tag %q for repository %#v: %#v", importTag.Name, repository, err) + // try to resolve the tag and fetch manifest by digest instead + desc, getTagErr := repo.Tags(ctx).Get(ctx, importTag.Name) + if getTagErr != nil { + glog.V(5).Infof("unable to get tag %q for repository %#v: %#v", importTag.Name, repository, getTagErr) + importTag.Err = formatRepositoryError(repository, importTag.Name, "", err) + continue + } + m, getManifestErr := s.Get(ctx, desc.Digest) + if getManifestErr != nil { + glog.V(5).Infof("unable to access digest %q for tag %q for repository %#v: %#v", desc.Digest, importTag.Name, repository, getManifestErr) + importTag.Err = formatRepositoryError(repository, importTag.Name, "", err) + continue + } + manifest = m } if signedManifest, isSchema1 := manifest.(*schema1.SignedManifest); isSchema1 { @@ -502,7 +509,7 @@ func (isi *ImageStreamImporter) importRepositoryFromDocker(ctx gocontext.Context } else if deserializedManifest, isSchema2 := manifest.(*schema2.DeserializedManifest); isSchema2 { imageConfig, err := b.Get(ctx, deserializedManifest.Config.Digest) if err != nil { - glog.V(5).Infof("unable to access image config using digest %q for tag %q for repository %#v: %#v", desc.Digest, importTag.Name, repository, err) + glog.V(5).Infof("unable to access image config using digest %q for tag %q for repository %#v: %#v", deserializedManifest.Config.Digest, importTag.Name, repository, err) importTag.Err = formatRepositoryError(repository, importTag.Name, "", err) continue } diff --git a/pkg/image/importer/importer_test.go b/pkg/image/importer/importer_test.go index 81ca7b27f08b..123850f6d1de 100644 --- a/pkg/image/importer/importer_test.go +++ b/pkg/image/importer/importer_test.go @@ -38,8 +38,9 @@ func TestImport(t *testing.T) { } insecureRetriever := &mockRetriever{ repo: &mockRepository{ - getTagErr: fmt.Errorf("no such tag"), - getErr: fmt.Errorf("no such digest"), + getTagErr: fmt.Errorf("no such tag"), + getByTagErr: fmt.Errorf("no such manifest tag"), + getErr: fmt.Errorf("no such digest"), }, } testCases := []struct { @@ -65,8 +66,9 @@ func TestImport(t *testing.T) { { retriever: &mockRetriever{ repo: &mockRepository{ - getTagErr: fmt.Errorf("no such tag"), - getErr: fmt.Errorf("no such digest"), + getTagErr: fmt.Errorf("no such tag"), + getByTagErr: fmt.Errorf("no such manifest tag"), + getErr: fmt.Errorf("no such digest"), }, }, isi: api.ImageStreamImport{ @@ -80,7 +82,7 @@ func TestImport(t *testing.T) { }, }, expect: func(isi *api.ImageStreamImport, t *testing.T) { - if !expectStatusError(isi.Status.Images[0].Status, "Internal error occurred: no such tag") { + if !expectStatusError(isi.Status.Images[0].Status, "Internal error occurred: no such manifest tag") { t.Errorf("unexpected status: %#v", isi.Status.Images[0].Status) } if !expectStatusError(isi.Status.Images[1].Status, "Internal error occurred: no such digest") { @@ -167,7 +169,8 @@ func TestImport(t *testing.T) { "3.1": "sha256:958608f8ecc1dc62c93b6c610f3a834dae4220c9642e6e8b4e0f2b3ad7cbd238", "abc": "sha256:958608f8ecc1dc62c93b6c610f3a834dae4220c9642e6e8b4e0f2b3ad7cbd238", }, - getTagErr: fmt.Errorf("no such tag"), + getTagErr: fmt.Errorf("no such tag"), + getByTagErr: fmt.Errorf("no such manifest tag"), }, }, isi: api.ImageStreamImport{ @@ -186,7 +189,7 @@ func TestImport(t *testing.T) { } expectedTags := []string{"3", "v2", "v1", "3.1", "abc"} for i, image := range isi.Status.Repository.Images { - if image.Status.Status != unversioned.StatusFailure || image.Status.Message != "Internal error occurred: no such tag" { + if image.Status.Status != unversioned.StatusFailure || image.Status.Message != "Internal error occurred: no such manifest tag" { t.Errorf("unexpected status %d: %#v", i, isi.Status.Repository.Images) } if image.Tag != expectedTags[i] {