testcontainers-go icon indicating copy to clipboard operation
testcontainers-go copied to clipboard

[Enhancement]: Add ability to specify dockerProvider for NewDockerComposeWith to be able to disable reaper

Open quolpr opened this issue 1 year ago • 15 comments

Proposal

The problem: on dev machine I want to have tests to run fast. We are using docker compose way to prepare test env, and after test run all containers are deleting. I actually want to keep them running, and to achieve this I need to specify env variables to disabled reaper. It's actually not comfortable way, and I want to be able to specify ryuk config manually.

What I suggest is to add to composeStackOptions new field dockerProvider *testcontainers.DockerProvider + new opt WithDockerProvider(provider *testcontainers.DockerProvider). So then I can call it with:

pr, err := testcontainers.NewDockerProvider()
if err != nil {
  return err
}
c := pr.Config().Config
c.RyukDisabled = true

tc.NewDockerComposeWith(tc.WithDockerProvider(pr))

quolpr avatar Jul 24 '24 08:07 quolpr

Hi, you can disable the reaper at the properties level https://golang.testcontainers.org/features/configuration/#customizing-ryuk-the-resource-reaper. Is that enough for your use case?

mdelapenya avatar Jul 25 '24 22:07 mdelapenya

@mdelapenya the problem is that I can't pass any configuration objects to NewDockerComposeWith. The only way is to pass env variables like TESTCONTAINERS_RYUK_DISABLED=true, which is not comfortable way for dev env. For sure I can make make script, but I will also need to set it for golang test runner/vscode test runner too. And ask to do this for the whole team.

The same for property file. I will need to ask the whole team to create this file.

So, it's better to have ability to configure the other default value without env way

quolpr avatar Jul 26 '24 07:07 quolpr

Also, other problem with property file is that it is global. When I put ryuk.disabled other project start disabling ryuk too, even those that don't use compose way.

@mdelapenya do you have any concerns with configurating doker provider? Otherwise, I could work on it

quolpr avatar Jul 29 '24 16:07 quolpr

We are planning to remove the DockerProvider abstraction in https://github.com/testcontainers/testcontainers-go/tree/v1. Please take a look and let me know how you feel with that code.

For historical reasons, https://github.com/testcontainers/testcontainers-go/pull/941, we removed the ability to skip the reaper by container, as we want a centralised way to do it, like in the rest of the testcontainers language libraries.

For your use case, I'd recommend setting up the environment at the project level in your CI, or setting it up in your build file (Make? Taskfile?...)

mdelapenya avatar Aug 05 '24 14:08 mdelapenya

I'm in a similar boat- we've gone through a lot of effort to make the go test . experience as vanilla as possible for our team- which makes things like the testing integrations in IDEs work out of the box. Newcomers can be onboarded very quickly because theres no additional config/services to worry about.

We're having trouble with not being able to control the ryuk/reaper params programmatically. We want to do things like extend the reconnection timeout from 10s -> 30s, force privileged=true (podman requires it), etc- but it's frustrating to instruct everyone to add a ~/.testcontainers.properties or add env-var config to their IDE test running config. Moreover, if we need to tweak these in the future, we have to instruct everyone to update their config, which isn't ideal.

I tried to work around these limitations by setting the env from my test

os.Setenv("TESTCONTAINERS_RYUK_RECONNECTION_TIMEOUT", "30s")

however, there's a package-level init() function in logger.go that triggers the readOnce config https://github.com/testcontainers/testcontainers-go/blob/b78a3512ff1e76f90f9b0bbd8b1795fb6be51345/logger.go#L18, so the config is already locked by the time any of my code runs.

Do you foresee a way around this?

seanlafferty-ibm avatar Aug 06 '24 11:08 seanlafferty-ibm

How would you see having a properties file in the project root dir that is versioned alongside the project and takes precedence over the user's home file? The precedence chain would be:

EnvVars < project properties file < home properties

Wdyt?

