Skip to content

[PLAT-5173] Add GOMEMLIMIT Helm snippet and chart lint warning - #162

Merged
prasiVenkat merged 2 commits into
mainfrom
plat-5173-gomemlimit-helm-snippet
Aug 11, 2026
Merged

[PLAT-5173] Add GOMEMLIMIT Helm snippet and chart lint warning#162
prasiVenkat merged 2 commits into
mainfrom
plat-5173-gomemlimit-helm-snippet

Conversation

@prasiVenkat

@prasiVenkat prasiVenkat commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Description

Platform tooling for PLAT-5173: add a canonical Helm helper and install guide for deriving GOMEMLIMIT from resources.limits.memory, and extend helm-chart-lint with Rule 8 to warn when a Go repo (go.mod present) renders manifests without GOMEMLIMIT.

  • helm/snippets/go-gomemlimit.tplapp.goMemoryLimit helper: resources.limits.memory × go.memoryLimitRatio (default 0.85), Mi suffix required, fails render on missing/invalid limit
  • helm/snippets/README.md — install steps for _helpers.tpl, values.yaml, and deployment.yaml, plus verification commands and scope (active: products-api, shipments-api)
  • .github/actions/helm-chart-lint/scripts/lint.sh — Rule 8 emits a ::warning:: when helm template output lacks GOMEMLIMIT (non-blocking; skips if template render fails)

Jira Issue: https://revolutionparts.atlassian.net/browse/PLAT-5173

Background

  • Platform audit under PLAT-5128 found no Go services with GOMEMLIMIT configured; Go auto-detects cgroup CPU limits but not memory, leading to kernel OOM kills (confirmed on products-api — PLAT-5155).
  • Per-service rollouts already landed in products-api (PLAT-5155) and shipments-api (PLAT-5164); this PR centralizes the reusable pattern in encodium/.github so future Go APIs adopt the same helper.
  • Deprecated APIs (prices-api, suppliers-api) and Argo WorkflowTemplates in encodium/workflows are out of scope here; workflows tracked under PLAT-5128.

Related PRs

Testing Information

Pre-merge

From a Go API repo that uses helm-chart-lint (e.g. products-api):

# Rule 8 — repo with go.mod + GOMEMLIMIT should pass (no Rule 8 warning)
# Run the chart lint action locally or via CI on a branch with the snippet applied

# Snippet happy path — 256Mi limit → 217MiB
helm template <release> deployments/ -f deployments/values.yaml --set image.tag=test | rg -A1 'name: GOMEMLIMIT'

# Custom limit — 512Mi → 435MiB
helm template <release> deployments/ -f deployments/values.yaml \
  --set image.tag=test --set resources.limits.memory=512Mi | rg -A1 'name: GOMEMLIMIT'

# Negative — must fail render
helm template <release> deployments/ -f deployments/values.yaml \
  --set image.tag=test --set resources.limits.memory=null
  
  Expected happy-path output
            - name: GOMEMLIMIT
              value: "217MiB"

Expected errors

  • resources.limits.memory must be set
  • resources.limits.memory must use Mi suffix for GOMEMLIMIT calculation, got "256Gi"

Pre-merge checklist

  • helm/snippets/go-gomemlimit.tpl renders expected GOMEMLIMIT for 256Mi and 512Mi limits
  • Template fails when resources.limits.memory is null or not Mi suffix
  • Rule 8 warns on a Go repo chart missing GOMEMLIMIT; no warning when snippet is applied
  • Rule 8 skips gracefully when helm template fails (warning only, does not fail lint)

Post-deploy follow-up

  • No runtime deploy from this repo. After consuming repos merge and deploy:
kubectl exec -n api deploy/<service> -- printenv GOMEMLIMIT

Datadog (48h soak on rolled-out services): source:oom_kill service: — confirm no new OOM kills attributable to missing memory limit.

@cursor

cursor Bot commented Aug 11, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Documentation and advisory CI warnings only; no runtime deploys or blocking lint rule changes beyond allowing the new go values key.

Overview
Adds platform tooling for PLAT-5173 so Go APIs can derive GOMEMLIMIT from Kubernetes memory limits and get nudged in CI when they do not.

helm/snippets/ introduces the reusable app.goMemoryLimit helper (limit × go.memoryLimitRatio, default 0.85; Mi suffix required) plus an install/verification README for copying into service charts.

helm-chart-lint now treats top-level go as an approved chart-control key (action default, lint.sh, and the bad-chart test env). Rule 8 runs only when go.mod exists at the repo root: it helm templates the chart and emits a non-blocking ::warning:: if rendered manifests lack a GOMEMLIMIT env var; failed renders skip the check with a warning instead of failing lint.

Reviewed by Cursor Bugbot for commit 149937f. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit f798114. Configure here.

Comment thread helm/snippets/README.md
Comment thread helm/snippets/go-gomemlimit.tpl Outdated
@prasiVenkat
prasiVenkat marked this pull request as ready for review August 11, 2026 14:05
@prasiVenkat
prasiVenkat requested a review from a team as a code owner August 11, 2026 14:05
@prasiVenkat
prasiVenkat requested a review from aspencer August 11, 2026 14:05
@prasiVenkat
prasiVenkat merged commit b0d895d into main Aug 11, 2026
6 checks passed
@prasiVenkat
prasiVenkat deleted the plat-5173-gomemlimit-helm-snippet branch August 11, 2026 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants