From 34481cf9027b79aaad4f6aa2dbdb7091dd9c49fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Thu, 16 Feb 2023 12:29:19 +0100 Subject: [PATCH] chore: move host-config and endpoint settings to a specific modifiers (#633) * feat: support preconfiguring Docker's host config and endpoint settings before container creation * chore: deprecate ExtraHosts from the container request * chore: deprecate Binds from the container request * chore: deprecate Tmpfs from the container request * chore: deprecate AutoRemove from the container request * chore: deprecate Privileged from the container request * chore: deprecate Resources from the container request * chore: deprecate ShmSize from the container request * chore: deprecate CapAdd from the container request * chore: deprecate CapDrop from the container request * chore: deprecate NetworkMode from the container request * chore: consistent name for endpointSettings variable * chore: rename callback to hook Callback suggests execution after an operation. Using hook following what git does with commit hooks * chore: extract default pre-creation hook to a function * fix: wording * chore: extract preCreation code to a function * chore: rename methods to use modifier * chore: extract life cycle to a separate file Preparation for the future * fix: update comments * chore: push Tmpfs back to layer 1 * chore: push Privileged back to layer 1 * fix: remove outdated comments * chore: bring ShmSize back to the first layer * chore: separate concerns for modifiers * fix: typo in variable * chore: add modifier for Docker config In the case of advanced setups, a user would be able to modify anything in the Docker config * chore: unit tests for the preCreateHook * fix: check for the existence of aliases when there are multiple networks * chore: adjust error messages in tests * chore: include mounts in the unit tests * fix: handle error in tests * fix: handle error in tests * chore: execute modifiers the last * chore: rename file * Revert "chore: execute modifiers the last" This reverts commit f1b6e26659a8e357ab450189d7c4f0294765f388. * docs: document the modifiers * fix: typo --- container.go | 64 +++--- docker.go | 78 ++------ docker_test.go | 74 ++++--- docs/features/creating_container.md | 11 + lifecycle.go | 88 ++++++++ lifecycle_test.go | 299 ++++++++++++++++++++++++++++ reaper.go | 13 +- reaper_test.go | 39 +++- 8 files changed, 532 insertions(+), 134 deletions(-) create mode 100644 lifecycle.go create mode 100644 lifecycle_test.go diff --git a/container.go b/container.go index 70146ab9a2..c2a7c9f35e 100644 --- a/container.go +++ b/container.go @@ -10,6 +10,7 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/network" "github.com/docker/docker/pkg/archive" "github.com/docker/go-connections/nat" @@ -95,36 +96,39 @@ type ContainerFile struct { // ContainerRequest represents the parameters used to get a running container type ContainerRequest struct { FromDockerfile - Image string - Entrypoint []string - Env map[string]string - ExposedPorts []string // allow specifying protocol info - Cmd []string - Labels map[string]string - Mounts ContainerMounts - Tmpfs map[string]string - RegistryCred string - WaitingFor wait.Strategy - Name string // for specifying container name - Hostname string - ExtraHosts []string - Privileged bool // for starting privileged container - Networks []string // for specifying network names - NetworkAliases map[string][]string // for specifying network aliases - NetworkMode container.NetworkMode - Resources container.Resources - Files []ContainerFile // files which will be copied when container starts - User string // for specifying uid:gid - SkipReaper bool // indicates whether we skip setting up a reaper for this - ReaperImage string // Deprecated: use WithImageName ContainerOption instead. Alternative reaper image - ReaperOptions []ContainerOption // options for the reaper - AutoRemove bool // if set to true, the container will be removed from the host when stopped - AlwaysPullImage bool // Always pull image - ImagePlatform string // ImagePlatform describes the platform which the image runs on. - Binds []string - ShmSize int64 // Amount of memory shared with the host (in bytes) - CapAdd []string // Add Linux capabilities - CapDrop []string // Drop Linux capabilities + Image string + Entrypoint []string + Env map[string]string + ExposedPorts []string // allow specifying protocol info + Cmd []string + Labels map[string]string + Mounts ContainerMounts + Tmpfs map[string]string + RegistryCred string + WaitingFor wait.Strategy + Name string // for specifying container name + Hostname string + ExtraHosts []string // Deprecated: Use HostConfigModifier instead + Privileged bool // For starting privileged container + Networks []string // for specifying network names + NetworkAliases map[string][]string // for specifying network aliases + NetworkMode container.NetworkMode // Deprecated: Use HostConfigModifier instead + Resources container.Resources // Deprecated: Use HostConfigModifier instead + Files []ContainerFile // files which will be copied when container starts + User string // for specifying uid:gid + SkipReaper bool // indicates whether we skip setting up a reaper for this + ReaperImage string // Deprecated: use WithImageName ContainerOption instead. Alternative reaper image + ReaperOptions []ContainerOption // options for the reaper + AutoRemove bool // Deprecated: Use HostConfigModifier instead. If set to true, the container will be removed from the host when stopped + AlwaysPullImage bool // Always pull image + ImagePlatform string // ImagePlatform describes the platform which the image runs on. + Binds []string // Deprecated: Use HostConfigModifier instead + ShmSize int64 // Amount of memory shared with the host (in bytes) + CapAdd []string // Deprecated: Use HostConfigModifier instead. Add Linux capabilities + CapDrop []string // Deprecated: Use HostConfigModifier instead. Drop Linux capabilities + ConfigModifier func(*container.Config) // Modifier for the config before container creation + HostConfigModifier func(*container.HostConfig) // Modifier for the host config before container creation + EnpointSettingsModifier func(map[string]*network.EndpointSettings) // Modifier for the network settings before container creation } type ( diff --git a/docker.go b/docker.go index 4f9fed6dee..436c72a2cc 100644 --- a/docker.go +++ b/docker.go @@ -1056,76 +1056,30 @@ func (p *DockerProvider) CreateContainer(ctx context.Context, req ContainerReque } } - exposedPorts := req.ExposedPorts - if len(exposedPorts) == 0 && !req.NetworkMode.IsContainer() { - image, _, err := p.client.ImageInspectWithRaw(ctx, tag) - if err != nil { - return nil, err - } - for p := range image.ContainerConfig.ExposedPorts { - exposedPorts = append(exposedPorts, string(p)) - } - } - - exposedPortSet, exposedPortMap, err := nat.ParsePortSpecs(exposedPorts) - if err != nil { - return nil, err - } - dockerInput := &container.Config{ - Entrypoint: req.Entrypoint, - Image: tag, - Env: env, - ExposedPorts: exposedPortSet, - Labels: req.Labels, - Cmd: req.Cmd, - Hostname: req.Hostname, - User: req.User, + Entrypoint: req.Entrypoint, + Image: tag, + Env: env, + Labels: req.Labels, + Cmd: req.Cmd, + Hostname: req.Hostname, + User: req.User, } - // prepare mounts - mounts := mapToDockerMounts(req.Mounts) - hostConfig := &container.HostConfig{ - ExtraHosts: req.ExtraHosts, - PortBindings: exposedPortMap, - Binds: req.Binds, - Mounts: mounts, - Tmpfs: req.Tmpfs, - AutoRemove: req.AutoRemove, - Privileged: req.Privileged, - NetworkMode: req.NetworkMode, - Resources: req.Resources, - ShmSize: req.ShmSize, - CapAdd: req.CapAdd, - CapDrop: req.CapDrop, - } - - endpointConfigs := map[string]*network.EndpointSettings{} - - // #248: Docker allows only one network to be specified during container creation - // If there is more than one network specified in the request container should be attached to them - // once it is created. We will take a first network if any specified in the request and use it to create container - if len(req.Networks) > 0 { - attachContainerTo := req.Networks[0] - - nw, err := p.GetNetwork(ctx, NetworkRequest{ - Name: attachContainerTo, - }) - if err == nil { - endpointSetting := network.EndpointSettings{ - Aliases: req.NetworkAliases[attachContainerTo], - NetworkID: nw.ID, - } - endpointConfigs[attachContainerTo] = &endpointSetting - } + Privileged: req.Privileged, + ShmSize: req.ShmSize, + Tmpfs: req.Tmpfs, } - networkingConfig := network.NetworkingConfig{ - EndpointsConfig: endpointConfigs, + networkingConfig := &network.NetworkingConfig{} + + err = p.preCreateContainerHook(ctx, req, dockerInput, hostConfig, networkingConfig) + if err != nil { + return nil, err } - resp, err := p.client.ContainerCreate(ctx, dockerInput, hostConfig, &networkingConfig, platform, req.Name) + resp, err := p.client.ContainerCreate(ctx, dockerInput, hostConfig, networkingConfig, platform, req.Name) if err != nil { return nil, err } diff --git a/docker_test.go b/docker_test.go index faa934d031..6dd72a0005 100644 --- a/docker_test.go +++ b/docker_test.go @@ -141,15 +141,17 @@ func TestContainerWithHostNetworkOptions(t *testing.T) { gcr := GenericContainerRequest{ ProviderType: providerType, ContainerRequest: ContainerRequest{ - Image: nginxAlpineImage, - Privileged: true, - SkipReaper: true, - NetworkMode: "host", - Mounts: Mounts(BindMount(absPath, "/etc/nginx/conf.d/default.conf")), + Image: nginxAlpineImage, + SkipReaper: true, + Mounts: Mounts(BindMount(absPath, "/etc/nginx/conf.d/default.conf")), ExposedPorts: []string{ nginxHighPort, }, + Privileged: true, WaitingFor: wait.ForListeningPort(nginxHighPort), + HostConfigModifier: func(hc *container.HostConfig) { + hc.NetworkMode = "host" + }, }, Started: true, } @@ -209,10 +211,12 @@ func TestContainerWithNetworkModeAndNetworkTogether(t *testing.T) { gcr := GenericContainerRequest{ ProviderType: providerType, ContainerRequest: ContainerRequest{ - Image: nginxImage, - SkipReaper: true, - NetworkMode: "host", - Networks: []string{"new-network"}, + Image: nginxImage, + SkipReaper: true, + Networks: []string{"new-network"}, + HostConfigModifier: func(hc *container.HostConfig) { + hc.NetworkMode = "host" + }, }, Started: true, } @@ -236,11 +240,13 @@ func TestContainerWithHostNetworkOptionsAndWaitStrategy(t *testing.T) { gcr := GenericContainerRequest{ ProviderType: providerType, ContainerRequest: ContainerRequest{ - Image: nginxAlpineImage, - SkipReaper: true, - NetworkMode: "host", - WaitingFor: wait.ForListeningPort(nginxHighPort), - Mounts: Mounts(BindMount(absPath, "/etc/nginx/conf.d/default.conf")), + Image: nginxAlpineImage, + SkipReaper: true, + WaitingFor: wait.ForListeningPort(nginxHighPort), + Mounts: Mounts(BindMount(absPath, "/etc/nginx/conf.d/default.conf")), + HostConfigModifier: func(hc *container.HostConfig) { + hc.NetworkMode = "host" + }, }, Started: true, } @@ -272,11 +278,13 @@ func TestContainerWithHostNetworkAndEndpoint(t *testing.T) { gcr := GenericContainerRequest{ ProviderType: providerType, ContainerRequest: ContainerRequest{ - Image: nginxAlpineImage, - SkipReaper: true, - NetworkMode: "host", - WaitingFor: wait.ForListeningPort(nginxHighPort), - Mounts: Mounts(BindMount(absPath, "/etc/nginx/conf.d/default.conf")), + Image: nginxAlpineImage, + SkipReaper: true, + WaitingFor: wait.ForListeningPort(nginxHighPort), + Mounts: Mounts(BindMount(absPath, "/etc/nginx/conf.d/default.conf")), + HostConfigModifier: func(hc *container.HostConfig) { + hc.NetworkMode = "host" + }, }, Started: true, } @@ -309,11 +317,13 @@ func TestContainerWithHostNetworkAndPortEndpoint(t *testing.T) { gcr := GenericContainerRequest{ ProviderType: providerType, ContainerRequest: ContainerRequest{ - Image: nginxAlpineImage, - SkipReaper: true, - NetworkMode: "host", - WaitingFor: wait.ForListeningPort(nginxHighPort), - Mounts: Mounts(BindMount(absPath, "/etc/nginx/conf.d/default.conf")), + Image: nginxAlpineImage, + SkipReaper: true, + WaitingFor: wait.ForListeningPort(nginxHighPort), + Mounts: Mounts(BindMount(absPath, "/etc/nginx/conf.d/default.conf")), + HostConfigModifier: func(hc *container.HostConfig) { + hc.NetworkMode = "host" + }, }, Started: true, } @@ -2317,8 +2327,10 @@ func TestDockerContainerResources(t *testing.T) { Image: nginxAlpineImage, ExposedPorts: []string{nginxDefaultPort}, WaitingFor: wait.ForListeningPort(nginxDefaultPort), - Resources: container.Resources{ - Ulimits: expected, + HostConfigModifier: func(hc *container.HostConfig) { + hc.Resources = container.Resources{ + Ulimits: expected, + } }, }, Started: true, @@ -2403,7 +2415,9 @@ func TestContainerCapAdd(t *testing.T) { Image: nginxAlpineImage, ExposedPorts: []string{nginxDefaultPort}, WaitingFor: wait.ForListeningPort(nginxDefaultPort), - CapAdd: []string{expected}, + HostConfigModifier: func(hc *container.HostConfig) { + hc.CapAdd = []string{expected} + }, }, Started: true, }) @@ -2547,8 +2561,10 @@ func TestNetworkModeWithContainerReference(t *testing.T) { nginxB, err := GenericContainer(ctx, GenericContainerRequest{ ProviderType: providerType, ContainerRequest: ContainerRequest{ - Image: nginxAlpineImage, - NetworkMode: container.NetworkMode(networkMode), + Image: nginxAlpineImage, + HostConfigModifier: func(hc *container.HostConfig) { + hc.NetworkMode = container.NetworkMode(networkMode) + }, }, Started: true, }) diff --git a/docs/features/creating_container.md b/docs/features/creating_container.md index 5035198775..51123afadc 100644 --- a/docs/features/creating_container.md +++ b/docs/features/creating_container.md @@ -87,6 +87,17 @@ func TestIntegrationNginxLatestReturn(t *testing.T) { } ``` +### Advanced Settings + +The aforementioned `GenericContainer` function and the `ContainerRequest` struct represent a straightforward manner to configure the containers, but you could need to create your containers with more advance settings regarding the config, host config and endpoint settings Docker types. For those more advance settings, _Testcontainers for Go_ offers a way to fully customise the container request and those internal Docker types. These customisations, called _modifiers_, will be applied just before the internal call to the Docker client to create the container. + + +[Using modifiers](../../lifecycle_test.go) inside_block:reqWithModifiers + + +!!!warning + The only special case where the modifiers are not applied last, is when there are no exposed ports in the container request and the container does not use a network mode from a container (e.g. `req.NetworkMode = container.NetworkMode("container:$CONTAINER_ID")`). In that case, _Testcontainers for Go_ will extract the ports from the underliying Docker image and export them. + ## Reusable container With `Reuse` option you can reuse an existing container. Reusing will work only if you pass an diff --git a/lifecycle.go b/lifecycle.go new file mode 100644 index 0000000000..dad8de2a9c --- /dev/null +++ b/lifecycle.go @@ -0,0 +1,88 @@ +package testcontainers + +import ( + "context" + + "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/network" + "github.com/docker/go-connections/nat" +) + +func (p *DockerProvider) preCreateContainerHook(ctx context.Context, req ContainerRequest, dockerInput *container.Config, hostConfig *container.HostConfig, networkingConfig *network.NetworkingConfig) error { + // prepare mounts + hostConfig.Mounts = mapToDockerMounts(req.Mounts) + + endpointSettings := map[string]*network.EndpointSettings{} + + // #248: Docker allows only one network to be specified during container creation + // If there is more than one network specified in the request container should be attached to them + // once it is created. We will take a first network if any specified in the request and use it to create container + if len(req.Networks) > 0 { + attachContainerTo := req.Networks[0] + + nw, err := p.GetNetwork(ctx, NetworkRequest{ + Name: attachContainerTo, + }) + if err == nil { + aliases := []string{} + if _, ok := req.NetworkAliases[attachContainerTo]; ok { + aliases = req.NetworkAliases[attachContainerTo] + } + endpointSetting := network.EndpointSettings{ + Aliases: aliases, + NetworkID: nw.ID, + } + endpointSettings[attachContainerTo] = &endpointSetting + } + } + + if req.ConfigModifier != nil { + req.ConfigModifier(dockerInput) + } + + if req.HostConfigModifier == nil { + req.HostConfigModifier = defaultHostConfigModifier(req) + } + req.HostConfigModifier(hostConfig) + + if req.EnpointSettingsModifier != nil { + req.EnpointSettingsModifier(endpointSettings) + } + + networkingConfig.EndpointsConfig = endpointSettings + + exposedPorts := req.ExposedPorts + // this check must be done after the pre-creation Modifiers are called, so the network mode is already set + if len(exposedPorts) == 0 && !hostConfig.NetworkMode.IsContainer() { + image, _, err := p.client.ImageInspectWithRaw(ctx, dockerInput.Image) + if err != nil { + return err + } + for p := range image.ContainerConfig.ExposedPorts { + exposedPorts = append(exposedPorts, string(p)) + } + } + + exposedPortSet, exposedPortMap, err := nat.ParsePortSpecs(exposedPorts) + if err != nil { + return err + } + + dockerInput.ExposedPorts = exposedPortSet + hostConfig.PortBindings = exposedPortMap + + return nil +} + +// defaultHostConfigModifier provides a default modifier including the deprecated fields +func defaultHostConfigModifier(req ContainerRequest) func(hostConfig *container.HostConfig) { + return func(hostConfig *container.HostConfig) { + hostConfig.AutoRemove = req.AutoRemove + hostConfig.CapAdd = req.CapAdd + hostConfig.CapDrop = req.CapDrop + hostConfig.Binds = req.Binds + hostConfig.ExtraHosts = req.ExtraHosts + hostConfig.NetworkMode = req.NetworkMode + hostConfig.Resources = req.Resources + } +} diff --git a/lifecycle_test.go b/lifecycle_test.go new file mode 100644 index 0000000000..5a9cad2124 --- /dev/null +++ b/lifecycle_test.go @@ -0,0 +1,299 @@ +package testcontainers + +import ( + "context" + "testing" + + "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/mount" + "github.com/docker/docker/api/types/network" + "github.com/docker/docker/api/types/strslice" + "github.com/docker/go-connections/nat" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestPreCreateModifierHook(t *testing.T) { + ctx := context.Background() + + provider, err := NewDockerProvider() + require.Nil(t, err) + + t.Run("No exposed ports", func(t *testing.T) { + // reqWithModifiers { + req := ContainerRequest{ + Image: nginxAlpineImage, // alpine image does expose port 80 + ConfigModifier: func(config *container.Config) { + config.Env = []string{"a=b"} + }, + Mounts: ContainerMounts{ + { + Source: DockerBindMountSource{ + HostPath: "/var/lib/app/data", + BindOptions: &mount.BindOptions{ + Propagation: mount.PropagationPrivate, + }, + }, + Target: "/data", + }, + }, + HostConfigModifier: func(hostConfig *container.HostConfig) { + hostConfig.PortBindings = nat.PortMap{ + "80/tcp": []nat.PortBinding{ + { + HostIP: "1", + HostPort: "2", + }, + }, + } + }, + EnpointSettingsModifier: func(endpointSettings map[string]*network.EndpointSettings) { + endpointSettings["a"] = &network.EndpointSettings{ + Aliases: []string{"b"}, + Links: []string{"link1", "link2"}, + } + }, + } + // } + + // define empty inputs to be overwritten by the pre create hook + inputConfig := &container.Config{ + Image: req.Image, + } + inputHostConfig := &container.HostConfig{} + inputNetworkingConfig := &network.NetworkingConfig{} + + err = provider.preCreateContainerHook(ctx, req, inputConfig, inputHostConfig, inputNetworkingConfig) + require.Nil(t, err) + + // assertions + + assert.Equal( + t, + []string{"a=b"}, + inputConfig.Env, + "Docker config's env should be overwritten by the modifier", + ) + assert.Equal(t, + nat.PortSet(nat.PortSet{"80/tcp": struct{}{}}), + inputConfig.ExposedPorts, + "Docker config's exposed ports should be overwritten by the modifier", + ) + assert.Equal( + t, + []mount.Mount{ + { + Type: mount.TypeBind, + Source: "/var/lib/app/data", + Target: "/data", + BindOptions: &mount.BindOptions{ + Propagation: mount.PropagationPrivate, + }, + }, + }, + inputHostConfig.Mounts, + "Host config's mounts should be mapped to Docker types", + ) + + assert.Equal(t, nat.PortMap{ + "80/tcp": []nat.PortBinding{ + { + HostIP: "", + HostPort: "", + }, + }, + }, inputHostConfig.PortBindings, + "Host config's port bindings should be overwritten by the modifier", + ) + + assert.Equal( + t, + []string{"b"}, + inputNetworkingConfig.EndpointsConfig["a"].Aliases, + "Networking config's aliases should be overwritten by the modifier", + ) + assert.Equal( + t, + []string{"link1", "link2"}, + inputNetworkingConfig.EndpointsConfig["a"].Links, + "Networking config's links should be overwritten by the modifier", + ) + }) + + t.Run("No exposed ports and network mode IsContainer", func(t *testing.T) { + req := ContainerRequest{ + Image: nginxAlpineImage, // alpine image does expose port 80 + HostConfigModifier: func(hostConfig *container.HostConfig) { + hostConfig.PortBindings = nat.PortMap{ + "80/tcp": []nat.PortBinding{ + { + HostIP: "1", + HostPort: "2", + }, + }, + } + hostConfig.NetworkMode = "container:foo" + }, + } + + // define empty inputs to be overwritten by the pre create hook + inputConfig := &container.Config{ + Image: req.Image, + } + inputHostConfig := &container.HostConfig{} + inputNetworkingConfig := &network.NetworkingConfig{} + + err = provider.preCreateContainerHook(ctx, req, inputConfig, inputHostConfig, inputNetworkingConfig) + require.Nil(t, err) + + // assertions + + assert.Equal( + t, + nat.PortSet(nat.PortSet{}), + inputConfig.ExposedPorts, + "Docker config's exposed ports should be empty", + ) + assert.Equal(t, + nat.PortMap{}, + inputHostConfig.PortBindings, + "Host config's portBinding should be empty", + ) + }) + + t.Run("Nil hostConfigModifier should apply default host config modifier", func(t *testing.T) { + req := ContainerRequest{ + Image: nginxAlpineImage, // alpine image does expose port 80 + AutoRemove: true, + CapAdd: []string{"addFoo", "addBar"}, + CapDrop: []string{"dropFoo", "dropBar"}, + Binds: []string{"bindFoo", "bindBar"}, + ExtraHosts: []string{"hostFoo", "hostBar"}, + NetworkMode: "networkModeFoo", + Resources: container.Resources{ + Memory: 2048, + NanoCPUs: 8, + }, + HostConfigModifier: nil, + } + + // define empty inputs to be overwritten by the pre create hook + inputConfig := &container.Config{ + Image: req.Image, + } + inputHostConfig := &container.HostConfig{} + inputNetworkingConfig := &network.NetworkingConfig{} + + err = provider.preCreateContainerHook(ctx, req, inputConfig, inputHostConfig, inputNetworkingConfig) + require.Nil(t, err) + + // assertions + + assert.Equal(t, req.AutoRemove, inputHostConfig.AutoRemove, "Deprecated AutoRemove should come from the container request") + assert.Equal(t, strslice.StrSlice(req.CapAdd), inputHostConfig.CapAdd, "Deprecated CapAdd should come from the container request") + assert.Equal(t, strslice.StrSlice(req.CapDrop), inputHostConfig.CapDrop, "Deprecated CapDrop should come from the container request") + assert.Equal(t, req.Binds, inputHostConfig.Binds, "Deprecated Binds should come from the container request") + assert.Equal(t, req.ExtraHosts, inputHostConfig.ExtraHosts, "Deprecated ExtraHosts should come from the container request") + assert.Equal(t, req.Resources, inputHostConfig.Resources, "Deprecated Resources should come from the container request") + }) + + t.Run("Request contains more than one network including aliases", func(t *testing.T) { + networkName := "foo" + net, err := provider.CreateNetwork(ctx, NetworkRequest{ + Name: networkName, + }) + require.Nil(t, err) + defer func() { + err := net.Remove(ctx) + if err != nil { + t.Logf("failed to remove network %s: %s", networkName, err) + } + }() + + dockerNetwork, err := provider.GetNetwork(ctx, NetworkRequest{ + Name: networkName, + }) + require.Nil(t, err) + + req := ContainerRequest{ + Image: nginxAlpineImage, // alpine image does expose port 80 + Networks: []string{networkName, "bar"}, + NetworkAliases: map[string][]string{ + "foo": {"foo1"}, // network aliases are needed at the moment there is a network + }, + } + + // define empty inputs to be overwritten by the pre create hook + inputConfig := &container.Config{ + Image: req.Image, + } + inputHostConfig := &container.HostConfig{} + inputNetworkingConfig := &network.NetworkingConfig{} + + err = provider.preCreateContainerHook(ctx, req, inputConfig, inputHostConfig, inputNetworkingConfig) + require.Nil(t, err) + + // assertions + + assert.Equal( + t, + req.NetworkAliases[networkName], + inputNetworkingConfig.EndpointsConfig[networkName].Aliases, + "Networking config's aliases should come from the container request", + ) + assert.Equal( + t, + dockerNetwork.ID, + inputNetworkingConfig.EndpointsConfig[networkName].NetworkID, + "Networking config's network ID should be retrieved from Docker", + ) + }) + + t.Run("Request contains more than one network without aliases", func(t *testing.T) { + networkName := "foo" + net, err := provider.CreateNetwork(ctx, NetworkRequest{ + Name: networkName, + }) + require.Nil(t, err) + defer func() { + err := net.Remove(ctx) + if err != nil { + t.Logf("failed to remove network %s: %s", networkName, err) + } + }() + + dockerNetwork, err := provider.GetNetwork(ctx, NetworkRequest{ + Name: networkName, + }) + require.Nil(t, err) + + req := ContainerRequest{ + Image: nginxAlpineImage, // alpine image does expose port 80 + Networks: []string{networkName, "bar"}, + } + + // define empty inputs to be overwritten by the pre create hook + inputConfig := &container.Config{ + Image: req.Image, + } + inputHostConfig := &container.HostConfig{} + inputNetworkingConfig := &network.NetworkingConfig{} + + err = provider.preCreateContainerHook(ctx, req, inputConfig, inputHostConfig, inputNetworkingConfig) + require.Nil(t, err) + + // assertions + + assert.Empty( + t, + inputNetworkingConfig.EndpointsConfig[networkName].Aliases, + "Networking config's aliases should be empty", + ) + assert.Equal( + t, + dockerNetwork.ID, + inputNetworkingConfig.EndpointsConfig[networkName].NetworkID, + "Networking config's network ID should be retrieved from Docker", + ) + }) +} diff --git a/reaper.go b/reaper.go index 76db4adb9f..cf74d6aaba 100644 --- a/reaper.go +++ b/reaper.go @@ -9,6 +9,7 @@ import ( "sync" "time" + "github.com/docker/docker/api/types/container" "github.com/docker/go-connections/nat" "github.com/testcontainers/testcontainers-go/internal" @@ -81,6 +82,8 @@ func newReaper(ctx context.Context, sessionID string, provider ReaperProvider, o listeningPort := nat.Port("8080/tcp") + tcConfig := provider.Config() + reaperOpts := containerOptions{} for _, opt := range opts { @@ -90,7 +93,6 @@ func newReaper(ctx context.Context, sessionID string, provider ReaperProvider, o req := ContainerRequest{ Image: reaperImage(reaperOpts.ImageName), ExposedPorts: []string{string(listeningPort)}, - NetworkMode: Bridge, Labels: map[string]string{ TestcontainerLabelIsReaper: "true", testcontainersdocker.LabelReaper: "true", @@ -98,9 +100,13 @@ func newReaper(ctx context.Context, sessionID string, provider ReaperProvider, o SkipReaper: true, RegistryCred: reaperOpts.RegistryCredentials, Mounts: Mounts(BindMount(dockerHost, "/var/run/docker.sock")), - AutoRemove: true, + Privileged: tcConfig.RyukPrivileged, WaitingFor: wait.ForListeningPort(listeningPort), ReaperOptions: opts, + HostConfigModifier: func(hc *container.HostConfig) { + hc.AutoRemove = true + hc.NetworkMode = Bridge + }, } // keep backwards compatibility @@ -114,9 +120,6 @@ func newReaper(ctx context.Context, sessionID string, provider ReaperProvider, o req.Labels[k] = v } - tcConfig := provider.Config() - req.Privileged = tcConfig.RyukPrivileged - // Attach reaper container to a requested network if it is specified if p, ok := provider.(*DockerProvider); ok { req.Networks = append(req.Networks, p.DefaultNetwork) diff --git a/reaper_test.go b/reaper_test.go index ba07b61306..b05b2ec0b1 100644 --- a/reaper_test.go +++ b/reaper_test.go @@ -5,6 +5,8 @@ import ( "errors" "testing" + "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/network" "github.com/docker/go-connections/nat" "github.com/stretchr/testify/assert" "github.com/testcontainers/testcontainers-go/internal" @@ -13,8 +15,10 @@ import ( ) type mockReaperProvider struct { - req ContainerRequest - config TestContainersConfig + req ContainerRequest + hostConfig *container.HostConfig + enpointSettings map[string]*network.EndpointSettings + config TestContainersConfig } var errExpected = errors.New("expected") @@ -22,6 +26,18 @@ var errExpected = errors.New("expected") func (m *mockReaperProvider) RunContainer(ctx context.Context, req ContainerRequest) (Container, error) { m.req = req + m.hostConfig = &container.HostConfig{} + m.enpointSettings = map[string]*network.EndpointSettings{} + + if req.HostConfigModifier == nil { + req.HostConfigModifier = defaultHostConfigModifier(req) + } + req.HostConfigModifier(m.hostConfig) + + if req.EnpointSettingsModifier != nil { + req.EnpointSettingsModifier(m.enpointSettings) + } + // we're only interested in the request, so instead of mocking the Docker client // we'll error out here return nil, errExpected @@ -44,11 +60,9 @@ func createContainerRequest(customize func(ContainerRequest) ContainerRequest) C testcontainersdocker.LabelLang: "go", testcontainersdocker.LabelVersion: internal.Version, }, - SkipReaper: true, - Mounts: Mounts(BindMount("/var/run/docker.sock", "/var/run/docker.sock")), - AutoRemove: true, - WaitingFor: wait.ForListeningPort(nat.Port("8080/tcp")), - NetworkMode: "bridge", + SkipReaper: true, + Mounts: Mounts(BindMount("/var/run/docker.sock", "/var/run/docker.sock")), + WaitingFor: wait.ForListeningPort(nat.Port("8080/tcp")), ReaperOptions: []ContainerOption{ WithImageName("reaperImage"), }, @@ -119,7 +133,16 @@ func Test_NewReaper(t *testing.T) { // we should have errored out see mockReaperProvider.RunContainer assert.EqualError(t, err, "expected") - assert.Equal(t, test.req, provider.req, "expected ContainerRequest doesn't match the submitted request") + assert.Equal(t, test.req.Image, provider.req.Image, "expected image doesn't match the submitted request") + assert.Equal(t, test.req.ExposedPorts, provider.req.ExposedPorts, "expected exposed ports don't match the submitted request") + assert.Equal(t, test.req.Labels, provider.req.Labels, "expected labels don't match the submitted request") + assert.Equal(t, test.req.SkipReaper, provider.req.SkipReaper, "expected skipReaper doesn't match the submitted request") + assert.Equal(t, test.req.Mounts, provider.req.Mounts, "expected mounts don't match the submitted request") + assert.Equal(t, test.req.WaitingFor, provider.req.WaitingFor, "expected waitingFor don't match the submitted request") + + // checks for reaper's preCreationCallback fields + assert.Equal(t, container.NetworkMode(Bridge), provider.hostConfig.NetworkMode, "expected networkMode doesn't match the submitted request") + assert.Equal(t, true, provider.hostConfig.AutoRemove, "expected networkMode doesn't match the submitted request") }) } }