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

Docker-compose expects ports to be exposed while finding containers to wait for them

Open mdelapenya opened this issue 4 years ago • 5 comments

Describe the bug

When using a docker-compose file that has no exposed ports, then it's not possible to apply any wait strategy, because it uses a port to create the filter when retrieving the running containers.

To Reproduce

Can you write a unit test to reproduce your issue?

docker-compose-no-exposed-ports.yml

version: '3'
services:
  nginx:
    image: nginx:stable-alpine
    ports:
     - "80"

Unit Test

func TestDockerComposeWithWaitStrategy_NoExposedPorts(t *testing.T) {
	path := "./testresources/docker-compose-no-exposed-ports.yml"

	identifier := strings.ToLower(uuid.New().String())

	compose := NewLocalDockerCompose([]string{path}, identifier)
	destroyFn := func() {
		err := compose.Down()
		checkIfError(t, err)
	}
	defer destroyFn()

	err := compose.
		WithCommand([]string{"up", "-d"}).
		WithExposedService("nginx_1", 9080, wait.NewHTTPStrategy("/").WithPort("80/tcp").WithStartupTimeout(10*time.Second)).
		Invoke()
	checkIfError(t, err)

	assert.Equal(t, 1, len(compose.Services))
	assert.Contains(t, compose.Services, "nginx")
}

Expected behavior

It's possible to reach the service by service name.

docker info

$ docker info
Client:
 Context:    default
 Debug Mode: false
 Plugins:
  app: Docker App (Docker Inc., v0.9.1-beta3)
  buildx: Build with BuildKit (Docker Inc., v0.5.1-docker)
  compose: Docker Compose (Docker Inc., 2.0.0-beta.1)
  scan: Docker Scan (Docker Inc., v0.8.0)

Server:
 Containers: 1
  Running: 1
  Paused: 0
  Stopped: 0
 Images: 141
 Server Version: 20.10.6
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Native Overlay Diff: true
  userxattr: false
 Logging Driver: json-file
 Cgroup Driver: cgroupfs
 Cgroup Version: 1
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local logentries splunk syslog
 Swarm: inactive
 Runtimes: io.containerd.runc.v2 io.containerd.runtime.v1.linux runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: 05f951a3781f4f2c1911b05e61c160e9c30eaa8e
 runc version: 12644e614e25b05da6fd08a38ffa0cfe1903fdec
 init version: de40ad0
 Security Options:
  seccomp
   Profile: default
 Kernel Version: 5.10.25-linuxkit
 Operating System: Docker Desktop
 OSType: linux
 Architecture: x86_64
 CPUs: 8
 Total Memory: 11.7GiB
 Name: docker-desktop
 ID: YKMI:4Y6G:M2QK:OC7L:RY5H:JONU:TFAN:NGUM:VAV5:XGOM:IIAM:Q7PU
 Docker Root Dir: /var/lib/docker
 Debug Mode: false
 HTTP Proxy: http.docker.internal:3128
 HTTPS Proxy: http.docker.internal:3128
 Registry: https://index.docker.io/v1/
 Labels:
 Experimental: false
 Insecure Registries:
  127.0.0.0/8
 Live Restore Enabled: false

mdelapenya avatar Jun 11 '21 13:06 mdelapenya

What happens if you set the second parameter to 0 in the WithExposedService call? I have used it that way before to wait for non-exposed services and it works, however never in addition to using NewHTTPStrategy, only wait.ForLog.

WithExposedService("nginx_1", 0, wait.NewHTTPStrategy("/").WithPort("80/tcp").WithStartupTimeout(10*time.Second)).

ajcasagrande avatar Aug 04 '21 04:08 ajcasagrande

Yes, for sure it's possible to pass zero, but for the sake of a clean API, I consider not forcing the client code to pass the port if not needed. Besides that, looking up the container by container name is more straight-forward than using name and port. I guess it was firstly implemented in that way because of a specific need.

Besides that, a downstream project was suffering this issue: https://github.com/apache/skywalking-infra-e2e/pull/19#discussion_r650607578

mdelapenya avatar Aug 05 '21 07:08 mdelapenya

@ajcasagrande if you agree, we can close this one. OTOH I'd be happy to discuss about any other consideration/implication you see.

Thanks!

mdelapenya avatar Aug 05 '21 07:08 mdelapenya

@ajcasagrande if you agree, we can close this one. OTOH I'd be happy to discuss about any other consideration/implication you see.

Thanks!

I think my preference would be to add a new WithService(name, waitStrategy) method that does not require the port as a parameter. That way there is no breaking change, and it avoids the confusion of passing 0 to WithExposedService.

ajcasagrande avatar Aug 05 '21 17:08 ajcasagrande

I have a local branch doing exactly that 😁

mdelapenya avatar Aug 05 '21 18:08 mdelapenya

Given #476 has been merged, I'm closing this issue. In there, it's possible to do what is requested in this issue:

compose, err := NewDockerCompose("./testresources/docker-compose-simple.yml")
assert.NoError(t, err, "NewDockerCompose()")
err = compose.
  // Appending with _1 as given in the Java Test-Containers Example
  WaitForService("mysql-1", wait.NewLogStrategy("started").WithStartupTimeout(10*time.Second).WithOccurrence(1)).
  Up(ctx, Wait(true))

mdelapenya avatar Oct 25 '22 10:10 mdelapenya