Skip to content

fix(harbor): give the nginx proxy explicit resources so it survives the tenant LimitRange - #4095

Merged
Aleksei Sviridkin (lexfrei) merged 1 commit into
mainfrom
fix/harbor-nginx-resources
Sep 6, 2026
Merged

Aleksei Sviridkin (lexfrei) merged 1 commit into
mainfrom
fix/harbor-nginx-resources

Conversation

@IvanHunters

@IvanHunters IvanHunters commented Sep 6, 2026 •

Copy link
Copy Markdown
Collaborator

What this PR does

With expose.type: clusterIP the goharbor chart deploys an nginx proxy pod, but the harbor chart set no resources for it. In a tenant namespace the tenant-quota LimitRange defaults any container without resources to a 128Mi memory limit, so the proxy is OOMKilled (exit 137) and Harbor never becomes ready.

This gives the nginx proxy t1.small (512Mi), and bumps the portal (also an nginx container) from t1.nano (128Mi) to t1.micro (256Mi), which sat at the same limit.

Reported from a tenant where the Harbor nginx pod was OOMKilled at a 128Mi limit.

Screenshots

Not a UI change.

Downstream repositories

Walked the trigger map against the diff. The change is limited to packages/apps/harbor/templates/harbor.yaml and only sets container resource presets already used elsewhere in the same file. No values.schema.json, values.yaml default, ApplicationDefinition, version enum, or release.prefix is touched, so nothing downstream (provider schemas, website reference pages, ansible role) is affected.

Release note

fix(harbor): set resources for the nginx proxy and raise the portal preset so neither is OOMKilled by the tenant LimitRange default (128Mi)

Summary by CodeRabbit

  • Performance
    • Updated Harbor’s web portal resource allocation to improve runtime capacity.
    • Added default resource settings for the NGINX component.

…he tenant LimitRange

With expose.type=clusterIP the goharbor chart deploys an nginx proxy pod, but
the chart set no resources for it. In a tenant namespace the LimitRange defaults
any container without resources to a 128Mi memory limit, so the proxy is
OOMKilled (exit 137) and Harbor never comes up.

Set the nginx proxy to t1.small (512Mi) and bump the portal (also nginx-based)
from t1.nano (128Mi) to t1.micro (256Mi) for the same reason.

Signed-off-by: IvanHunters <[email protected]>
@github-actions github-actions Bot added size/XS This PR changes 0-9 lines, ignoring generated files area/storage Issues or PRs related to storage (linstor, seaweedfs, bucket, velero, harbor) kind/bug Categorizes issue or PR as related to a bug labels Sep 6, 2026
@coderabbitai

coderabbitai Bot commented Sep 6, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 647c8ed0-e07f-4561-b55e-24d459dc79b8

📥 Commits

Reviewing files that changed from the base of the PR and between 41f92ec and 5a7e2d5.

📒 Files selected for processing (1)
  • packages/apps/harbor/templates/harbor.yaml

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

The Harbor Helm values add resource settings for nginx and increase the portal resource preset from t1.nano to t1.micro.

Changes

Harbor resource presets

Layer / File(s) Summary
Update Harbor resource presets
packages/apps/harbor/templates/harbor.yaml
Adds nginx resources with the t1.small preset and changes the portal preset to t1.micro.

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

Merge Risk: ⚪ Minimal · up to 5a7e2

Harbor now assigns explicit nginx resources and increases portal resources, reducing OOMKill exposure from tenant LimitRange defaults. The configuration change is ready to merge.

🚥 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 summarizes the main change: it gives the Harbor nginx proxy explicit resources to prevent OOMKills caused by tenant LimitRange defaults.
Docstring Coverage ✅ Passed 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 0…
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 fix/harbor-nginx-resources

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.

@lexfrei
Aleksei Sviridkin (lexfrei) merged commit 86d62bb into main Sep 6, 2026
36 checks passed
@lexfrei
Aleksei Sviridkin (lexfrei) deleted the fix/harbor-nginx-resources branch September 6, 2026 20:05
myasnikovdaniil added a commit that referenced this pull request Oct 23, 2026
)

## What this PR does

#4095 gave the Harbor nginx proxy the `t1.small` preset and moved the
portal from `t1.nano` to `t1.micro`, so neither falls back to the
LimitRange default of a tenant namespace, 250m CPU and 128Mi. It came
without a test. This adds a helm-unittest suite that pins both presets
in the nested HelmRelease and checks that they stay the same when the
other components are resized, so dropping them or tying them to another
component's values fails the suite.

Both containers run nginx with `worker_processes auto`, which starts one
worker per CPU of the node whatever the container's CPU limit is. With
the proxy config the chart renders and `goharbor/nginx-photon:v2.15.1`,
96 workers idle at about 205 MiB, so on a node with many CPUs the proxy
is OOMKilled at 128Mi before it serves a request. That is what happened
on a v1.6.5 cluster, where both nginx pods of a default Harbor restarted
5 and 6 times within minutes, because `release-1.6` never got #4095. The
backport, with this suite and the measurements, is a separate PR.

`helm unittest .` in `packages/apps/harbor` passes 47 of 47 with
helm-unittest 1.1.2. With the #4095 hunk reverted the three new cases
fail and the 44 existing ones pass. Each of seven template mutations
fails at least one new case. They drop the nginx block, give nginx
`t1.micro` or the portal `t1.nano`, tie nginx to the core values or the
portal to the jobservice values, misspell the `resources` key under
nginx, and make core ignore its preset, which the resize case checks so
that it cannot pass on overrides that never reach the release.

### Screenshots

Not a UI change.

### Downstream repositories

One new helm-unittest suite in `packages/apps/harbor/tests`. No
template, values key, schema, default, README or package change, so no
trigger map entry applies.

- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:

### Release note

```release-note
NONE
```

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
<!-- coderabbit-ai-generated:v1 -->

## Summary by CodeRabbit

* **Tests**
* Added Helm coverage verifying default resource settings for Harbor’s
nginx and portal components.
* Added coverage confirming nginx and portal memory allocations remain
consistent when related service resources are customized.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/storage Issues or PRs related to storage (linstor, seaweedfs, bucket, velero, harbor) kind/bug Categorizes issue or PR as related to a bug size/XS This PR changes 0-9 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants