Repository navigation
fix(opensearch): give the securityconfig update job its own resources - #4228
Aleksei Sviridkin (lexfrei) merged 1 commit into
Conversation
The operator hands spec.security.config.updateJob.resources straight to the Job container and the chart left it unset, so a tenant namespace applied its LimitRange default of 128Mi instead. securityadmin.sh runs java with no -Xmx and is OOMKilled at that size, and the operator builds the Job with backoffLimit 0, so that one kill is final: the security index is never written and every node stays unready. Constants rather than a preset value, because the tool configures the cluster once and exits. Assisted-by: LLM Signed-off-by: IvanHunters <[email protected]>
github-actions
Bot
added
size/M
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe OpenSearch chart assigns fixed CPU and memory resources to the security configuration update job. New tests verify the values under default sizing and with the ChangesOpenSearch update job resources
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The update job receives sufficient fixed resources without being overridden by the nano preset, with no concrete merge-blocking risk identified. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
LGTM.
The thing that could have made this inert is whether the operator reads the field at all, so I checked that first. It does, at the pinned version: NewSecurityconfigUpdateJob in opensearch-operator/pkg/builders/cluster.go takes Spec.Security.GetConfig().GetUpdateJob().Resources on line 1116 and puts it on the updater container on 1129. Chart and appVersion are 2.8.0 per packages/system/opensearch-operator/Makefile. backoffLimit := int32(0) sits two lines above, so the single-attempt part holds too, and securityadmin.sh at security 2.11.1.0 execs java with $JAVA_OPTS alone, which the Job never sets, so the heap really does come from the container limit.
It also works on a cluster. The e2e run on this PR has the job at Normal Completed, with no OOMKilled, no exitCode: 137 and no BackoffLimitExceeded anywhere in the log. The same job on a run from a change that cannot reach OpenSearch ends at BackoffLimitExceeded with 157 .opendistro_security failures behind it.
The LimitRange cannot refuse the new numbers. packages/apps/tenant/templates/quota.yaml carries default and defaultRequest and nothing else: no min, no max, no maxLimitRequestRatio. Explicit values only displace the defaults, and ephemeral-storage keeps getting one. The sibling ResourceQuota is the only object that could reject the pod, and for the e2e tenant it caps limits.memory and limits.cpu at 128Gi and 60 cores, against 512Mi and 1 here.
I mutated each asserted line separately. 256Mi, 100m and "1" each redden one assertion, 512Mi reddens two, one per test, and replacing the literal block with the preset include reddens all five. So the second test really does hold the preset off. hack/helm-unit-tests.sh picks the suite up from the package test target, so it runs in the unit lane.
Two things for the body, neither blocking.
The claim that this is what has been failing the opensearch chainsaw suite in every run since 2026-09-11 is too strong, and the run on this PR is the counterexample. The suite is still red, but at verify-statefulset-rolled-onto-the-new-mount-plan instead of verify-operator-managed-listener, with the job completed and the rolled node pod logging Not yet initialized three times and not one .opendistro_security error. That looks like #4230 rather than this bug, and the two are ordered: Status.Initialized comes from AllMastersReady in controllers/opensearchController.go:244, and pkg/reconcilers/cluster.go:100-103 sends the bootstrap pod to StateAbsent once it flips, so the node going ready is the event that removes it. While the job was being killed the node never went ready, so #4230 could not fire. Fixing this one is what lets it. Worth saying so, otherwise the next person reads a still-red suite as the fix not working.
Second, resourceQuotas is empty by default in packages/apps/tenant/values.yaml, and quota.yaml renders the LimitRange only when it is set, so a namespace without one is the ordinary case rather than an exotic one. There the job had no memory ceiling at all and now gets 512Mi. Probably fine, since the measurement puts that at about twice what the tool uses and a JVM with no -Xmx sizes itself to fit, but the body describes only the LimitRange half. Same for the numbers themselves: 2.11.1 is what files/versions.yaml maps v2 to, so they cover the default and not v1 or v3.
2 participants
What this PR does
The chart rendered
spec.security.configwithoutupdateJob, and the operator passes that field straight through to the Job container it builds forsecurityadmin. With nothing set, a tenant namespace applies its owntenant-range-limitsLimitRange default of 128Mi instead.securityadmin.shrunsjavawith no-Xmxof its own, so the JVM sizes its heap from the container limit and does not fit in 128Mi. The operator builds the Job withbackoffLimit: 0, so the single OOM kill is final. Nothing writes the.opendistro_securityindex after that, every node sits behindNot yet initialized (you may need to run securityadmin), and the release never reaches ready.This is what has been failing the
opensearchchainsaw suite in every run that executes it since 2026-09-11. The suite waits 12 minutes forstatus.readyReplicasonopensearch-test-nodes, which is the whole 748s runtime of the red test. Full write-up in #4227.Where the numbers come from
Measured against
opensearchproject/opensearch:2.11.1onlinux/amd64, running the operator's own command against a fresh single-node cluster:Having no
-Xmx, the tool grows into whatever it is given, so the peak tracks the limit and the headroom is what matters rather than the peak itself. The values are constants and not aresourcesPresetlookup: the tool configures the cluster once and exits, so what it needs does not scale with the nodes it configures, and the smallest preset would hand it back the 128Mi it dies at.Upgrading an already-stuck release
This does not unstick one on its own. The operator skips the Job entirely while the existing one carries a matching securityconfig checksum, and bumping the chart does not change that checksum, so the failed Job stays where it is. Delete it once and the operator rebuilds it with the resources above:
A fresh install needs nothing.
Not fixed here
backoffLimit: 0is the operator's, and it is what turns one transient kill into a permanent dead cluster. Worth raising upstream separately.Screenshots
Not a UI change.
Downstream repositories
Release note
Summary by CodeRabbit