Skip to content

Commit

Permalink
chore: cleanup dependency on parent termination or if failed to start
Browse files Browse the repository at this point in the history
  • Loading branch information
Mathew-Estafanous committed Apr 12, 2024
1 parent cf5e0b0 commit da11872
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 36 deletions.
4 changes: 2 additions & 2 deletions container.go
Expand Up @@ -132,8 +132,8 @@ type ContainerRequest struct {
Tmpfs map[string]string
RegistryCred string // Deprecated: Testcontainers will detect registry credentials automatically
WaitingFor wait.Strategy
DependsOn []Dependency // specify other containers that must be running before starting this container.
Name string // for specifying container name
DependsOn []*ContainerDependency // specify other containers that must be running before starting this container.
Name string // for specifying container name
Hostname string
WorkingDir string // specify the working directory of the container
ExtraHosts []string // Deprecated: Use HostConfigModifier instead
Expand Down
78 changes: 56 additions & 22 deletions dependency.go
Expand Up @@ -9,28 +9,42 @@ import (
"github.com/pkg/errors"
)

type Dependency interface {
StartDependency(context.Context, string) (string, error)
type ContainerDependency struct {
Request ContainerRequest
EnvKey string
CallbackFunc func(Container)
}

type ContainerDependency struct {
request ContainerRequest
envKey string
callbackFn func(Container)
func NewContainerDependency(containerRequest ContainerRequest, envKey string) *ContainerDependency {
return &ContainerDependency{
Request: containerRequest,
EnvKey: envKey,
CallbackFunc: func(c Container) {},
}
}

func (c *ContainerDependency) WithCallback(callbackFunc func(Container)) *ContainerDependency {
c.CallbackFunc = callbackFunc
return c

}

func (c *ContainerDependency) StartDependency(ctx context.Context, network string) (string, error) {
c.request.Networks = append(c.request.Networks, network)
func (c *ContainerDependency) StartDependency(ctx context.Context, network string) (Container, error) {
c.Request.Networks = append(c.Request.Networks, network)
dependency, err := GenericContainer(ctx, GenericContainerRequest{
ContainerRequest: c.request,
ContainerRequest: c.Request,
Started: true,
})
if err != nil {
return "", err
return nil, err
}

c.callbackFn(dependency)
networkAlias, err := dependency.NetworkAliases(ctx)
c.CallbackFunc(dependency)
return dependency, nil
}

func resolveDNSName(ctx context.Context, container Container, network string) (string, error) {
networkAlias, err := container.NetworkAliases(ctx)
if err != nil {
return "", err
}
Expand All @@ -41,22 +55,35 @@ func (c *ContainerDependency) StartDependency(ctx context.Context, network strin
return aliases[0], nil
}

func NewContainerDependency(containerRequest ContainerRequest, envVar string) *ContainerDependency {
return &ContainerDependency{
request: containerRequest,
envKey: envVar,
callbackFn: func(c Container) {},
func cleanupDependency(ctx context.Context, dependencies []Container, network *DockerNetwork) error {
if network == nil {
return nil
}

for _, dependency := range dependencies {
err := dependency.Terminate(ctx)
if err != nil {
return err
}
}
return network.Remove(ctx)
}

var defaultDependencyHook = func(dockerInput *container.Config, client client.APIClient) ContainerLifecycleHooks {
var depNetwork *DockerNetwork
depContainers := make([]Container, 0)
return ContainerLifecycleHooks{
PreCreates: []ContainerRequestHook{
func(ctx context.Context, req ContainerRequest) error {
func(ctx context.Context, req ContainerRequest) (err error) {
if len(req.DependsOn) == 0 {
return nil
}
defer func() {
if err != nil {
cleanupErr := cleanupDependency(ctx, depContainers, depNetwork)
Logger.Printf("Could not cleanup dependencies after an error occured: %v", cleanupErr)
}
}()

net, err := GenericNetwork(ctx, GenericNetworkRequest{
NetworkRequest: NetworkRequest{
Expand All @@ -72,12 +99,19 @@ var defaultDependencyHook = func(dockerInput *container.Config, client client.AP
}

for _, dep := range req.DependsOn {
name, err := dep.StartDependency(ctx, depNetwork.Name)
if dep.EnvKey == "" {
return errors.New("cannot create dependency with empty environment key.")
}
container, err := dep.StartDependency(ctx, depNetwork.Name)
if err != nil {
return err
}
depContainers = append(depContainers, container)
name, err := resolveDNSName(ctx, container, depNetwork.Name)
if err != nil {
return err
}
envKey := dep.(*ContainerDependency).envKey
dockerInput.Env = append(dockerInput.Env, envKey+"="+name)
dockerInput.Env = append(dockerInput.Env, dep.EnvKey+"="+name)
}
return nil
},
Expand All @@ -92,7 +126,7 @@ var defaultDependencyHook = func(dockerInput *container.Config, client client.AP
},
PostTerminates: []ContainerHook{
func(ctx context.Context, container Container) error {
return depNetwork.Remove(ctx)
return cleanupDependency(ctx, depContainers, depNetwork)
},
},
}
Expand Down
33 changes: 23 additions & 10 deletions dependency_test.go
Expand Up @@ -12,7 +12,7 @@ import (
func Test_ContainerDependsOn(t *testing.T) {
type TestCase struct {
name string
configureDependants func(ctx context.Context, t *testing.T) []Dependency
configureDependants func(ctx context.Context, t *testing.T) []*ContainerDependency
containerRequest ContainerRequest
expectedEnv []string
expectedError string
Expand All @@ -21,14 +21,14 @@ func Test_ContainerDependsOn(t *testing.T) {
testCases := []TestCase{
{
name: "dependency is started and the dns name is passed as an environment variable",
configureDependants: func(ctx context.Context, t *testing.T) []Dependency {
configureDependants: func(ctx context.Context, t *testing.T) []*ContainerDependency {
nginxReq := ContainerRequest{
Image: nginxAlpineImage,
ExposedPorts: []string{nginxDefaultPort},
WaitingFor: wait.ForListeningPort(nginxDefaultPort),
}

return []Dependency{
return []*ContainerDependency{
NewContainerDependency(nginxReq, "FIRST_DEPENDENCY"),
NewContainerDependency(nginxReq, "SECOND_DEPENDENCY"),
}
Expand All @@ -41,13 +41,13 @@ func Test_ContainerDependsOn(t *testing.T) {
},
{
name: "container fails to start when dependency fails to start",
configureDependants: func(ctx context.Context, t *testing.T) []Dependency {
configureDependants: func(ctx context.Context, t *testing.T) []*ContainerDependency {
badReq := ContainerRequest{
Image: "bad image name",
ExposedPorts: []string{"80/tcp"},
}

return []Dependency{
return []*ContainerDependency{
NewContainerDependency(badReq, "FIRST_DEPENDENCY"),
}
},
Expand All @@ -57,6 +57,24 @@ func Test_ContainerDependsOn(t *testing.T) {
},
expectedError: "failed to create container",
},
{
name: "fails to start dependency when key is empty",
configureDependants: func(ctx context.Context, t *testing.T) []*ContainerDependency {
nginxReq := ContainerRequest{
Image: nginxAlpineImage,
ExposedPorts: []string{nginxDefaultPort},
WaitingFor: wait.ForListeningPort(nginxDefaultPort),
}

dependency := NewContainerDependency(nginxReq, "")
return []*ContainerDependency{dependency}
},
containerRequest: ContainerRequest{
Image: "curlimages/curl:8.7.1",
Entrypoint: []string{"tail", "-f", "/dev/null"},
},
expectedError: "cannot create dependency with empty environment key",
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
Expand Down Expand Up @@ -96,11 +114,6 @@ func Test_ContainerDependsOn(t *testing.T) {
require.Equal(t, 0, exitCode)
}
}

terminateContainerOnEnd(t, ctx, c)
//for _, dc := range dependantContainers {
// terminateContainerOnEnd(t, ctx, dc)
//}
})
}
}
4 changes: 2 additions & 2 deletions docker.go
Expand Up @@ -58,7 +58,7 @@ type DockerContainer struct {
// Container ID from Docker
ID string
WaitingFor wait.Strategy
DependsOn []Dependency
DependsOn []*ContainerDependency
Image string

isRunning bool
Expand Down Expand Up @@ -1123,7 +1123,7 @@ func (p *DockerProvider) CreateContainer(ctx context.Context, req ContainerReque
c := &DockerContainer{
ID: resp.ID,
WaitingFor: req.WaitingFor,
DependsOn: req.DependsOn,
DependsOn: req.DependsOn,
Image: imageName,
imageWasBuilt: req.ShouldBuildImage(),
keepBuiltImage: req.ShouldKeepBuiltImage(),
Expand Down

0 comments on commit da11872

Please sign in to comment.