Repository navigation
feat(opensearch): make setVMMaxMapCount configurable - #4026
europrinter (yankawai) wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (7)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe OpenSearch API and Helm chart now expose ChangesOpenSearch vm.max_map_count configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The configurable setting preserves existing behavior by default and adds an opt-out for environments that manage the node sysctl themselves; no actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. (6 skipped: 6 unsupported.) ✨ 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 |
Signed-off-by: Yan Bondarenko <[email protected]>
ddcc871 to
19e86aa
Compare
|
This PR conflicts with |
|
Closing this one, main has moved past it. #4152 raises vm.max_map_count on every node from a DaemonSet in the opensearch-operator package, and #2682 then set |
What this PR does
packages/apps/opensearch/templates/opensearch.yamlhardcodesspec.general.setVMMaxMapCount: true, so the opster operator always injects a privilegedinit-sysctlinitContainer. Tenant namespaces carry nopod-security.kubernetes.io/enforcelabel and therefore inherit the cluster-wide default, which isbaseline— and baseline forbids privileged containers. The StatefulSet never creates a Pod, the*-externalServices get their LoadBalancer IPs but keep empty EndpointSlices, and theOpenSearchClusterstill reportsPHASE: RUNNINGbecause only the non-privileged securityconfig Job runs. That is #3022, open since June with no fix.The chart offers no way out: there is no values knob, so an operator who has already raised
vm.max_map_counton the nodes still cannot stop the operator from adding the privileged container. This PR addssetVMMaxMapCount, defaulting totrueso nothing changes for existing installations, and threads it into the CR.Why the default stays
trueFlipping the default to
falsewould be the wrong trade.vm.max_map_countis a node-level sysctl, not a namespaced one, so it cannot be set through a PodsecurityContext.sysctls— a cluster whose nodes still sit at the Linux default of 65530 would get a Pod that starts and then fails OpenSearch's own bootstrap check instead of failing admission. Worse, anyone who worked around #3022 by labelling the namespaceprivilegedwould silently lose the sysctl. Keepingtruemeans this PR is purely additive.Verification
Measured on a cluster reproducing #3022: nodes report
vm.max_map_count = 65530, the machine config carries novm.*sysctl, and the tenant namespace has no pod-security label, so the cluster-wideenforce: baselineapplies and the StatefulSet logs theFailedCreatefrom the issue verbatim.New suite
tests/vm-max-map-count_test.yamlasserts both branches. Positive control: hardcoding the template back totrueturns one of the two cases red, so the asserts really follow the value rather than the literal. Full chart suite 60/60 across 5 suites.values.schema.json,README.md, the generated Go type and theopensearch-rdcozyrds are all updated in step; the cozyrdsopenAPISchemais byte-identical tojq -cof the schema.What this does not fix
Half the problem lives outside the chart. Cozystack ships no node sysctls —
packages/core/talosbuilds images only — so an operator still has to setvm.max_map_count=262144on the nodes before turning this knob off. This PR gives them the switch; whether the platform should ship the sysctl itself is the other half of #3022 and belongs in a separate change.Refs #3022
Summary by CodeRabbit
New Features
setVMMaxMapCountoption for OpenSearch deployments, enabled by default.vm.max_map_countsystem setting using a privileged initialization container.Documentation
Tests