Skip to content

Commit

Permalink
Fix transient mounts in Docker 1.12 and add test case
Browse files Browse the repository at this point in the history
Use a temporary container to receive the copy instead of using host
mounts.
  • Loading branch information
smarterclayton committed Oct 19, 2016
1 parent 787a548 commit 6dfd613
Show file tree
Hide file tree
Showing 3 changed files with 137 additions and 39 deletions.
2 changes: 1 addition & 1 deletion cmd/imagebuilder/imagebuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func build(dockerfilePath string, e *dockerclient.ClientExecutor, arguments map[
defer f.Close()

// TODO: handle signals
if err := e.Cleanup(); err != nil {
if err := e.Cleanup(e.Container); err != nil {
fmt.Fprintf(e.ErrOut, "error: Unable to clean up build: %v\n", err)
}

Expand Down
96 changes: 58 additions & 38 deletions dockerclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,13 @@ func (e *ClientExecutor) Build(r io.Reader, args map[string]string) error {
Config: &docker.Config{
Image: from,
},
HostConfig: &docker.HostConfig{},
}
if e.HostConfig != nil {
opts.HostConfig = e.HostConfig
}
originalBinds := opts.HostConfig.Binds

if mustStart {
// Transient mounts only make sense on images that will be running processes
if len(e.TransientMounts) > 0 {
Expand Down Expand Up @@ -181,53 +187,36 @@ func (e *ClientExecutor) Build(r io.Reader, args map[string]string) error {
opts.Config.Entrypoint = []string{"/bin/sh", "-c"}
}
}

if len(opts.Config.Cmd) == 0 {
opts.Config.Entrypoint = []string{"/bin/sh", "-c", "# NOP"}
}

// copy any source content into the temporary mount path
if mustStart && len(e.TransientMounts) > 0 {
if len(sharedMount) == 0 {
return fmt.Errorf("no mount point available for temporary mounts")
}
binds, err := e.StartPartialContainer(opts, e.TransientMounts, sharedMount)
if err != nil {
return err
}
opts.HostConfig.Binds = append(originalBinds, binds...)
}

container, err := e.Client.CreateContainer(opts)
if err != nil {
return fmt.Errorf("unable to create build container: %v", err)
}
e.Container = container

// if we create the container, take responsibilty for cleaning up
defer e.Cleanup()
}

// copy any source content into the temporary mount path
if mustStart && len(e.TransientMounts) > 0 {
var copies []imagebuilder.Copy
for i, mount := range e.TransientMounts {
source := mount.SourcePath
copies = append(copies, imagebuilder.Copy{
Src: source,
Dest: []string{path.Join("/tmp/__temporarymount", strconv.Itoa(i))},
})
}
if err := e.Copy(copies...); err != nil {
return fmt.Errorf("unable to copy build context into container: %v", err)
}
defer e.Cleanup(container)
}

// TODO: lazy start
if mustStart && !e.Container.State.Running {
var hostConfig docker.HostConfig
if e.HostConfig != nil {
hostConfig = *e.HostConfig
}

// mount individual items temporarily
for i, mount := range e.TransientMounts {
if len(sharedMount) == 0 {
return fmt.Errorf("no mount point available for temporary mounts")
}
hostConfig.Binds = append(
hostConfig.Binds,
fmt.Sprintf("%s:%s:%s", path.Join(sharedMount, strconv.Itoa(i)), mount.DestinationPath, "ro"),
)
}

if err := e.Client.StartContainer(e.Container.ID, &hostConfig); err != nil {
if err := e.Client.StartContainer(e.Container.ID, nil); err != nil {
return fmt.Errorf("unable to start build container: %v", err)
}
// TODO: is this racy? may have to loop wait in the actual run step
Expand Down Expand Up @@ -291,20 +280,46 @@ func (e *ClientExecutor) Build(r io.Reader, args map[string]string) error {
return nil
}

func (e *ClientExecutor) StartPartialContainer(opts docker.CreateContainerOptions, transientMounts []Mount, sharedMount string) ([]string, error) {
container, err := e.Client.CreateContainer(opts)
if err != nil {
return nil, fmt.Errorf("unable to create build container: %v", err)
}
defer e.Cleanup(container)

var copies []imagebuilder.Copy
for i, mount := range transientMounts {
source := mount.SourcePath
copies = append(copies, imagebuilder.Copy{
Src: source,
Dest: []string{path.Join("/tmp/__temporarymount", strconv.Itoa(i))},
})
}
if err := e.CopyContainer(container, copies...); err != nil {
return nil, fmt.Errorf("unable to copy build context into container: %v", err)
}

// mount individual items temporarily
var binds []string
for i, mount := range e.TransientMounts {
binds = append(binds, fmt.Sprintf("%s:%s:%s", path.Join(sharedMount, strconv.Itoa(i)), mount.DestinationPath, "ro"))
}
return binds, nil
}

// Cleanup will remove the container that created the build.
func (e *ClientExecutor) Cleanup() error {
if e.Container == nil {
func (e *ClientExecutor) Cleanup(container *docker.Container) error {
if container == nil {
return nil
}
err := e.Client.RemoveContainer(docker.RemoveContainerOptions{
ID: e.Container.ID,
ID: container.ID,
RemoveVolumes: true,
Force: true,
})
if _, ok := err.(*docker.NoSuchContainer); err != nil && !ok {
return fmt.Errorf("unable to cleanup build container: %v", err)
}
e.Container = nil
return nil
}

Expand Down Expand Up @@ -481,8 +496,13 @@ func (e *ClientExecutor) Run(run imagebuilder.Run, config docker.Config) error {
return nil
}

// Copy implements the executor copy function.
func (e *ClientExecutor) Copy(copies ...imagebuilder.Copy) error {
container := e.Container
return e.CopyContainer(e.Container, copies...)
}

// CopyContainer copies the provided content into a destination container.
func (e *ClientExecutor) CopyContainer(container *docker.Container, copies ...imagebuilder.Copy) error {
for _, c := range copies {
// TODO: reuse source
for _, dst := range c.Dest {
Expand Down
78 changes: 78 additions & 0 deletions dockerclient/conformance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"fmt"
"io"
"io/ioutil"
"math/rand"
"os"
"os/exec"
"path/filepath"
Expand Down Expand Up @@ -142,6 +143,83 @@ func TestConformanceExternal(t *testing.T) {
}
}

func TestTransientMount(t *testing.T) {
c, err := docker.NewClientFromEnv()
if err != nil {
t.Fatal(err)
}

e := &ClientExecutor{}
e.LogFn = func(_ string, _ ...interface{}) {}
e.Client = c
e.AllowPull = true
e.Directory = "testdata"
e.TransientMounts = []Mount{
{SourcePath: "dir", DestinationPath: "/mountdir"},
{SourcePath: "Dockerfile.env", DestinationPath: "/mountfile"},
}
e.Tag = fmt.Sprintf("conformance%d", rand.Int63())

defer c.RemoveImage(e.Tag)

out := &bytes.Buffer{}
e.Out = out
if err := e.Build(bytes.NewBufferString("FROM busybox\nRUN ls /mountdir/subdir\nRUN cat /mountfile\n"), nil); err != nil {
t.Fatalf("unable to build image: %v", err)
}
if !strings.Contains(out.String(), "ENV name=value\n") {
t.Errorf("did not find expected output:\n%s", out.String())
}
if !strings.Contains(out.String(), "file2\n") {
t.Errorf("did not find expected output:\n%s", out.String())
}

result, err := testContainerOutput(c, e.Tag, []string{"/bin/sh", "-c", "ls -al /mountdir"})
if err != nil {
t.Fatal(err)
}
if strings.Contains(result, "subdir") {
t.Errorf("did not find expected output:\n%s", result)
}
result, err = testContainerOutput(c, e.Tag, []string{"/bin/sh", "-c", "cat /mountfile"})
if err != nil {
t.Fatal(err)
}
if strings.Contains(result, "ENV name=value\n") {
t.Errorf("did not find expected output:\n%s", result)
}
}

func testContainerOutput(c *docker.Client, tag string, command []string) (string, error) {
container, err := c.CreateContainer(docker.CreateContainerOptions{
Name: tag + "-test",
Config: &docker.Config{
Image: tag,
Entrypoint: command,
Cmd: nil,
},
})
if err != nil {
return "", err
}
defer c.RemoveContainer(docker.RemoveContainerOptions{ID: container.ID})
if err := c.StartContainer(container.ID, nil); err != nil {
return "", err
}
code, err := c.WaitContainer(container.ID)
if err != nil {
return "", err
}
if code != 0 {
return "", fmt.Errorf("unrecognized exit code: %d", code)
}
out := &bytes.Buffer{}
if err := c.Logs(docker.LogsOptions{Container: container.ID, Stdout: true, OutputStream: out}); err != nil {
return "", fmt.Errorf("unable to get logs: %v", err)
}
return out.String(), nil
}

func conformanceTester(t *testing.T, c *docker.Client, test conformanceTest, i int, deep bool) {
dockerfile := test.Dockerfile
if len(dockerfile) == 0 {
Expand Down

0 comments on commit 6dfd613

Please sign in to comment.