spring-attic / spring-cloud-stream-binder-kafka

Spring Cloud Stream binders for Apache Kafka and Kafka Streams
Apache License 2.0
331 stars 301 forks source link

Kafka bindings from previous test class remains active in the next test class #1204

Closed ofirgrm closed 2 years ago

ofirgrm commented 2 years ago

Hi,

The code I have is using Spring Boot 2.5.1 (Spring Cloud Kafka Binders) with Confluent Kafka 6.1.1 For testing, I'm using TestContainers to start a Kafka broker and Schema Registry to run against the test code For this issue, I've written a minimum code that allows me to reproduce the issue:

@Testcontainers
@SpringBootTest
public class Test1 {

    private static KafkaContainersOrchestration kafkaContainersOrchestration;

    @DynamicPropertySource
    protected static void dynamicProperties(DynamicPropertyRegistry registry) throws IOException {
        Network network = Network.newNetwork();
        kafkaContainersOrchestration = new KafkaContainersOrchestration(registry);
        kafkaContainersOrchestration.start(network);
    }

    @AfterAll
    static void afterAll() {
        kafkaContainersOrchestration.shutdown();
    }

    @Test
    public void testNothing() { }

}

The code is using KafkaContainersOrchestration to create the Kafka broker and Schema registry (using test containers) and register the configuration to Spring Boot. KafkaContainersOrchestration provides methods to start and shut down the Kafka container. As a side note:

Assume you have Test2 which is an identical copy of Test1 and you run both of them. The first test will start creating the streams, consumers and providers. The following log is from the first test:

2022-03-03 11:51:42.634  INFO [AbstractConfig      ] AdminClientConfig values: 
    bootstrap.servers = [PLAINTEXT://localhost:49310]
    client.dns.lookup = use_all_dns_ips
    client.id = sow-consumer-supplier-update_consumer-0067ee10-5449-4573-9013-b2a95ff42f09-admin

It will finish the test and shut down the containers, moving to the next test. Instead of having a "clean" application context, it seems that all the previous binding entities linger: The following log is from the second test:

2022-03-03 11:52:28.322 DEBUG [Selector            ] [AdminClient clientId=sow-consumer-supplier-update_consumer-0067ee10-5449-4573-9013-b2a95ff42f09-admin] Connection with localhost/127.0.0.1 disconnected

java.net.ConnectException: Connection refused: no further information
    at java.base/sun.nio.ch.SocketChannelImpl.checkConnect(Native Method)
    at java.base/sun.nio.ch.SocketChannelImpl.finishConnect(SocketChannelImpl.java:777)
    at org.apache.kafka.common.network.PlaintextTransportLayer.finishConnect(PlaintextTransportLayer.java:50)
    at org.apache.kafka.common.network.KafkaChannel.finishConnect(KafkaChannel.java:219)
    at org.apache.kafka.common.network.Selector.pollSelectionKeys(Selector.java:526)
    at org.apache.kafka.common.network.Selector.poll(Selector.java:481)
    at org.apache.kafka.clients.NetworkClient.poll(NetworkClient.java:563)
    at org.apache.kafka.clients.admin.KafkaAdminClient$AdminClientRunnable.processRequests(KafkaAdminClient.java:1329)
    at org.apache.kafka.clients.admin.KafkaAdminClient$AdminClientRunnable.run(KafkaAdminClient.java:1260)
    at java.base/java.lang.Thread.run(Thread.java:829)

Further, in the same log of the second test:

2022-03-03 11:51:52.323  WARN [NetworkClient       ] [AdminClient clientId=sow-consumer-supplier-update_consumer-0067ee10-5449-4573-9013-b2a95ff42f09-admin] Connection to node 1 (localhost/127.0.0.1:49310) could not be established. Broker may not be available.

Port 49310, mentioned in the log, belonged to the previous instance of the Kafka broker which was shut down by the end of Test1.

These error doesn't affect the test outcome, however, they make the log dirty (you will get dozens of these errors printed in the log for the second test, which will get even worse for any consecutive test). Also, they are a potential issue because a "state" of a previous test is leaking into the next, which shouldn't happen, as each test should start with a clean "state".

garyrussell commented 2 years ago

Add @DirtiesContext to the test class to shut down the application context.

ofirgrm commented 2 years ago

thanks a lot, Gary for the quick response. I can confirm that it fixes the issue