From ec9050f7664e1413e8471d33055d6a5ab0c3c6e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 10 Apr 2018 16:02:49 +0200 Subject: [PATCH 1/3] Fix the example in doc.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- doc.go | 44 +++++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/doc.go b/doc.go index 8d59503a75..d9c358cf4e 100644 --- a/doc.go +++ b/doc.go @@ -1,30 +1,32 @@ // Package image provides libraries and commands to interact with containers images. // -// package main +// package main // -// import ( -// "fmt" +// import ( +// "context" +// "fmt" // -// "github.com/containers/image/docker" -// ) +// "github.com/containers/image/docker" +// ) // -// func main() { -// ref, err := docker.ParseReference("//fedora") -// if err != nil { -// panic(err) -// } -// ctx := context.Background() -// img, err := ref.NewImage(ctx) -// if err != nil { -// panic(err) -// } -// defer img.Close() -// b, _, err := img.Manifest(ctx) -// if err != nil { -// panic(err) -// } -// fmt.Printf("%s", string(b)) +// func main() { +// ref, err := docker.ParseReference("//fedora") +// if err != nil { +// panic(err) // } +// ctx := context.Background() +// img, err := ref.NewImage(ctx, nil) +// if err != nil { +// panic(err) +// } +// defer img.Close() +// b, _, err := img.Manifest(ctx) +// if err != nil { +// panic(err) +// } +// fmt.Printf("%s", string(b)) +// } +// // // TODO(runcom) package image From 2f122a276021e4fe97dc801da7a99d97f76dcf16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 10 Apr 2018 16:25:26 +0200 Subject: [PATCH 2/3] Add TODO notes for places which do a lot of work on the local filesystem MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It would be nice if those could be canceled, although so far we seem not to have any users which would benefit. Many would be easily handled with a cancelable variant of io.Copy, but a few might be much more involved. Signed-off-by: Miloslav Trmač --- directory/directory_dest.go | 1 + docker/tarfile/dest.go | 2 ++ docker/tarfile/src.go | 1 + oci/archive/oci_dest.go | 1 + oci/archive/oci_transport.go | 1 + oci/layout/oci_dest.go | 1 + ostree/ostree_dest.go | 3 +++ storage/storage_image.go | 2 ++ tarball/tarball_src.go | 1 + 9 files changed, 13 insertions(+) diff --git a/directory/directory_dest.go b/directory/directory_dest.go index a303393779..a38d7d5cfb 100644 --- a/directory/directory_dest.go +++ b/directory/directory_dest.go @@ -139,6 +139,7 @@ func (d *dirImageDestination) PutBlob(ctx context.Context, stream io.Reader, inp digester := digest.Canonical.Digester() tee := io.TeeReader(stream, digester.Hash()) + // TODO: This can take quite some time, and should ideally be cancellable using ctx.Done(). size, err := io.Copy(blobFile, tee) if err != nil { return types.BlobInfo{}, err diff --git a/docker/tarfile/dest.go b/docker/tarfile/dest.go index 223eb9f10d..2bbaf43206 100644 --- a/docker/tarfile/dest.go +++ b/docker/tarfile/dest.go @@ -86,6 +86,7 @@ func (d *Destination) PutBlob(ctx context.Context, stream io.Reader, inputInfo t digester := digest.Canonical.Digester() tee := io.TeeReader(stream, digester.Hash()) + // TODO: This can take quite some time, and should ideally be cancellable using ctx.Done(). size, err := io.Copy(streamCopy, tee) if err != nil { return types.BlobInfo{}, err @@ -352,6 +353,7 @@ func (d *Destination) sendFile(path string, expectedSize int64, stream io.Reader if err := d.tar.WriteHeader(hdr); err != nil { return err } + // TODO: This can take quite some time, and should ideally be cancellable using a context.Context. size, err := io.Copy(d.tar, stream) if err != nil { return err diff --git a/docker/tarfile/src.go b/docker/tarfile/src.go index 8c2a70de02..2a0d9966e6 100644 --- a/docker/tarfile/src.go +++ b/docker/tarfile/src.go @@ -81,6 +81,7 @@ func NewSourceFromStream(inputStream io.Reader) (*Source, error) { } }() + // TODO: This can take quite some time, and should ideally be cancellable using a context.Context. if _, err := io.Copy(tarCopyFile, inputStream); err != nil { return nil, errors.Wrapf(err, "error copying contents to temporary file %q", tarCopyFile.Name()) } diff --git a/oci/archive/oci_dest.go b/oci/archive/oci_dest.go index 105742f55f..432b310b42 100644 --- a/oci/archive/oci_dest.go +++ b/oci/archive/oci_dest.go @@ -125,6 +125,7 @@ func tarDirectory(src, dst string) error { defer outFile.Close() // copies the contents of the directory to the tar file + // TODO: This can take quite some time, and should ideally be cancellable using a context.Context. _, err = io.Copy(outFile, input) return err diff --git a/oci/archive/oci_transport.go b/oci/archive/oci_transport.go index 90600dee7d..7c1d26ba82 100644 --- a/oci/archive/oci_transport.go +++ b/oci/archive/oci_transport.go @@ -181,6 +181,7 @@ func createUntarTempDir(ref ociArchiveReference) (tempDirOCIRef, error) { } src := ref.resolvedFile dst := tempDirRef.tempDirectory + // TODO: This can take quite some time, and should ideally be cancellable using a context.Context. if err := archive.UntarPath(src, dst); err != nil { if err := tempDirRef.deleteTempDir(); err != nil { return tempDirOCIRef{}, errors.Wrapf(err, "error deleting temp directory %q", tempDirRef.tempDirectory) diff --git a/oci/layout/oci_dest.go b/oci/layout/oci_dest.go index 3a0097bbee..f7a6e7125f 100644 --- a/oci/layout/oci_dest.go +++ b/oci/layout/oci_dest.go @@ -125,6 +125,7 @@ func (d *ociImageDestination) PutBlob(ctx context.Context, stream io.Reader, inp digester := digest.Canonical.Digester() tee := io.TeeReader(stream, digester.Hash()) + // TODO: This can take quite some time, and should ideally be cancellable using ctx.Done(). size, err := io.Copy(blobFile, tee) if err != nil { return types.BlobInfo{}, err diff --git a/ostree/ostree_dest.go b/ostree/ostree_dest.go index 395d956ed9..2e0d7fcff5 100644 --- a/ostree/ostree_dest.go +++ b/ostree/ostree_dest.go @@ -140,6 +140,7 @@ func (d *ostreeImageDestination) PutBlob(ctx context.Context, stream io.Reader, digester := digest.Canonical.Digester() tee := io.TeeReader(stream, digester.Hash()) + // TODO: This can take quite some time, and should ideally be cancellable using ctx.Done(). size, err := io.Copy(blobFile, tee) if err != nil { return types.BlobInfo{}, err @@ -264,6 +265,8 @@ func generateTarSplitMetadata(output *bytes.Buffer, file string) (digest.Digest, } func (d *ostreeImageDestination) importBlob(selinuxHnd *C.struct_selabel_handle, repo *otbuiltin.Repo, blob *blobToImport) error { + // TODO: This can take quite some time, and should ideally be cancellable using a context.Context. + ostreeBranch := fmt.Sprintf("ociimage/%s", blob.Digest.Hex()) destinationPath := filepath.Join(d.tmpDirPath, blob.Digest.Hex(), "root") if err := ensureDirectoryExists(destinationPath); err != nil { diff --git a/storage/storage_image.go b/storage/storage_image.go index d485610166..e38fdb949a 100644 --- a/storage/storage_image.go +++ b/storage/storage_image.go @@ -309,6 +309,7 @@ func (s *storageImageDestination) PutBlob(ctx context.Context, stream io.Reader, return errorBlobInfo, errors.Wrap(err, "error setting up to decompress blob") } // Copy the data to the file. + // TODO: This can take quite some time, and should ideally be cancellable using ctx.Done(). _, err = io.Copy(diffID.Hash(), decompressed) decompressed.Close() if err != nil { @@ -544,6 +545,7 @@ func (s *storageImageDestination) Commit(ctx context.Context) error { return errors.Errorf("error applying blob %q: content not found", blob.Digest) } // Build the new layer using the diff, regardless of where it came from. + // TODO: This can take quite some time, and should ideally be cancellable using ctx.Done(). layer, _, err := s.imageRef.transport.store.PutLayer(id, lastLayer, nil, "", false, nil, diff) if err != nil { return errors.Wrapf(err, "error adding layer with blob %q", blob.Digest) diff --git a/tarball/tarball_src.go b/tarball/tarball_src.go index 4fa5eb94ce..17af60b30c 100644 --- a/tarball/tarball_src.go +++ b/tarball/tarball_src.go @@ -87,6 +87,7 @@ func (r *tarballReference) NewImageSource(ctx context.Context, sys *types.System layerType = imgspecv1.MediaTypeImageLayer uncompressed = nil } + // TODO: This can take quite some time, and should ideally be cancellable using ctx.Done(). n, err := io.Copy(ioutil.Discard, reader) if err != nil { return nil, fmt.Errorf("error reading %q: %v", filename, err) From 8968eb0bc3171f6502e0d040e8fc798201c87f44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 10 Apr 2018 16:54:32 +0200 Subject: [PATCH 3/3] Remove a few unused context.context parameters from private functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- docker/archive/dest.go | 2 +- docker/archive/transport.go | 2 +- docker/docker_client.go | 2 +- docker/docker_image.go | 2 +- docker/docker_image_dest.go | 4 ++-- docker/docker_image_src.go | 6 +++--- docker/docker_transport.go | 4 ++-- oci/layout/oci_dest.go | 2 +- oci/layout/oci_dest_test.go | 2 +- oci/layout/oci_src.go | 2 +- oci/layout/oci_transport.go | 6 +++--- openshift/openshift.go | 2 +- openshift/openshift_transport.go | 4 ++-- ostree/ostree_src.go | 2 +- ostree/ostree_transport.go | 4 ++-- storage/storage_image.go | 2 +- storage/storage_reference.go | 2 +- 17 files changed, 25 insertions(+), 25 deletions(-) diff --git a/docker/archive/dest.go b/docker/archive/dest.go index 7db778db8e..12bbfa7474 100644 --- a/docker/archive/dest.go +++ b/docker/archive/dest.go @@ -16,7 +16,7 @@ type archiveImageDestination struct { writer io.Closer } -func newImageDestination(ctx context.Context, ref archiveReference) (types.ImageDestination, error) { +func newImageDestination(ref archiveReference) (types.ImageDestination, error) { if ref.destinationRef == nil { return nil, errors.Errorf("docker-archive: destination reference not supplied (must be of form :)") } diff --git a/docker/archive/transport.go b/docker/archive/transport.go index baef83e940..169a2b2e2f 100644 --- a/docker/archive/transport.go +++ b/docker/archive/transport.go @@ -148,7 +148,7 @@ func (ref archiveReference) NewImageSource(ctx context.Context, sys *types.Syste // NewImageDestination returns a types.ImageDestination for this reference. // The caller must call .Close() on the returned ImageDestination. func (ref archiveReference) NewImageDestination(ctx context.Context, sys *types.SystemContext) (types.ImageDestination, error) { - return newImageDestination(ctx, ref) + return newImageDestination(ref) } // DeleteImage deletes the named image from the registry, if supported. diff --git a/docker/docker_client.go b/docker/docker_client.go index c5bb95c17e..4a1d421496 100644 --- a/docker/docker_client.go +++ b/docker/docker_client.go @@ -171,7 +171,7 @@ func dockerCertDir(sys *types.SystemContext, hostPort string) (string, error) { // newDockerClientFromRef returns a new dockerClient instance for refHostname (a host a specified in the Docker image reference, not canonicalized to dockerRegistry) // “write” specifies whether the client will be used for "write" access (in particular passed to lookaside.go:toplevelFromSection) -func newDockerClientFromRef(ctx context.Context, sys *types.SystemContext, ref dockerReference, write bool, actions string) (*dockerClient, error) { +func newDockerClientFromRef(sys *types.SystemContext, ref dockerReference, write bool, actions string) (*dockerClient, error) { registry := reference.Domain(ref.ref) username, password, err := config.GetAuthentication(sys, reference.Domain(ref.ref)) if err != nil { diff --git a/docker/docker_image.go b/docker/docker_image.go index e6511046ed..7f8ad858ef 100644 --- a/docker/docker_image.go +++ b/docker/docker_image.go @@ -23,7 +23,7 @@ type Image struct { // a client to the registry hosting the given image. // The caller must call .Close() on the returned Image. func newImage(ctx context.Context, sys *types.SystemContext, ref dockerReference) (types.ImageCloser, error) { - s, err := newImageSource(ctx, sys, ref) + s, err := newImageSource(sys, ref) if err != nil { return nil, err } diff --git a/docker/docker_image_dest.go b/docker/docker_image_dest.go index 7ef32b6386..782a048cec 100644 --- a/docker/docker_image_dest.go +++ b/docker/docker_image_dest.go @@ -33,8 +33,8 @@ type dockerImageDestination struct { } // newImageDestination creates a new ImageDestination for the specified image reference. -func newImageDestination(ctx context.Context, sys *types.SystemContext, ref dockerReference) (types.ImageDestination, error) { - c, err := newDockerClientFromRef(ctx, sys, ref, true, "pull,push") +func newImageDestination(sys *types.SystemContext, ref dockerReference) (types.ImageDestination, error) { + c, err := newDockerClientFromRef(sys, ref, true, "pull,push") if err != nil { return nil, err } diff --git a/docker/docker_image_src.go b/docker/docker_image_src.go index 924f3cbbf5..98f6067e1e 100644 --- a/docker/docker_image_src.go +++ b/docker/docker_image_src.go @@ -30,8 +30,8 @@ type dockerImageSource struct { // newImageSource creates a new ImageSource for the specified image reference. // The caller must call .Close() on the returned ImageSource. -func newImageSource(ctx context.Context, sys *types.SystemContext, ref dockerReference) (*dockerImageSource, error) { - c, err := newDockerClientFromRef(ctx, sys, ref, false, "pull") +func newImageSource(sys *types.SystemContext, ref dockerReference) (*dockerImageSource, error) { + c, err := newDockerClientFromRef(sys, ref, false, "pull") if err != nil { return nil, err } @@ -310,7 +310,7 @@ func (s *dockerImageSource) getSignaturesFromAPIExtension(ctx context.Context, i // deleteImage deletes the named image from the registry, if supported. func deleteImage(ctx context.Context, sys *types.SystemContext, ref dockerReference) error { - c, err := newDockerClientFromRef(ctx, sys, ref, true, "push") + c, err := newDockerClientFromRef(sys, ref, true, "push") if err != nil { return err } diff --git a/docker/docker_transport.go b/docker/docker_transport.go index 24a2337ff7..3c67efb4ac 100644 --- a/docker/docker_transport.go +++ b/docker/docker_transport.go @@ -135,13 +135,13 @@ func (ref dockerReference) NewImage(ctx context.Context, sys *types.SystemContex // NewImageSource returns a types.ImageSource for this reference. // The caller must call .Close() on the returned ImageSource. func (ref dockerReference) NewImageSource(ctx context.Context, sys *types.SystemContext) (types.ImageSource, error) { - return newImageSource(ctx, sys, ref) + return newImageSource(sys, ref) } // NewImageDestination returns a types.ImageDestination for this reference. // The caller must call .Close() on the returned ImageDestination. func (ref dockerReference) NewImageDestination(ctx context.Context, sys *types.SystemContext) (types.ImageDestination, error) { - return newImageDestination(ctx, sys, ref) + return newImageDestination(sys, ref) } // DeleteImage deletes the named image from the registry, if supported. diff --git a/oci/layout/oci_dest.go b/oci/layout/oci_dest.go index f7a6e7125f..386109339f 100644 --- a/oci/layout/oci_dest.go +++ b/oci/layout/oci_dest.go @@ -25,7 +25,7 @@ type ociImageDestination struct { } // newImageDestination returns an ImageDestination for writing to an existing directory. -func newImageDestination(ctx context.Context, sys *types.SystemContext, ref ociReference) (types.ImageDestination, error) { +func newImageDestination(sys *types.SystemContext, ref ociReference) (types.ImageDestination, error) { if ref.image == "" { return nil, errors.Errorf("cannot save image with empty image.ref.name") } diff --git a/oci/layout/oci_dest_test.go b/oci/layout/oci_dest_test.go index 12f763a291..bb79db87d8 100644 --- a/oci/layout/oci_dest_test.go +++ b/oci/layout/oci_dest_test.go @@ -104,7 +104,7 @@ func TestPutManifestTwice(t *testing.T) { } func putTestManifest(t *testing.T, ociRef ociReference, tmpDir string) { - imageDest, err := newImageDestination(context.Background(), nil, ociRef) + imageDest, err := newImageDestination(nil, ociRef) assert.NoError(t, err) data := []byte("abc") diff --git a/oci/layout/oci_src.go b/oci/layout/oci_src.go index ba314bba86..33115c00db 100644 --- a/oci/layout/oci_src.go +++ b/oci/layout/oci_src.go @@ -24,7 +24,7 @@ type ociImageSource struct { } // newImageSource returns an ImageSource for reading from an existing directory. -func newImageSource(ctx context.Context, sys *types.SystemContext, ref ociReference) (types.ImageSource, error) { +func newImageSource(sys *types.SystemContext, ref ociReference) (types.ImageSource, error) { tr := tlsclientconfig.NewTransport() tr.TLSClientConfig = tlsconfig.ServerDefault() diff --git a/oci/layout/oci_transport.go b/oci/layout/oci_transport.go index 72b415f06e..95a9def2ca 100644 --- a/oci/layout/oci_transport.go +++ b/oci/layout/oci_transport.go @@ -146,7 +146,7 @@ func (ref ociReference) PolicyConfigurationNamespaces() []string { // verify that UnparsedImage, and convert it into a real Image via image.FromUnparsedImage. // WARNING: This may not do the right thing for a manifest list, see image.FromSource for details. func (ref ociReference) NewImage(ctx context.Context, sys *types.SystemContext) (types.ImageCloser, error) { - src, err := newImageSource(ctx, sys, ref) + src, err := newImageSource(sys, ref) if err != nil { return nil, err } @@ -219,13 +219,13 @@ func LoadManifestDescriptor(imgRef types.ImageReference) (imgspecv1.Descriptor, // NewImageSource returns a types.ImageSource for this reference. // The caller must call .Close() on the returned ImageSource. func (ref ociReference) NewImageSource(ctx context.Context, sys *types.SystemContext) (types.ImageSource, error) { - return newImageSource(ctx, sys, ref) + return newImageSource(sys, ref) } // NewImageDestination returns a types.ImageDestination for this reference. // The caller must call .Close() on the returned ImageDestination. func (ref ociReference) NewImageDestination(ctx context.Context, sys *types.SystemContext) (types.ImageDestination, error) { - return newImageDestination(ctx, sys, ref) + return newImageDestination(sys, ref) } // DeleteImage deletes the named image from the registry, if supported. diff --git a/openshift/openshift.go b/openshift/openshift.go index 4db4b40f65..3504d8ce14 100644 --- a/openshift/openshift.go +++ b/openshift/openshift.go @@ -170,7 +170,7 @@ type openshiftImageSource struct { // newImageSource creates a new ImageSource for the specified reference. // The caller must call .Close() on the returned ImageSource. -func newImageSource(ctx context.Context, sys *types.SystemContext, ref openshiftReference) (types.ImageSource, error) { +func newImageSource(sys *types.SystemContext, ref openshiftReference) (types.ImageSource, error) { client, err := newOpenshiftClient(ref) if err != nil { return nil, err diff --git a/openshift/openshift_transport.go b/openshift/openshift_transport.go index b8dfee6654..b27867a0d6 100644 --- a/openshift/openshift_transport.go +++ b/openshift/openshift_transport.go @@ -132,7 +132,7 @@ func (ref openshiftReference) PolicyConfigurationNamespaces() []string { // verify that UnparsedImage, and convert it into a real Image via image.FromUnparsedImage. // WARNING: This may not do the right thing for a manifest list, see image.FromSource for details. func (ref openshiftReference) NewImage(ctx context.Context, sys *types.SystemContext) (types.ImageCloser, error) { - src, err := newImageSource(ctx, sys, ref) + src, err := newImageSource(sys, ref) if err != nil { return nil, err } @@ -142,7 +142,7 @@ func (ref openshiftReference) NewImage(ctx context.Context, sys *types.SystemCon // NewImageSource returns a types.ImageSource for this reference. // The caller must call .Close() on the returned ImageSource. func (ref openshiftReference) NewImageSource(ctx context.Context, sys *types.SystemContext) (types.ImageSource, error) { - return newImageSource(ctx, sys, ref) + return newImageSource(sys, ref) } // NewImageDestination returns a types.ImageDestination for this reference. diff --git a/ostree/ostree_src.go b/ostree/ostree_src.go index 1766695140..1f325b2a73 100644 --- a/ostree/ostree_src.go +++ b/ostree/ostree_src.go @@ -42,7 +42,7 @@ type ostreeImageSource struct { } // newImageSource returns an ImageSource for reading from an existing directory. -func newImageSource(ctx context.Context, tmpDir string, ref ostreeReference) (types.ImageSource, error) { +func newImageSource(tmpDir string, ref ostreeReference) (types.ImageSource, error) { return &ostreeImageSource{ref: ref, tmpDir: tmpDir, compressed: nil}, nil } diff --git a/ostree/ostree_transport.go b/ostree/ostree_transport.go index 5d5dd7abd7..c9856530b8 100644 --- a/ostree/ostree_transport.go +++ b/ostree/ostree_transport.go @@ -189,7 +189,7 @@ func (ref ostreeReference) NewImage(ctx context.Context, sys *types.SystemContex } else { tmpDir = sys.OSTreeTmpDirPath } - src, err := newImageSource(ctx, tmpDir, ref) + src, err := newImageSource(tmpDir, ref) if err != nil { return nil, err } @@ -205,7 +205,7 @@ func (ref ostreeReference) NewImageSource(ctx context.Context, sys *types.System } else { tmpDir = sys.OSTreeTmpDirPath } - return newImageSource(ctx, tmpDir, ref) + return newImageSource(tmpDir, ref) } // NewImageDestination returns a types.ImageDestination for this reference. diff --git a/storage/storage_image.go b/storage/storage_image.go index e38fdb949a..e6e759456c 100644 --- a/storage/storage_image.go +++ b/storage/storage_image.go @@ -239,7 +239,7 @@ func (s *storageImageSource) GetSignatures(ctx context.Context, instanceDigest * // newImageDestination sets us up to write a new image, caching blobs in a temporary directory until // it's time to Commit() the image -func newImageDestination(ctx context.Context, imageRef storageReference) (*storageImageDestination, error) { +func newImageDestination(imageRef storageReference) (*storageImageDestination, error) { directory, err := ioutil.TempDir(temporaryDirectoryForBigFiles, "storage") if err != nil { return nil, errors.Wrapf(err, "error creating a temporary directory") diff --git a/storage/storage_reference.go b/storage/storage_reference.go index aafcecaf90..5327169411 100644 --- a/storage/storage_reference.go +++ b/storage/storage_reference.go @@ -206,5 +206,5 @@ func (s storageReference) NewImageSource(ctx context.Context, sys *types.SystemC } func (s storageReference) NewImageDestination(ctx context.Context, sys *types.SystemContext) (types.ImageDestination, error) { - return newImageDestination(ctx, s) + return newImageDestination(s) }