Merge pull request #23327 from dmcgowan/refactor-foreign-layer

Refactor foreign sources to describable interfaces

distribution/pull_v2.go

@@ -133,7 +133,7 @@ type v2LayerDescriptor struct {
 	V2MetadataService *metadata.V2MetadataService
 	tmpFile           *os.File
 	verifier          digest.Verifier
-	foreignSrc        *distribution.Descriptor
+	src               distribution.Descriptor
 }
 
 func (ld *v2LayerDescriptor) Key() string {
@@ -511,14 +511,7 @@ func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *s
 			repo:              p.repo,
 			repoInfo:          p.repoInfo,
 			V2MetadataService: p.V2MetadataService,
-		}
+			src:               d,
-
-		if d.MediaType == schema2.MediaTypeForeignLayer && len(d.URLs) > 0 {
-			if !layer.ForeignSourceSupported() {
-				return "", "", errors.New("foreign layers are not supported on this OS")
-			}
-
-			layerDescriptor.foreignSrc = &d
 		}
 
 		descriptors = append(descriptors, layerDescriptor)

distribution/pull_v2_windows.go

@@ -11,9 +11,9 @@ import (
 	"github.com/docker/distribution"
 	"github.com/docker/distribution/context"
 	"github.com/docker/distribution/manifest/schema1"
+	"github.com/docker/distribution/manifest/schema2"
 	"github.com/docker/distribution/registry/client/transport"
 	"github.com/docker/docker/image"
-	"github.com/docker/docker/layer"
 )
 
 func detectBaseLayer(is image.Store, m *schema1.Manifest, rootFS *image.RootFS) error {
@@ -35,14 +35,17 @@ func detectBaseLayer(is image.Store, m *schema1.Manifest, rootFS *image.RootFS)
 	return fmt.Errorf("Invalid base layer %q", v1img.Parent)
 }
 
-var _ layer.ForeignSourcer = &v2LayerDescriptor{}
+var _ distribution.Describable = &v2LayerDescriptor{}
 
