feat: remove product config & clean up#1041
Open
maltesander wants to merge 16 commits into
Open
Conversation
Point stackable-operator at the smooth-operator branch (via [patch]) to gain access to the v2:: framework modules, in preparation for removing the product-config dependency. Bump dependencies (cargo update) to satisfy the branch's snafu >=0.9.1 requirement. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a vendored framework module (mirroring stackable_operator::v2) and move the Java .properties writer into framework/writer.rs, backed by the same java-properties crate as product_config::writer so the rendered output is byte-identical. Swap the zoo.cfg / security.properties rendering in zk_controller from product_config::writer to the local writer. This removes the first product-config touch point. Properties only; the Hadoop XML writer from the hdfs-operator source is not needed here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add framework/role_utils.rs mirroring stackable_operator::v2::role_utils from the smooth-operator branch (as trino-operator does): RoleGroupConfig plus with_validated_config, which merges default <- role <- rolegroup config fragments and validates them via FromFragment. Gated with allow(dead_code) until the reconciler is switched to produce a ValidatedCluster in the following commit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ster
Replace the product-config-based validation pipeline with a typed, framework-
based one mirroring trino-operator:
- validate() now produces a ValidatedCluster { name, image, cluster_config,
role_group_configs } via framework::role_utils::with_validated_config, instead
of the product-config PropertyNameKind map. server.<myid> quorum entries are
precomputed into cluster_config so the build step never needs the CRD.
- Add zk_controller/build/config_map.rs which builds zoo.cfg and
security.properties directly from the typed config, referencing the
ZookeeperCluster only for the owner reference. The operator defaults that
product-config used to inject from deploy/config-spec/properties.yaml
(admin.serverPort, clientPort, dataDir, initLimit, syncLimit, tickTime,
metricsProvider.*, networkaddress.cache.*) are now seeded here, byte-identical
to before (pinned by the kuttl ConfigMap snapshot).
- Drop the product_config Configuration impl from the CRD; add a manual Merge
impl for ZookeeperConfigOverrides (CRD schema unchanged) and Ord for
ZookeeperRole.
- Minimal consumer changes: the StatefulSet derives MYID_OFFSET/ZOOCFGDIR from
the merged config plus envOverrides; the metrics Service takes a resolved port.
- Remove ProductConfigManager from main.rs/Ctx and the product-config dependency
from both Cargo.toml. It remains only as a transitive dependency of
stackable-operator itself.
Unit tests now exercise the product-config-free path and assert the seeded
defaults and security.properties byte-for-byte. The generated CRD is unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
product-config no longer reads this file, but it is kept (empty) because the build/packaging process still expects it (it is COPYd into the operator image). Mirrors trino-operator. To be removed entirely in a later cleanup. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Restructure zk_controller/build to mirror trino-operator and hdfs-operator: - build/properties/mod.rs holds the ConfigFileName enum and shared helpers (apply_overrides, into_optional_values). - build/properties/zoo_cfg.rs and security_properties.rs each render one file; metrics_http_port moves alongside the zoo.cfg builder. - build/properties/logging.rs takes over the logback/log4j and vector.yaml rendering, replacing the top-level product_logging.rs module (now removed). - build/config_map.rs becomes a thin orchestrator that assembles the data map via ConfigFileName and the property/logging builders, then .data(data).build(). Pure restructuring: no behavior change. The generated CRD is unchanged and all 16 unit tests (which render the full ConfigMap) still pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mirror trino-operator and hdfs-operator:
- Switch ZookeeperConfigOverrides to stackable_operator::v2::config_overrides::
KeyValueConfigOverrides with non-optional fields and a derived Merge impl,
replacing the manual Merge impl and the KeyValueOverridesProvider/
get_key_value_overrides indirection. The zoo.cfg/security.properties builders
now read rg.config_overrides.<field> directly and resolve via resolved_overrides
(None values are dropped).
- Drop the hard-coded ZOOKEEPER_PROPERTIES_FILE / JVM_SECURITY_PROPERTIES_FILE
crd constants. The ConfigMap file names now come solely from the ConfigFileName
enum; jvm.rs keeps a local security.properties const for the JVM system property
(as trino does).
CRD regenerated: the configOverrides fields become non-nullable objects
(default {}) with nullable string values, matching trino/hdfs. Backwards-
compatible for existing string-valued overrides. All 16 tests pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mirror trino-operator and hdfs-operator by relocating discovery.rs to zk_controller/build/discovery.rs. The zk_controller `build` module is now pub(crate) so the znode controller can reach the shared builder; both controllers build discovery ConfigMaps for a ZookeeperCluster. Pure move: no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…idated struct - build_discovery_configmap drops the redundant `zk` parameter (it was only used for an error ObjectRef; the owner reference comes from `owner`) and renames `resolved_product_image` to `image`. It keeps `image` + `zookeeper_security` as explicit shared params, so both controllers can call it without a shared trait or a full ValidatedCluster. - Rename the znode controller's ValidatedInputs to ValidatedZnode and its resolved_product_image field to image, for naming consistency with ValidatedCluster. No behavior change. Build, clippy, the 16 unit tests, and the generated CRD are all unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
These constants are no longer used in crd/mod.rs. Following hdfs-operator (logging constants live in the logging builder) and trino-operator (MAX_PREPARE_LOG_FILE_SIZE lives in the controller): - LOGBACK_CONFIG_FILE, LOG4J_CONFIG_FILE, ZOOKEEPER_LOG_FILE and MAX_ZK_LOG_FILES_SIZE move to zk_controller/build/properties/logging.rs. - MAX_PREPARE_LOG_FILE_SIZE (the prepare init container) moves to zk_controller.rs. - config/jvm.rs keeps its own local log-config-file consts (as it already does for security.properties) to avoid coupling config to the controller build module. No behavior change. Build, clippy, the 16 unit tests, and the generated CRD are all unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
16 tasks
Replace the vendored Java-properties writer (rust/operator-binary/src/framework/writer.rs) with stackable_operator::v2::config_file_writer (moved there via operator-rs #1217 on the smooth-operator branch). ZooKeeper's copy was a java-only subset of the canonical hdfs writer; the upstream module's additional to_hadoop_xml simply goes unimported. Drop the now-unused java-properties dependency. The framework module now only contains role_utils. No behaviour change; rendered .properties output is byte-identical by construction (same code, new home). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Description
Definition of Done Checklist
Author
Reviewer
Acceptance
type/deprecationlabel & add to the deprecation scheduletype/experimentallabel & add to the experimental features tracker