fix(kafka): set entity-operator resources to avoid OOM under tenant LimitRange - #2934
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughThe Strimzi Kafka Helm template adds explicit CPU/memory requests and a 512Mi memory limit to both ChangesKafka entityOperator Resource Constraints
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue where the Kafka entity-operator containers would crash due to insufficient memory when running in tenant namespaces with default LimitRanges. By explicitly defining resource requirements, the entity-operator can now start reliably, ensuring that KafkaTopic and KafkaUser resources are properly reconciled. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request configures explicit resource requests and limits for the topicOperator and userOperator in the Kafka entity operator template to prevent OOMKill issues caused by default LimitRanges in tenant namespaces. It also adds a corresponding test case to verify the memory limits. The reviewer suggests expanding the test assertions to also verify the CPU and memory requests for both operators.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| - equal: | ||
| path: spec.entityOperator.topicOperator.resources.limits.memory | ||
| value: 512Mi | ||
| - equal: | ||
| path: spec.entityOperator.userOperator.resources.limits.memory | ||
| value: 512Mi |
There was a problem hiding this comment.
To ensure comprehensive test coverage and prevent regressions, consider also asserting the CPU and memory requests for both the topicOperator and userOperator.
- equal:
path: spec.entityOperator.topicOperator.resources.requests.cpu
value: 100m
- equal:
path: spec.entityOperator.topicOperator.resources.requests.memory
value: 256Mi
- equal:
path: spec.entityOperator.topicOperator.resources.limits.memory
value: 512Mi
- equal:
path: spec.entityOperator.userOperator.resources.requests.cpu
value: 100m
- equal:
path: spec.entityOperator.userOperator.resources.requests.memory
value: 256Mi
- equal:
path: spec.entityOperator.userOperator.resources.limits.memory
value: 512MiThere was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/apps/kafka/tests/entityoperator_template_test.yaml (1)
24-34: ⚡ Quick winExpand assertions to cover requested resources too.
The test currently validates only
limits.memory, so it can miss regressions inrequests.cpuandrequests.memoryeven though those are part of the explicit resource contract.Suggested test extension
- it: topic-operator and user-operator set explicit resources (avoid 128Mi LimitRange OOM) release: name: test-kafka namespace: tenant-test asserts: + - equal: + path: spec.entityOperator.topicOperator.resources.requests.cpu + value: 100m + - equal: + path: spec.entityOperator.topicOperator.resources.requests.memory + value: 256Mi - equal: path: spec.entityOperator.topicOperator.resources.limits.memory value: 512Mi + - equal: + path: spec.entityOperator.userOperator.resources.requests.cpu + value: 100m + - equal: + path: spec.entityOperator.userOperator.resources.requests.memory + value: 256Mi - equal: path: spec.entityOperator.userOperator.resources.limits.memory value: 512Mi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/apps/kafka/tests/entityoperator_template_test.yaml` around lines 24 - 34, The test case "topic-operator and user-operator set explicit resources (avoid 128Mi LimitRange OOM)" currently only validates limits.memory for both topicOperator and userOperator, but does not verify the requested resources (requests.cpu and requests.memory). Add four additional equal assertions to the asserts section to validate spec.entityOperator.topicOperator.resources.requests.cpu, spec.entityOperator.topicOperator.resources.requests.memory, spec.entityOperator.userOperator.resources.requests.cpu, and spec.entityOperator.userOperator.resources.requests.memory with their expected values to ensure complete coverage of the explicit resource contract and prevent regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/apps/kafka/tests/entityoperator_template_test.yaml`:
- Around line 24-34: The test case "topic-operator and user-operator set
explicit resources (avoid 128Mi LimitRange OOM)" currently only validates
limits.memory for both topicOperator and userOperator, but does not verify the
requested resources (requests.cpu and requests.memory). Add four additional
equal assertions to the asserts section to validate
spec.entityOperator.topicOperator.resources.requests.cpu,
spec.entityOperator.topicOperator.resources.requests.memory,
spec.entityOperator.userOperator.resources.requests.cpu, and
spec.entityOperator.userOperator.resources.requests.memory with their expected
values to ensure complete coverage of the explicit resource contract and prevent
regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4d983303-5a66-4164-ad8a-85f58b84883e
📒 Files selected for processing (2)
packages/apps/kafka/templates/kafka.yamlpackages/apps/kafka/tests/entityoperator_template_test.yaml
…imitRange entityOperator.topicOperator/userOperator carried no resources, so in tenant namespaces (the LimitRange defaults containers to 128Mi) the JVM topic-operator and user-operator are OOMKilled on startup. The entity-operator stays in CrashLoopBackOff, KafkaTopics/KafkaUsers never reconcile, and on teardown topics get stuck on the strimzi.io/topic-operator finalizer. Set explicit requests/limits (256Mi/512Mi) so they no longer inherit the namespace default; add a template test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
3ee03da to
1ab5f17
Compare
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
LGTM (APPROVE)
Business context: entityOperator.topicOperator/userOperator rendered without resources, so under the tenant LimitRange default (memory: 128Mi) the topic-operator and user-operator JVM containers OOMKill and never reconcile KafkaTopic/KafkaUser. This PR sets explicit resources so they no longer inherit that default.
Blockers: none.
The problem is real and the fix is plumbed correctly:
- The tenant LimitRange applies
default.memory: 128Mito every container, which the JVM operator containers inherit and OOM on. Evidence: packages/apps/tenant/templates/quota.yaml:18-26. resourcesis a valid field under bothtopicOperatoranduserOperatorinkafka.strimzi.io/v1beta2, so the values reach the operator containers. Evidence: packages/system/kafka-operator/charts/strimzi-kafka-operator/crds/040-Crd-kafka.yaml:3463,3653.- The chosen limit cannot break scheduling: that LimitRange defines only
default/defaultRequest— nomax,min, ormaxLimitRequestRatio— so 256Mi/512Mi has no upper bound to violate, and 512Mi sits well above the 128Mi that OOMs (and in line with the cluster-operator's own 384Mi). Evidence: packages/apps/tenant/templates/quota.yaml:16-26. - helm-unittest passes (35/35), including the new assertion.
Non-blocking:
- The test asserts only
limits.memoryfor each operator. The fix is the wholeresourcesblock, sorequests.cpu/requests.memoryare part of the contract and currently unguarded — a later edit could drop them without failing the suite. Addequalassertions forresources.requests.cpu(100m) andresources.requests.memory(256Mi) on both operators. Evidence: packages/apps/kafka/tests/entityoperator_template_test.yaml:31-34. - The resources are hardcoded with no values knob. That matches the platform's sensible-default approach and is fine here; exposing entity-operator sizing the way
kafka/zookeeperexposeresourcesPresetwould be a reasonable follow-up if tuning is ever needed. Evidence: packages/apps/kafka/values.yaml (noentityOperatorsection).
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin release-1.4
git worktree add -d .worktree/backport-2934-to-release-1.4 origin/release-1.4
cd .worktree/backport-2934-to-release-1.4
git switch --create backport-2934-to-release-1.4
git cherry-pick -x 1ab5f17caca0a950596f6dfd6e64eda318739e1e |
|
Successfully created backport PR for |
What this PR does
packages/apps/kafkarendered the StrimziKafkaCR withentityOperator.topicOperator: {}/userOperator: {}and noresources. Tenant namespaces ship aLimitRangethat defaults containers tomemory: 128Mi, so the topic-operator and user-operator JVM containers (Kafka 3.9.1 + fabric8/vertx) are OOMKilled (exit 137) on startup. The<cluster>-entity-operatorDeployment never becomes Ready (CrashLoopBackOff), so:KafkaTopic/KafkaUserCRs never reconcile — topic creation effectively fails;Exceeded timeout of 300000ms while waiting for Deployment ...-entity-operator ... to be ready;KafkaTopics get stuck permanently with thestrimzi.io/topic-operatorfinalizer (no operator left to clear them), blocking namespace deletion.The Kafka broker + ZooKeeper come up fine — only the entity-operator dies. The
apps.cozystack.io/Kafkaschema exposes no field for entity-operator resources, so there is no user-side workaround.Fix
Set explicit
resources(requests256Mi/100m, limit512Mimemory) onentityOperator.topicOperatoranduserOperatorso they no longer inherit the 128Mi namespace default. Adds ahelm-unittestassertion for the rendered limits.Testing
Reproduced on a clean cluster (Cozystack v1.4.3 and v1.5.0-rc.2; Strimzi 0.45.1, Kafka 3.9.1): both entity-operator containers
OOMKilledat 128Mi → CrashLoopBackOff →KafkaTopicneverREADY. After bumping to 512Mi the entity-operator rolls cleanly (both containers Ready, 0 restarts) and theKafkaTopicreconciles toREADY=True.Release note
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests