Fix kafka internal docker connection

[catinapoke]

What does this PR do?

I fixed passing alias name in startup script when it's possible.

Why is it important?

You can't connect to kafka container from other container at the same docker network.
Details: https://www.confluent.io/blog/kafka-listeners-explained/

Related issues

I have stumbled into issue with connecting from app docker container to kafka docker container at the same network. I tried many options including altering KAFKA_ADVERTISED_LISTENERS, but none of them worked. Finally, I found out that you overwrite KAFKA_ADVERTISED_LISTENERS in startup script.

Options I tried:

1. Alter KAFKA_ADVERTISED_LISTENERS
2. Change network type from bridge to host (it's impossible)
3. Add new listeners

How to test this PR

I have tested it in my ProofOfConcept repo https://github.com/catinapoke/testcontainers-poc in kafka_test.go file You can run it go test -v ./tests/integration/kafka_test.go

[netlify[bot]]

✅ Deploy Preview for testcontainers-go ready!

Name	Link
🔨 Latest commit	c8c47f45c7618f172146971ee2c19b1bdd5f38f3
🔍 Latest deploy log	https://app.netlify.com/sites/testcontainers-go/deploys/66f1ac6cd2840a0008cf8fa7
😎 Deploy Preview	https://deploy-preview-2490--testcontainers-go.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[catinapoke]

@mdelapenya hi, will you make code review?

[mdelapenya]

@eddumelendez wdyt about this one? It LGTM from the code point of view, but want to double check with you if you see anything else 🙏

[eddumelendez]

Hi, I think the approach should be consistent between Redpanda and Kafka regarding adding additional listeners https://golang.testcontainers.org/modules/redpanda/#additional-listener

[catinapoke]

Okay, I will check this

[mdelapenya]

I think that, after #1989, we can close this one. @catinapoke could you double check it? 🙏

[mdelapenya]

BTW I'm going to add support for registering listeners, like in the redpanda implementation.

[catinapoke]

I think that, after #1989, we can close this one. @catinapoke could you double check it? 🙏

I checked and it is doing different things, so not yet

[mdelapenya]

The code to register listeners will shadow what the redpanda module is currently doing (see https://github.com/testcontainers/testcontainers-go/blob/main/modules/redpanda/redpanda.go#L295), but instead of using the YAML template to render the listeners, we are going to build the proper environment variables for kafka: KAFKA_ADVERTISED_LISTENERS, KAFKA_LISTENERS, etc

[catinapoke]

@mdelapenya I added listeners and you can check it's working with this branch of my test repo, but you will have to clone my kafka module locally

@eddumelendez fyi

Old tests are passed btw and fixed assertAdvertisedListeners cause I use Host instead of Inspect.Hostname

That's how it is used:

KafkaContainer, err = kafka.RunContainer(ctx,
		kafka.WithClusterID("test-cluster"),
		testcontainers.WithImage("confluentinc/confluent-local:7.6.1"),
		network.WithNetwork([]string{"kafka"}, Network),
		kafka.WithListener([]kafka.KafkaListener{
			{
				Name: "INTERNAL",
				Ip:   "kafka",
				Port: "9092",
			},
		}),
	)

There is console output for convenience

=== RUN   TestKafkaTests
2024/05/10 21:12:03 github.com/testcontainers/testcontainers-go - Connected to docker:
  Server Version: 25.0.4
  API Version: 1.44
  Operating System: Debian GNU/Linux 11 (bullseye)
  Total Memory: 7918 MB
  Resolved Docker Host: unix:///var/run/docker.sock
  Resolved Docker Socket Path: /var/run/docker.sock
  Test SessionID: 46d84fc1f454f00ea235cd3704cf2a9cd34e7f0ad278632b6845237407425ab2
  Test ProcessID: 7bf1657e-427e-4328-a767-915f2073b99e
2024/05/10 21:12:03 🐳 Creating container for image testcontainers/ryuk:0.7.0
2024/05/10 21:12:04 ✅ Container created: bc124ab81e28
2024/05/10 21:12:04 🐳 Starting container: bc124ab81e28
2024/05/10 21:12:04 ✅ Container started: bc124ab81e28
2024/05/10 21:12:04 🚧 Waiting for container id bc124ab81e28 image: testcontainers/ryuk:0.7.0. Waiting for: &{Port:8080/tcp timeout:<nil> PollInterval:100ms}
2024/05/10 21:12:04 🔔 Container is ready: bc124ab81e28
2024/05/10 21:12:04 🐳 Creating container for image confluentinc/confluent-local:7.6.1
2024/05/10 21:12:05 ✅ Container created: 18074a289352
2024/05/10 21:12:05 🐳 Starting container: 18074a289352
2024/05/10 21:12:10 ✅ Container started: 18074a289352
2024/05/10 21:12:10 🔔 Container is ready: 18074a289352
2024/05/10 21:12:10 🐳 Creating container for image postgres:16.2-alpine3.19
2024/05/10 21:12:10 ✅ Container created: d1d7187c9c31
2024/05/10 21:12:10 🐳 Starting container: d1d7187c9c31
2024/05/10 21:12:10 ✅ Container started: d1d7187c9c31
2024/05/10 21:12:10 🚧 Waiting for container id d1d7187c9c31 image: postgres:16.2-alpine3.19. Waiting for: &{timeout:<nil> deadline:0xc0002de300 Strategies:[0xc000478090]}
2024/05/10 21:12:11 🔔 Container is ready: d1d7187c9c31
2024/05/10 21:12:11 true
succesfully created topics
time="2024-05-10T21:12:12+03:00" level=error msg="Tar: Can't stat file /home/impossible/vkusvill/testcontainers/tests/docker/kafka_rw_test to tar: lstat /home/impossible/vkusvill/testcontainers/tests/docker/kafka_rw_test//home/impossible/vkusvill/testcontainers/tests/docker/kafka_rw_test/.dockerignore: no such file or directory"
Step 1/8 : FROM golang:bullseye
 ---> 6023337f69a5
Step 2/8 : WORKDIR /app
 ---> Using cache
 ---> b4dc351741d0
Step 3/8 : COPY go.mod .
 ---> Using cache
 ---> babb994d22b5
Step 4/8 : COPY go.sum .
 ---> Using cache
 ---> 6800ebf38938
Step 5/8 : RUN go mod tidy
 ---> Using cache
 ---> b577bdb6096a
Step 6/8 : COPY . .
 ---> Using cache
 ---> 9b726456719e
Step 7/8 : RUN go build -o ./pg_test
 ---> Using cache
 ---> 1b06125cd391
Step 8/8 : CMD /app/pg_test
 ---> Using cache
 ---> 0f76970b95dc
Successfully built 0f76970b95dc
Successfully tagged d3d5b7cc-3010-40fb-8dc9-4d7e65445ba5:a840a8f1-5151-4857-81d9-a36add67b840
2024/05/10 21:12:12 🐳 Creating container for image
2024/05/10 21:12:12 ✅ Container created: 6101fa184022
2024/05/10 21:12:12 🐳 Starting container: 6101fa184022
2024/05/10 21:12:12 ✅ Container started: 6101fa184022
2024/05/10 21:12:12 🚧 Waiting for container id 6101fa184022 image: d3d5b7cc-3010-40fb-8dc9-4d7e65445ba5:a840a8f1-5151-4857-81d9-a36add67b840. Waiting for: &{timeout:<nil> Log:start consuming events IsRegexp:false Occurrence:1 PollInterval:100ms}
2024/05/10 21:12:12 🔔 Container is ready: 6101fa184022
=== RUN   TestKafkaTests/TestKafkaConnectivity
2024/05/10 21:12:12 INFO kafka producer connectivity-producer created
2024/05/10 21:12:12 INFO kafka consumer connectivity-consumer created
--- PASS: TestKafkaTests/TestKafkaConnectivity (1.24s)
2024/05/10 21:12:14 🐳 Terminating container: 18074a289352
2024/05/10 21:12:15 🚫 Container terminated: 18074a289352
2024/05/10 21:12:15 🐳 Terminating container: d1d7187c9c31
2024/05/10 21:12:16 🚫 Container terminated: d1d7187c9c31
2024/05/10 21:12:16 🐳 Terminating container: 6101fa184022
2024/05/10 21:12:19 🚫 Container terminated: 6101fa184022
--- PASS: TestKafkaTests (16.40s)
PASS
ok  	testcontainers/tests/integration	16.418s

[catinapoke]

@mdelapenya I have fixed docs and added new test that shows connectivity with option WithListeners

[catinapoke]

Btw looks like failed tests are the ones that I didn't touch and couldn't touch as I only renamed folder and test

[catinapoke]

@mdelapenya @eddumelendez have finished edits, please review and give approval if everything is okay

[hwax0r]

LGTM

[catinapoke]

Some day @eddumelendez will approve this PR, but probably not today....

[catinapoke]

@mdelapenya got several minutes and added unit test btw do you have rights to approve this PR?

[mdelapenya]

Some day @eddumelendez will approve this PR, but probably not today....

He is currently in a conference, so not sure how available is he. Will double check with him today.

do you have rights to approve this PR?

I'm the maintainer of the repo, but as you can imagine, I cannot be an expert in all the technologies that we support, that's why we usually check with other team members, our community, and even vendors, about PRs touching certain modules. And kafka is a tough one 😅 Sorry if it's taking that long, I'm in the middle of a major refactor of the container APIs looking for a more idiomatic candidate for v1.0.0, so it's my fault if the repo is not properly attended. In any case, we'll focus on this one ASAP.

Thanks!

[mdelapenya]

@catinapoke could you take a look at this PR? We'd like to start discussions for the new native Kafka image, but from a solid foundation. I'd appreciate if you could address Eddu's comments

Thanks for your time 🙇

[catinapoke]

@catinapoke could you take a look at this PR? We'd like to start discussions for the new native Kafka image, but from a solid foundation. I'd appreciate if you could address Eddu's comments

Thanks for your time you

If you mean to add all the stuff Eddu suggested than it will take a while because I don't have much time, also I think it could be approved at this point and finished in next stage.

In my opinion, your comments was enough to release this PR as it already fixed the problem and had several iterations. Honestly, I burned out a bit after month late response that says do one more fix, write one more test, redo this part of code. I expected little changes at most. btw you have access to my fork as I checked "Allow edits and access to secrets by maintainers "

so, yeah, I definitely love testcontainers and was willing to be contributor of it, but you can't polish things forever

fyi @mdelapenya

[mdelapenya]

Honestly, I burned out a bit after month late response that says do one more fix, write one more test, redo this part of code. I expected little changes at most. btw you have access to my fork as I checked "Allow edits and access to secrets by maintainers "

@catinapoke I'm sorry to hear that. Please consider that I'm not as expert in Kafka as you are, so please consider this me being cautious before merging the code. I'm really sorry if that caused frustration.

so, yeah, I definitely love testcontainers and was willing to be contributor of it, but you can't polish things forever

Eddu is maintainer in the Java project so he sometimes jumps in when I ping him, so sorry again if we were juggling with this PR too much. Kafka is a very complex project (to me) and there are some other ongoing issues that could be affected by the changes introduced here so I always want to make sure the PR tries to fit as many of them as possible (see https://github.com/testcontainers/testcontainers-go/issues?q=is%3Aissue+is%3Aopen+kafka)

If we add that travelling to conferences and summer vacations have played their role here, then the result is having the PR more time in the back burner.

In any case, I acknowledge it's taking more time than expected (you opened this PR back in April), so I want to offer my apologies for that. On the other hand, I also hope you understand the position of OSS maintainers and the stress it takes to maintain a popular project like this one. In this particular PR we tried to come with meaningful questions and I already told you about my lack of expertise in Kafka and the amount of work we had at that time.

In any case, I personally appreciate any contribution without exception, and I always try to put in the developer's feet whenever I review a PR. So I suffer in this situations too when a PR gets stuck for months, but I'd understand if you want to step back with your contributions.

[mdelapenya]

@catinapoke I did not mention it, but if you're swamped or cannot attend this PR, I can step up and add commits on top of yours, if I may 🙏

[catinapoke]

@mdelapenya u can, I kinda started to work on PR again, but still not much time

[catinapoke]

@mdelapenya can you help me with network connectivity test?

It has problems at external read step. I'm using kcat to send message to topic, but message never come to external consumer