Skip to content

Commit

Permalink
chore: deprecate BindMount APIs (#1907)
Browse files Browse the repository at this point in the history
* chore: deprecate BindMount APIs

* fix: lint

* chore: update deprecation message
  • Loading branch information
mdelapenya committed Nov 8, 2023
1 parent fc966d5 commit 555cb76
Show file tree
Hide file tree
Showing 14 changed files with 201 additions and 276 deletions.
26 changes: 26 additions & 0 deletions container.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,8 @@ func (c *ContainerRequest) validateContextOrImageIsSpecified() error {
return nil
}

// validateMounts ensures that the mounts do not have duplicate targets.
// It will check the Mounts and HostConfigModifier.Binds fields.
func (c *ContainerRequest) validateMounts() error {
targets := make(map[string]bool, len(c.Mounts))

Expand All @@ -317,5 +319,29 @@ func (c *ContainerRequest) validateMounts() error {
targets[targetPath] = true
}
}

if c.HostConfigModifier == nil {
return nil
}

hostConfig := container.HostConfig{}

c.HostConfigModifier(&hostConfig)

if hostConfig.Binds != nil && len(hostConfig.Binds) > 0 {
for _, bind := range hostConfig.Binds {
parts := strings.Split(bind, ":")
if len(parts) != 2 {
return fmt.Errorf("%w: %s", ErrInvalidBindMount, bind)
}
targetPath := parts[1]
if targets[targetPath] {
return fmt.Errorf("%w: %s", ErrDuplicateMountTarget, targetPath)
} else {
targets[targetPath] = true
}
}
}

return nil
}
23 changes: 19 additions & 4 deletions container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"testing"
"time"

"github.com/docker/docker/api/types/container"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

Expand Down Expand Up @@ -55,16 +56,30 @@ func Test_ContainerValidation(t *testing.T) {
Name: "Can mount same source to multiple targets",
ExpectedError: nil,
ContainerRequest: ContainerRequest{
Image: "redis:latest",
Mounts: Mounts(BindMount("/data", "/srv"), BindMount("/data", "/data")),
Image: "redis:latest",
HostConfigModifier: func(hc *container.HostConfig) {
hc.Binds = []string{"/data:/srv", "/data:/data"}
},
},
},
{
Name: "Cannot mount multiple sources to same target",
ExpectedError: errors.New("duplicate mount target detected: /data"),
ContainerRequest: ContainerRequest{
Image: "redis:latest",
Mounts: Mounts(BindMount("/srv", "/data"), BindMount("/data", "/data")),
Image: "redis:latest",
HostConfigModifier: func(hc *container.HostConfig) {
hc.Binds = []string{"/data:/data", "/data:/data"}
},
},
},
{
Name: "Invalid bind mount",
ExpectedError: errors.New("invalid bind mount: /data:/data:/data"),
ContainerRequest: ContainerRequest{
Image: "redis:latest",
HostConfigModifier: func(hc *container.HostConfig) {
hc.Binds = []string{"/data:/data:/data"}
},
},
},
}
Expand Down
8 changes: 2 additions & 6 deletions docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,8 @@ import (
"github.com/testcontainers/testcontainers-go/wait"
)

var (
// Implement interfaces
_ Container = (*DockerContainer)(nil)

ErrDuplicateMountTarget = errors.New("duplicate mount target detected")
)
// Implement interfaces
var _ Container = (*DockerContainer)(nil)

