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

[Bug]: WithAfterReadyCommand incorrect iterator scope

Open itsnotapt opened this issue 1 year ago • 3 comments

Testcontainers version

0.32.0

Using the latest Testcontainers version?

Yes

Host OS

MacOS

Host arch

ARM64

Go version

1.22.4

Docker version

Server: Docker Desktop 4.22.1 (118664)
 Engine:
  Version:          24.0.5
  API version:      1.43 (minimum version 1.12)
  Go version:       go1.20.6
  Git commit:       a61e2b4
  Built:            Fri Jul 21 20:35:38 2023
  OS/Arch:          linux/arm64
  Experimental:     false
 containerd:
  Version:          1.6.21
  GitCommit:        3dce8eb055cbb6872793272b4f20ed16117344f8
 runc:
  Version:          1.1.7
  GitCommit:        v1.1.7-0-g860f061
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0

Docker info

Server:
 Containers: 10
  Running: 9
  Paused: 0
  Stopped: 1
 Images: 24
 Server Version: 24.0.5
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Using metacopy: false
  Native Overlay Diff: true
  userxattr: false
 Logging Driver: json-file
 Cgroup Driver: cgroupfs
 Cgroup Version: 2
 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 runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: 3dce8eb055cbb6872793272b4f20ed16117344f8
 runc version: v1.1.7-0-g860f061
 init version: de40ad0
 Security Options:
  seccomp
   Profile: unconfined
  cgroupns
 Kernel Version: 5.15.49-linuxkit-pr
 Operating System: Docker Desktop
 OSType: linux
 Architecture: aarch64
 CPUs: 5
 Total Memory: 7.667GiB
 Name: docker-desktop
 ID: 8e7dabbb-25ee-4f34-9fe5-2df589d4f8b3
 Docker Root Dir: /var/lib/docker
 Debug Mode: false
 HTTP Proxy: http.docker.internal:3128
 HTTPS Proxy: http.docker.internal:3128
 No Proxy: hubproxy.docker.internal
 Experimental: false
 Insecure Registries:
  hubproxy.docker.internal:5555
  127.0.0.0/8
 Live Restore Enabled: false

What happened?

The for loop for generating the functions associated with WithAfterReadyCommand is referencing the wrong scope.

Consider this test:

func TestWithAfterReadyCommand(t *testing.T) {
	req := testcontainers.GenericContainerRequest{
		ContainerRequest: testcontainers.ContainerRequest{
			Image:      "alpine",
			Entrypoint: []string{"tail", "-f", "/dev/null"},
		},
		Started: true,
	}

	testExec := []testcontainers.Executable{
		testcontainers.NewRawCommand([]string{"touch", "/tmp/.testcontainers-1"}),
		testcontainers.NewRawCommand([]string{"touch", "/tmp/.testcontainers-2"}),
		testcontainers.NewRawCommand([]string{"touch", "/tmp/.testcontainers-3"}),
	}

	err := testcontainers.WithAfterReadyCommand(testExec...)(&req)
	require.NoError(t, err)

	c, err := testcontainers.GenericContainer(context.Background(), req)
	require.NoError(t, err)
	defer func() {
		err = c.Terminate(context.Background())
		require.NoError(t, err)
	}()

	for i := 1; i <= 3; i++ {
		_, reader, err := c.Exec(context.Background(), []string{"ls", fmt.Sprintf("/tmp/.testcontainers-%d", i)}, exec.Multiplexed())
		require.NoError(t, err)

		content, err := io.ReadAll(reader)
		require.NoError(t, err)
		assert.Equal(t, fmt.Sprintf("/tmp/.testcontainers-%d\n", i), string(content))
	}
}

This will fail with go 1.21. However, if you update to go 1.22 or use the env var GOEXPERIMENT=loopvar the test will pass.

Simple fix is to update the scope for exec at https://github.com/testcontainers/testcontainers-go/blob/70b90cce99b42b5c5e64646e9fd30471c85e0d9d/options.go#L275

exec := exec
execFn := func(ctx context.Context, c Container) error {
	_, _, err := c.Exec(ctx, exec.AsCommand(), exec.Options()...)
	return err
}

Relevant log output

No response

Additional information

No response

itsnotapt avatar Jul 23 '24 02:07 itsnotapt

Looks like WithStartupCommand has the same bug

itsnotapt avatar Jul 23 '24 02:07 itsnotapt

Bump module requirement to 1.22?

stevenh avatar Aug 08 '24 20:08 stevenh

Hi @itsnotapt thanks for reporting this. We usually test honouring the Go version matrix (current two minors: e.g. 1.22 and 1.23) so at some point we for sure were covering 1.21 but I'm afraid we did not have a test honouring the execution order.

I'd expect that users of the library are in the same Go support matrix, but I see it could not be the case. It would be super easy to fix this with your patch, although I'd like to know more about your use case and your reason to stay in 1.21.

mdelapenya avatar Sep 02 '24 12:09 mdelapenya

Hi @mdelapenya, we've updated to 1.22, so keeping 1.21 isn't necessary for us. We ended up using a different onload script and are no longer affected by this issue anymore. Thanks for your response and looking into this, feel free to close.

itsnotapt avatar Nov 02 '24 19:11 itsnotapt