Expose Redpanda schema registry by gustavomonarin · Pull Request #5994 · testcontainers/testcontainers-java

gustavomonarin · 2022-10-17T19:34:28Z

This change is intended to make the redpanda implementation of the schema registry easily available on test containers.

The schema registry port 8081 was added to the list of exposed ports and a supporting method getSchemaRegistryAddress was added to easily use for configuring schema.registry.url serializer configuration.

A minor change on the internal advertised address was made as the previous kafka alias was not available within the container and the panda proxy would get lost with the following message:

WARN  2022-10-17 19:08:37,176 [shard 0] kafka/client - broker.cc:52 - std::system_error: kafka: Not found
ERROR 2022-10-17 19:08:37,177 [shard 0] pandaproxy - service.cc:137 - Schema registry failed to initialize internal topic: kafka::client::broker_error ({ node: -1 }, { error_code: broker_not_available [8] })

Since this is only an internal advertised address and the dev-container is an alias to only one node, using 127.0.0.1 should not be an issue. An alternative approach for configuring the kafka alias within the container should work, any suggestion on configuring the internal container address lookup?

Also, a simple test to check if the registry is available is made. Please let me know if you feel like we should actually perform some schema registration.

eddumelendez

thanks for the PR @gustavomonarin ! Exposing the schema registry sounds great. Do you mind improving the docs too, please?

Regarding to the schema registry test. I think it would be great to have such a great example to publish a schema, retrieved and send a message to test that everything is working as expected.

gustavomonarin · 2022-10-17T20:06:10Z

Hi Eddu, thank you for the quick reply :)

Sure. I will try to work on the docs tomorrow.

Regarding the test, i would like to extend the discussion / ask your suggestion:
We could test in different ways:

Test against the schema registry public api (post/get a schema) then assert.

This is still only on the api level, over rest.

Actually remove the schema registry test and extend the kafka producing / consumer to use the schema registry.

This will require to add the confluent schema registry serializers, a schema (ie protobuf), a generation code plugin for the schema, a proper message. I am afraid of adding the gradle plugin only for test to be honest, beyond some extra energy :).

Do you think is really worth the energy? What are is the value that you are looking for? For developer example/guidance on our tests? For actually checking the redpanda schema registry fully functional?

gustavomonarin · 2022-10-17T20:07:10Z

By the way, i had a quick look and the pipeline and the errors seem unrelated.

building locally here only the redpanda module works like a charm with ./gradlew :redpanda:check

Do you have any hints regarding the ci?

eddumelendez · 2022-10-17T20:13:39Z

the issue is related to connection issues with maven central.

Regarding to the test I think publish and retrieve will be enough then.

gustavomonarin · 2022-10-19T11:09:45Z

Hi @eddumelendez ,

I have updated the documentation and the tests as discussed. Could you please have another look?

This change is intended to make the redpanda implementation of the schema registry easily available on test containers. The schema registry port `8081` was added to the list of exposed ports and a supporting method `getSchemaRegistryAddress` was added to easily use for configuring `schema.registry.url` serializer configuration. A minor change on the internal advertised address was made as the `kafka` alias was not available within the container and the panda proxy was getting lost with the following message: ``` INFO 2022-10-17 19:08:37,164 [shard 0] kafka/client - broker.cc:41 - connected to broker:-1 - 0.0.0.0:29092 WARN 2022-10-17 19:08:37,176 [shard 0] kafka/client - broker.cc:52 - std::system_error: kafka: Not found ERROR 2022-10-17 19:08:37,177 [shard 0] pandaproxy - service.cc:137 - Schema registry failed to initialize internal topic: kafka::client::broker_error ({ node: -1 }, { error_code: broker_not_available [8] }) ``` Since this is only an internal advertised address and the `dev-container` is an alias to only one node, this should not be an issue. An alternative approach for configuring the `kafka` alias within the container node would be welcome. Also, a simple test to check if the registry is available is made. Please let me know if you feel like we should actually perform some schema registration.

Describes how to retrieve the schema registry address

Schema registry tests for publishing and consuming new schemas. The tests only cover the api availability. Further reference should follow the official documentation as this is a quite extensive / complex subject, which would require code generation, plugins to have as example while the [official documentation](https://docs.confluent.io/platform/current/schema-registry/index.html) is quite good.

eddumelendez

Thanks @gustavomonarin ! Just minor suggestions but LGTM in general

kiview · 2022-10-20T14:21:56Z

 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.assertj.core.api.Assertions.tuple;
+import static org.hamcrest.Matchers.hasItems;


Should be replaced with Assertj usage.

@gustavomonarin are you able to fix this? If not, we can do it. The PR is almost done.

Of course. Should be fixed on 7a36f32.

Let me know if i missed something.

The branch is now slightly behind the origin / master. Should i rebase? (I am not sure if rebase could affect anything with the review process...)

no need to do so. I'm waiting for the checks to pass and I will merge it. Thanks @gustavomonarin !

eddumelendez · 2022-10-27T17:04:38Z

thanks @gustavomonarin ! this is now part of main branch

gustavomonarin requested a review from a team October 17, 2022 19:34

github-actions Bot added the modules/redpanda label Oct 17, 2022

eddumelendez reviewed Oct 17, 2022

View reviewed changes

github-actions Bot added the type/docs label Oct 19, 2022

gustavo.monarin added 3 commits October 19, 2022 13:10

Add documentation

25e9519

Describes how to retrieve the schema registry address

gustavomonarin force-pushed the enable-redpanda-out-of-box-schema-registry branch from 17ce951 to 4453ac1 Compare October 19, 2022 11:10

eddumelendez approved these changes Oct 19, 2022

View reviewed changes

Comment thread modules/redpanda/src/test/java/org/testcontainers/redpanda/RedpandaContainerTest.java Outdated

Comment thread modules/redpanda/src/test/java/org/testcontainers/redpanda/RedpandaContainerTest.java Outdated

eddumelendez added this to the next milestone Oct 19, 2022

Remove unecessary test comments

b501e7f

kiview approved these changes Oct 20, 2022

View reviewed changes

kiview reviewed Oct 20, 2022

View reviewed changes

Comment thread modules/redpanda/src/main/java/org/testcontainers/redpanda/RedpandaContainer.java

kiview reviewed Oct 20, 2022

View reviewed changes

switch from Hamcrest to AssertJ matchers

7a36f32

eddumelendez merged commit 0bb6925 into testcontainers:main Oct 27, 2022

Uh oh!

Conversation

gustavomonarin commented Oct 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eddumelendez left a comment

Choose a reason for hiding this comment

Uh oh!

gustavomonarin commented Oct 17, 2022

Uh oh!

gustavomonarin commented Oct 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eddumelendez commented Oct 17, 2022

Uh oh!

gustavomonarin commented Oct 19, 2022

Uh oh!

eddumelendez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kiview Oct 20, 2022

Choose a reason for hiding this comment

Uh oh!

eddumelendez Oct 26, 2022

Choose a reason for hiding this comment

Uh oh!

gustavomonarin Oct 27, 2022

Choose a reason for hiding this comment

Uh oh!

gustavomonarin Oct 27, 2022

Choose a reason for hiding this comment

Uh oh!

eddumelendez Oct 27, 2022

Choose a reason for hiding this comment

Uh oh!

eddumelendez commented Oct 27, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gustavomonarin commented Oct 17, 2022 •

edited

Loading

gustavomonarin commented Oct 17, 2022 •

edited

Loading