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

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

Closed
kernle32dll opened this issue Jul 26, 2022 · 9 comments
Closed

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

kernle32dll opened this issue Jul 26, 2022 · 9 comments

Comments

@kernle32dll
Copy link

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

@kiview
Copy link
Member

kiview commented Jul 26, 2022

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.

@kernle32dll
Copy link
Author

kernle32dll commented Jul 26, 2022

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

@kiview
Copy link
Member

kiview commented Jul 26, 2022

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

kiview commented Jul 28, 2022

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

@fedinskiy
Copy link

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] #2088 (comment)
[3] containers/podman#15306

@kiview
Copy link
Member

kiview commented Oct 18, 2022

@fedinskiy did the documentation solution as mentioned by the Podman team work for you? containers/podman#15306 (comment)

@fedinskiy
Copy link

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?

@kiview
Copy link
Member

kiview commented Oct 19, 2022

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 #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.

@eddumelendez
Copy link
Member

Fixed via #6174 and it will be part of the next release.

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