config: add gets_restored tag to properties #25105
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I intend on adding a test and would prefer it being a GTest.
Cluster properties that _don't_ get restored during cluster restore were previously managed in a static list in the cluster restore reconciler. This made it easy to miss when adding new properties. As a step towards making this list more future proof, this commit adds a gets_restored flag to property metadata, and updates the reconciler to generate its list dynamically based on the cluster's current properties. The generated list is identical to the one previously defined in the reconciler.
dotnwat
approved these changes
Feb 19, 2025
Member
dotnwat
left a comment
dotnwat
left a comment
There was a problem hiding this comment.
seems ok to have the aliases, but they aren't real configuration option names, so i'm not sure if that has any kind of downstream affect.
Comment on lines
+34
to
+35
| for (const auto& alias : prop.aliases()) { | ||
| ignore_list.emplace(alias); |
Member
There was a problem hiding this comment.
will aliases matter or would aliases cause problems? afaik they aren't actually strings that are ever serialized to disk/wire, and they only exist as strings in the binary.
Contributor
Author
There was a problem hiding this comment.
Yea good question. I added aliases because aliasing is one way we implement renames of properties. Since the restored controller snapshots may exist from older versions of Redpanda, for such properties, there's no guarantee that the snapshotted names will be identical to the used in the latest version of Redpanda.
Collaborator
CI test resultstest results on build#61981
|
JakeSCahill
added a commit
to redpanda-data/docs-extensions-and-macros
that referenced
this pull request
Apr 28, 2025
JakeSCahill
added a commit
to redpanda-data/docs-extensions-and-macros
that referenced
this pull request
May 2, 2025
…ve access to a consistent set of tools (#99) * Add a doc-tools CLI command so that writers and our tests have access to a consistent set of tools * Bump version * Add submodule * Add tree-sitter-cpp submodule * Add tree-sitter-c grammar as dependency of C++ parser * Add tree-sitter-c grammar as required by tree-sitter-cpp * Add property automation to CLI * Add CLI help * doc-tools CLI * Remove tree‑sitter‑cpp submodule; clone on demand instead * Fix paths * Fix makefile * Update package.json * Update package-lock.json * Fix packages * Cleanup * Apply suggestions from code review Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Refactor * Add Iceberg support in test cluster * Fix error * Apply suggestions from code review Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Final fixes * Apply suggestions * Add support for the new gets_restored metadata introduced in redpanda-data/redpanda#25105 * Add fetch command for saving files from other repos locally * Apply suggestions from code review Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Add support for a --diff flag --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
JakeSCahill
added a commit
to redpanda-data/docs-extensions-and-macros
that referenced
this pull request
May 6, 2025
* Add a doc-tools CLI command so that writers and our tests have access to a consistent set of tools * Bump version * Add submodule * Add tree-sitter-cpp submodule * Add tree-sitter-c grammar as dependency of C++ parser * Add tree-sitter-c grammar as required by tree-sitter-cpp * Add property automation to CLI * Add CLI help * doc-tools CLI * Remove tree‑sitter‑cpp submodule; clone on demand instead * Fix paths * Fix makefile * Update package.json * Update package-lock.json * Fix packages * Cleanup * Apply suggestions from code review Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Refactor * Add Iceberg support in test cluster * Fix error * Apply suggestions from code review Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Final fixes * Apply suggestions * Add support for the new gets_restored metadata introduced in redpanda-data/redpanda#25105 * Add fetch command for saving files from other repos locally * Apply suggestions from code review Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Add support for a --diff flag * Use tags for Redpanda labs * Add symlink util for labs to the CLI * Remove utils --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Cluster properties that don't get restored during cluster restore were
previously managed in a static list in the cluster restore reconciler.
This made it easy to miss when adding new properties.
As a step towards making this list more future proof, this commit adds a
gets_restored flag to property metadata, and updates the reconciler to
generate its list dynamically based on the cluster's current properties.
The generated list is identical to the one previously defined in the
reconciler.
There are no major functional changes in this PR.
Backports Required
Release Notes