Skip to content

fix(eks): set resource requests/limits for Calico/Tigera components#343

Open
amdove wants to merge 2 commits into
mainfrom
helm-resource-requests-limits
Open

fix(eks): set resource requests/limits for Calico/Tigera components#343
amdove wants to merge 2 commits into
mainfrom
helm-resource-requests-limits

Conversation

@amdove

@amdove amdove commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Description

Sets resource requests and limits on the Calico/Tigera components installed by the tigera-operator Helm chart on EKS. This should resolve resource contention issues with Calico: previously the chart shipped resources: {} and the operator's built-in component defaults set no memory bounds, so these pods had no guaranteed reservations and no ceilings, leaving them to compete unbounded with workloads on busy nodes.

Policy applied (from the review discussion):

  • Memory: request == limit. Memory is non-compressible — an equal request/limit gives each pod a guaranteed floor and a bounded ceiling that protects the node from a runaway component.
  • CPU: request only, no CPU limit. Avoids CFS-throttling the dataplane (a throttled calico-node degrades networking cluster-wide).
Component CPU request Memory req = limit
tigera-operator 100m 256Mi
calico-node 250m 512Mi
calico-typha 100m 256Mi
calico-kube-controllers 50m 128Mi
calico-apiserver 100m 256Mi

calico-node gets the largest memory bound and keeps the operator's built-in 250m CPU floor, since Felix memory scales with the number of endpoints/policies.

Code Flow

All changes are in deployTigeraOperator (lib/steps/eks_helpers.go), which builds the tigera-operator Helm release for the EKS path. Two small helpers were added:

  • calicoResources(cpuRequest, memory) builds the {requests: {cpu, memory}, limits: {memory}} block encoding the memory-bounded / CPU-unbounded policy.
  • calicoComponentOverride(container, cpuRequest, memory) wraps a per-container resource override in the operator's component-deployment strategic-merge shape.

The dataplane overrides (calicoNodeDaemonSet, typhaDeployment, calicoKubeControllersDeployment) are added under the existing installation values (the same passthrough already used for calicoNetwork/cni); the operator pod gets top-level resources; and calico-apiserver gets apiServer.apiServerDeployment (the chart defaults apiServer.enabled: true and PTD does not disable it).

Scope — what this does and does not fix

EKS: fixed. PTD installs the tigera-operator Helm chart on the EKS path, so these reservations attach directly to the release. Past resource-contention problems we've seen on EKS clusters should be resolved by this, and applying sane, explicit defaults here is the right thing to do regardless — it replaces "unbounded / no reservations" with predictable, logical values.

AKS: not fixed by this change. AKS clusters also run Calico, but it comes from the Azure-managed network-policy add-on, not a PTD-installed chart. That add-on does not expose resource requests/limits — Azure owns and reconciles the installation, so the components (calico-node, calico-typha, tigera-operator) ship with no requests/limits and cannot be tuned by us. This is a known, open Azure limitation: Azure/AKS#3496. So recent resource-contention issues observed on AKS clusters will not be addressed here and need a separate approach (e.g. node right-sizing, or migrating the network-policy dataplane to Cilium, which Azure recommends).

Watch after applying

  • OOMKills — because memory request == limit, a component that exceeds its ceiling is OOMKilled rather than allowed to burst. Headroom is comfortable today, but memory scales with cluster size; watch calico-node (512Mi) and calico-typha (256Mi) OOM/restart counts on the largest clusters and bump if needed.
  • Rollout churn — applying resources restarts the calico-node DaemonSet and the operator-managed deployments; expect a brief per-node CNI reconcile as they roll.

Category of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • I have reviewed my own diff and added inline comments on lines I want reviewers to focus on or that I am uncertain about

Testing

  • just format, go vet, and go test ./steps/ pass.
  • Added TestAWSEKSDeployTigeraResourceRequests asserting each component's request/limit values and the no-CPU-limit invariant.

The tigera-operator chart ships resources: {} and the operator's
built-in component defaults set no memory bounds, so the Calico pods
had no guaranteed reservations and no ceilings — a source of resource
contention on busy nodes.

Pin predictable reservations on the operator pod, calico-node,
calico-typha, calico-kube-controllers, and calico-apiserver, following
a memory-bounded / CPU-unbounded policy: memory request == limit
(non-compressible; guaranteed floor + bounded ceiling), CPU request
only with no CPU limit (avoids CFS-throttling the dataplane).

EKS only; AKS uses the managed Calico add-on.
@amdove
amdove marked this pull request as ready for review July 10, 2026 21:25
@amdove
amdove requested a review from a team as a code owner July 10, 2026 21:25
stevenolen
stevenolen previously approved these changes Jul 13, 2026
@amdove

amdove commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

@claude review

@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown

Claude finished @amdove's task in 1m 0s —— View job


Code Review

  • Read changed files
  • Review implementation
  • Review tests

This is a clean, well-scoped change. The implementation is correct and the rationale is sound. A few observations:

lib/steps/eks_helpers.go

The two helpers are a nice abstraction. calicoResources correctly encodes the memory-bounded / CPU-unbounded policy — memory appears in both requests and limits, CPU appears only in requests. calicoComponentOverride mirrors the operator's strategic-merge shape accurately.

One minor note: calicoComponentOverride only patches containers, not initContainers. For calico-node in particular, the operator injects an install-cni init container. Init containers are ephemeral (they exit after CNI installation and don't run during normal operation), so omitting them here is fine — they don't contribute to steady-state resource pressure. Worth being aware of if OOM pressure appears during node bootstrap.

The calico-apiserver override is placed under apiServer.apiServerDeployment rather than under installation, which matches the chart's schema for that component. That's the right call — a comment noting why it lives outside installation would help future readers who might wonder why it's inconsistent, but it's not blocking.

Resource values

The chosen values look reasonable for typical cluster sizes. calico-node at 512Mi is comfortable headroom for Felix on moderately-sized clusters. The note in the PR body about watching OOM/restart counts on the largest clusters is important operational context.

One thing to verify if you haven't already: the tigera-operator chart's resources key at the top level applies to the operator Deployment (not the Installation CR), which is what you want. The chart's values schema does support a top-level resources block for the operator pod, so this should work as written, but it's worth confirming against the chart version in use.

lib/steps/eks_aws_test.go

TestAWSEKSDeployTigeraResourceRequests is thorough and correctly asserts the no-CPU-limit invariant (assert.NotContains(..., "cpu")). The containerResources helper navigates the deep nested path cleanly. The test correctly checks all five components including the operator pod's top-level resources block.

Overall

No issues blocking merge. The change is correct, well-tested, and appropriately scoped to EKS. The AKS limitation is clearly documented. Approved by stevenolen — LGTM.

Address review feedback: note that calicoComponentOverride patches only
the main container (not ephemeral initContainers like install-cni), and
explain why calico-apiserver's resources live under apiServer rather than
installation (they map to the separate APIServer CR).
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