From 8c58934106e91a9330daf6a97bf0db1def0aa307 Mon Sep 17 00:00:00 2001 From: Laurent Goderre Date: Mon, 28 Oct 2024 12:15:40 -0400 Subject: [PATCH] Implement mount from image Signed-off-by: Laurent Goderre --- .../router/container/container_routes.go | 5 + api/swagger.yaml | 4 + api/types/mount/mount.go | 2 + daemon/containerd/image_snapshot.go | 5 + daemon/image_service.go | 1 + daemon/images/image_delete.go | 14 +- daemon/images/service.go | 19 ++- daemon/mounts.go | 67 +++++--- daemon/volumes.go | 39 +++++ docs/api/version-history.md | 2 + integration/volume/mount_test.go | 147 ++++++++++++++++++ layer/mounted_layer.go | 15 +- volume/mounts/linux_parser.go | 19 +++ volume/mounts/mounts.go | 17 ++ volume/mounts/parser_test.go | 11 ++ volume/mounts/validate_test.go | 29 ++++ 16 files changed, 370 insertions(+), 26 deletions(-) diff --git a/api/server/router/container/container_routes.go b/api/server/router/container/container_routes.go index 011808b721..5b07db5ebc 100644 --- a/api/server/router/container/container_routes.go +++ b/api/server/router/container/container_routes.go @@ -649,6 +649,11 @@ func (c *containerRouter) postContainersCreate(ctx context.Context, w http.Respo // reinitialize this field. epConfig.GwPriority = 0 } + for _, m := range hostConfig.Mounts { + if m.Type == mount.TypeImage { + return errdefs.InvalidParameter(errors.New(`Mount type "Image" needs API v1.48 or newer`)) + } + } } var warnings []string diff --git a/api/swagger.yaml b/api/swagger.yaml index 829e9a058e..9ca6f67b9d 100644 --- a/api/swagger.yaml +++ b/api/swagger.yaml @@ -212,6 +212,7 @@ definitions: - `bind` a mount of a file or directory from the host into the container. - `volume` a docker volume with the given `Name`. + - `image` a docker image - `tmpfs` a `tmpfs`. - `npipe` a named pipe from the host into the container. - `cluster` a Swarm cluster volume @@ -219,6 +220,7 @@ definitions: enum: - "bind" - "volume" + - "image" - "tmpfs" - "npipe" - "cluster" @@ -350,6 +352,7 @@ definitions: - `bind` Mounts a file or directory from the host into the container. Must exist prior to creating the container. - `volume` Creates a volume with the given name and options (or uses a pre-existing volume with the same name and options). These are **not** removed when the container is removed. + - `image` Mounts an image. - `tmpfs` Create a tmpfs with the given options. The mount source cannot be specified for tmpfs. - `npipe` Mounts a named pipe from the host into the container. Must exist prior to creating the container. - `cluster` a Swarm cluster volume @@ -357,6 +360,7 @@ definitions: enum: - "bind" - "volume" + - "image" - "tmpfs" - "npipe" - "cluster" diff --git a/api/types/mount/mount.go b/api/types/mount/mount.go index c68dcf65bd..15538147c4 100644 --- a/api/types/mount/mount.go +++ b/api/types/mount/mount.go @@ -19,6 +19,8 @@ const ( TypeNamedPipe Type = "npipe" // TypeCluster is the type for Swarm Cluster Volumes. TypeCluster Type = "cluster" + // TypeImage is the type for mounting another image's filesystem + TypeImage Type = "image" ) // Mount represents a mount (volume). diff --git a/daemon/containerd/image_snapshot.go b/daemon/containerd/image_snapshot.go index 78362e6d3c..663a396743 100644 --- a/daemon/containerd/image_snapshot.go +++ b/daemon/containerd/image_snapshot.go @@ -13,6 +13,7 @@ import ( "github.com/docker/docker/container" "github.com/docker/docker/daemon/snapshotter" "github.com/docker/docker/errdefs" + "github.com/docker/docker/image" "github.com/docker/docker/layer" "github.com/opencontainers/image-spec/identity" "github.com/pkg/errors" @@ -89,6 +90,10 @@ func (i *ImageService) CreateLayer(ctr *container.Container, initFunc layer.Moun }, nil } +func (i *ImageService) CreateLayerFromImage(img *image.Image, layerName string, rwLayerOpts *layer.CreateRWLayerOpts) (container.RWLayer, error) { + return nil, errdefs.NotImplemented(errdefs.NotImplemented(errors.New("not implemented"))) +} + type rwLayer struct { id string snapshotter snapshots.Snapshotter diff --git a/daemon/image_service.go b/daemon/image_service.go index dfc4a0b8f7..a24ffe91a8 100644 --- a/daemon/image_service.go +++ b/daemon/image_service.go @@ -48,6 +48,7 @@ type ImageService interface { GetImageAndReleasableLayer(ctx context.Context, refOrID string, opts backend.GetImageAndLayerOptions) (builder.Image, builder.ROLayer, error) CreateLayer(container *container.Container, initFunc layer.MountInit) (container.RWLayer, error) + CreateLayerFromImage(img *image.Image, layerName string, rwLayerOpts *layer.CreateRWLayerOpts) (container.RWLayer, error) GetLayerByID(cid string) (container.RWLayer, error) LayerStoreStatus() [][2]string GetLayerMountID(cid string) (string, error) diff --git a/daemon/images/image_delete.go b/daemon/images/image_delete.go index 00d84ee4f3..552ec722d9 100644 --- a/daemon/images/image_delete.go +++ b/daemon/images/image_delete.go @@ -75,7 +75,19 @@ func (i *ImageService) ImageDelete(ctx context.Context, imageRef string, force, repoRefs := i.referenceStore.References(imgID.Digest()) using := func(c *container.Container) bool { - return c.ImageID == imgID + if c.ImageID == imgID { + return true + } + + for _, mp := range c.MountPoints { + if mp.Type == "image" { + if mp.Spec.Source == string(imgID) { + return true + } + } + } + + return false } var removedRepositoryRef bool diff --git a/daemon/images/service.go b/daemon/images/service.go index 213eec28bd..1c6167197c 100644 --- a/daemon/images/service.go +++ b/daemon/images/service.go @@ -119,13 +119,13 @@ func (i *ImageService) Children(_ context.Context, id image.ID) ([]image.ID, err // called from create.go // TODO: accept an opt struct instead of container? func (i *ImageService) CreateLayer(container *container.Container, initFunc layer.MountInit) (container.RWLayer, error) { - var layerID layer.ChainID + var img *image.Image if container.ImageID != "" { - img, err := i.imageStore.Get(container.ImageID) + containerImg, err := i.imageStore.Get(container.ImageID) if err != nil { return nil, err } - layerID = img.RootFS.ChainID() + img = containerImg } rwLayerOpts := &layer.CreateRWLayerOpts{ @@ -134,7 +134,18 @@ func (i *ImageService) CreateLayer(container *container.Container, initFunc laye StorageOpt: container.HostConfig.StorageOpt, } - return i.layerStore.CreateRWLayer(container.ID, layerID, rwLayerOpts) + return i.CreateLayerFromImage(img, container.ID, rwLayerOpts) +} + +// CreateLayerFromImage creates a file system from an arbitrary image +// Used to mount an image inside another +func (i *ImageService) CreateLayerFromImage(img *image.Image, layerName string, rwLayerOpts *layer.CreateRWLayerOpts) (container.RWLayer, error) { + var layerID layer.ChainID + if img != nil { + layerID = img.RootFS.ChainID() + } + + return i.layerStore.CreateRWLayer(layerName, layerID, rwLayerOpts) } // GetLayerByID returns a layer by ID diff --git a/daemon/mounts.go b/daemon/mounts.go index cfc27052ac..10ad9cc684 100644 --- a/daemon/mounts.go +++ b/daemon/mounts.go @@ -17,6 +17,17 @@ func (daemon *Daemon) prepareMountPoints(container *container.Container) error { if err := daemon.lazyInitializeVolume(container.ID, config); err != nil { return err } + + // Restore reference to image mount layer + if config.Type == mounttypes.TypeImage && config.Layer == nil { + layer, err := daemon.imageService.GetLayerByID(config.ID) + if err != nil { + return err + } + + config.Layer = layer + } + if config.Volume == nil { // FIXME(thaJeztah): should we check for config.Type here as well? (i.e., skip bind-mounts etc) continue @@ -38,28 +49,44 @@ func (daemon *Daemon) removeMountPoints(container *container.Container, rm bool) var rmErrors []string ctx := context.TODO() for _, m := range container.MountPoints { - if m.Type != mounttypes.TypeVolume || m.Volume == nil { - continue - } - daemon.volumes.Release(ctx, m.Volume.Name(), container.ID) - if !rm { - continue + if m.Type == mounttypes.TypeVolume { + if m.Volume == nil { + continue + } + daemon.volumes.Release(ctx, m.Volume.Name(), container.ID) + if !rm { + continue + } + + // Do not remove named mountpoints + // these are mountpoints specified like `docker run -v :/foo` + if m.Spec.Source != "" { + continue + } + + err := daemon.volumes.Remove(ctx, m.Volume.Name()) + // Ignore volume in use errors because having this + // volume being referenced by other container is + // not an error, but an implementation detail. + // This prevents docker from logging "ERROR: Volume in use" + // where there is another container using the volume. + if err != nil && !volumesservice.IsInUse(err) { + rmErrors = append(rmErrors, err.Error()) + } } - // Do not remove named mountpoints - // these are mountpoints specified like `docker run -v :/foo` - if m.Spec.Source != "" { - continue - } - - err := daemon.volumes.Remove(ctx, m.Volume.Name()) - // Ignore volume in use errors because having this - // volume being referenced by other container is - // not an error, but an implementation detail. - // This prevents docker from logging "ERROR: Volume in use" - // where there is another container using the volume. - if err != nil && !volumesservice.IsInUse(err) { - rmErrors = append(rmErrors, err.Error()) + if m.Type == mounttypes.TypeImage { + layer := m.Layer + err := layer.Unmount() + if err != nil { + rmErrors = append(rmErrors, err.Error()) + continue + } + err = daemon.imageService.ReleaseLayer(layer) + if err != nil { + rmErrors = append(rmErrors, err.Error()) + continue + } } } diff --git a/daemon/volumes.go b/daemon/volumes.go index e48dacf961..f42465d60c 100644 --- a/daemon/volumes.go +++ b/daemon/volumes.go @@ -2,6 +2,7 @@ package daemon // import "github.com/docker/docker/daemon" import ( "context" + "fmt" "os" "path/filepath" "sort" @@ -9,12 +10,14 @@ import ( "time" "github.com/containerd/log" + "github.com/docker/docker/api/types/backend" containertypes "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/mount" mounttypes "github.com/docker/docker/api/types/mount" volumetypes "github.com/docker/docker/api/types/volume" "github.com/docker/docker/container" "github.com/docker/docker/errdefs" + "github.com/docker/docker/layer" "github.com/docker/docker/volume" volumemounts "github.com/docker/docker/volume/mounts" "github.com/docker/docker/volume/service" @@ -245,6 +248,42 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo } } + if mp.Type == mounttypes.TypeImage { + img, err := daemon.imageService.GetImage(ctx, mp.Source, backend.GetImageOpts{}) + if err != nil { + return err + } + + rwLayerOpts := &layer.CreateRWLayerOpts{ + StorageOpt: container.HostConfig.StorageOpt, + } + + layerName := fmt.Sprintf("%s-%s", container.ID, mp.Source) + layer, err := daemon.imageService.CreateLayerFromImage(img, layerName, rwLayerOpts) + if err != nil { + return err + } + metadata, err := layer.Metadata() + if err != nil { + return err + } + + path, err := layer.Mount("") + if err != nil { + return err + } + + if metadata["ID"] != "" { + mp.ID = metadata["ID"] + } + + mp.Name = mp.Spec.Source + mp.Spec.Source = img.ID().String() + mp.Source = path + mp.Layer = layer + mp.RW = false + } + binds[mp.Destination] = true dereferenceIfExists(mp.Destination) mountPoints[mp.Destination] = mp diff --git a/docs/api/version-history.md b/docs/api/version-history.md index b0b02482a6..bab01957bc 100644 --- a/docs/api/version-history.md +++ b/docs/api/version-history.md @@ -38,6 +38,8 @@ keywords: "API, Docker, rcli, REST, documentation" defined through `Mounts` because the `VolumeDriver` option has no effect on those volumes. This warning was previously generated by the CLI, but now moved to the daemon so that other clients can also get this warning. +* `POST /containers/create` now supports `Mount` of type `image` for mounting + an image inside a container. * Deprecated: The `ContainerdCommit.Expected`, `RuncCommit.Expected`, and `InitCommit.Expected` fields in the `GET /info` endpoint are deprecated and will be omitted in API v1.49. diff --git a/integration/volume/mount_test.go b/integration/volume/mount_test.go index 1c6b4cd507..26cc9f8d52 100644 --- a/integration/volume/mount_test.go +++ b/integration/volume/mount_test.go @@ -1,12 +1,17 @@ package volume import ( + "bytes" "context" + "fmt" + "io" "path/filepath" "strings" "testing" + "github.com/docker/docker/api/types" containertypes "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/image" "github.com/docker/docker/api/types/mount" "github.com/docker/docker/api/types/network" "github.com/docker/docker/api/types/versions" @@ -14,6 +19,7 @@ import ( "github.com/docker/docker/client" "github.com/docker/docker/integration/internal/container" "github.com/docker/docker/internal/safepath" + "github.com/docker/docker/testutil/fakecontext" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" "gotest.tools/v3/skip" @@ -112,6 +118,104 @@ func TestRunMountVolumeSubdir(t *testing.T) { } } +func TestRunMountImage(t *testing.T) { + skip.If(t, versions.LessThan(testEnv.DaemonAPIVersion(), "1.48"), "skip test from new feature") + skip.If(t, testEnv.UsingSnapshotter(), "snapshotter not supported") + skip.If(t, testEnv.DaemonInfo.OSType == "windows", "image mounts not supported on Windows") + + ctx := setupTest(t) + apiClient := testEnv.APIClient() + + for _, tc := range []struct { + name string + cmd []string + volumeTarget string + createErr string + startErr string + expected string + skipPlatform string + }{ + {name: "image", cmd: []string{"cat", "/image/foo"}, expected: "bar"}, + {name: "image_tag", cmd: []string{"cat", "/image/foo"}, expected: "bar"}, + {name: "image_remove", cmd: []string{"cat", "/image/foo"}, expected: "bar"}, + // Expected is duplicated because the container runs twice + {name: "image_remove_force", cmd: []string{"cat", "/image/foo"}, expected: "barbar"}, + } { + t.Run(tc.name, func(t *testing.T) { + testImage := setupTestImage(t, ctx, apiClient, tc.name) + if testImage != "" { + defer apiClient.ImageRemove(ctx, testImage, image.RemoveOptions{Force: true}) + } + + cfg := containertypes.Config{ + Image: "busybox", + Cmd: tc.cmd, + } + + hostCfg := containertypes.HostConfig{ + Mounts: []mount.Mount{ + { + Type: mount.TypeImage, + Source: testImage, + Target: "/image", + }, + }, + } + + startContainer := func(id string) { + startErr := apiClient.ContainerStart(ctx, id, containertypes.StartOptions{}) + if tc.startErr != "" { + assert.ErrorContains(t, startErr, tc.startErr) + return + } + assert.NilError(t, startErr) + } + + ctrName := strings.ReplaceAll(t.Name(), "/", "_") + create, creatErr := apiClient.ContainerCreate(ctx, &cfg, &hostCfg, &network.NetworkingConfig{}, nil, ctrName) + id := create.ID + if id != "" { + defer apiClient.ContainerRemove(ctx, id, containertypes.RemoveOptions{Force: true}) + } + + if tc.createErr != "" { + assert.ErrorContains(t, creatErr, tc.createErr) + return + } + assert.NilError(t, creatErr, "container creation failed") + startContainer(id) + + // Test image mounted is in use logic + if tc.name == "image_remove" { + _, removeErr := apiClient.ImageRemove(ctx, testImage, image.RemoveOptions{}) + assert.ErrorContains(t, removeErr, fmt.Sprintf(`unable to remove repository reference "test-image" (must force) - container %s is using its referenced image`, id[:12])) + } + + // Test that the container servives a restart when mounted image is removed + if tc.name == "image_remove_force" { + stopErr := apiClient.ContainerStop(ctx, id, containertypes.StopOptions{}) + assert.NilError(t, stopErr) + + _, removeErr := apiClient.ImageRemove(ctx, testImage, image.RemoveOptions{Force: true}) + assert.NilError(t, removeErr) + + startContainer(id) + } + + output, err := container.Output(ctx, apiClient, id) + assert.Check(t, err) + + inspect, err := apiClient.ContainerInspect(ctx, id) + if assert.Check(t, err) { + assert.Check(t, is.Equal(inspect.State.ExitCode, 0)) + } + + assert.Check(t, is.Equal(strings.TrimSpace(output.Stderr), "")) + assert.Check(t, is.Equal(strings.TrimSpace(output.Stdout), tc.expected)) + }) + } +} + // setupTestVolume sets up a volume with: // . // |-- bar.txt (file with "foo") @@ -183,3 +287,46 @@ func setupTestVolume(t *testing.T, client client.APIClient) string { return volumeName } + +func setupTestImage(t *testing.T, ctx context.Context, client client.APIClient, test string) string { + imgName := "test-image" + + if test == "image_tag" { + imgName += ":foo" + } + + dockerfile := ` + FROM scratch + ADD foo / + ` + + source := fakecontext.New( + t, + "", + fakecontext.WithDockerfile(dockerfile), + fakecontext.WithFile("foo", "bar"), + ) + defer source.Close() + + resp, err := client.ImageBuild(ctx, + source.AsTarReader(t), + types.ImageBuildOptions{ + Remove: false, + ForceRemove: false, + Tags: []string{imgName}, + }) + assert.NilError(t, err) + + out := bytes.NewBuffer(nil) + _, err = io.Copy(out, resp.Body) + assert.Check(t, resp.Body.Close()) + assert.NilError(t, err) + + _, _, err = client.ImageInspectWithRaw(ctx, imgName) + if err != nil { + t.Log(out) + } + assert.NilError(t, err) + + return imgName +} diff --git a/layer/mounted_layer.go b/layer/mounted_layer.go index 05f98f5f3d..0e6d3655b3 100644 --- a/layer/mounted_layer.go +++ b/layer/mounted_layer.go @@ -55,7 +55,20 @@ func (ml *mountedLayer) Changes() ([]archive.Change, error) { } func (ml *mountedLayer) Metadata() (map[string]string, error) { - return ml.layerStore.driver.GetMetadata(ml.mountID) + m, err := ml.layerStore.driver.GetMetadata(ml.mountID) + if err != nil { + return nil, err + } + + if m == nil { + m = make(map[string]string) + } + + if m["ID"] == "" { + m["ID"] = ml.name + } + + return m, nil } func (ml *mountedLayer) getReference() RWLayer { diff --git a/volume/mounts/linux_parser.go b/volume/mounts/linux_parser.go index 507003a086..73f4b69c3f 100644 --- a/volume/mounts/linux_parser.go +++ b/volume/mounts/linux_parser.go @@ -119,6 +119,16 @@ func (p *linuxParser) validateMountConfigImpl(mnt *mount.Mount, validateBindSour if _, err := p.ConvertTmpfsOptions(mnt.TmpfsOptions, mnt.ReadOnly); err != nil { return &errMountConfig{mnt, err} } + case mount.TypeImage: + if mnt.BindOptions != nil { + return &errMountConfig{mnt, errExtraField("BindOptions")} + } + if mnt.VolumeOptions != nil { + return &errMountConfig{mnt, errExtraField("VolumeOptions")} + } + if len(mnt.Source) == 0 { + return &errMountConfig{mnt, errMissingField("Source")} + } default: return &errMountConfig{mnt, errors.New("mount type unknown")} } @@ -353,6 +363,15 @@ func (p *linuxParser) parseMountSpec(cfg mount.Mount, validateBindSourceExists b } case mount.TypeTmpfs: // NOP + case mount.TypeImage: + mp.Source = cfg.Source + if cfg.BindOptions != nil && len(cfg.BindOptions.Propagation) > 0 { + mp.Propagation = cfg.BindOptions.Propagation + } else { + // If user did not specify a propagation mode, get + // default propagation mode. + mp.Propagation = linuxDefaultPropagationMode + } } return mp, nil } diff --git a/volume/mounts/mounts.go b/volume/mounts/mounts.go index 7a518a046f..7278f21e7e 100644 --- a/volume/mounts/mounts.go +++ b/volume/mounts/mounts.go @@ -18,6 +18,21 @@ import ( "github.com/pkg/errors" ) +// RWLayer represents a writable layer. +type RWLayer interface { + // Mount mounts the RWLayer and returns the filesystem path + // to the writable layer. + Mount(mountLabel string) (string, error) + + // Unmount unmounts the RWLayer. This should be called + // for every mount. If there are multiple mount calls + // this operation will only decrement the internal mount counter. + Unmount() error + + // Metadata returns the low level metadata for the mutable layer + Metadata() (map[string]string, error) +} + // MountPoint is the intersection point between a volume and a container. It // specifies which volume is to be used and where inside a container it should // be mounted. @@ -80,6 +95,8 @@ type MountPoint struct { // SafePaths created by Setup that should be cleaned up before unmounting // the volume. safePaths []*safepath.SafePath + + Layer RWLayer `json:"-"` } // Cleanup frees resources used by the mountpoint and cleans up all the paths diff --git a/volume/mounts/parser_test.go b/volume/mounts/parser_test.go index bfafffdf7e..0adb6927db 100644 --- a/volume/mounts/parser_test.go +++ b/volume/mounts/parser_test.go @@ -2,6 +2,7 @@ package mounts // import "github.com/docker/docker/volume/mounts" import ( "os" + "runtime" "testing" "github.com/docker/docker/api/types/mount" @@ -78,6 +79,16 @@ func TestParseMountSpec(t *testing.T) { }, } + if runtime.GOOS != "windows" { + tests = append(tests, struct { + input mount.Mount + expected MountPoint + }{ + input: mount.Mount{Type: mount.TypeImage, Source: "alpine", Target: testDestinationPath}, + expected: MountPoint{Type: mount.TypeImage, Source: "alpine", Destination: testDestinationPath, RW: true, Propagation: parser.DefaultPropagationMode()}, + }) + } + for _, tc := range tests { t.Run("", func(t *testing.T) { mp, err := parser.ParseMountSpec(tc.input) diff --git a/volume/mounts/validate_test.go b/volume/mounts/validate_test.go index f0d8e8a5e9..9266062582 100644 --- a/volume/mounts/validate_test.go +++ b/volume/mounts/validate_test.go @@ -55,6 +55,35 @@ func TestValidateMount(t *testing.T) { expected: errBindSourceDoesNotExist(testSourcePath), }, } + + if runtime.GOOS != "windows" { + imageTests := []struct { + input mount.Mount + expected error + }{ + { + input: mount.Mount{Type: mount.TypeImage}, + expected: errMissingField("Target"), + }, + { + input: mount.Mount{Type: mount.TypeImage, Target: testDestinationPath}, + expected: errMissingField("Source"), + }, + { + input: mount.Mount{Type: mount.TypeImage, Target: testDestinationPath, Source: "hello"}, + }, + { + input: mount.Mount{Type: mount.TypeImage, Target: testDestinationPath, Source: "hello", BindOptions: &mount.BindOptions{}}, + expected: errExtraField("BindOptions"), + }, + { + input: mount.Mount{Type: mount.TypeImage, Target: testDestinationPath, Source: "hello", VolumeOptions: &mount.VolumeOptions{}}, + expected: errExtraField("VolumeOptions"), + }, + } + tests = append(tests, imageTests...) + } + for _, tc := range tests { t.Run("", func(t *testing.T) { err := parser.ValidateMountConfig(&tc.input)