mdelapenya avatar Aug 22 '24 12:08 mdelapenya

@quolpr reading your initial request more carefully:

We are using docker compose way to prepare test env, and after test run all containers are deleting. I actually want to keep them running, and to achieve this I need to specify env variables to disabled reaper.

I think you are more interested in the reuse mode I guess. Setting a container name (this will eventually change) and the Reuse field in the container request struct, you tell Ryuk to not remove a container. Is that what you need?

mdelapenya avatar Aug 26 '24 15:08 mdelapenya

@mdelapenya hmm, I will check, thanks!

As for

EnvVars < project properties file < home properties

That sounds good actually 🙂

quolpr avatar Aug 26 '24 15:08 quolpr

For reference, the reusable docs: https://golang.testcontainers.org/features/creating_container/#reusable-container

mdelapenya avatar Aug 26 '24 15:08 mdelapenya

@mdelapenya but is it possible to use reusable container with docker compose? Here how I make up now:

cmp, err := tc.NewDockerComposeWith(tc.WithStackReaders(bytes.NewReader(dockerCompose)), tc.StackIdentifier("mag_collect_test"))

if err != nil {
	return
}

err = cmp.Up(
	ctx,
	tc.WithRecreate(api.RecreateNever),
	tc.WithRecreateDependencies(api.RecreateNever),
	tc.Wait(true),
)

Not sure how to pass to compose container that they should be reusable

quolpr avatar Aug 27 '24 15:08 quolpr

No, reusable mode is. not affecting to compose containers, although it could be a desired feature request.

mdelapenya avatar Aug 27 '24 16:08 mdelapenya

@mdelapenya oh yeah, it seems I need this.

Btw, my initial issue was connected with compose, that I can't disable ryuk with it 🙂. And yeah, reusable will help with it

quolpr avatar Aug 27 '24 16:08 quolpr

Btw, as temporary workaround I made this:

package docker

import (
	"bytes"
	"context"
	_ "embed"
	"fmt"
	"os"
	"sync"

	"github.com/docker/compose/v2/pkg/api"
	tc "github.com/testcontainers/testcontainers-go/modules/compose"
)

var (
	containerInit sync.Once
	compose       tc.ComposeStack
	errCompose    error
)

//go:embed docker-compose.yml
var dockerCompose []byte

func init() {
	// https://github.com/testcontainers/testcontainers-go/issues/2669#issuecomment-2271056446
	os.Setenv("TESTCONTAINERS_RYUK_DISABLED", "true")
}

func startCompose(ctx context.Context) (tc.ComposeStack, error) {
	containerInit.Do(func() {
		cmp, err := tc.NewDockerComposeWith(tc.WithStackReaders(bytes.NewReader(dockerCompose)), tc.StackIdentifier("mag_collect_test"))

		if err != nil {
			return
		}

		err = cmp.Up(
			ctx,
			tc.WithRecreate(api.RecreateNever),
			tc.WithRecreateDependencies(api.RecreateNever),
			tc.Wait(true),
		)

		if err != nil {
			return
		}

		compose = cmp
		errCompose = err
	})

	if errCompose != nil {
		return nil, fmt.Errorf("failed to start compose: %w", errCompose)
	}

	return compose, nil
}

Set env variable in init

quolpr avatar Aug 27 '24 16:08 quolpr

How would you see having a properties file in the project root dir that is versioned alongside the project and takes precedence over the user's home file? The precedence chain would be:

EnvVars < project properties file < home properties

Wdyt?

I think this would be a great enhancement, as it could guarantee any newcomers to our project have an out-of-the-box experience that "just works" 🙂 . I could spin off a separate issue for that if we think it's a good idea.

seanlafferty-ibm avatar Aug 28 '24 12:08 seanlafferty-ibm

Envvars can now be set from within go without them getting nuked by the package init https://github.com/testcontainers/testcontainers-go/pull/2725

seanlafferty-ibm avatar Sep 28 '24 01:09 seanlafferty-ibm