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

GenericContainer tries to talk to remote registry too soon (during instance initialization) #302

Open
ldcasillas-progreso opened this issue Mar 2, 2017 · 4 comments

Comments

@ldcasillas-progreso
Copy link

ldcasillas-progreso commented Mar 2, 2017

I'm running tests on a machine that doesn't currently have connectivity to the Docker Hub. Certainly, I expect TestContainer-based cases to fail in that case. So I have written a JUnit rule to conditionally disable these test cases when a certain configuration setting has been made, and write something like this in my test case:

public static ConditionalIgnore skip =
        ConditionalIgnore.needsDocker(ConfigFactory.load("test"));

// VaultContainer is my own subclass of GenericContainer
public static VaultContainer VAULT = new VaultContainer();

@ClassRule
public static final RuleChain rules = RuleChain.outerRule(skip)
        .around(VAULT);

The problem I'm seeing is, however, that this requires instantiating the VaultContainer class, which will call the GenericContainer constructors, and those constructors will actually try to talk to the Docker Hub:

Caused by: org.testcontainers.containers.ContainerFetchException: Can't get Docker image name from org.testcontainers.images.RemoteDockerImage@4a0952ab
	at org.testcontainers.containers.GenericContainer.getDockerImageName(GenericContainer.java:711)
	at org.testcontainers.containers.GenericContainer.setDockerImageName(GenericContainer.java:699)
	at org.testcontainers.containers.GenericContainer.<init>(GenericContainer.java:145)
	at com.progressfin.foundation.secret.VaultContainer.<init>(VaultContainer.java:33)
	at com.progressfin.foundation.secret.VaultContainer.<init>(VaultContainer.java:29)
	at com.progressfin.foundation.secret.VaultSecretsTest.<clinit>(VaultSecretsTest.java:30)
	... 40 more
Caused by: org.testcontainers.shaded.com.github.dockerjava.api.exception.InternalServerErrorException: {"message":"Get https://registry-1.docker.io/v2/: read tcp 1.2.3.4:54526->4.3.2.1:443: read: connection reset by peer"}

The problem is here (line numbers don't match up because the stack trace is from 1.1.6):

There's a very interesting comment there ("Mimic old behavior where we resolve image once it's set") that tells us that this behavior is intentional. What I would suggest is that the behavior is at odds with the sort of uses that JUnit rules are meant to support; the test rule should arguably take no "risky" actions at all unless the Statement is actually invoked, i.e. in the GenericContainer.start() method, which already does call the getDockerImageName() method anyway. (And is it just me, or is it also weird that a String getter like getDockerImageName() would talk to the network?)

@bsideup bsideup added this to the 2.0 milestone Mar 2, 2017
@bsideup
Copy link
Member

bsideup commented Mar 2, 2017

@ldcasillas-progreso this is something I want to change as well, yes :) You just hit the leftover from a change to allow Dockerfiles.
However, this is a breaking change, and we have to target 2.x milestone for it.
Good news - we're going to start 2.x soon :)

As a workaround, you can use

new GenericContainer(CompletableFuture.completed("vault:1.2.3"))

public GenericContainer(@NonNull final Future<String> image) {
this.image = image;
}

@bsideup bsideup added the type/breaking-api-change Public APIs are being changed label Mar 2, 2017
@ldcasillas-progreso
Copy link
Author

@bsideup Thanks for the workaround! It definitely helped in our narrow case, but I would observe that I still see TestContainers (now upgraded to 1.1.9) doing some stuff:

2017-03-02 11:04:21:645 -0800 [main] [INFO] org.testcontainers.dockerclient.DockerClientProviderStrategy - Accessing unix domain socket via TCP proxy (/var/run/docker.sock via localhost:49553)
2017-03-02 11:04:21:647 -0800 [main] [INFO] org.testcontainers.dockerclient.DockerClientProviderStrategy - Looking for Docker environment. Tried local Unix socket (via TCP proxy)
2017-03-02 11:04:21:651 -0800 [main] [INFO] org.testcontainers.DockerClientFactory - Docker host IP address is localhost
2017-03-02 11:04:22:146 -0800 [main] [INFO] org.testcontainers.DockerClientFactory - Connected to docker: 
  Server Version: 1.13.1
  API Version: 1.26
  Operating System: Alpine Linux v3.5
  Total Memory: 7973 MB
2017-03-02 11:04:23:531 -0800 [main] [INFO] org.testcontainers.DockerClientFactory - Disk utilization in Docker environment is unknown (unknown available )
2017-03-02 11:04:23:547 -0800 [main] [INFO] com.progressfin.foundation.utils.test.ConditionalIgnore - Skipping test case: com.progressfin.foundation.secret.VaultSecretsTest

I haven't tried running in a system without Docker, so I didn't establish whether this would cause a failure in that case.

@rnorth
Copy link
Member

rnorth commented Jun 4, 2017

Relates to #343

@stale
Copy link

stale bot commented Oct 28, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you believe this is a mistake, please reply to this comment to keep it open. If there isn't one already, a PR to fix or at least reproduce the problem in a test case will always help us get back on track to tackle this.

@stale stale bot added the stale label Oct 28, 2018
@stale stale bot removed the stale label Nov 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants