Set GOMEMLIMIT from container memory limit by fwiesel · Pull Request #319 · cobaltcore-dev/openstack-hypervisor-operator

fwiesel · 2026-06-19T09:49:28Z

Without GOMEMLIMIT the Go GC is unaware of the container limit and may
let the heap grow until the kernel OOM-kills the process.

Summary by CodeRabbit

New Features
- The manager now automatically adjusts its Go memory limit based on the container memory limit, helping the service operate more predictably under constrained resources.

coderabbitai · 2026-06-19T09:49:37Z

📝 Walkthrough

Walkthrough

The controller-manager Deployment template adds a GOMEMLIMIT environment variable to the manager container, sourced from the container's memory limit via resourceFieldRef with a divisor of "1".

Changes

GOMEMLIMIT Configuration

Layer / File(s)	Summary
Add GOMEMLIMIT environment variable `charts/openstack-hypervisor-operator/templates/deployment.yaml`	Adds a `GOMEMLIMIT` environment variable to the manager container, sourced from `limits.memory` via `resourceFieldRef` with divisor `"1"`.

Estimated code review effort: 1 (Trivial) | ~3 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and accurately summarizes the main change: deriving GOMEMLIMIT from the container memory limit.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch set-gomemlimit

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

github-actions · 2026-06-19T09:53:40Z

Merging this branch will increase overall coverage

Impacted Packages	Coverage Δ	🤖
github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1	0.00% (ø)
github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller	69.37% (+0.39%)	👍

Coverage by file

Changed files (no unit tests)

Changed File	Coverage Δ	Total	Covered	Missed	🤖
github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1/hypervisor_types.go	0.00% (ø)	0	0	0
github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller/constants.go	0.00% (ø)	0	0	0
github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller/gardener_node_lifecycle_controller.go	63.95% (-8.46%)	86 (+28)	55 (+13)	31 (+15)	👎
github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller/hypervisor_controller.go	81.08% (+5.41%)	111 (+37)	90 (+34)	21 (+3)	👍
github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller/offboarding_controller.go	73.91% (+2.04%)	69 (+5)	51 (+5)	18	👍

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files

github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller/gardener_node_lifecycle_controller_test.go
github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller/hypervisor_controller_test.go
github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller/offboarding_controller_test.go

Copilot

Pull request overview

This PR updates the Helm deployment for the controller-manager to set GOMEMLIMIT from the container’s configured memory limit so the Go GC can respect cgroup/container limits and reduce OOM-kill risk.

Changes:

Add a GOMEMLIMIT environment variable to the controller-manager container, sourced from limits.memory via resourceFieldRef.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Without GOMEMLIMIT the Go GC is unaware of the container limit and may let the heap grow until the kernel OOM-kills the process.

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

charts/openstack-hypervisor-operator/templates/deployment.yaml (1)
50-54: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Leave headroom in GOMEMLIMIT
charts/openstack-hypervisor-operator/templates/deployment.yaml:50-54 sets GOMEMLIMIT to limits.memory exactly. Go still needs room for stacks, cgo, and other non-heap memory, so this can still OOM-kill the container. Set it to ~85–90% of the pod memory limit instead.
🤖 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 `@charts/openstack-hypervisor-operator/templates/deployment.yaml` around lines
50 - 54, The GOMEMLIMIT setting in the deployment template is using the full
memory limit, leaving no room for non-heap usage. Update the deployment’s
GOMEMLIMIT environment variable in the pod spec to use a fraction of the
container memory limit (around 85–90%) instead of mapping directly to
limits.memory. Use the existing GOMEMLIMIT env entry in the deployment template
to make the change.

🤖 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.

Inline comments:
In `@charts/openstack-hypervisor-operator/templates/deployment.yaml`:
- Around line 50-54: The GOMEMLIMIT env in the deployment template is always
rendered through resourceFieldRef, which can silently fall back to node
allocatable memory if controllerManager.manager.resources.limits.memory is
removed by overrides. Update the deployment.yaml logic around the GOMEMLIMIT
entry to only emit it when a memory limit exists, or add a chart guard that
checks the manager resources limit before rendering this env var so it stays
tied to the container limit.

---

Nitpick comments:
In `@charts/openstack-hypervisor-operator/templates/deployment.yaml`:
- Around line 50-54: The GOMEMLIMIT setting in the deployment template is using
the full memory limit, leaving no room for non-heap usage. Update the
deployment’s GOMEMLIMIT environment variable in the pod spec to use a fraction
of the container memory limit (around 85–90%) instead of mapping directly to
limits.memory. Use the existing GOMEMLIMIT env entry in the deployment template
to make the change.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 44971fd1-a88b-473d-825c-cabb87613c2a

📥 Commits

Reviewing files that changed from the base of the PR and between 744a801 and 02985d0.

📒 Files selected for processing (1)

charts/openstack-hypervisor-operator/templates/deployment.yaml

mchristianl previously approved these changes Jun 19, 2026

View reviewed changes

fwiesel dismissed mchristianl’s stale review via 15714f5 June 19, 2026 10:16

fwiesel force-pushed the set-gomemlimit branch from fe6e070 to 15714f5 Compare June 19, 2026 10:16

fwiesel requested review from Copilot and mchristianl June 19, 2026 10:18

Copilot started reviewing on behalf of fwiesel June 19, 2026 10:18 View session

Copilot AI reviewed Jun 19, 2026

View reviewed changes

Comment thread charts/openstack-hypervisor-operator/templates/deployment.yaml

Set GOMEMLIMIT from container memory limit

02985d0

Without GOMEMLIMIT the Go GC is unaware of the container limit and may let the heap grow until the kernel OOM-kills the process.

fwiesel force-pushed the set-gomemlimit branch from 15714f5 to 02985d0 Compare July 1, 2026 08:21

coderabbitai Bot reviewed Jul 1, 2026

View reviewed changes

Comment thread charts/openstack-hypervisor-operator/templates/deployment.yaml

notandy approved these changes Jul 1, 2026

View reviewed changes

fwiesel removed the request for review from mchristianl July 2, 2026 07:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Set GOMEMLIMIT from container memory limit#319

Set GOMEMLIMIT from container memory limit#319
fwiesel wants to merge 1 commit into
mainfrom
set-gomemlimit

fwiesel commented Jun 19, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 19, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

github-actions Bot commented Jun 19, 2026

Changed files (no unit tests)

Changed unit test files

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

fwiesel commented Jun 19, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

github-actions Bot commented Jun 19, 2026

Merging this branch will increase overall coverage

Changed files (no unit tests)

Changed unit test files

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fwiesel commented Jun 19, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 19, 2026 •

edited

Loading