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

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

Open
quolpr opened this issue Jul 24, 2024 · 15 comments
Labels
compose Docker Compose. enhancement New feature or request

Comments

@quolpr
Copy link

quolpr commented Jul 24, 2024

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 quolpr added the enhancement New feature or request label Jul 24, 2024
@mdelapenya
Copy link
Member

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?

@quolpr
Copy link
Author

quolpr commented Jul 26, 2024

@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
Copy link
Author

quolpr commented Jul 29, 2024

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

@mdelapenya
Copy link
Member

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, #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?...)

@seanlafferty-ibm
Copy link

seanlafferty-ibm commented Aug 6, 2024

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

func init() {
, so the config is already locked by the time any of my code runs.

Do you foresee a way around this?

@mdelapenya mdelapenya added the compose Docker Compose. label Aug 22, 2024
@mdelapenya
Copy link
Member

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
Copy link
Member

@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?

@quolpr
Copy link
Author

quolpr commented Aug 26, 2024

@mdelapenya hmm, I will check, thanks!

As for

EnvVars < project properties file < home properties

That sounds good actually 🙂

@mdelapenya
Copy link
Member

@quolpr
Copy link
Author

quolpr commented Aug 27, 2024

@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

@mdelapenya
Copy link
Member

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

@quolpr
Copy link
Author

quolpr commented Aug 27, 2024

@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
Copy link
Author

quolpr commented Aug 27, 2024

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

@seanlafferty-ibm
Copy link

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
Copy link

Envvars can now be set from within go without them getting nuked by the package init #2725

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compose Docker Compose. enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants