Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: support disabling reaper at the configuration level #561

Closed
wants to merge 26 commits into from
Closed
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
4cda3b2
chore: extract properties to a separate file
mdelapenya Oct 5, 2022
0b4476c
chore: consistency about project name
mdelapenya Oct 5, 2022
ff730fa
chore: use testing package to set the environment in tests
mdelapenya Oct 5, 2022
4bff11f
chore: use testing package to create temp dirs
mdelapenya Oct 5, 2022
e9fddec
chore: convert tc properties into a pointer
mdelapenya Oct 5, 2022
061c71a
chore: initialize tc properties just once
mdelapenya Oct 5, 2022
8e7fb0f
chore: move properties to an internal package
mdelapenya Oct 5, 2022
8bcb82b
fix: use pointers in tests
mdelapenya Oct 5, 2022
0f222e8
chore: simplify
mdelapenya Oct 5, 2022
fa34199
fix: check if the input is a valid bool
mdelapenya Oct 5, 2022
9d23de6
feat: support for setting if ryuk is enabled at the properties/env level
mdelapenya Oct 5, 2022
bc55cdf
chore: deprecate SkipReaper in favor of the global config
mdelapenya Oct 6, 2022
74bf58e
chore: do not return the tc properties
mdelapenya Oct 7, 2022
6ae0d97
chore: do not expose the docker client
mdelapenya Oct 7, 2022
819f15d
chore: optimise singleton for reaper
mdelapenya Oct 7, 2022
784f8ed
chore: skip reaper for reaper
mdelapenya Oct 7, 2022
358ae7c
fix: connect to the reaper immediately
mdelapenya Oct 7, 2022
66bd04d
docs: document the new properties
mdelapenya Oct 7, 2022
750582f
fix: clean up container properly
mdelapenya Oct 10, 2022
a323581
fix: clean up container properly even more
mdelapenya Oct 10, 2022
1995609
fix: wrong copy paste in docs
mdelapenya Oct 10, 2022
7cdb1a5
chore: use constant for Nginx test container
mdelapenya Oct 10, 2022
57a06e1
chore: run tests with and without the reaper enabled
mdelapenya Oct 10, 2022
e01ae00
chore: skip tests if reaper is disabled
mdelapenya Oct 10, 2022
15b6462
fix: create properties on linux, only
mdelapenya Oct 10, 2022
5f655d4
Merge branch 'main' into refactor-ryuk
mdelapenya Oct 13, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions compose_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ func TestLocalDockerComposeWithVolume(t *testing.T) {
}

func assertVolumeDoesNotExist(tb testing.TB, volumeName string) {
containerClient, _, _, err := NewDockerClient()
containerClient, _, err := newDockerClient()
if err != nil {
tb.Fatalf("Failed to get provider: %v", err)
}
Expand All @@ -468,7 +468,7 @@ func assertContainerEnvironmentVariables(
present map[string]string,
absent map[string]string,
) {
containerClient, _, _, err := NewDockerClient()
containerClient, _, err := newDockerClient()
if err != nil {
tb.Fatalf("Failed to get provider: %v", err)
}
Expand Down
54 changes: 28 additions & 26 deletions container.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/docker/docker/pkg/archive"
"github.com/docker/go-connections/nat"

"github.com/testcontainers/testcontainers-go/internal/properties"
"github.com/testcontainers/testcontainers-go/wait"
)

Expand All @@ -30,7 +31,7 @@ type ContainerProvider interface {
ReuseOrCreateContainer(context.Context, ContainerRequest) (Container, error) // reuses a container if it exists or creates a container without starting
RunContainer(context.Context, ContainerRequest) (Container, error) // create a container and start it
Health(context.Context) error
Config() TestContainersConfig
Config() *properties.TestcontainersConfig
}

// Container allows getting info about and controlling a single container instance
Expand Down Expand Up @@ -90,31 +91,32 @@ 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 // alternative reaper image
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.
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
// Deprecated: The reaper is globally controlled by the .testcontainers.properties file or the TESTCONTAINERS_RYUK_DISABLED environment variable
SkipReaper bool
ReaperImage string // alternative reaper image
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)
}
Expand Down
85 changes: 18 additions & 67 deletions docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ import (
"github.com/docker/docker/pkg/jsonmessage"
"github.com/docker/go-connections/nat"
"github.com/google/uuid"
"github.com/magiconair/properties"
"github.com/moby/term"
specs "github.com/opencontainers/image-spec/specs-go/v1"

"github.com/testcontainers/testcontainers-go/internal/properties"
"github.com/testcontainers/testcontainers-go/wait"
)

Expand Down Expand Up @@ -649,19 +649,11 @@ type DockerProvider struct {
client *client.Client
host string
hostCache string
config TestContainersConfig
config *properties.TestcontainersConfig
}

var _ ContainerProvider = (*DockerProvider)(nil)

// or through Decode
type TestContainersConfig struct {
Host string `properties:"docker.host,default="`
TLSVerify int `properties:"docker.tls.verify,default=0"`
CertPath string `properties:"docker.cert.path,default="`
RyukPrivileged bool `properties:"ryuk.container.privileged,default=false"`
}

type (
// DockerProviderOptions defines options applicable to DockerProvider
DockerProviderOptions struct {
Expand Down Expand Up @@ -705,8 +697,8 @@ func WithDefaultBridgeNetwork(bridgeNetworkName string) DockerProviderOption {
})
}

