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

Bugfix: kafka not found port error #2552

Conversation

wilsouza
Copy link
Contributor

What does this PR do?

This PR addresses the Kafka startup failure related to issue #2543. It implements a workaround to ensure that the container maps the ports before attempting to retrieve them.

Why is it important?

This fix resolves a problem in the Kafka module that was causing startup failures due to port mapping issues.

Related issues

How to test this PR

package main

import (
	"context"
	"log"

	"github.com/testcontainers/testcontainers-go/modules/kafka"
)

func main() {
	ctx := context.Background()

	kafkaContainer, err := kafka.RunContainer(ctx,
		kafka.WithClusterID("test-cluster"),
	)
	if err != nil {
		log.Fatalf("failed to start container: %s", err)
	}

	// Clean up the container after
	defer func() {
		if err := kafkaContainer.Terminate(ctx); err != nil {
			log.Fatalf("failed to terminate container: %s", err)
		}
	}()
}

@wilsouza wilsouza requested a review from a team as a code owner May 23, 2024 16:59
Copy link

netlify bot commented May 23, 2024

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit 7960122
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-go/deploys/666cfd007d7acd0008c5d54a
😎 Deploy Preview https://deploy-preview-2552--testcontainers-go.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

Copy link

@AhmedNSidd AhmedNSidd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried using this for my kafka port not found error cases, and this fixed the issues I was running into.

@wilsouza wilsouza force-pushed the fix-workaround-for-kafka-not-found-port branch 2 times, most recently from c5fb1e2 to 8b27632 Compare May 27, 2024 15:06
@mgilbir
Copy link

mgilbir commented Jun 14, 2024

I've tried it and it fixes most of the kafka port not found issues. Thanks!
I am still hitting some port not found: creating reaper failed: failed to create container

@wilsouza wilsouza force-pushed the fix-workaround-for-kafka-not-found-port branch from 3bae926 to 7960122 Compare June 15, 2024 02:31
@mdelapenya
Copy link
Collaborator

Hi @wilsouza thanks for submitting this quick fix to the original issue. Unfortunately, I think we should not add a time.Sleep here as it would not make the code deterministic.

I'm working on #2606, which adds a condition to check that all the exposed ports have been already made available by the container runtime, and this check happens before the wait strategies. I'd appreciate your feedback in that issue if possible.

So because of that, I'd close this one.

@wilsouza
Copy link
Contributor Author

wilsouza commented Jul 3, 2024

Hi @wilsouza thanks for submitting this quick fix to the original issue. Unfortunately, I think we should not add a time.Sleep here as it would not make the code deterministic.

I'm working on #2606, which adds a condition to check that all the exposed ports have been already made available by the container runtime, and this check happens before the wait strategies. I'd appreciate your feedback in that issue if possible.

So because of that, I'd close this one.

No worries. I understand. My intent was to solve the issue without altering the core features of Testcontainers.
I considered the possibility of removing the cached Inspect specifically for the MappedPort API, but I was unsure of the potential impacts of this change.
If there’s anything else I can assist with, please let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Fail to start Kafka container (KRaft) with basic setup
4 participants