-func (ld *v2LayerDescriptor) ForeignSource() *distribution.Descriptor {
+func (ld *v2LayerDescriptor) Descriptor() distribution.Descriptor {
-	return ld.foreignSrc
+	if ld.src.MediaType == schema2.MediaTypeForeignLayer && len(ld.src.URLs) > 0 {
+		return ld.src
+	}
+	return distribution.Descriptor{}
 }
 
 func (ld *v2LayerDescriptor) open(ctx context.Context) (distribution.ReadSeekCloser, error) {
-	if ld.foreignSrc == nil {
+	if len(ld.src.URLs) == 0 {
 		blobs := ld.repo.Blobs(ctx)
 		return blobs.Open(ctx, ld.digest)
 	}
@@ -53,7 +56,7 @@ func (ld *v2LayerDescriptor) open(ctx context.Context) (distribution.ReadSeekClo
 	)
 
 	// Find the first URL that results in a 200 result code.
-	for _, url := range ld.foreignSrc.URLs {
+	for _, url := range ld.src.URLs {
 		rsc = transport.NewHTTPReadSeeker(http.DefaultClient, url, nil)
 		_, err = rsc.Seek(0, os.SEEK_SET)
 		if err == nil {

distribution/push_v2.go

@@ -240,10 +240,10 @@ func (pd *v2PushDescriptor) DiffID() layer.DiffID {
 }
 
 func (pd *v2PushDescriptor) Upload(ctx context.Context, progressOutput progress.Output) (distribution.Descriptor, error) {
-	if fs, ok := pd.layer.(layer.ForeignSourcer); ok {
+	if fs, ok := pd.layer.(distribution.Describable); ok {
-		if d := fs.ForeignSource(); d != nil {
+		if d := fs.Descriptor(); len(d.URLs) > 0 {
 			progress.Update(progressOutput, pd.ID(), "Skipped foreign layer")
-			return *d, nil
+			return d, nil
 		}
 	}
 

distribution/xfer/download.go

@@ -319,11 +319,15 @@ func (ldm *LayerDownloadManager) makeDownloadFunc(descriptor DownloadDescriptor,
 				return
 			}
 
-			var src *distribution.Descriptor
+			var src distribution.Descriptor
-			if fs, ok := descriptor.(layer.ForeignSourcer); ok {
+			if fs, ok := descriptor.(distribution.Describable); ok {
-				src = fs.ForeignSource()
+				src = fs.Descriptor()
+			}
+			if ds, ok := d.layerStore.(layer.DescribableStore); ok {
+				d.layer, err = ds.RegisterWithDescriptor(inflatedLayerData, parentLayer, src)
+			} else {
+				d.layer, err = d.layerStore.Register(inflatedLayerData, parentLayer)
 			}
-			d.layer, err = d.layerStore.RegisterForeign(inflatedLayerData, parentLayer, src)
 			if err != nil {
 				select {
 				case <-d.Transfer.Context().Done():
@@ -414,11 +418,15 @@ func (ldm *LayerDownloadManager) makeDownloadFuncFromDownload(descriptor Downloa
 			}
 			defer layerReader.Close()
 
-			var src *distribution.Descriptor
+			var src distribution.Descriptor
-			if fs, ok := l.(layer.ForeignSourcer); ok {
+			if fs, ok := l.(distribution.Describable); ok {
-				src = fs.ForeignSource()
+				src = fs.Descriptor()
+			}
+			if ds, ok := d.layerStore.(layer.DescribableStore); ok {
+				d.layer, err = ds.RegisterWithDescriptor(layerReader, parentLayer, src)
+			} else {
+				d.layer, err = d.layerStore.Register(layerReader, parentLayer)
 			}
-			d.layer, err = d.layerStore.RegisterForeign(layerReader, parentLayer, src)
 			if err != nil {
 				d.err = fmt.Errorf("failed to register layer: %v", err)
 				return

distribution/xfer/download_test.go

@@ -72,10 +72,10 @@ func createChainIDFromParent(parent layer.ChainID, dgsts ...layer.DiffID) layer.
 }
 
 func (ls *mockLayerStore) Register(reader io.Reader, parentID layer.ChainID) (layer.Layer, error) {
-	return ls.RegisterForeign(reader, parentID, nil)
+	return ls.RegisterWithDescriptor(reader, parentID, distribution.Descriptor{})
 }
 
-func (ls *mockLayerStore) RegisterForeign(reader io.Reader, parentID layer.ChainID, _ *distribution.Descriptor) (layer.Layer, error) {
+func (ls *mockLayerStore) RegisterWithDescriptor(reader io.Reader, parentID layer.ChainID, _ distribution.Descriptor) (layer.Layer, error) {
 	var (
 		parent layer.Layer
 		err    error

image/tarexport/load.go

@@ -64,10 +64,6 @@ func (l *tarexporter) Load(inTar io.ReadCloser, outStream io.Writer, quiet bool)
 	var parentLinks []parentLink
 
 	for _, m := range manifest {
-		if m.LayerSources != nil && !layer.ForeignSourceSupported() {
-			return fmt.Errorf("invalid manifest, foreign layers not supported on this operating system")
-		}
-
 		configPath, err := safePath(tmpDir, m.Config)
 		if err != nil {
 			return err
@@ -156,7 +152,7 @@ func (l *tarexporter) setParentID(id, parentID image.ID) error {
 	return l.is.SetParent(id, parentID)
 }
 
-func (l *tarexporter) loadLayer(filename string, rootFS image.RootFS, id string, foreignSrc *distribution.Descriptor, progressOutput progress.Output) (layer.Layer, error) {
+func (l *tarexporter) loadLayer(filename string, rootFS image.RootFS, id string, foreignSrc distribution.Descriptor, progressOutput progress.Output) (layer.Layer, error) {
 	rawTar, err := os.Open(filename)
 	if err != nil {
 		logrus.Debugf("Error reading embedded tar: %v", err)
@@ -179,9 +175,17 @@ func (l *tarexporter) loadLayer(filename string, rootFS image.RootFS, id string,
 
 		progressReader := progress.NewProgressReader(inflatedLayerData, progressOutput, fileInfo.Size(), stringid.TruncateID(id), "Loading layer")
 
-		return l.ls.RegisterForeign(progressReader, rootFS.ChainID(), foreignSrc)
+		if ds, ok := l.ls.(layer.DescribableStore); ok {
+			return ds.RegisterWithDescriptor(progressReader, rootFS.ChainID(), foreignSrc)
+		}
+		return l.ls.Register(progressReader, rootFS.ChainID())
+
 	}
-	return l.ls.RegisterForeign(inflatedLayerData, rootFS.ChainID(), foreignSrc)
+
+	if ds, ok := l.ls.(layer.DescribableStore); ok {
+		return ds.RegisterWithDescriptor(inflatedLayerData, rootFS.ChainID(), foreignSrc)
+	}
+	return l.ls.Register(inflatedLayerData, rootFS.ChainID())
 }
 
 func (l *tarexporter) setLoadedTag(ref reference.NamedTagged, imgID image.ID, outStream io.Writer) error {
@@ -303,7 +307,7 @@ func (l *tarexporter) legacyLoadImage(oldID, sourceDir string, loadedMap map[str
 	if err != nil {
 		return err
 	}
-	newLayer, err := l.loadLayer(layerPath, *rootFS, oldID, nil, progressOutput)
+	newLayer, err := l.loadLayer(layerPath, *rootFS, oldID, distribution.Descriptor{}, progressOutput)
 	if err != nil {
 		return err
 	}

image/tarexport/save.go

@@ -216,7 +216,7 @@ func (s *saveSession) save(outStream io.Writer) error {
 	return nil
 }
 
-func (s *saveSession) saveImage(id image.ID) (map[layer.DiffID]*distribution.Descriptor, error) {
+func (s *saveSession) saveImage(id image.ID) (map[layer.DiffID]distribution.Descriptor, error) {
 	img, err := s.is.Get(id)
 	if err != nil {
 		return nil, err
@@ -228,7 +228,7 @@ func (s *saveSession) saveImage(id image.ID) (map[layer.DiffID]*distribution.Des
 
 	var parent digest.Digest
 	var layers []string
-	var foreignSrcs map[layer.DiffID]*distribution.Descriptor
+	var foreignSrcs map[layer.DiffID]distribution.Descriptor
 	for i := range img.RootFS.DiffIDs {
 		v1Img := image.V1Image{}
 		if i == len(img.RootFS.DiffIDs)-1 {
@@ -252,9 +252,9 @@ func (s *saveSession) saveImage(id image.ID) (map[layer.DiffID]*distribution.Des
 		}
 		layers = append(layers, v1Img.ID)
 		parent = v1ID
-		if src != nil {
+		if src.Digest != "" {
 			if foreignSrcs == nil {
-				foreignSrcs = make(map[layer.DiffID]*distribution.Descriptor)
+				foreignSrcs = make(map[layer.DiffID]distribution.Descriptor)
 			}
 			foreignSrcs[img.RootFS.DiffIDs[i]] = src
 		}
@@ -272,65 +272,65 @@ func (s *saveSession) saveImage(id image.ID) (map[layer.DiffID]*distribution.Des
 	return foreignSrcs, nil
 }
 
-func (s *saveSession) saveLayer(id layer.ChainID, legacyImg image.V1Image, createdTime time.Time) (*distribution.Descriptor, error) {
+func (s *saveSession) saveLayer(id layer.ChainID, legacyImg image.V1Image, createdTime time.Time) (distribution.Descriptor, error) {
 	if _, exists := s.savedLayers[legacyImg.ID]; exists {
-		return nil, nil
+		return distribution.Descriptor{}, nil
 	}
 
 	outDir := filepath.Join(s.outDir, legacyImg.ID)
 	if err := os.Mkdir(outDir, 0755); err != nil {
-		return nil, err
+		return distribution.Descriptor{}, err
 	}
 
 	// todo: why is this version file here?
 	if err := ioutil.WriteFile(filepath.Join(outDir, legacyVersionFileName), []byte("1.0"), 0644); err != nil {
-		return nil, err
+		return distribution.Descriptor{}, err
 	}
 
 	imageConfig, err := json.Marshal(legacyImg)
 	if err != nil {
-		return nil, err
+		return distribution.Descriptor{}, err
 	}
 
 	if err := ioutil.WriteFile(filepath.Join(outDir, legacyConfigFileName), imageConfig, 0644); err != nil {
-		return nil, err
+		return distribution.Descriptor{}, err
 	}
 
 	// serialize filesystem
 	tarFile, err := os.Create(filepath.Join(outDir, legacyLayerFileName))
 	if err != nil {
-		return nil, err
+		return distribution.Descriptor{}, err
 	}
 	defer tarFile.Close()
 
 	l, err := s.ls.Get(id)
 	if err != nil {
-		return nil, err
+		return distribution.Descriptor{}, err
 	}
 	defer layer.ReleaseAndLog(s.ls, l)
 
 	arch, err := l.TarStream()
 	if err != nil {
-		return nil, err
+		return distribution.Descriptor{}, err
 	}
 	defer arch.Close()
 
 	if _, err := io.Copy(tarFile, arch); err != nil {
-		return nil, err
+		return distribution.Descriptor{}, err
 	}
 
 	for _, fname := range []string{"", legacyVersionFileName, legacyConfigFileName, legacyLayerFileName} {
 		// todo: maybe save layer created timestamp?
 		if err := system.Chtimes(filepath.Join(outDir, fname), createdTime, createdTime); err != nil {
-			return nil, err
+			return distribution.Descriptor{}, err
 		}
 	}
 
 	s.savedLayers[legacyImg.ID] = struct{}{}
 
-	var src *distribution.Descriptor
+	var src distribution.Descriptor
-	if fs, ok := l.(layer.ForeignSourcer); ok {
+	if fs, ok := l.(distribution.Describable); ok {
-		src = fs.ForeignSource()
+		src = fs.Descriptor()
 	}
 	return src, nil
 }

image/tarexport/tarexport.go

@@ -19,8 +19,8 @@ type manifestItem struct {
 	Config       string
 	RepoTags     []string
 	Layers       []string
-	Parent       image.ID                                  `json:",omitempty"`
+	Parent       image.ID                                 `json:",omitempty"`
-	LayerSources map[layer.DiffID]*distribution.Descriptor `json:",omitempty"`
+	LayerSources map[layer.DiffID]distribution.Descriptor `json:",omitempty"`
 }
 
 type tarexporter struct {

layer/filestore.go

@@ -26,9 +26,6 @@ var (
 		// digest.SHA384, // Currently not used
 		// digest.SHA512, // Currently not used
 	}
-
-	// ErrNoForeignSource is returned when no foreign source is set for a layer.
-	ErrNoForeignSource = errors.New("layer does not have a foreign source")
 )
 
 type fileMetadataStore struct {
@@ -103,7 +100,7 @@ func (fm *fileMetadataTransaction) SetCacheID(cacheID string) error {
 	return ioutil.WriteFile(filepath.Join(fm.root, "cache-id"), []byte(cacheID), 0644)
 }
 
-func (fm *fileMetadataTransaction) SetForeignSource(ref distribution.Descriptor) error {
+func (fm *fileMetadataTransaction) SetDescriptor(ref distribution.Descriptor) error {
 	jsonRef, err := json.Marshal(ref)
 	if err != nil {
 		return err
@@ -204,11 +201,12 @@ func (fms *fileMetadataStore) GetCacheID(layer ChainID) (string, error) {
 	return content, nil
 }
 
-func (fms *fileMetadataStore) GetForeignSource(layer ChainID) (distribution.Descriptor, error) {
+func (fms *fileMetadataStore) GetDescriptor(layer ChainID) (distribution.Descriptor, error) {
 	content, err := ioutil.ReadFile(fms.getLayerFilename(layer, "descriptor.json"))
 	if err != nil {
 		if os.IsNotExist(err) {
-			return distribution.Descriptor{}, ErrNoForeignSource
+			// only return empty descriptor to represent what is stored
+			return distribution.Descriptor{}, nil
 		}
 		return distribution.Descriptor{}, err
 	}

layer/layer.go

@@ -108,14 +108,6 @@ type Layer interface {
 	Metadata() (map[string]string, error)
 }
 
-// ForeignSourcer is an interface used to describe the source of layers
-// and objects representing layers, when the source is a foreign URL.
-type ForeignSourcer interface {
-	// ForeignSource returns the descriptor for this layer if it is
-	// a foreign layer, or nil for ordinary layers.
-	ForeignSource() *distribution.Descriptor
-}
-
 // RWLayer represents a layer which is
 // read and writable
 type RWLayer interface {
@@ -177,7 +169,6 @@ type MountInit func(root string) error
 // read-only and read-write layers.
 type Store interface {
 	Register(io.Reader, ChainID) (Layer, error)
-	RegisterForeign(io.Reader, ChainID, *distribution.Descriptor) (Layer, error)
 	Get(ChainID) (Layer, error)
 	Release(Layer) ([]Metadata, error)
 
@@ -191,6 +182,12 @@ type Store interface {
 	DriverName() string
 }
 
+// DescribableStore represents a layer store capable of storing
+// descriptors for layers.
+type DescribableStore interface {
+	RegisterWithDescriptor(io.Reader, ChainID, distribution.Descriptor) (Layer, error)
+}
+
 // MetadataTransaction represents functions for setting layer metadata
 // with a single transaction.
 type MetadataTransaction interface {
@@ -198,7 +195,7 @@ type MetadataTransaction interface {
 	SetParent(parent ChainID) error
 	SetDiffID(DiffID) error
 	SetCacheID(string) error
-	SetForeignSource(distribution.Descriptor) error
+	SetDescriptor(distribution.Descriptor) error
 	TarSplitWriter(compressInput bool) (io.WriteCloser, error)
 
 	Commit(ChainID) error
@@ -218,7 +215,7 @@ type MetadataStore interface {
 	GetParent(ChainID) (ChainID, error)
 	GetDiffID(ChainID) (DiffID, error)
 	GetCacheID(ChainID) (string, error)
-	GetForeignSource(ChainID) (distribution.Descriptor, error)
+	GetDescriptor(ChainID) (distribution.Descriptor, error)
 	TarSplitReader(ChainID) (io.ReadCloser, error)
 
 	SetMountID(string, string) error

layer/layer_store.go

@@ -129,6 +129,11 @@ func (ls *layerStore) loadLayer(layer ChainID) (*roLayer, error) {
 		return nil, fmt.Errorf("failed to get parent for %s: %s", layer, err)
 	}
 
+	descriptor, err := ls.store.GetDescriptor(layer)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get descriptor for %s: %s", layer, err)
+	}
+
 	cl = &roLayer{
 		chainID:    layer,
 		diffID:     diff,
@@ -136,13 +141,7 @@ func (ls *layerStore) loadLayer(layer ChainID) (*roLayer, error) {
 		cacheID:    cacheID,
 		layerStore: ls,
 		references: map[Layer]struct{}{},
-	}
+		descriptor: descriptor,
-
-	foreignSrc, err := ls.store.GetForeignSource(layer)
-	if err == nil {
-		cl.foreignSrc = &foreignSrc
-	} else if err != ErrNoForeignSource {
-		return nil, fmt.Errorf("failed to get foreign reference for %s: %s", layer, err)
 	}
 
 	if parent != "" {
@@ -236,10 +235,10 @@ func (ls *layerStore) applyTar(tx MetadataTransaction, ts io.Reader, parent stri
 }
 
 func (ls *layerStore) Register(ts io.Reader, parent ChainID) (Layer, error) {
-	return ls.RegisterForeign(ts, parent, nil)
+	return ls.registerWithDescriptor(ts, parent, distribution.Descriptor{})
 }
 
-func (ls *layerStore) RegisterForeign(ts io.Reader, parent ChainID, foreignSrc *distribution.Descriptor) (Layer, error) {
+func (ls *layerStore) registerWithDescriptor(ts io.Reader, parent ChainID, descriptor distribution.Descriptor) (Layer, error) {
 	// err is used to hold the error which will always trigger
 	// cleanup of creates sources but may not be an error returned
 	// to the caller (already exists).
@@ -270,10 +269,10 @@ func (ls *layerStore) RegisterForeign(ts io.Reader, parent ChainID, foreignSrc *
 	layer := &roLayer{
 		parent:         p,
 		cacheID:        stringid.GenerateRandomID(),
-		foreignSrc:     foreignSrc,
 		referenceCount: 1,
 		layerStore:     ls,
 		references:     map[Layer]struct{}{},
+		descriptor:     descriptor,
 	}
 
 	if err = ls.driver.Create(layer.cacheID, pid, "", nil); err != nil {

layer/layer_store_windows.go

@@ -0,0 +1,11 @@
+package layer
+
+import (
+	"io"
+
+	"github.com/docker/distribution"
+)
+
+func (ls *layerStore) RegisterWithDescriptor(ts io.Reader, parent ChainID, descriptor distribution.Descriptor) (Layer, error) {
+	return ls.registerWithDescriptor(ts, parent, descriptor)
+}

layer/layer_unix.go

@@ -7,9 +7,3 @@ import "github.com/docker/docker/pkg/stringid"
 func (ls *layerStore) mountID(name string) string {
 	return stringid.GenerateRandomID()
 }
-
-// ForeignSourceSupported returns whether layers downloaded from foreign sources are
-// supported in this daemon.
-func ForeignSourceSupported() bool {
-	return false
-}

layer/layer_windows.go

@@ -96,9 +96,3 @@ func (ls *layerStore) mountID(name string) string {
 func (ls *layerStore) GraphDriver() graphdriver.Driver {
 	return ls.driver
 }
-
-// ForeignSourceSupported returns whether layers downloaded from foreign sources are
-// supported in this daemon.
-func ForeignSourceSupported() bool {
-	return true
-}

layer/ro_layer.go

@@ -15,7 +15,7 @@ type roLayer struct {
 	cacheID    string
 	size       int64
 	layerStore *layerStore
-	foreignSrc *distribution.Descriptor
+	descriptor distribution.Descriptor
 
 	referenceCount int
 	references     map[Layer]struct{}
@@ -120,13 +120,14 @@ func storeLayer(tx MetadataTransaction, layer *roLayer) error {
 	if err := tx.SetCacheID(layer.cacheID); err != nil {
 		return err
 	}
-	if layer.parent != nil {
+	// Do not store empty descriptors
-		if err := tx.SetParent(layer.parent.chainID); err != nil {
+	if layer.descriptor.Digest != "" {
+		if err := tx.SetDescriptor(layer.descriptor); err != nil {
 			return err
 		}
 	}
-	if layer.foreignSrc != nil {
+	if layer.parent != nil {
-		if err := tx.SetForeignSource(*layer.foreignSrc); err != nil {
+		if err := tx.SetParent(layer.parent.chainID); err != nil {
 			return err
 		}
 	}

layer/ro_layer_windows.go

@@ -2,8 +2,8 @@ package layer
 
 import "github.com/docker/distribution"
 
-var _ ForeignSourcer = &roLayer{}
+var _ distribution.Describable = &roLayer{}
 
-func (rl *roLayer) ForeignSource() *distribution.Descriptor {
+func (rl *roLayer) Descriptor() distribution.Descriptor {
-	return rl.foreignSrc
+	return rl.descriptor
 }