func NewDockerClient() (cli *client.Client, host string, tcConfig TestContainersConfig, err error) {
tcConfig = configureTC()
func newDockerClient() (cli *client.Client, host string, err error) {
tcConfig := properties.Get()

host = tcConfig.Host

Expand All @@ -731,12 +723,12 @@ func NewDockerClient() (cli *client.Client, host string, tcConfig TestContainers
cli, err = client.NewClientWithOpts(opts...)

if err != nil {
return nil, "", TestContainersConfig{}, err
return nil, "", err
}

cli.NegotiateAPIVersion(context.Background())

return cli, host, tcConfig, nil
return cli, host, nil
}

// NewDockerProvider creates a Docker provider with the EnvClient
Expand All @@ -751,7 +743,7 @@ func NewDockerProvider(provOpts ...DockerProviderOption) (*DockerProvider, error
provOpts[idx].ApplyDockerTo(o)
}

c, host, tcConfig, err := NewDockerClient()
c, host, err := newDockerClient()
if err != nil {
return nil, err
}
Expand All @@ -770,7 +762,7 @@ func NewDockerProvider(provOpts ...DockerProviderOption) (*DockerProvider, error
DockerProviderOptions: o,
host: host,
client: c,
config: tcConfig,
config: properties.Get(),
}

// log docker server info only once
Expand All @@ -797,40 +789,6 @@ func (p *DockerProvider) logDockerServerInfo() {
info.OperatingSystem, info.MemTotal/1024/1024)
}

// configureTC reads from testcontainers properties file, if it exists
// it is possible that certain values get overridden when set as environment variables
func configureTC() TestContainersConfig {
config := TestContainersConfig{}

applyEnvironmentConfiguration := func(config TestContainersConfig) TestContainersConfig {
ryukPrivilegedEnv := os.Getenv("TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED")
if ryukPrivilegedEnv != "" {
config.RyukPrivileged = ryukPrivilegedEnv == "true"
}

return config
}

home, err := os.UserHomeDir()
if err != nil {
return applyEnvironmentConfiguration(config)
}

tcProp := filepath.Join(home, ".testcontainers.properties")
// init from a file
properties, err := properties.LoadFile(tcProp, properties.UTF8)
if err != nil {
return applyEnvironmentConfiguration(config)
}

if err := properties.Decode(&config); err != nil {
fmt.Printf("invalid testcontainers properties file, returning an empty Testcontainers configuration: %v\n", err)
return applyEnvironmentConfiguration(config)
}

return applyEnvironmentConfiguration(config)
}

// BuildImage will build and image from context and Dockerfile, then return the tag
func (p *DockerProvider) BuildImage(ctx context.Context, img ImageBuildInfo) (string, error) {
repo := uuid.New()
Expand Down Expand Up @@ -920,15 +878,14 @@ func (p *DockerProvider) CreateContainer(ctx context.Context, req ContainerReque
sessionID := uuid.New()

var termSignal chan bool
if !req.SkipReaper {
// the reaper does not need to start a reaper for itself
if !properties.Get().RyukDisabled && !strings.EqualFold(req.Image, reaperImage(req.ReaperImage)) {
r, err := NewReaper(context.WithValue(ctx, dockerHostContextKey, p.host), sessionID.String(), p, req.ReaperImage)
if err != nil {
return nil, fmt.Errorf("%w: creating reaper failed", err)
}
termSignal, err = r.Connect()
if err != nil {
return nil, fmt.Errorf("%w: connecting to reaper failed", err)
}
termSignal = r.termSignal

for k, v := range r.Labels() {
if _, ok := req.Labels[k]; !ok {
req.Labels[k] = v
Expand Down Expand Up @@ -1134,15 +1091,12 @@ func (p *DockerProvider) ReuseOrCreateContainer(ctx context.Context, req Contain

sessionID := uuid.New()
var termSignal chan bool
if !req.SkipReaper {
if !properties.Get().RyukDisabled {
r, err := NewReaper(ctx, sessionID.String(), p, req.ReaperImage)
if err != nil {
return nil, fmt.Errorf("%w: creating reaper failed", err)
}
termSignal, err = r.Connect()
if err != nil {
return nil, fmt.Errorf("%w: connecting to reaper failed", err)
}
termSignal = r.termSignal
}
dc := &DockerContainer{
ID: c.ID,
Expand Down Expand Up @@ -1209,9 +1163,9 @@ func (p *DockerProvider) RunContainer(ctx context.Context, req ContainerRequest)
return c, nil
}

// Config provides the TestContainersConfig read from $HOME/.testcontainers.properties or
// Config provides the TestcontainersConfig read from $HOME/.testcontainers.properties or
// the environment variables
func (p *DockerProvider) Config() TestContainersConfig {
func (p *DockerProvider) Config() *properties.TestcontainersConfig {
return p.config
}

Expand Down Expand Up @@ -1288,15 +1242,12 @@ func (p *DockerProvider) CreateNetwork(ctx context.Context, req NetworkRequest)
sessionID := uuid.New()

var termSignal chan bool
if !req.SkipReaper {
if !properties.Get().RyukDisabled {
r, err := NewReaper(context.WithValue(ctx, dockerHostContextKey, p.host), sessionID.String(), p, req.ReaperImage)
if err != nil {
return nil, fmt.Errorf("%w: creating network reaper failed", err)
}
termSignal, err = r.Connect()
if err != nil {
return nil, fmt.Errorf("%w: connecting to network reaper failed", err)
}
termSignal = r.termSignal
for k, v := range r.Labels() {
if _, ok := req.Labels[k]; !ok {
req.Labels[k] = v
Expand Down
Loading