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

[Bug]: library/mysql is not a valid substitute for mysql

Open kernle32dll opened this issue 3 years ago • 8 comments

Module

MySQL

Testcontainers version

1.17.3

Using the latest Testcontainers version?

Yes

Host OS

Linux

Host Arch

x86

Docker version

Client:
 Version:           20.10.17
 API version:       1.41
 Go version:        go1.18.3
 Git commit:        100c70180f
 Built:             Sat Jun 11 23:27:28 2022
 OS/Arch:           linux/amd64
 Context:           default
 Experimental:      true

Server:
 Engine:
  Version:          20.10.17
  API version:      1.41 (minimum version 1.12)
  Go version:       go1.18.3
  Git commit:       a89b84221c
  Built:            Sat Jun 11 23:27:14 2022
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          v1.6.6
  GitCommit:        10c12954828e7c7c9b6e0ea9b0c02b01407d3ae1.m
 runc:
  Version:          1.1.3
  GitCommit:        
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0

What happened?

We are using new MySQLContainer<>("mysql:8.0.16") to create a mysql container locally. All is working fine.

However, in our CI we now introduced a Docker proxy via Nexus. Since we don't want to use the proxy locally, but only in the CI, we integrated it via an environment variable TESTCONTAINERS_HUB_IMAGE_NAME_PREFIX: "registry-proxy.awesomecorp.eu/".

This did not work, as Nexus internally stores the mysql image not as mysql, but library/mysql (which is the same, e.g. you can do docker pull library/mysql:latest on your machine right now, and it will work). So we adjusted the code to: new MySQLContainer<>("library/mysql:8.0.16"). But now the test fails at startup with Failed to verify that image 'library/mysql:8.0.16' is a compatible substitute for 'mysql'.. It provides the asCompatibleSubstituteFor method for a fix, and that works. However, I would argue it should not be necessary, as its essentially the same image.

Alas, DockerImageName.parse("mysql:8.0.16").equal(DockerImageName.parse("library/mysql:8.0.16")) should be true.

Relevant log output

No response

Additional Information

No response

kernle32dll avatar Jul 26 '22 10:07 kernle32dll

Hey @kernle32dll, thanks for reporting. You are right, mysql is an alias for library/mysql, with library/ being a very special case in the context of Docker Hub and Docker CLI. Using asCompatibleSubstituteFor was a valid workaround for you, wasn't it?

I am not yet sure if we want to add a special case for DockerImageName.parse("mysql:8.0.16").equal(DockerImageName.parse("library/mysql:8.0.16")), but it might be the expected behavior, you are right.

kiview avatar Jul 26 '22 10:07 kiview

Using asCompatibleSubstituteFor was a valid workaround for you, wasn't it?

Yeah.

I think it comes down to a question of usability. One could argue that testcontainers should use images with library/ prefixed in the first place, but I don't really want to open that discussion here and now :D

kernle32dll avatar Jul 26 '22 13:07 kernle32dll

All good, I think you have a point here. We just need to ponder if such a change will have other unintended sideffects and particularly, how well it will play with other Docker-API compatible container engines, that are not Docker (thinking of podman here).

kiview avatar Jul 26 '22 14:07 kiview

In a way, I think this is related to #5275 as well and should be tackled in conjunction.

kiview avatar Jul 28 '22 10:07 kiview

I would like to point out, that this issue is not specific for mysql, but for any(most?) fully-qualified names. I checked Localstack and Nginx and got errors Failed to verify that image 'docker.io/localstack/localstack' is a compatible substitute for 'localstack/localstack'. and Failed to verify that image 'docker.io/library/nginx' is a compatible substitute for 'nginx' for both cases respectively. This is especially a problem for podman, since I receive NPE[1] when running with a partial name, see this comment[2] for a similar issue. This is probably related to this issue[3] in podman project.

[1]

Caused by: org.testcontainers.containers.ContainerFetchException: Can't get Docker image: RemoteDockerImage(imageName=nginx:latest, imagePullPolicy=DefaultPullPolicy(), imageNameSubstitutor=org.testcontainers.utility.ImageNameSubstitutor$LogWrappedImageNameSubstitutor@4215838f)
	at org.testcontainers.containers.GenericContainer.getDockerImageName(GenericContainer.java:1371)
	at org.testcontainers.containers.GenericContainer.logger(GenericContainer.java:651)
	at org.testcontainers.containers.GenericContainer.doStart(GenericContainer.java:331)
	... 27 more
Caused by: java.lang.NullPointerException
	at com.github.dockerjava.api.command.PullImageResultCallback.checkForDockerSwarmResponse(PullImageResultCallback.java:48)
	at com.github.dockerjava.api.command.PullImageResultCallback.onNext(PullImageResultCallback.java:35)
	at org.testcontainers.images.LoggedPullImageResultCallback.onNext(LoggedPullImageResultCallback.java:48)
	at org.testcontainers.images.TimeLimitedLoggedPullImageResultCallback.onNext(TimeLimitedLoggedPullImageResultCallback.java:73)
	at org.testcontainers.images.TimeLimitedLoggedPullImageResultCallback.onNext(TimeLimitedLoggedPullImageResultCallback.java:24)
	at org.testcontainers.shaded.com.github.dockerjava.core.exec.AbstrAsyncDockerCmdExec$1.onNext(AbstrAsyncDockerCmdExec.java:41)
	at org.testcontainers.shaded.com.github.dockerjava.core.DefaultInvocationBuilder$JsonSink.accept(DefaultInvocationBuilder.java:315)
	at org.testcontainers.shaded.com.github.dockerjava.core.DefaultInvocationBuilder$JsonSink.accept(DefaultInvocationBuilder.java:298)
	at org.testcontainers.shaded.com.github.dockerjava.core.DefaultInvocationBuilder.lambda$executeAndStream$1(DefaultInvocationBuilder.java:275)
	at java.base/java.lang.Thread.run(Thread.java:829)


[2] https://github.com/testcontainers/testcontainers-java/issues/2088#issuecomment-1273456194 [3] https://github.com/containers/podman/issues/15306

fedinskiy avatar Oct 17 '22 14:10 fedinskiy

@fedinskiy did the documentation solution as mentioned by the Podman team work for you? https://github.com/containers/podman/issues/15306#issuecomment-1282109732

kiview avatar Oct 18 '22 10:10 kiview

I uncommented compat_api_enforce_docker_hub = true in /usr/share/containers/containers.conf and it eased a problem a bit. There is another problem(test-containers is falling during image pull), but I presume it's worth a separate issue :)

At the same time, I still think, that test-containers should consider docker.io/localstack/localstack to be a compatible substitute for localstack/localstack. If you are not outright opposed to the idea, I can prepare a PR with that change in a couple of days(probably even today). WDYT?

fedinskiy avatar Oct 19 '22 07:10 fedinskiy

I am not opposed to it in general, but I feel that instead of sprinkling the code with making docker.io prefixed image names, we need more of an architectural solution to the codebase as a whole, especially since it also relates to https://github.com/testcontainers/testcontainers-java/issues/5275.

As you can see in this issue, besides the docker.io default registry topic, there is also the implicit behavior regarding the library/ prefixing for Docker Hub images.

kiview avatar Oct 19 '22 07:10 kiview

Fixed via https://github.com/testcontainers/testcontainers-java/pull/6174 and it will be part of the next release.

eddumelendez avatar Feb 10 '23 23:02 eddumelendez