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]: WithAfterReadyCommand incorrect iterator scope #2668

Closed
itsnotapt opened this issue Jul 23, 2024 · 4 comments
Closed

[Bug]: WithAfterReadyCommand incorrect iterator scope #2668

itsnotapt opened this issue Jul 23, 2024 · 4 comments
Labels
bug An issue with the library

Comments

@itsnotapt
Copy link

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

execFn := func(ctx context.Context, c Container) error {

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 itsnotapt added the bug An issue with the library label Jul 23, 2024
@itsnotapt
Copy link
Author

Looks like WithStartupCommand has the same bug

@stevenh
Copy link
Collaborator

stevenh commented Aug 8, 2024

Bump module requirement to 1.22?

@mdelapenya
Copy link
Member

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.

@itsnotapt
Copy link
Author

itsnotapt commented Nov 2, 2024

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.

@stevenh stevenh closed this as completed Nov 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue with the library
Projects
None yet
Development

No branches or pull requests

3 participants