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]: Reuse flag to start exited or paused containers #2470

Open
FredrikMorstad opened this issue Apr 7, 2024 · 2 comments · May be fixed by #2768
Open

[Enhancement]: Reuse flag to start exited or paused containers #2470

FredrikMorstad opened this issue Apr 7, 2024 · 2 comments · May be fixed by #2768
Labels
enhancement New feature or request

Comments

@FredrikMorstad
Copy link

FredrikMorstad commented Apr 7, 2024

Proposal

As of now using the reuse flag on container who's not running(but is created) will cause a the program to ping and wait for the container to be running for 15 minutes DefaultMaxElapsedTime before exiting(with ryku disabled). My proposal is to restart the container if it's not running as I think it's a natural extension of the reuse name. As stetted in the documentation With Reuse option you can reuse an existing container. . I interpret existing covering both running containers as well as exited or paused containers, so I think this will be an enhancement for the reuse flag to restart a paused or exited container.

Why is this useful?

In my case I have an interface providing the necessary functions for running a local authentication emulator, I want to keep using the same container rather than recreating the container as this image rarely changes. By extending the reuse capability we can avoid the need for calling the docker api to start the container before actually using testcontainers.

Here is the code for some context:

func (testauth *TestAuth) Start() error {
	containerPort := "9099"

	// emulator specific endpoint used to check if emulator is ready
	firebaseConfigURI := "/emulator/v1/projects/demo-project/config"

	request := testcontainers.ContainerRequest{
		Image:        "our_ghcr_image",
		ExposedPorts: []string{containerPort},
		WaitingFor:   wait.NewHTTPStrategy(firebaseConfigURI),
	}

	if testauth.reuse {
		request.Name = "our_testcontainer_name"
	}
	
	// Must add code here to check if container of name `req.Name` is paused or exited and possible restart the container for reuse to work
	
	container, err := testcontainers.GenericContainer(testauth.containerCtx, testcontainers.GenericContainerRequest{
		ContainerRequest: request,
		Started:          true,
		Reuse:            testauth.reuse,
	})

	if err != nil {
		return err
	}

	// ... some other code 
}

Some extra info

I have ryku disabled as I had a problem with it and haven't gotten around to investigate the problem. So this may not be a problem if the garbage collector cleans up in the background so when reuse is enabled it's almost always needing to create a new container. However if this is the case I still belive that this feature will improve the reuse functionality

If it turns out to be something we want to do I can create the PR as I have already gotten the bear minimum version on my computer to work

@FredrikMorstad FredrikMorstad added the enhancement New feature or request label Apr 7, 2024
@mdelapenya
Copy link
Member

Hey @FredrikMorstad I'd like to know more about your setup to understand better your use case, more specifically the number of packages using that "singleton" container.

Depending on your setup, having a TestMain per package would help you out to create a singleton instance of the container you mention, started just once per package and not being created for every test. I'd go with this approach if your codebase allows that. And I'd do that even having multiple packages consuming the same container, as each package should be decoupled from the rest, and accessing a container that out-scopes the package would mean there is "some state" elsewhere that need to be maintained.

I see that reuse could be not deterministic with its current implementation (it started as something experimental), but you know, at the moment it lands into the public API it is "production-ready" 😅 , but in any case, my response does not justify that reuse is experimental :D so please consider it as part of my learnings on how its used in the wild, and how to workaround it using Go built-in capabilities.

@mdelapenya mdelapenya linked a pull request Sep 4, 2024 that will close this issue
@mdelapenya
Copy link
Member

I think #2768 will solve this. Would appreciate your review 🙏

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

Successfully merging a pull request may close this issue.

2 participants