Skip to content

Commit

Permalink
Put context.Context arguments on almost everything
Browse files Browse the repository at this point in the history
- Network IO paths should react to cancels now.
- File IO paths generally still won't.
- `SystemContext` objects have been renamed to `sys` to leave `ctx`
  available for the stdlib context objects.

Signed-off-by: Mike Lundy <mike@fluffypenguin.org>
  • Loading branch information
novas0x2a committed Apr 7, 2018
1 parent a69b38f commit 369c442
Show file tree
Hide file tree
Showing 78 changed files with 832 additions and 766 deletions.
102 changes: 53 additions & 49 deletions copy/copy.go

Large diffs are not rendered by default.

9 changes: 5 additions & 4 deletions copy/manifest.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package copy

import (
"context"
"strings"

"github.com/containers/image/manifest"
Expand Down Expand Up @@ -41,8 +42,8 @@ func (os *orderedSet) append(s string) {
// Note that the conversion will only happen later, through ic.src.UpdatedImage
// Returns the preferred manifest MIME type (whether we are converting to it or using it unmodified),
// and a list of other possible alternatives, in order.
func (ic *imageCopier) determineManifestConversion(destSupportedManifestMIMETypes []string, forceManifestMIMEType string) (string, []string, error) {
_, srcType, err := ic.src.Manifest()
func (ic *imageCopier) determineManifestConversion(ctx context.Context, destSupportedManifestMIMETypes []string, forceManifestMIMEType string) (string, []string, error) {
_, srcType, err := ic.src.Manifest(ctx)
if err != nil { // This should have been cached?!
return "", nil, errors.Wrap(err, "Error reading manifest")
}
Expand Down Expand Up @@ -111,8 +112,8 @@ func (ic *imageCopier) determineManifestConversion(destSupportedManifestMIMEType
}

// isMultiImage returns true if img is a list of images
func isMultiImage(img types.UnparsedImage) (bool, error) {
_, mt, err := img.Manifest()
func isMultiImage(ctx context.Context, img types.UnparsedImage) (bool, error) {
_, mt, err := img.Manifest(ctx)
if err != nil {
return false, err
}
Expand Down
24 changes: 12 additions & 12 deletions copy/manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ type fakeImageSource string
func (f fakeImageSource) Reference() types.ImageReference {
panic("Unexpected call to a mock function")
}
func (f fakeImageSource) Manifest() ([]byte, string, error) {
func (f fakeImageSource) Manifest(ctx context.Context) ([]byte, string, error) {
if string(f) == "" {
return nil, "", errors.New("Manifest() directed to fail")
}
Expand All @@ -47,28 +47,28 @@ func (f fakeImageSource) Signatures(context.Context) ([][]byte, error) {
func (f fakeImageSource) ConfigInfo() types.BlobInfo {
panic("Unexpected call to a mock function")
}
func (f fakeImageSource) ConfigBlob() ([]byte, error) {
func (f fakeImageSource) ConfigBlob(context.Context) ([]byte, error) {
panic("Unexpected call to a mock function")
}
func (f fakeImageSource) OCIConfig() (*v1.Image, error) {
func (f fakeImageSource) OCIConfig(context.Context) (*v1.Image, error) {
panic("Unexpected call to a mock function")
}
func (f fakeImageSource) LayerInfos() []types.BlobInfo {
panic("Unexpected call to a mock function")
}
func (f fakeImageSource) LayerInfosForCopy() ([]types.BlobInfo, error) {
func (f fakeImageSource) LayerInfosForCopy(ctx context.Context) ([]types.BlobInfo, error) {
panic("Unexpected call to a mock function")
}
func (f fakeImageSource) EmbeddedDockerReferenceConflicts(ref reference.Named) bool {
panic("Unexpected call to a mock function")
}
func (f fakeImageSource) Inspect() (*types.ImageInspectInfo, error) {
func (f fakeImageSource) Inspect(context.Context) (*types.ImageInspectInfo, error) {
panic("Unexpected call to a mock function")
}
func (f fakeImageSource) UpdatedImageNeedsLayerDiffIDs(options types.ManifestUpdateOptions) bool {
panic("Unexpected call to a mock function")
}
func (f fakeImageSource) UpdatedImage(options types.ManifestUpdateOptions) (types.Image, error) {
func (f fakeImageSource) UpdatedImage(ctx context.Context, options types.ManifestUpdateOptions) (types.Image, error) {
panic("Unexpected call to a mock function")
}
func (f fakeImageSource) Size() (int64, error) {
Expand Down Expand Up @@ -144,7 +144,7 @@ func TestDetermineManifestConversion(t *testing.T) {
src: src,
canModifyManifest: true,
}
preferredMIMEType, otherCandidates, err := ic.determineManifestConversion(c.destTypes, "")
preferredMIMEType, otherCandidates, err := ic.determineManifestConversion(context.Background(), c.destTypes, "")
require.NoError(t, err, c.description)
assert.Equal(t, c.expectedUpdate, ic.manifestUpdates.ManifestMIMEType, c.description)
if c.expectedUpdate == "" {
Expand All @@ -163,7 +163,7 @@ func TestDetermineManifestConversion(t *testing.T) {
src: src,
canModifyManifest: false,
}
preferredMIMEType, otherCandidates, err := ic.determineManifestConversion(c.destTypes, "")
preferredMIMEType, otherCandidates, err := ic.determineManifestConversion(context.Background(), c.destTypes, "")
require.NoError(t, err, c.description)
assert.Equal(t, "", ic.manifestUpdates.ManifestMIMEType, c.description)
assert.Equal(t, manifest.NormalizedMIMEType(c.sourceType), preferredMIMEType, c.description)
Expand All @@ -178,7 +178,7 @@ func TestDetermineManifestConversion(t *testing.T) {
src: src,
canModifyManifest: true,
}
preferredMIMEType, otherCandidates, err := ic.determineManifestConversion(c.destTypes, v1.MediaTypeImageManifest)
preferredMIMEType, otherCandidates, err := ic.determineManifestConversion(context.Background(), c.destTypes, v1.MediaTypeImageManifest)
require.NoError(t, err, c.description)
assert.Equal(t, v1.MediaTypeImageManifest, ic.manifestUpdates.ManifestMIMEType, c.description)
assert.Equal(t, v1.MediaTypeImageManifest, preferredMIMEType, c.description)
Expand All @@ -191,7 +191,7 @@ func TestDetermineManifestConversion(t *testing.T) {
src: fakeImageSource(""),
canModifyManifest: true,
}
_, _, err := ic.determineManifestConversion(supportS1S2, "")
_, _, err := ic.determineManifestConversion(context.Background(), supportS1S2, "")
assert.Error(t, err)
}

Expand All @@ -205,13 +205,13 @@ func TestIsMultiImage(t *testing.T) {
{manifest.DockerV2Schema2MediaType, false},
} {
src := fakeImageSource(c.mt)
res, err := isMultiImage(src)
res, err := isMultiImage(context.Background(), src)
require.NoError(t, err)
assert.Equal(t, c.expected, res, c.mt)
}

// Error getting manifest MIME type
src := fakeImageSource("")
_, err := isMultiImage(src)
_, err := isMultiImage(context.Background(), src)
assert.Error(t, err)
}
6 changes: 4 additions & 2 deletions copy/sign_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package copy

import (
"context"
"io/ioutil"
"os"
"testing"
Expand Down Expand Up @@ -42,7 +43,7 @@ func TestCreateSignature(t *testing.T) {
defer os.RemoveAll(tempDir)
dirRef, err := directory.NewReference(tempDir)
require.NoError(t, err)
dirDest, err := dirRef.NewImageDestination(nil)
dirDest, err := dirRef.NewImageDestination(context.Background(), nil)
require.NoError(t, err)
defer dirDest.Close()
c := &copier{
Expand All @@ -55,7 +56,8 @@ func TestCreateSignature(t *testing.T) {
// Set up a docker: reference
dockerRef, err := docker.ParseReference("//busybox")
require.NoError(t, err)
dockerDest, err := dockerRef.NewImageDestination(&types.SystemContext{RegistriesDirPath: "/this/doesnt/exist", DockerPerHostCertDirPath: "/this/doesnt/exist"})
dockerDest, err := dockerRef.NewImageDestination(context.Background(),
&types.SystemContext{RegistriesDirPath: "/this/doesnt/exist", DockerPerHostCertDirPath: "/this/doesnt/exist"})
require.NoError(t, err)
defer dockerDest.Close()
c = &copier{
Expand Down
15 changes: 8 additions & 7 deletions directory/directory_dest.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package directory

import (
"context"
"io"
"io/ioutil"
"os"
Expand Down Expand Up @@ -94,7 +95,7 @@ func (d *dirImageDestination) SupportedManifestMIMETypes() []string {

// SupportsSignatures returns an error (to be displayed to the user) if the destination certainly can't store signatures.
// Note: It is still possible for PutSignatures to fail if SupportsSignatures returns nil.
func (d *dirImageDestination) SupportsSignatures() error {
func (d *dirImageDestination) SupportsSignatures(ctx context.Context) error {
return nil
}

Expand Down Expand Up @@ -122,7 +123,7 @@ func (d *dirImageDestination) MustMatchRuntimeOS() bool {
// WARNING: The contents of stream are being verified on the fly. Until stream.Read() returns io.EOF, the contents of the data SHOULD NOT be available
// to any other readers for download using the supplied digest.
// If stream.Read() at any time, ESPECIALLY at end of input, returns an error, PutBlob MUST 1) fail, and 2) delete any data stored so far.
func (d *dirImageDestination) PutBlob(stream io.Reader, inputInfo types.BlobInfo, isConfig bool) (types.BlobInfo, error) {
func (d *dirImageDestination) PutBlob(ctx context.Context, stream io.Reader, inputInfo types.BlobInfo, isConfig bool) (types.BlobInfo, error) {
blobFile, err := ioutil.TempFile(d.ref.path, "dir-put-blob")
if err != nil {
return types.BlobInfo{}, err
Expand Down Expand Up @@ -164,7 +165,7 @@ func (d *dirImageDestination) PutBlob(stream io.Reader, inputInfo types.BlobInfo
// Unlike PutBlob, the digest can not be empty. If HasBlob returns true, the size of the blob must also be returned.
// If the destination does not contain the blob, or it is unknown, HasBlob ordinarily returns (false, -1, nil);
// it returns a non-nil error only on an unexpected failure.
func (d *dirImageDestination) HasBlob(info types.BlobInfo) (bool, int64, error) {
func (d *dirImageDestination) HasBlob(ctx context.Context, info types.BlobInfo) (bool, int64, error) {
if info.Digest == "" {
return false, -1, errors.Errorf(`"Can not check for a blob with unknown digest`)
}
Expand All @@ -179,19 +180,19 @@ func (d *dirImageDestination) HasBlob(info types.BlobInfo) (bool, int64, error)
return true, finfo.Size(), nil
}

func (d *dirImageDestination) ReapplyBlob(info types.BlobInfo) (types.BlobInfo, error) {
func (d *dirImageDestination) ReapplyBlob(ctx context.Context, info types.BlobInfo) (types.BlobInfo, error) {
return info, nil
}

// PutManifest writes manifest to the destination.
// FIXME? This should also receive a MIME type if known, to differentiate between schema versions.
// If the destination is in principle available, refuses this manifest type (e.g. it does not recognize the schema),
// but may accept a different manifest type, the returned error must be an ManifestTypeRejectedError.
func (d *dirImageDestination) PutManifest(manifest []byte) error {
func (d *dirImageDestination) PutManifest(ctx context.Context, manifest []byte) error {
return ioutil.WriteFile(d.ref.manifestPath(), manifest, 0644)
}

func (d *dirImageDestination) PutSignatures(signatures [][]byte) error {
func (d *dirImageDestination) PutSignatures(ctx context.Context, signatures [][]byte) error {
for i, sig := range signatures {
if err := ioutil.WriteFile(d.ref.signaturePath(i), sig, 0644); err != nil {
return err
Expand All @@ -204,7 +205,7 @@ func (d *dirImageDestination) PutSignatures(signatures [][]byte) error {
// WARNING: This does not have any transactional semantics:
// - Uploaded data MAY be visible to others before Commit() is called
// - Uploaded data MAY be removed or MAY remain around if Close() is called without Commit() (i.e. rollback is allowed but not guaranteed)
func (d *dirImageDestination) Commit() error {
func (d *dirImageDestination) Commit(ctx context.Context) error {
return nil
}

Expand Down
6 changes: 3 additions & 3 deletions directory/directory_src.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func (s *dirImageSource) Close() error {
// It may use a remote (= slow) service.
// If instanceDigest is not nil, it contains a digest of the specific manifest instance to retrieve (when the primary manifest is a manifest list);
// this never happens if the primary manifest is not a manifest list (e.g. if the source never returns manifest lists).
func (s *dirImageSource) GetManifest(instanceDigest *digest.Digest) ([]byte, string, error) {
func (s *dirImageSource) GetManifest(ctx context.Context, instanceDigest *digest.Digest) ([]byte, string, error) {
if instanceDigest != nil {
return nil, "", errors.Errorf(`Getting target manifest not supported by "dir:"`)
}
Expand All @@ -49,7 +49,7 @@ func (s *dirImageSource) GetManifest(instanceDigest *digest.Digest) ([]byte, str
}

// GetBlob returns a stream for the specified blob, and the blob’s size (or -1 if unknown).
func (s *dirImageSource) GetBlob(info types.BlobInfo) (io.ReadCloser, int64, error) {
func (s *dirImageSource) GetBlob(ctx context.Context, info types.BlobInfo) (io.ReadCloser, int64, error) {
r, err := os.Open(s.ref.layerPath(info.Digest))
if err != nil {
return nil, -1, err
Expand Down Expand Up @@ -84,6 +84,6 @@ func (s *dirImageSource) GetSignatures(ctx context.Context, instanceDigest *dige
}

// LayerInfosForCopy() returns updated layer info that should be used when copying, in preference to values in the manifest, if specified.
func (s *dirImageSource) LayerInfosForCopy() ([]types.BlobInfo, error) {
func (s *dirImageSource) LayerInfosForCopy(ctx context.Context) ([]types.BlobInfo, error) {
return nil, nil
}
44 changes: 22 additions & 22 deletions directory/directory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func TestDestinationReference(t *testing.T) {
ref, tmpDir := refToTempDir(t)
defer os.RemoveAll(tmpDir)

dest, err := ref.NewImageDestination(nil)
dest, err := ref.NewImageDestination(context.Background(), nil)
require.NoError(t, err)
defer dest.Close()
ref2 := dest.Reference()
Expand All @@ -31,26 +31,26 @@ func TestGetPutManifest(t *testing.T) {
defer os.RemoveAll(tmpDir)

man := []byte("test-manifest")
dest, err := ref.NewImageDestination(nil)
dest, err := ref.NewImageDestination(context.Background(), nil)
require.NoError(t, err)
defer dest.Close()
err = dest.PutManifest(man)
err = dest.PutManifest(context.Background(), man)
assert.NoError(t, err)
err = dest.Commit()
err = dest.Commit(context.Background())
assert.NoError(t, err)

src, err := ref.NewImageSource(nil)
src, err := ref.NewImageSource(context.Background(), nil)
require.NoError(t, err)
defer src.Close()
m, mt, err := src.GetManifest(nil)
m, mt, err := src.GetManifest(context.Background(), nil)
assert.NoError(t, err)
assert.Equal(t, man, m)
assert.Equal(t, "", mt)

// Non-default instances are not supported
md, err := manifest.Digest(man)
require.NoError(t, err)
_, _, err = src.GetManifest(&md)
_, _, err = src.GetManifest(context.Background(), &md)
assert.Error(t, err)
}

Expand All @@ -59,21 +59,21 @@ func TestGetPutBlob(t *testing.T) {
defer os.RemoveAll(tmpDir)

blob := []byte("test-blob")
dest, err := ref.NewImageDestination(nil)
dest, err := ref.NewImageDestination(context.Background(), nil)
require.NoError(t, err)
defer dest.Close()
assert.Equal(t, types.PreserveOriginal, dest.DesiredLayerCompression())
info, err := dest.PutBlob(bytes.NewReader(blob), types.BlobInfo{Digest: digest.Digest("sha256:digest-test"), Size: int64(9)}, false)
info, err := dest.PutBlob(context.Background(), bytes.NewReader(blob), types.BlobInfo{Digest: digest.Digest("sha256:digest-test"), Size: int64(9)}, false)
assert.NoError(t, err)
err = dest.Commit()
err = dest.Commit(context.Background())
assert.NoError(t, err)
assert.Equal(t, int64(9), info.Size)
assert.Equal(t, digest.FromBytes(blob), info.Digest)

src, err := ref.NewImageSource(nil)
src, err := ref.NewImageSource(context.Background(), nil)
require.NoError(t, err)
defer src.Close()
rc, size, err := src.GetBlob(info)
rc, size, err := src.GetBlob(context.Background(), info)
assert.NoError(t, err)
defer rc.Close()
b, err := ioutil.ReadAll(rc)
Expand Down Expand Up @@ -117,13 +117,13 @@ func TestPutBlobDigestFailure(t *testing.T) {
return 0, errors.Errorf(digestErrorString)
})

dest, err := ref.NewImageDestination(nil)
dest, err := ref.NewImageDestination(context.Background(), nil)
require.NoError(t, err)
defer dest.Close()
_, err = dest.PutBlob(reader, types.BlobInfo{Digest: blobDigest, Size: -1}, false)
_, err = dest.PutBlob(context.Background(), reader, types.BlobInfo{Digest: blobDigest, Size: -1}, false)
assert.Error(t, err)
assert.Contains(t, digestErrorString, err.Error())
err = dest.Commit()
err = dest.Commit(context.Background())
assert.NoError(t, err)

_, err = os.Lstat(blobPath)
Expand All @@ -135,25 +135,25 @@ func TestGetPutSignatures(t *testing.T) {
ref, tmpDir := refToTempDir(t)
defer os.RemoveAll(tmpDir)

dest, err := ref.NewImageDestination(nil)
dest, err := ref.NewImageDestination(context.Background(), nil)
require.NoError(t, err)
defer dest.Close()
man := []byte("test-manifest")
signatures := [][]byte{
[]byte("sig1"),
[]byte("sig2"),
}
err = dest.SupportsSignatures()
err = dest.SupportsSignatures(context.Background())
assert.NoError(t, err)
err = dest.PutManifest(man)
err = dest.PutManifest(context.Background(), man)
require.NoError(t, err)

err = dest.PutSignatures(signatures)
err = dest.PutSignatures(context.Background(), signatures)
assert.NoError(t, err)
err = dest.Commit()
err = dest.Commit(context.Background())
assert.NoError(t, err)

src, err := ref.NewImageSource(nil)
src, err := ref.NewImageSource(context.Background(), nil)
require.NoError(t, err)
defer src.Close()
sigs, err := src.GetSignatures(context.Background(), nil)
Expand All @@ -171,7 +171,7 @@ func TestSourceReference(t *testing.T) {
ref, tmpDir := refToTempDir(t)
defer os.RemoveAll(tmpDir)

src, err := ref.NewImageSource(nil)
src, err := ref.NewImageSource(context.Background(), nil)
require.NoError(t, err)
defer src.Close()
ref2 := src.Reference()
Expand Down
Loading

0 comments on commit 369c442

Please sign in to comment.