Repository: google/go-containerregistry
Pull request #798 (2020-10-24 00:36:42)

Add remote.MultiWrite
This method takes multiple images or manifest lists, and attempts to
efficiently upload all of them in parallel, deduplicating shared layer
and config blobs and avoiding unnecessary auth token exchanges.

Blobs are uploaded in parallel first, then manifests are tagged in order
with as much parallelism as we can support.

This method is currently limited in that:
- all images must be getting uploaded to the same repository (to
  simplify auth and transports), and
- no image may contain streaming layers (to simplify digests), but both
  of these can be removed in future changes.
- blobs are uploaded and manifests are tagged in parallel with a
  constant max parallelism, which should be a user-controlled option.


codecov-io: # [Codecov](https://codecov.io/gh/google/go-containerregistry/pull/798?src=pr&el=h1) Report
> Merging [#798](https://codecov.io/gh/google/go-containerregistry/pull/798?src=pr&el=desc) into [master](https://codecov.io/gh/google/go-containerregistry/commit/1f7f30495bce7f2539be25ab4508a7e55a5f6faf?el=desc) will **decrease** coverage by `0.46%`.
> The diff coverage is `50.63%`.

[![Impacted file tree graph](https://codecov.io/gh/google/go-containerregistry/pull/798/graphs/tree.svg?width=650&height=150&src=pr&token=wG7iiDCdZ4)](https://codecov.io/gh/google/go-containerregistry/pull/798?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #798      +/-   ##
==========================================
- Coverage   75.54%   75.07%   -0.47%     
==========================================
  Files         102      103       +1     
  Lines        4171     4249      +78     
==========================================
+ Hits         3151     3190      +39     
- Misses        566      590      +24     
- Partials      454      469      +15     
```


| [Impacted Files](https://codecov.io/gh/google/go-containerregistry/pull/798?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/v1/remote/multi\_write.go](https://codecov.io/gh/google/go-containerregistry/pull/798/diff?src=pr&el=tree#diff-cGtnL3YxL3JlbW90ZS9tdWx0aV93cml0ZS5nbw==) | `48.68% <48.68%> (ø)` | |
| [pkg/crane/push.go](https://codecov.io/gh/google/go-containerregistry/pull/798/diff?src=pr&el=tree#diff-cGtnL2NyYW5lL3B1c2guZ28=) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/google/go-containerregistry/pull/798?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/google/go-containerregistry/pull/798?src=pr&el=footer). Last update [1f7f304...ca0bd82](https://codecov.io/gh/google/go-containerregistry/pull/798?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

ImJasonH: Talked with Jon yesterday, some of this is more complex than we'd expected, since some registries (e.g., GCR) require that all of a manifest list's referenced manifests already be uploaded, so we can't just blindly copy manifests up at once, we have to do it in an order.

Since a manifest list can reference other manifest lists, etc., we get a tree of them that has to be uploaded, leafs before roots. Each level can be uploaded in parallel, but has to complete before the next level up starts.

I'm adding this check to our test registry in `pkg/registry` so we'll be able to test it's done right.

Patch:
diff --git a/.github/workflows/knative-releasability.yaml b/.github/workflows/knative-releasability.yaml
index fd934ab7..e3e92801 100644
--- a/.github/workflows/knative-releasability.yaml
+++ b/.github/workflows/knative-releasability.yaml
@@ -36,8 +36,8 @@ jobs:
     env:
       #########################################
       #   Update this section each release.   #
-      RELEASE: 'v0.18'
-      SLACK_CHANNEL: 'release-18'
+      RELEASE: 'v0.19'
+      SLACK_CHANNEL: 'release-19'
       #########################################
 
     steps:
diff --git a/.github/workflows/knative-style.yaml b/.github/workflows/knative-style.yaml
index 116c5a13..a57a71a2 100644
--- a/.github/workflows/knative-style.yaml
+++ b/.github/workflows/knative-style.yaml
@@ -121,7 +121,7 @@ jobs:
         if: steps.golangci_configuration.outputs.files_exists == 'true'
         uses: golangci/golangci-lint-action@v2
         with:
-          version: v1.30
+          version: v1.32
 
       - name: misspell
         shell: bash
diff --git a/hack/update-codegen.sh b/hack/update-codegen.sh
index 4ea76931..9032a42d 100755
--- a/hack/update-codegen.sh
+++ b/hack/update-codegen.sh
@@ -30,4 +30,7 @@ export PATH="${GOPATH}/bin:${PATH}"
 go mod tidy
 go mod vendor
 go run $PROJECT_ROOT/cmd/crane/help/main.go --dir=$PROJECT_ROOT/cmd/crane/doc/
-go generate ./...
+
+go run ./vendor/github.com/maxbrunsfeld/counterfeiter/v6/ -o pkg/v1/fake/index.go ${PROJECT_ROOT}/pkg/v1 ImageIndex
+go run ./vendor/github.com/maxbrunsfeld/counterfeiter/v6/ -o pkg/v1/fake/image.go ${PROJECT_ROOT}/pkg/v1 Image
+go run ./vendor/k8s.io/code-generator/cmd/deepcopy-gen -O zz_deepcopy_generated --go-header-file $BOILER_PLATE_FILE -i github.com/google/go-containerregistry/pkg/v1
diff --git a/pkg/crane/push.go b/pkg/crane/push.go
index 52f4fc48..8cddb366 100644
--- a/pkg/crane/push.go
+++ b/pkg/crane/push.go
@@ -36,5 +36,7 @@ func Push(img v1.Image, dst string, opt ...Option) error {
 	if err != nil {
 		return fmt.Errorf("parsing tag %q: %v", dst, err)
 	}
-	return remote.Write(tag, img, o.remote...)
+	return remote.MultiWrite(map[name.Reference]remote.Taggable{
+		tag: img,
+	}, o.remote...)
 }
diff --git a/pkg/registry/manifest.go b/pkg/registry/manifest.go
index 736b7fc9..a22af346 100644
--- a/pkg/registry/manifest.go
+++ b/pkg/registry/manifest.go
@@ -23,6 +23,9 @@ import (
 	"net/http"
 	"strings"
 	"sync"
+
+	v1 "github.com/google/go-containerregistry/pkg/v1"
+	"github.com/google/go-containerregistry/pkg/v1/types"
 )
 
 type manifest struct {
@@ -123,6 +126,33 @@ func (m *manifests) handle(resp http.ResponseWriter, req *http.Request) *regErro
 			blob:        b.Bytes(),
 			contentType: req.Header.Get("Content-Type"),
 		}
+
+		// If the manifest is a manifest list, check that the manifest
+		// list's constituent manifests are already uploaded.
+		// This isn't strictly required by the registry API, but some
+		// registries require this.
+		if mf.contentType == string(types.OCIImageIndex) ||
+			mf.contentType == string(types.DockerManifestList) {
+
+			im, err := v1.ParseIndexManifest(b)
+			if err != nil {
+				return &regError{
+					Status:  http.StatusNotFound,
+					Code:    "MANIFEST_UNKNOWN",
+					Message: err.Error(),
+				}
+			}
+			for _, desc := range im.Manifests {
+				if _, found := m.manifests[repo][desc.Digest.String()]; !found {
+					return &regError{
+						Status:  http.StatusNotFound,
+						Code:    "MANIFEST_UNKNOWN",
+						Message: fmt.Sprintf("Sub-manifest %q not found", desc.Digest),
+					}
+				}
+			}
+		}
+
 		// Allow future references by target (tag) and immutable digest.
 		// See https://docs.docker.com/engine/reference/commandline/pull/#pull-an-image-by-digest-immutable-identifier.
 		m.manifests[repo][target] = mf
diff --git a/pkg/v1/doc.go b/pkg/v1/doc.go
index a686b6e5..7a84736b 100644
--- a/pkg/v1/doc.go
+++ b/pkg/v1/doc.go
@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-//go:generate go run ./../../vendor/k8s.io/code-generator/cmd/deepcopy-gen/ -O zz_deepcopy_generated --go-header-file $BOILER_PLATE_FILE -i .
 // +k8s:deepcopy-gen=package
 
 // Package v1 defines structured types for OCI v1 images
diff --git a/pkg/v1/image.go b/pkg/v1/image.go
index 228fa879..8de9e476 100644
--- a/pkg/v1/image.go
+++ b/pkg/v1/image.go
@@ -19,7 +19,6 @@ import (
 )
 
 // Image defines the interface for interacting with an OCI v1 image.
-//go:generate go run ./../../vendor/github.com/maxbrunsfeld/counterfeiter/v6/ -o fake/image.go . Image
 type Image interface {
 	// Layers returns the ordered collection of filesystem layers that comprise this image.
 	// The order of the list is oldest/base layer first, and most-recent/top layer last.
diff --git a/pkg/v1/index.go b/pkg/v1/index.go
index 063e1b67..8e7bc8eb 100644
--- a/pkg/v1/index.go
+++ b/pkg/v1/index.go
@@ -19,7 +19,6 @@ import (
 )
 
 // ImageIndex defines the interface for interacting with an OCI image index.
-//go:generate go run ./../../vendor/github.com/maxbrunsfeld/counterfeiter/v6/ -o fake/index.go . ImageIndex
 type ImageIndex interface {
 	// MediaType of this image's manifest.
 	MediaType() (types.MediaType, error)
diff --git a/pkg/v1/remote/multi_write.go b/pkg/v1/remote/multi_write.go
new file mode 100644
index 00000000..769fee8c
--- /dev/null
+++ b/pkg/v1/remote/multi_write.go
@@ -0,0 +1,245 @@
+// Copyright 2020 Google LLC All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package remote
+
+import (
+	"fmt"
+	"net/http"
+
+	"github.com/google/go-containerregistry/pkg/name"
+	v1 "github.com/google/go-containerregistry/pkg/v1"
+	"github.com/google/go-containerregistry/pkg/v1/partial"
+	"github.com/google/go-containerregistry/pkg/v1/remote/transport"
+	"github.com/google/go-containerregistry/pkg/v1/types"
+	"golang.org/x/sync/errgroup"
+)
+
+// Parallelism of blob and manifest uploads
+// TODO(jasonhall): Make this an Option.
+const jobs = 4
+
+// MultiWrite writes the given Images or ImageIndexes to the given refs, as
+// efficiently as possible, by deduping shared layer blobs and uploading layers
+// in parallel, then uploading all manifests in parallel.
+//
+// Current limitations:
+// - All refs must share the same repository.
+// - Images cannot consist of stream.Layers.
+func MultiWrite(m map[name.Reference]Taggable, options ...Option) error {
+	// Determine the repository being pushed to; if asked to push to
+	// multiple repositories, give up.
+	var repo, zero name.Repository
+	for ref := range m {
+		if repo == zero {
+			repo = ref.Context()
+		} else if ref.Context() != repo {
+			return fmt.Errorf("MultiWrite can only push to the same repository (saw %q and %q)", repo, ref.Context())
+		}
+	}
+
+	// Collect unique blobs (layers and config blobs).
+	blobs := map[v1.Hash]v1.Layer{}
+	newManifests := []map[name.Reference]Taggable{}
+	// Separate originally requested images and indexes, so we can push images first.
+	images, indexes := map[name.Reference]Taggable{}, map[name.Reference]Taggable{}
+	var err error
+	for ref, i := range m {
+		if img, ok := i.(v1.Image); ok {
+			images[ref] = i
+			if err := addImageBlobs(img, blobs); err != nil {
+				return err
+			}
+			continue
+		}
+		if idx, ok := i.(v1.ImageIndex); ok {
+			indexes[ref] = i
+			newManifests, err = addIndexBlobs(idx, blobs, repo, newManifests, 0)
+			if err != nil {
+				return err
+			}
+			continue
+		}
+		return fmt.Errorf("pushable resource was not Image or ImageIndex: %T", i)
+	}
+
+	o, err := makeOptions(repo, options...)
+	if err != nil {
+		return err
+	}
+	// Determine if any of the layers are Mountable, because if so we need
+	// to request Pull scope too.
+	ls := []v1.Layer{}
+	for _, l := range blobs {
+		ls = append(ls, l)
+	}
+	scopes := scopesForUploadingImage(repo, ls)
+	tr, err := transport.New(repo.Registry, o.auth, o.transport, scopes)
+	if err != nil {
+		return err
+	}
+	w := writer{
+		repo:    repo,
+		client:  &http.Client{Transport: tr},
+		context: o.context,
+	}
+
+	// Upload individual blobs and collect any errors.
+	blobChan := make(chan v1.Layer, 2*jobs)
+	var g errgroup.Group
+	for i := 0; i < jobs; i++ {
+		// Start N workers consuming blobs to upload.
+		g.Go(func() error {
+			for b := range blobChan {
+				if err := w.uploadOne(b); err != nil {
+					return err
+				}
+			}
+			return nil
+		})
+	}
+	go func() {
+		for _, b := range blobs {
+			blobChan <- b
+		}
+		close(blobChan)
+	}()
+	if err := g.Wait(); err != nil {
+		return err
+	}
+
+	commitMany := func(m map[name.Reference]Taggable) error {
+		// With all of the constituent elements uploaded, upload the manifests
+		// to commit the images and indexes, and collect any errors.
+		type task struct {
+			i   Taggable
+			ref name.Reference
+		}
+		taskChan := make(chan task, 2*jobs)
+		for i := 0; i < jobs; i++ {
+			// Start N workers consuming tasks to upload manifests.
+			g.Go(func() error {
+				for t := range taskChan {
+					if err := w.commitImage(t.i, t.ref); err != nil {
+						return err
+					}
+				}
+				return nil
+			})
+		}
+		go func() {
+			for ref, i := range m {
+				taskChan <- task{i, ref}
+			}
+			close(taskChan)
+		}()
+		return g.Wait()
+	}
+	// Push originally requested image manifests. These have no
+	// dependencies.
+	if err := commitMany(images); err != nil {
+		return err
+	}
+	// Push new manifests from lowest levels up.
+	for i := len(newManifests) - 1; i >= 0; i-- {
+		if err := commitMany(newManifests[i]); err != nil {
+			return err
+		}
+	}
+	// Push originally requested index manifests, which might depend on
+	// newly discovered manifests.
+	return commitMany(indexes)
+
+}
+
+// addIndexBlobs adds blobs to the set of blobs we intend to upload, and
+// returns the latest copy of the ordered collection of manifests to upload.
+func addIndexBlobs(idx v1.ImageIndex, blobs map[v1.Hash]v1.Layer, repo name.Repository, newManifests []map[name.Reference]Taggable, lvl int) ([]map[name.Reference]Taggable, error) {
+	if lvl > len(newManifests)-1 {
+		newManifests = append(newManifests, map[name.Reference]Taggable{})
+	}
+
+	im, err := idx.IndexManifest()
+	if err != nil {
+		return nil, err
+	}
+	for _, desc := range im.Manifests {
+		switch desc.MediaType {
+		case types.OCIImageIndex, types.DockerManifestList:
+			idx, err := idx.ImageIndex(desc.Digest)
+			if err != nil {
+				return nil, err
+			}
+			newManifests, err = addIndexBlobs(idx, blobs, repo, newManifests, lvl+1)
+			if err != nil {
+				return nil, err
+			}
+
+			// Also track the sub-index manifest to upload later by digest.
+			newManifests[lvl][repo.Digest(desc.Digest.String())] = idx
+		case types.OCIManifestSchema1, types.DockerManifestSchema2:
+			img, err := idx.Image(desc.Digest)
+			if err != nil {
+				return nil, err
+			}
+			if err := addImageBlobs(img, blobs); err != nil {
+				return nil, err
+			}
+
+			// Also track the sub-image manifest to upload later by digest.
+			newManifests[lvl][repo.Digest(desc.Digest.String())] = img
+		default:
+			return nil, fmt.Errorf("unknown media type: %v", desc.MediaType)
+		}
+	}
+	return newManifests, nil
+}
+
+func addImageBlobs(img v1.Image, blobs map[v1.Hash]v1.Layer) error {
+	ls, err := img.Layers()
+	if err != nil {
+		return err
+	}
+	// Collect all layers.
+	for _, l := range ls {
+		d, err := l.Digest()
+		if err != nil {
+			return err
+		}
+
+		// Ignore foreign layers.
+		mt, err := l.MediaType()
+		if err != nil {
+			return err
+		}
+		if !mt.IsDistributable() {
+			// TODO(jonjohnsonjr): Add "allow-nondistributable-artifacts" option.
+			continue
+		}
+
+		blobs[d] = l
+	}
+
+	// Collect config blob.
+	cl, err := partial.ConfigLayer(img)
+	if err != nil {
+		return err
+	}
+	cld, err := cl.Digest()
+	if err != nil {
+		return err
+	}
+	blobs[cld] = cl
+	return nil
+}


Test patch:
diff --git a/pkg/v1/remote/multi_write_test.go b/pkg/v1/remote/multi_write_test.go
new file mode 100644
index 00000000..fc54fd89
--- /dev/null
+++ b/pkg/v1/remote/multi_write_test.go
@@ -0,0 +1,143 @@
+// Copyright 2020 Google LLC All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package remote
+
+import (
+	"net/http/httptest"
+	"net/url"
+	"testing"
+
+	"github.com/google/go-containerregistry/pkg/name"
+	"github.com/google/go-containerregistry/pkg/registry"
+	"github.com/google/go-containerregistry/pkg/v1/empty"
+	"github.com/google/go-containerregistry/pkg/v1/mutate"
+	"github.com/google/go-containerregistry/pkg/v1/random"
+	"github.com/google/go-containerregistry/pkg/v1/types"
+	"github.com/google/go-containerregistry/pkg/v1/validate"
+)
+
+func TestMultiWrite(t *testing.T) {
+	// Create a random image.
+	img1, err := random.Image(1024, 2)
+	if err != nil {
+		t.Fatal("random.Image:", err)
+	}
+
+	// Create another image that's based on the first.
+	rl, err := random.Layer(1024, types.OCIUncompressedLayer)
+	if err != nil {
+		t.Fatal("random.Layer:", err)
+	}
+	img2, err := mutate.AppendLayers(img1, rl)
+	if err != nil {
+		t.Fatal("mutate.AppendLayers:", err)
+	}
+
+	// Also create a random index of images.
+	subidx, err := random.Index(1024, 2, 3)
+	if err != nil {
+		t.Fatal("random.Index:", err)
+	}
+
+	// Add a sub-sub-index of random images.
+	subsubidx, err := random.Index(1024, 3, 4)
+	if err != nil {
+		t.Fatal("random.Index:", err)
+	}
+	subidx = mutate.AppendManifests(subidx, mutate.IndexAddendum{Add: subsubidx})
+
+	// Create an index containing both images and the index above.
+	idx := mutate.AppendManifests(empty.Index,
+		mutate.IndexAddendum{Add: img1},
+		mutate.IndexAddendum{Add: img2},
+		mutate.IndexAddendum{Add: subidx},
+	)
+
+	// Set up a fake registry.
+	s := httptest.NewServer(registry.New())
+	defer s.Close()
+	u, err := url.Parse(s.URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Write both images and the manifest list.
+	tag1, tag2, tag3 := mustNewTag(t, u.Host+"/repo:tag1"), mustNewTag(t, u.Host+"/repo:tag2"), mustNewTag(t, u.Host+"/repo:tag3")
+	if err := MultiWrite(map[name.Reference]Taggable{
+		tag1: img1,
+		tag2: img2,
+		tag3: idx,
+	}); err != nil {
+		t.Error("Write:", err)
+	}
+
+	// Check that tagged images are present.
+	for _, tag := range []name.Tag{tag1, tag2} {
+		got, err := Image(tag)
+		if err != nil {
+			t.Error(err)
+			continue
+		}
+		if err := validate.Image(got); err != nil {
+			t.Error("Validate() =", err)
+		}
+	}
+
+	// Check that tagged manfest list is present and valid.
+	got, err := Index(tag3)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := validate.Index(got); err != nil {
+		t.Error("Validate() =", err)
+	}
+}
+
+// TestMultiWrite_Deep tests that a deeply nested tree of manifest lists gets
+// pushed in the correct order (i.e., each level in sequence).
+func TestMultiWrite_Deep(t *testing.T) {
+	idx, err := random.Index(1024, 2, 2)
+	if err != nil {
+		t.Fatal("random.Image:", err)
+	}
+	for i := 0; i < 10; i++ {
+		idx = mutate.AppendManifests(idx, mutate.IndexAddendum{Add: idx})
+	}
+
+	// Set up a fake registry.
+	s := httptest.NewServer(registry.New())
+	defer s.Close()
+	u, err := url.Parse(s.URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Write both images and the manifest list.
+	tag := mustNewTag(t, u.Host+"/repo:tag")
+	if err := MultiWrite(map[name.Reference]Taggable{
+		tag: idx,
+	}); err != nil {
+		t.Error("Write:", err)
+	}
+
+	// Check that tagged manfest list is present and valid.
+	got, err := Index(tag)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := validate.Index(got); err != nil {
+		t.Error("Validate() =", err)
+	}
+}