const (
Bridge = "bridge" // Bridge network name (as well as driver)
Expand Down
20 changes: 8 additions & 12 deletions docker_auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ func prepareLocalRegistryWithAuth(t *testing.T) {
ctx := context.Background()
wd, err := os.Getwd()
assert.NoError(t, err)
// bindMounts {
// copyDirectoryToContainer {
req := ContainerRequest{
Image: "registry:2",
ExposedPorts: []string{"5001:5000/tcp"},
Expand All @@ -253,18 +253,14 @@ func prepareLocalRegistryWithAuth(t *testing.T) {
"REGISTRY_AUTH_HTPASSWD_PATH": "/auth/htpasswd",
"REGISTRY_STORAGE_FILESYSTEM_ROOTDIRECTORY": "/data",
},
Mounts: ContainerMounts{
ContainerMount{
Source: GenericBindMountSource{
HostPath: fmt.Sprintf("%s/testdata/auth", wd),
},
Target: "/auth",
Files: []ContainerFile{
{
HostFilePath: fmt.Sprintf("%s/testdata/auth", wd),
ContainerFilePath: "/auth",
},
ContainerMount{
Source: GenericBindMountSource{
HostPath: fmt.Sprintf("%s/testdata/data", wd),
},
Target: "/data",
{
HostFilePath: fmt.Sprintf("%s/testdata/data", wd),
ContainerFilePath: "/data",
},
},
WaitingFor: wait.ForExposedPort(),
Expand Down
10 changes: 7 additions & 3 deletions docker_mounts.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ package testcontainers
import "github.com/docker/docker/api/types/mount"

var mountTypeMapping = map[MountType]mount.Type{
MountTypeBind: mount.TypeBind,
MountTypeVolume: mount.TypeVolume,
MountTypeTmpfs: mount.TypeTmpfs,
MountTypePipe: mount.TypeNamedPipe,
}

// Deprecated: use Files or HostConfigModifier in the ContainerRequest, or copy files container APIs to make containers portable across Docker environments
// BindMounter can optionally be implemented by mount sources
// to support advanced scenarios based on mount.BindOptions
type BindMounter interface {
Expand All @@ -27,6 +27,7 @@ type TmpfsMounter interface {
GetTmpfsOptions() *mount.TmpfsOptions
}

// Deprecated: use Files or HostConfigModifier in the ContainerRequest, or copy files container APIs to make containers portable across Docker environments
type DockerBindMountSource struct {
*mount.BindOptions

Expand All @@ -35,14 +36,17 @@ type DockerBindMountSource struct {
HostPath string
}

// Deprecated: use Files or HostConfigModifier in the ContainerRequest, or copy files container APIs to make containers portable across Docker environments
func (s DockerBindMountSource) Source() string {
return s.HostPath
}

// Deprecated: use Files or HostConfigModifier in the ContainerRequest, or copy files container APIs to make containers portable across Docker environments
func (DockerBindMountSource) Type() MountType {
return MountTypeBind
}

// Deprecated: use Files or HostConfigModifier in the ContainerRequest, or copy files container APIs to make containers portable across Docker environments
func (s DockerBindMountSource) GetBindOptions() *mount.BindOptions {
return s.BindOptions
}
Expand Down Expand Up @@ -99,12 +103,12 @@ func mapToDockerMounts(containerMounts ContainerMounts) []mount.Mount {
}

switch typedMounter := m.Source.(type) {
case BindMounter:
containerMount.BindOptions = typedMounter.GetBindOptions()
case VolumeMounter:
containerMount.VolumeOptions = typedMounter.GetVolumeOptions()
case TmpfsMounter:
containerMount.TmpfsOptions = typedMounter.GetTmpfsOptions()
default:
Logger.Printf("Mount type %s is not supported by Testcontainers for Go", m.Source.Type())
}

mounts = append(mounts, containerMount)
Expand Down
51 changes: 23 additions & 28 deletions docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/container"
"github.com/docker/docker/api/types/strslice"
"github.com/docker/docker/api/types/volume"
"github.com/docker/docker/errdefs"
"github.com/docker/go-units"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -142,8 +141,13 @@ func TestContainerWithHostNetworkOptions(t *testing.T) {
gcr := GenericContainerRequest{
ProviderType: providerType,
ContainerRequest: ContainerRequest{
Image: nginxAlpineImage,
Mounts: Mounts(BindMount(absPath, "/etc/nginx/conf.d/default.conf")),
Image: nginxAlpineImage,
Files: []ContainerFile{
{
HostFilePath: absPath,
ContainerFilePath: "/etc/nginx/conf.d/default.conf",
},
},
ExposedPorts: []string{
nginxHighPort,
},
Expand Down Expand Up @@ -258,7 +262,12 @@ func TestContainerWithHostNetwork(t *testing.T) {
ContainerRequest: ContainerRequest{
Image: nginxAlpineImage,
WaitingFor: wait.ForListeningPort(nginxHighPort),
Mounts: Mounts(BindMount(absPath, "/etc/nginx/conf.d/default.conf")),
Files: []ContainerFile{
{
HostFilePath: absPath,
ContainerFilePath: "/etc/nginx/conf.d/default.conf",
},
},
HostConfigModifier: func(hc *container.HostConfig) {
hc.NetworkMode = "host"
},
Expand Down Expand Up @@ -1187,43 +1196,29 @@ func ExampleContainer_MappedPort() {
// }
}

func TestContainerCreationWithBindAndVolume(t *testing.T) {
func TestContainerCreationWithVolumeAndFileWritingToIt(t *testing.T) {
absPath, err := filepath.Abs(filepath.Join(".", "testdata", "hello.sh"))
if err != nil {
t.Fatal(err)
}
ctx, cnl := context.WithTimeout(context.Background(), 30*time.Second)
defer cnl()
// Create a Docker client.
dockerCli, err := NewDockerClientWithOpts(context.Background())
if err != nil {
t.Fatal(err)
}

// Create the volume.
vol, err := dockerCli.VolumeCreate(ctx, volume.CreateOptions{
Driver: "local",
})
if err != nil {
t.Fatal(err)
}
volumeName := vol.Name
t.Cleanup(func() {
ctx, cnl := context.WithTimeout(context.Background(), 5*time.Second)
defer cnl()
defer dockerCli.Close()
volumeName := "volumeName"

err := dockerCli.VolumeRemove(ctx, volumeName, true)
if err != nil {
t.Fatal(err)
}
})
// Create the container that writes into the mounted volume.
bashC, err := GenericContainer(ctx, GenericContainerRequest{
ProviderType: providerType,
ContainerRequest: ContainerRequest{
Image: "docker.io/bash",
Mounts: Mounts(BindMount(absPath, "/hello.sh"), VolumeMount(volumeName, "/data")),
Image: "docker.io/bash",
Files: []ContainerFile{
{
HostFilePath: absPath,
ContainerFilePath: "/hello.sh",
},
},
Mounts: Mounts(VolumeMount(volumeName, "/data")),
Cmd: []string{"bash", "/hello.sh"},
WaitingFor: wait.ForLog("done"),
},
Expand Down
113 changes: 0 additions & 113 deletions docs/features/copy_file.md

This file was deleted.

Loading

0 comments on commit 555cb76

Please sign in to comment.