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

Expand serde coverage #9816

Merged

Conversation

michael-redpanda
Copy link
Contributor

@michael-redpanda michael-redpanda commented Apr 4, 2023

Expands test coverage of schema registry by adding in a Golang and Java based Kafka client to exercise the schema registry

Fixes #9211

Force push 95ea7d0:

  • Fixed python linting error

Force push c32d751:

  • Added Makefile that was missed

Force push a2b9898:

  • Updated ducktape test

Force push 5f11ad5:

  • Fixed commit message

Force push a7df33e:

  • Added clang format rules for protobuf files

Force push f834a30:

  • Fixed test
  • Fixed makefile

Force push 8cc46f6:

  • serde clients now running on independent node
  • added missing comments

Force push d037aae:

  • Fixed issue with missing python script

Force push d2b2901:

  • Building same verison of protobufs in Dockerfile as we use in Redpanda

Force push 8c3e3ec:

  • Fixed issue with CDT failing

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v23.1.x
  • v22.3.x
  • v22.2.x

Release Notes

  • None

@michael-redpanda michael-redpanda self-assigned this Apr 4, 2023
@michael-redpanda michael-redpanda requested a review from a team as a code owner April 4, 2023 13:31
@michael-redpanda michael-redpanda requested review from gousteris and removed request for a team April 4, 2023 13:31
@michael-redpanda michael-redpanda force-pushed the expand-serde-coverage branch 5 times, most recently from 5f11ad5 to a7df33e Compare April 4, 2023 19:06
tests/go/go-kafka-serde/go.mod Show resolved Hide resolved
tests/go/go-kafka-serde/Makefile Outdated Show resolved Hide resolved
@@ -37,6 +37,7 @@ function install_system_deps() {
npm \
openssh-server \
netcat-openbsd \
protobuf-compiler \
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we can/should use the one we build in the build, e.g.:

vbuild/debug/clang/rp_deps_install/bin/protoc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll see if it's accessible within the docker container

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The docker container is built independent of Redpanda. Your local build of Redpanda is mounted within the containers when the cluster is composed.

Copy link
Member

Choose a reason for hiding this comment

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

I've had huge problems getting an arbitrary version of protoc to generate code that works with an arbitrary version of a protobuf library dependency. That's across C++, Go, and Python. Maybe I'm just doing something wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not as familiar with protobufs as I am with flatbuffers... I'm compiling with version A has such issues working with library version B, if they are at least "close".

Maybe the correct solution for docker, and anything, is to build and use the same protoc across all use cases. I can update the docker build script to do just that.

Copy link
Member

Choose a reason for hiding this comment

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

Right, but protoc is already built. My well-known PR updates the Python lib to the same version as what we build for Redpanda. (Don't let the version number fool you).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, but protoc is already built.

The build for the test Docker image doesn't reference any build artifacts for Redpanda. Instead, the container is mounted with your build directory as a volume after it is built.

(Don't let the version number fool you).

It did

Copy link
Contributor

@NyaliaLui NyaliaLui left a comment

Choose a reason for hiding this comment

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

I still need to review the java and go code, but I left comments for python!

tests/rptest/clients/java_serde_client.py Outdated Show resolved Hide resolved
tests/rptest/clients/golang_serde_client.py Outdated Show resolved Hide resolved
Added java based client that will exercise the schema registry

Signed-off-by: Michael Boquard <michael@redpanda.com>
Signed-off-by: Michael Boquard <michael@redpanda.com>
@michael-redpanda
Copy link
Contributor Author

/cdt
tests/rptest/tests/schema_registry_test.py

Signed-off-by: Michael Boquard <michael@redpanda.com>
Added golang serde client based off of Confluent's kafka-go library

Signed-off-by: Michael Boquard <michael@redpanda.com>
@michael-redpanda
Copy link
Contributor Author

/cdt
tests/rptest/tests/schema_registry_test.py

@michael-redpanda
Copy link
Contributor Author

/cdt
tests/rptest/tests/schema_registry_test.py

@michael-redpanda
Copy link
Contributor Author

michael-redpanda commented Apr 7, 2023

@michael-redpanda
Copy link
Contributor Author

/cdt
tests/rptest/tests/schema_registry_test.py

Signed-off-by: Michael Boquard <michael@redpanda.com>
Created a SerdeClient BackgroundThreadService that will fire off the
different clients based on which client type to use.

Moved python_librdkafka_serde_client.py to the remote_scripts
directory so it can be accessed in the docker container.

Updated python_librdkafka_serde_client.py to have the same command line
arguments as the other clients.

Signed-off-by: Michael Boquard <michael@redpanda.com>
@michael-redpanda
Copy link
Contributor Author

/cdt
tests/rptest/tests/schema_registry_test.py

Copy link
Member

@BenPope BenPope left a comment

Choose a reason for hiding this comment

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

LGTM

@michael-redpanda michael-redpanda merged commit 1ceb333 into redpanda-data:dev Apr 10, 2023
@BenPope
Copy link
Member

BenPope commented Apr 10, 2023

/backport v23.1.x

@BenPope
Copy link
Member

BenPope commented Apr 10, 2023

/backport v22.3.x

@vbotbuildovich
Copy link
Collaborator

Failed to run cherry-pick command. I executed the below command:

git cherry-pick -x 3b7d390af094d106390525e9de981bd2185ad4cb d910151d037c9993a5800dff147e01814edc8099 ab1177eb529da0b9a7e66b848480463f85e512ed 1b0c24a4235459caeb2cf314d632518fb9e5e446 a0215d6dff7b73da89bfbb07dd4557f0febb08fc 8c3e3ecc7d1aeadce823f910287919467797e266

Workflow run logs.

@michael-redpanda
Copy link
Contributor Author

/backport v22.2.x

@vbotbuildovich
Copy link
Collaborator

Failed to run cherry-pick command. I executed the below command:

git cherry-pick -x 3b7d390af094d106390525e9de981bd2185ad4cb d910151d037c9993a5800dff147e01814edc8099 ab1177eb529da0b9a7e66b848480463f85e512ed 1b0c24a4235459caeb2cf314d632518fb9e5e446 a0215d6dff7b73da89bfbb07dd4557f0febb08fc 8c3e3ecc7d1aeadce823f910287919467797e266

Workflow run logs.

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.

sr: expand serde coverage
5 participants