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

Support for multiple kafka brokers #881

Merged
merged 5 commits into from
Nov 8, 2023

Conversation

wcmitchell
Copy link
Contributor

@wcmitchell wcmitchell commented Oct 25, 2023

This allows a hostnames key to be defined in the managed kafka secret as a comma,separated,list of broker hostnames. If it is defined, the cdappconfig's BrokerConfig array will contain all the possible broker hostnames that a client can connect to.

If the hostnames key is not found, then we fall back to looking for the hostname key as we have always done and only have a single BrokerConfig in the array.

Signed-off-by: Chris Mitchell <cmitchel@redhat.com>
@wcmitchell wcmitchell marked this pull request as draft October 25, 2023 12:34
@wcmitchell
Copy link
Contributor Author

/retest

@wcmitchell
Copy link
Contributor Author

/retest

Signed-off-by: Chris Mitchell <cmitchel@redhat.com>
@bsquizz bsquizz marked this pull request as ready for review November 7, 2023 17:01
@bsquizz
Copy link
Contributor

bsquizz commented Nov 7, 2023

/retest

Copy link
Contributor

@bsquizz bsquizz left a comment

Choose a reason for hiding this comment

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

Looks good to me. The old test-kafka-managed is making sure the plain old hostname key still works... and test-kafka-managed-multibroker is validating the new plural hostnames key works.

@bsquizz bsquizz changed the title WIP: Initial multibroker work Support for multiple kafka brokers Nov 7, 2023
@bsquizz bsquizz merged commit 2084576 into RedHatInsights:master Nov 8, 2023
7 checks passed
@lzap
Copy link

lzap commented Nov 13, 2023

Sup! When can we expected to see a new release with this?

@bsquizz
Copy link
Contributor

bsquizz commented Nov 13, 2023

This was released to stage last Friday (but we haven't tagged a new official release on github yet)

@lzap
Copy link

lzap commented Nov 14, 2023

Ok let us know when we can actually start upgrading dependencies and trying it out on stage. Thanks.

@bsquizz
Copy link
Contributor

bsquizz commented Nov 14, 2023

Which dependencies do you need to upgrade? I would expect that you actually should not need to change dependencies because the cdappconfig schema did not change. Clowder is sending 3 broker configurations to apps in stage already.

@lzap
Copy link

lzap commented Nov 16, 2023

Oh I thought a clowder bump is needed, disregard then. Thanks.

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.

None yet

4 participants