Fix kafka internal docker connection #2490

catinapoke · 2024-04-19T11:48:04Z

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:

Alter KAFKA_ADVERTISED_LISTENERS
Change network type from bridge to host (it's impossible)
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 · 2024-04-19T11:48:19Z

✅ Deploy Preview for testcontainers-go ready!

Name	Link
🔨 Latest commit	`128d31a`
🔍 Latest deploy log	https://app.netlify.com/sites/testcontainers-go/deploys/664f7fd3a6157000081ace88
😎 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 · 2024-04-22T09:41:36Z

@mdelapenya hi, will you make code review?

mdelapenya · 2024-04-22T10:51:32Z

@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 · 2024-04-22T13:21:57Z

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 · 2024-04-22T13:39:46Z

Okay, I will check this

mdelapenya · 2024-05-08T12:52:14Z

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

mdelapenya · 2024-05-08T12:52:39Z

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

catinapoke · 2024-05-09T11:20:18Z

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 · 2024-05-09T15:20:16Z

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 · 2024-05-10T18:19:02Z

@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

modules/kafka/options.go

mdelapenya

Added some comments regarding docs format (my fault, as it's not documented at all)

Thanks!

docs/modules/kafka.md

Co-authored-by: Manuel de la Peña <social.mdelapenya@gmail.com>

catinapoke · 2024-05-16T13:49:15Z

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

modules/kafka/kafka_test.go

catinapoke · 2024-05-17T06:26:00Z

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

mdelapenya

@catinapoke I think this PR looks good to me. Let's address my comments and wait for @eddumelendez's approval.

@eddumelendez could you do a final review? 🙏

modules/kafka/kafka.go

catinapoke · 2024-05-23T17:43:52Z

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

hwax0r · 2024-05-23T18:01:42Z

LGTM

mdelapenya · 2024-05-24T10:10:55Z

modules/kafka/kafka_test.go

@@ -190,6 +190,117 @@ func TestKafka_networkConnectivity(t *testing.T) {
 	}
 }

+func TestKafka_listenersValidation(t *testing.T) {


If you need to test the trimValidateListeners function at the unit level, because this file is in the kafka_test package, you can create another test file for the validation, with access to the private functions of the package. Then you can just run the tests without creating the container, and just doing real unit-testing on that function. Doing this, you can have multiple subtests with the different use cases, speeding up the build time. Wdyt?

I guess it's unnecessary as it's already pretty fast test and just higher type in pyramid of testing 🤔

fixed kafka internal docker connection

320e54d

catinapoke requested a review from a team as a code owner April 19, 2024 11:48

mdelapenya self-assigned this Apr 22, 2024

mdelapenya added the bug An issue with the library label Apr 22, 2024

added new option for kafka listeners

9b6537d

Merge remote-tracking branch 'original/main'

15f4ca3

mdelapenya reviewed May 12, 2024

View reviewed changes

modules/kafka/options.go Show resolved Hide resolved

fixed kafka docs with new options

c7552f4

mdelapenya reviewed May 13, 2024

View reviewed changes

catinapoke and others added 4 commits May 14, 2024 10:00

Update docs/modules/kafka.md

8904938

Co-authored-by: Manuel de la Peña <social.mdelapenya@gmail.com>

Apply suggestions from code review for docs

7b38c28

Co-authored-by: Manuel de la Peña <social.mdelapenya@gmail.com>

added new test, fixed docs from code review

98793d7

Merge branch 'main' of https://github.com/catinapoke/testcontainers-go

62a68b5

Merge remote-tracking branch 'Original/main'

a7c4a9f

mdelapenya reviewed May 16, 2024

View reviewed changes

modules/kafka/kafka_test.go Outdated Show resolved Hide resolved

renamed docker folder to testdata and fixed test name

bfa9191

mdelapenya requested a review from eddumelendez May 17, 2024 08:21

mdelapenya reviewed May 21, 2024

View reviewed changes

modules/kafka/kafka.go Outdated Show resolved Hide resolved

modules/kafka/kafka.go Outdated Show resolved Hide resolved

modules/kafka/kafka.go Show resolved Hide resolved

catinapoke added 2 commits May 23, 2024 20:28

Merge remote-tracking branch 'Original/main'

f1b0d48

added test for input listeners validation and refactor from code review

128d31a

mdelapenya reviewed May 24, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix kafka internal docker connection #2490

Fix kafka internal docker connection #2490

catinapoke commented Apr 19, 2024

netlify bot commented Apr 19, 2024 •

edited

catinapoke commented Apr 22, 2024

mdelapenya commented Apr 22, 2024

eddumelendez commented Apr 22, 2024

catinapoke commented Apr 22, 2024

mdelapenya commented May 8, 2024

mdelapenya commented May 8, 2024

catinapoke commented May 9, 2024

mdelapenya commented May 9, 2024

catinapoke commented May 10, 2024 •

edited

mdelapenya left a comment

catinapoke commented May 16, 2024

catinapoke commented May 17, 2024 •

edited

mdelapenya left a comment •

edited

catinapoke commented May 23, 2024

hwax0r commented May 23, 2024

mdelapenya May 24, 2024 •

edited

catinapoke May 24, 2024

Fix kafka internal docker connection #2490

Are you sure you want to change the base?

Fix kafka internal docker connection #2490

Conversation

catinapoke commented Apr 19, 2024

What does this PR do?

Why is it important?

Related issues

How to test this PR

netlify bot commented Apr 19, 2024 • edited

✅ Deploy Preview for testcontainers-go ready!

catinapoke commented Apr 22, 2024

mdelapenya commented Apr 22, 2024

eddumelendez commented Apr 22, 2024

catinapoke commented Apr 22, 2024

mdelapenya commented May 8, 2024

mdelapenya commented May 8, 2024

catinapoke commented May 9, 2024

mdelapenya commented May 9, 2024

catinapoke commented May 10, 2024 • edited

mdelapenya left a comment

Choose a reason for hiding this comment

catinapoke commented May 16, 2024

catinapoke commented May 17, 2024 • edited

mdelapenya left a comment • edited

Choose a reason for hiding this comment

catinapoke commented May 23, 2024

hwax0r commented May 23, 2024

mdelapenya May 24, 2024 • edited

Choose a reason for hiding this comment

catinapoke May 24, 2024

Choose a reason for hiding this comment

netlify bot commented Apr 19, 2024 •

edited

catinapoke commented May 10, 2024 •

edited

catinapoke commented May 17, 2024 •

edited

mdelapenya left a comment •

edited

mdelapenya May 24, 2024 •

edited