Skip to content

fix(kafka): regenerate config constraints#3002

Draft
cjc7373 wants to merge 8 commits into
release-1.1from
bugfix/kafka-various-fix
Draft

fix(kafka): regenerate config constraints#3002
cjc7373 wants to merge 8 commits into
release-1.1from
bugfix/kafka-various-fix

Conversation

@cjc7373

@cjc7373 cjc7373 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Below are AI generated description.

Fixes #3027.

Problem

This draft PR covers two separate Kafka 1.1 problems. They are related only because both affect Kafka config/reconfigure behavior.

1. Kafka 3.x config constraints drifted from Kafka runtime configs

The old hand-written ParametersDefinition schema was stale and not role-aware.

Concrete examples from the old schema:

  • valid Kafka 3.x keys such as listeners, controller.quorum.voters, and group.consumer.session.timeout.ms were absent;
  • combined, controller, and broker components all used the same schema, so wrong-role keys could not be rejected per component role;
  • LIST-valued keys could accept a CUE string list in the runtime schema while the reachable external schema still required a scalar string.

2. JMX_PORT was set as a container-wide env var

The old component definitions put JMX_PORT=5555 in the container env. That means ordinary Kafka CLI commands started inside the container can inherit JMX_PORT and try to open the same JMX port as the Kafka process.

This PR moves JMX_PORT into the Kafka startup script path so the Kafka process still gets JMX, but ad-hoc CLI commands do not inherit it from the container env.

Changes

  • Generate Kafka 3.x CUE constraints from Kafka ConfigDef with tools/gen-cue.
  • Split Kafka 3.x schemas by role: combined, controller, broker.
  • Wire each Kafka 3.x ParamConfigRenderer to its matching role schema.
  • Emit string | [...string] for LIST-valued external keys while keeping non-LIST external keys as string.
  • Keep the Kafka 2.7 path unchanged.
  • Move JMX_PORT=5555 out of component env and into set_jvm_configuration().
  • Remove stale rendered default key allow.everyone.if.no.acl.found, which is not present in the generated Kafka 3.9 schema.
  • Add generator tests so live keys in configs/kafka-server.prop.tpl remain covered and LIST/non-LIST external contracts cannot diverge again.

Local and repository validation

Current head: 6e6dc0d9f5417c1d0b180943d60edea95dc7fcaf.

Passed:

  • helm dependency build addons/kafka
  • helm lint addons/kafka
  • helm template kafka addons/kafka --namespace kafka-pr3002
  • static check: every live key in configs/kafka-server.prop.tpl is present in configs/kafka-server-constraint.cue
  • CUE positive check: valid broker/controller/combined examples pass
  • CUE negative check: broker rejects a controller-only key; controller rejects a broker-only key; combined rejects an unknown key
  • render check: no container env entry named JMX_PORT; only the Kafka startup script exports it
  • Temurin 17 / Maven focused generator test rendersScopedSchemas: 1 test, 0 failures, 0 errors
  • the other five generator test methods: 5 tests, 0 failures, 0 errors
  • unchanged local CUE fixture matrix: 8/8 exit with rc=0; the three LIST cases that failed at the previous head now pass
  • repository CI: 8/8 checks pass at the current head

The full generator module is 6 tests / 1 failure at the existing log.retention.ms expectation in rendersRealKafkaSources. The untouched parent head reproduces the same logical failure, so this is a pre-existing baseline red, not a regression introduced by this patch.

These results close repository CI and the local CUE scalar/list contract only. They do not prove Controller/Broker runtime behavior.

Runtime validation still required before leaving draft

Fresh runtime validation at the current head has not started: Kubernetes invocation is 0 and runtime N=0. The current environment blocker is the fresh release-1.1 vcluster handoff owned by Mason.

The runtime target must use KubeBlocks source pin apecloud/kubeblocks release-1.1@00a42539bc879c0748371fd51fecfb62dca77461 with matching CRDs and full controller imageID/digest readback. Prior r11/r12 scenes are cleanup-closed and must not be reused.

Required fresh runtime matrix:

  • Helm upgrade path: install the old release-1.1 chart, create a Kafka cluster, upgrade to this PR chart, and verify the kafka-pd -> role-specific PD rename does not cause unexpected config re-render/restart and reconfigure still works.
  • Provision + reconfigure on combined topology.
  • Provision + reconfigure on separated controller topology.
  • Provision + reconfigure on separated broker topology.
  • Negative cases: wrong-role key rejected on controller and broker role schemas.

This PR is not release-ready until the fresh environment handoff, reviewed invocation-0 launch packet, and runtime evidence are complete.

@codecov-commenter

codecov-commenter commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (e242a73) to head (1b80419).
⚠️ Report is 7 commits behind head on release-1.1.

Additional details and impacted files
@@             Coverage Diff             @@
##           release-1.1   #3002   +/-   ##
===========================================
  Coverage         0.00%   0.00%           
===========================================
  Files               73      73           
  Lines             9259    9259           
===========================================
  Misses            9259    9259           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@weicao

weicao commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Early review while this is still draft (classes 1–8 walked on the current head). The direction — regenerating constraints from Kafka's own ConfigDef via a checked-in generator, and splitting the ParametersDefinition per role — is sound, and the 2.7 path (KafkaParameter + configs/2.7/...cue) is correctly left untouched. Four things to settle before this leaves draft:

1. Empty body / no issue. Repo rule: every code PR needs an issue describing what it fixes, the failing scenario, and reproduction. Please state what the old hand-written constraints broke (rejected valid reconfigure values? missing keys? wrong types?) with one concrete example.

2. Scope: the JMX_PORT move is a second root cause bundled in. Moving JMX_PORT=5555 from container-level cmpd env into set_jvm_configuration() fixes the classic "any kafka CLI invocation in the container inherits JMX_PORT and dies with Port already in use" problem — a real fix (I verified nothing else at head consumes the removed env), but it is unrelated to constraint regeneration. Either split it into its own PR or document both root causes explicitly in the body/issue so the behavioral deltas stay attributable.

3. Upgrade path for the ParametersDefinition rename. kafka-pd is deleted and replaced by kafka-pd-{combined,controller,broker}, and the PCRs are re-pointed. For existing 1.1 clusters: what happens to in-flight ComponentParameter objects bound via the old PD on helm upgrade — clean re-bind, no-op, or unexpected re-render/restart? Needs an explicit upgrade test (install old chart → create cluster → upgrade chart → verify no config re-render/restart and reconfigure still works).

4. Closed-struct validation per role needs runtime evidence. #Controller, #Broker (and #Combined = #Controller & #Broker) are CUE definitions, i.e. closed structs — any key present in a rendered server.properties that is missing from the definition now fails validation for that role. That is the point of the split, but it means the evidence bar is: provision + reconfigure passing on all three topologies (combined, separate broker+controller) on release-1.1, plus one negative case per role showing a wrong-role key is rejected. mvn test on the generator does not cover the rendered-config-vs-schema contract.

Nice touches: generator README documents the DistributedConfig boundary; pinned kafka.version=3.9.0 makes regeneration reproducible.

@weicao weicao added nopick Not auto cherry-pick when PR merged chart-release-1.1 Auto release chart when PR merged release-1.1. labels Jul 6, 2026
@cjc7373

cjc7373 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

@weicao don't modify this PR. It is still under development.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chart-release-1.1 Auto release chart when PR merged release-1.1. nopick Not auto cherry-pick when PR merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants