Skip to content
This repository has been archived by the owner on Oct 13, 2023. It is now read-only.

Commit

Permalink
LCOW: Pass platform through into layer store
Browse files Browse the repository at this point in the history
Signed-off-by: John Howard <jhoward@microsoft.com>
Upstream-commit: 42c5c1a
Component: engine
  • Loading branch information
John Howard committed Jun 20, 2017
1 parent 04857f4 commit 8508f49
Show file tree
Hide file tree
Showing 18 changed files with 125 additions and 74 deletions.
7 changes: 6 additions & 1 deletion components/engine/daemon/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ func (daemon *Daemon) Commit(name string, c *backend.ContainerCommitConfig) (str
}()

var parent *image.Image
os := runtime.GOOS
if container.ImageID == "" {
parent = new(image.Image)
parent.RootFS = image.NewRootFS()
Expand All @@ -168,9 +169,13 @@ func (daemon *Daemon) Commit(name string, c *backend.ContainerCommitConfig) (str
if err != nil {
return "", err
}
// To support LCOW, Windows needs to pass the platform in when registering the layer in the store
if runtime.GOOS == "windows" {
os = parent.OS
}
}

l, err := daemon.layerStore.Register(rwTar, parent.RootFS.ChainID())
l, err := daemon.layerStore.Register(rwTar, parent.RootFS.ChainID(), layer.Platform(os))
if err != nil {
return "", err
}
Expand Down
9 changes: 8 additions & 1 deletion components/engine/daemon/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package daemon
import (
"encoding/json"
"fmt"
"runtime"
"sort"
"time"

Expand Down Expand Up @@ -272,7 +273,13 @@ func (daemon *Daemon) SquashImage(id, parent string) (string, error) {
}
defer ts.Close()

newL, err := daemon.layerStore.Register(ts, parentChainID)
// To support LCOW, Windows needs to pass the platform into the store when registering the layer.
platform := layer.Platform("")
if runtime.GOOS == "windows" {
platform = l.Platform()
}

newL, err := daemon.layerStore.Register(ts, parentChainID, platform)
if err != nil {
return "", errors.Wrap(err, "error registering layer")
}
Expand Down
7 changes: 6 additions & 1 deletion components/engine/daemon/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,12 @@ func (daemon *Daemon) ImportImage(src string, repository, tag string, msg string
return err
}
// TODO: support windows baselayer?
l, err := daemon.layerStore.Register(inflatedLayerData, "")
// TODO: LCOW support @jhowardmsft. For now, pass in a null platform when
// registering the layer. Windows doesn't currently support import,
// but for Linux images, there's no reason it couldn't. However it
// would need another CLI flag as there's no meta-data indicating
// the OS of the thing being imported.
l, err := daemon.layerStore.Register(inflatedLayerData, "", "")
if err != nil {
return err
}
Expand Down
18 changes: 11 additions & 7 deletions components/engine/distribution/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ type ImagePushConfig struct {
type ImageConfigStore interface {
Put([]byte) (digest.Digest, error)
Get(digest.Digest) ([]byte, error)
RootFSFromConfig([]byte) (*image.RootFS, error)
RootFSAndPlatformFromConfig([]byte) (*image.RootFS, layer.Platform, error)
}

// PushLayerProvider provides layers to be pushed by ChainID.
Expand All @@ -109,7 +109,7 @@ type RootFSDownloadManager interface {
// returns the final rootfs.
// Given progress output to track download progress
// Returns function to release download resources
Download(ctx context.Context, initialRootFS image.RootFS, layers []xfer.DownloadDescriptor, progressOutput progress.Output) (image.RootFS, func(), error)
Download(ctx context.Context, initialRootFS image.RootFS, platform layer.Platform, layers []xfer.DownloadDescriptor, progressOutput progress.Output) (image.RootFS, func(), error)
}

type imageConfigStore struct {
Expand Down Expand Up @@ -137,21 +137,25 @@ func (s *imageConfigStore) Get(d digest.Digest) ([]byte, error) {
return img.RawJSON(), nil
}

func (s *imageConfigStore) RootFSFromConfig(c []byte) (*image.RootFS, error) {
func (s *imageConfigStore) RootFSAndPlatformFromConfig(c []byte) (*image.RootFS, layer.Platform, error) {
var unmarshalledConfig image.Image
if err := json.Unmarshal(c, &unmarshalledConfig); err != nil {
return nil, err
return nil, "", err
}

// fail immediately on Windows when downloading a non-Windows image
// and vice versa. Exception on Windows if Linux Containers are enabled.
if runtime.GOOS == "windows" && unmarshalledConfig.OS == "linux" && !system.LCOWSupported() {
return nil, fmt.Errorf("image operating system %q cannot be used on this platform", unmarshalledConfig.OS)
return nil, "", fmt.Errorf("image operating system %q cannot be used on this platform", unmarshalledConfig.OS)
} else if runtime.GOOS != "windows" && unmarshalledConfig.OS == "windows" {
return nil, fmt.Errorf("image operating system %q cannot be used on this platform", unmarshalledConfig.OS)
return nil, "", fmt.Errorf("image operating system %q cannot be used on this platform", unmarshalledConfig.OS)
}

return unmarshalledConfig.RootFS, nil
platform := ""
if runtime.GOOS == "windows" {
platform = unmarshalledConfig.OS
}
return unmarshalledConfig.RootFS, layer.Platform(platform), nil
}

type storeLayerProvider struct {
Expand Down
2 changes: 1 addition & 1 deletion components/engine/distribution/pull_v1.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ func (p *v1Puller) pullImage(ctx context.Context, v1ID, endpoint string, localNa
}

rootFS := image.NewRootFS()
resultRootFS, release, err := p.config.DownloadManager.Download(ctx, *rootFS, descriptors, p.config.ProgressOutput)
resultRootFS, release, err := p.config.DownloadManager.Download(ctx, *rootFS, "", descriptors, p.config.ProgressOutput)
if err != nil {
return err
}
Expand Down
27 changes: 14 additions & 13 deletions components/engine/distribution/pull_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ func (p *v2Puller) pullSchema1(ctx context.Context, ref reference.Named, unverif
descriptors = append(descriptors, layerDescriptor)
}

resultRootFS, release, err := p.config.DownloadManager.Download(ctx, *rootFS, descriptors, p.config.ProgressOutput)
resultRootFS, release, err := p.config.DownloadManager.Download(ctx, *rootFS, "", descriptors, p.config.ProgressOutput)
if err != nil {
return "", "", err
}
Expand Down Expand Up @@ -556,10 +556,11 @@ func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *s
}()

var (
configJSON []byte // raw serialized image config
downloadedRootFS *image.RootFS // rootFS from registered layers
configRootFS *image.RootFS // rootFS from configuration
release func() // release resources from rootFS download
configJSON []byte // raw serialized image config
downloadedRootFS *image.RootFS // rootFS from registered layers
configRootFS *image.RootFS // rootFS from configuration
release func() // release resources from rootFS download
platform layer.Platform // for LCOW when registering downloaded layers
)

// https://github.com/docker/docker/issues/24766 - Err on the side of caution,
Expand All @@ -571,7 +572,7 @@ func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *s
// check to block Windows images being pulled on Linux is implemented, it
// may be necessary to perform the same type of serialisation.
if runtime.GOOS == "windows" {
configJSON, configRootFS, err = receiveConfig(p.config.ImageStore, configChan, configErrChan)
configJSON, configRootFS, platform, err = receiveConfig(p.config.ImageStore, configChan, configErrChan)
if err != nil {
return "", "", err
}
Expand All @@ -598,7 +599,7 @@ func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *s
rootFS image.RootFS
)
downloadRootFS := *image.NewRootFS()
rootFS, release, err = p.config.DownloadManager.Download(ctx, downloadRootFS, descriptors, p.config.ProgressOutput)
rootFS, release, err = p.config.DownloadManager.Download(ctx, downloadRootFS, platform, descriptors, p.config.ProgressOutput)
if err != nil {
// Intentionally do not cancel the config download here
// as the error from config download (if there is one)
Expand All @@ -616,7 +617,7 @@ func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *s
}

if configJSON == nil {
configJSON, configRootFS, err = receiveConfig(p.config.ImageStore, configChan, configErrChan)
configJSON, configRootFS, _, err = receiveConfig(p.config.ImageStore, configChan, configErrChan)
if err == nil && configRootFS == nil {
err = errRootFSInvalid
}
Expand Down Expand Up @@ -663,16 +664,16 @@ func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *s
return imageID, manifestDigest, nil
}

func receiveConfig(s ImageConfigStore, configChan <-chan []byte, errChan <-chan error) ([]byte, *image.RootFS, error) {
func receiveConfig(s ImageConfigStore, configChan <-chan []byte, errChan <-chan error) ([]byte, *image.RootFS, layer.Platform, error) {
select {
case configJSON := <-configChan:
rootfs, err := s.RootFSFromConfig(configJSON)
rootfs, platform, err := s.RootFSAndPlatformFromConfig(configJSON)
if err != nil {
return nil, nil, err
return nil, nil, "", err
}
return configJSON, rootfs, nil
return configJSON, rootfs, platform, nil
case err := <-errChan:
return nil, nil, err
return nil, nil, "", err
// Don't need a case for ctx.Done in the select because cancellation
// will trigger an error in p.pullSchema2ImageConfig.
}
Expand Down
2 changes: 1 addition & 1 deletion components/engine/distribution/push_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func (p *v2Pusher) pushV2Tag(ctx context.Context, ref reference.NamedTagged, id
return fmt.Errorf("could not find image from tag %s: %v", reference.FamiliarString(ref), err)
}

rootfs, err := p.config.ImageStore.RootFSFromConfig(imgConfig)
rootfs, _, err := p.config.ImageStore.RootFSAndPlatformFromConfig(imgConfig)
if err != nil {
return fmt.Errorf("unable to get rootfs for image %s: %s", reference.FamiliarString(ref), err)
}
Expand Down
20 changes: 10 additions & 10 deletions components/engine/distribution/xfer/download.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ type DownloadDescriptorWithRegistered interface {
// Download method is called to get the layer tar data. Layers are then
// registered in the appropriate order. The caller must call the returned
// release function once it is done with the returned RootFS object.
func (ldm *LayerDownloadManager) Download(ctx context.Context, initialRootFS image.RootFS, layers []DownloadDescriptor, progressOutput progress.Output) (image.RootFS, func(), error) {
func (ldm *LayerDownloadManager) Download(ctx context.Context, initialRootFS image.RootFS, platform layer.Platform, layers []DownloadDescriptor, progressOutput progress.Output) (image.RootFS, func(), error) {
var (
topLayer layer.Layer
topDownload *downloadTransfer
Expand Down Expand Up @@ -140,7 +140,7 @@ func (ldm *LayerDownloadManager) Download(ctx context.Context, initialRootFS ima
// the stack? If so, avoid downloading it more than once.
var topDownloadUncasted Transfer
if existingDownload, ok := downloadsByKey[key]; ok {
xferFunc := ldm.makeDownloadFuncFromDownload(descriptor, existingDownload, topDownload)
xferFunc := ldm.makeDownloadFuncFromDownload(descriptor, existingDownload, topDownload, platform)
defer topDownload.Transfer.Release(watcher)
topDownloadUncasted, watcher = ldm.tm.Transfer(transferKey, xferFunc, progressOutput)
topDownload = topDownloadUncasted.(*downloadTransfer)
Expand All @@ -152,10 +152,10 @@ func (ldm *LayerDownloadManager) Download(ctx context.Context, initialRootFS ima

var xferFunc DoFunc
if topDownload != nil {
xferFunc = ldm.makeDownloadFunc(descriptor, "", topDownload)
xferFunc = ldm.makeDownloadFunc(descriptor, "", topDownload, platform)
defer topDownload.Transfer.Release(watcher)
} else {
xferFunc = ldm.makeDownloadFunc(descriptor, rootFS.ChainID(), nil)
xferFunc = ldm.makeDownloadFunc(descriptor, rootFS.ChainID(), nil, platform)
}
topDownloadUncasted, watcher = ldm.tm.Transfer(transferKey, xferFunc, progressOutput)
topDownload = topDownloadUncasted.(*downloadTransfer)
Expand Down Expand Up @@ -212,7 +212,7 @@ func (ldm *LayerDownloadManager) Download(ctx context.Context, initialRootFS ima
// complete before the registration step, and registers the downloaded data
// on top of parentDownload's resulting layer. Otherwise, it registers the
// layer on top of the ChainID given by parentLayer.
func (ldm *LayerDownloadManager) makeDownloadFunc(descriptor DownloadDescriptor, parentLayer layer.ChainID, parentDownload *downloadTransfer) DoFunc {
func (ldm *LayerDownloadManager) makeDownloadFunc(descriptor DownloadDescriptor, parentLayer layer.ChainID, parentDownload *downloadTransfer, platform layer.Platform) DoFunc {
return func(progressChan chan<- progress.Progress, start <-chan struct{}, inactive chan<- struct{}) Transfer {
d := &downloadTransfer{
Transfer: NewTransfer(),
Expand Down Expand Up @@ -335,9 +335,9 @@ func (ldm *LayerDownloadManager) makeDownloadFunc(descriptor DownloadDescriptor,
src = fs.Descriptor()
}
if ds, ok := d.layerStore.(layer.DescribableStore); ok {
d.layer, err = ds.RegisterWithDescriptor(inflatedLayerData, parentLayer, src)
d.layer, err = ds.RegisterWithDescriptor(inflatedLayerData, parentLayer, platform, src)
} else {
d.layer, err = d.layerStore.Register(inflatedLayerData, parentLayer)
d.layer, err = d.layerStore.Register(inflatedLayerData, parentLayer, platform)
}
if err != nil {
select {
Expand Down Expand Up @@ -376,7 +376,7 @@ func (ldm *LayerDownloadManager) makeDownloadFunc(descriptor DownloadDescriptor,
// parentDownload. This function does not log progress output because it would
// interfere with the progress reporting for sourceDownload, which has the same
// Key.
func (ldm *LayerDownloadManager) makeDownloadFuncFromDownload(descriptor DownloadDescriptor, sourceDownload *downloadTransfer, parentDownload *downloadTransfer) DoFunc {
func (ldm *LayerDownloadManager) makeDownloadFuncFromDownload(descriptor DownloadDescriptor, sourceDownload *downloadTransfer, parentDownload *downloadTransfer, platform layer.Platform) DoFunc {
return func(progressChan chan<- progress.Progress, start <-chan struct{}, inactive chan<- struct{}) Transfer {
d := &downloadTransfer{
Transfer: NewTransfer(),
Expand Down Expand Up @@ -434,9 +434,9 @@ func (ldm *LayerDownloadManager) makeDownloadFuncFromDownload(descriptor Downloa
src = fs.Descriptor()
}
if ds, ok := d.layerStore.(layer.DescribableStore); ok {
d.layer, err = ds.RegisterWithDescriptor(layerReader, parentLayer, src)
d.layer, err = ds.RegisterWithDescriptor(layerReader, parentLayer, platform, src)
} else {
d.layer, err = d.layerStore.Register(layerReader, parentLayer)
d.layer, err = d.layerStore.Register(layerReader, parentLayer, platform)
}
if err != nil {
d.err = fmt.Errorf("failed to register layer: %v", err)
Expand Down
17 changes: 11 additions & 6 deletions components/engine/distribution/xfer/download_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func (ls *mockLayerStore) Map() map[layer.ChainID]layer.Layer {
return layers
}

func (ls *mockLayerStore) Register(reader io.Reader, parentID layer.ChainID) (layer.Layer, error) {
func (ls *mockLayerStore) Register(reader io.Reader, parentID layer.ChainID, platform layer.Platform) (layer.Layer, error) {
return ls.RegisterWithDescriptor(reader, parentID, distribution.Descriptor{})
}

Expand Down Expand Up @@ -272,7 +272,9 @@ func TestSuccessfulDownload(t *testing.T) {
}

layerStore := &mockLayerStore{make(map[layer.ChainID]*mockLayer)}
ldm := NewLayerDownloadManager(layerStore, maxDownloadConcurrency, func(m *LayerDownloadManager) { m.waitDuration = time.Millisecond })
lsMap := make(map[string]layer.Store)
lsMap[runtime.GOOS] = layerStore
ldm := NewLayerDownloadManager(lsMap, maxDownloadConcurrency, func(m *LayerDownloadManager) { m.waitDuration = time.Millisecond })

progressChan := make(chan progress.Progress)
progressDone := make(chan struct{})
Expand All @@ -291,13 +293,13 @@ func TestSuccessfulDownload(t *testing.T) {
firstDescriptor := descriptors[0].(*mockDownloadDescriptor)

// Pre-register the first layer to simulate an already-existing layer
l, err := layerStore.Register(firstDescriptor.mockTarStream(), "")
l, err := layerStore.Register(firstDescriptor.mockTarStream(), "", layer.Platform(runtime.GOOS))
if err != nil {
t.Fatal(err)
}
firstDescriptor.diffID = l.DiffID()

rootFS, releaseFunc, err := ldm.Download(context.Background(), *image.NewRootFS(), descriptors, progress.ChanOutput(progressChan))
rootFS, releaseFunc, err := ldm.Download(context.Background(), *image.NewRootFS(), layer.Platform(runtime.GOOS), descriptors, progress.ChanOutput(progressChan))
if err != nil {
t.Fatalf("download error: %v", err)
}
Expand Down Expand Up @@ -333,7 +335,10 @@ func TestSuccessfulDownload(t *testing.T) {
}

func TestCancelledDownload(t *testing.T) {
ldm := NewLayerDownloadManager(&mockLayerStore{make(map[layer.ChainID]*mockLayer)}, maxDownloadConcurrency, func(m *LayerDownloadManager) { m.waitDuration = time.Millisecond })
layerStore := &mockLayerStore{make(map[layer.ChainID]*mockLayer)}
lsMap := make(map[string]layer.Store)
lsMap[runtime.GOOS] = layerStore
ldm := NewLayerDownloadManager(lsMap, maxDownloadConcurrency, func(m *LayerDownloadManager) { m.waitDuration = time.Millisecond })

progressChan := make(chan progress.Progress)
progressDone := make(chan struct{})
Expand All @@ -352,7 +357,7 @@ func TestCancelledDownload(t *testing.T) {
}()

descriptors := downloadDescriptors(nil)
_, _, err := ldm.Download(ctx, *image.NewRootFS(), descriptors, progress.ChanOutput(progressChan))
_, _, err := ldm.Download(ctx, *image.NewRootFS(), layer.Platform(runtime.GOOS), descriptors, progress.ChanOutput(progressChan))
if err != context.Canceled {
t.Fatal("expected download to be cancelled")
}
Expand Down
Loading

0 comments on commit 8508f49

Please sign in to comment.