-
Notifications
You must be signed in to change notification settings - Fork 580
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
kafka: fix config source for default cleanup.policy config #17456
kafka: fix config source for default cleanup.policy config #17456
Conversation
33b34fd
to
98876be
Compare
Currently Redpanda returns source=1 (topic-specific overwrite) for the cleanup.policy config. This is different to Apache Kafka, which returns source=5 (default config), even though the config value they return (delete) is the same, so I believe this behaviour is a bug and this commit fixes it by using the hide_default_override helper field.
98876be
to
78fcff7
Compare
# New brokers may not report the version yet, so gather the cluster version | ||
# based on multiple brokers' reported version | ||
versions = list( | ||
set(broker['version'] for broker in brokers_resp | ||
if 'version' in broker)) | ||
assert len(versions) == 1, f"Unexpected versions: {versions}" | ||
version_line = versions[0] | ||
|
||
self.version = ver_triple(version_line) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Force-pushed to make the version uniformity check here more lenient by allowing missing versions from some nodes. This is to make the test_join_restart_catch_up
test less flaky.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test changes LGTM!
Kicking off a rebuild of CI as something in the infrastructure got messed up |
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/47172#018e9b9f-9f5e-41df-8f46-dfadf2feaca8 |
Can we backport this PR at least to v23.3.x? We would like to have this in the latest versions for our Terraform Provider. 😄 |
/backport v23.3.x |
This fixes a bug in the reported config source of
cleanup.policy
. If I create a topic without a cleanup policy, the default cleanup policy is as configured on the broker (initially, cleanup.policy=delete). Describing a topic's configs shows:when it should show
Fixes #2225
Backports Required
Release Notes
Bug Fixes