fix: add istio resource defaults

[patrickleet]

Summary:

• Add explicit resource requests/limits for istiod, istio-cni, and ztunnel chart defaults.
• Preserve existing Helm values override behavior by merging user values over chart defaults.
• Cover default resource requests in render tests.

Verification:

• make test: 10/10 passed.
• hops config install --path xrs/stacks/k8s/istio --context colima: completed, package healthy.
• Colima IstioStack/default pat-local: Synced=True Ready=True.
• Colima Helm Releases pat-local-base, pat-local-cni, pat-local-istiod, pat-local-ztunnel: observed current generation, Synced=True Ready=True, state=deployed.
• Target istio-system/istiod: 1/1 ready, requests cpu=23m memory=121Mi, limits cpu=1 memory=512Mi.
• Target istio-system/istio-cni-node: DaemonSet rolled out 10/10, requests cpu=15m memory=100Mi, limits cpu=100m memory=256Mi.
• Target istio-system/ztunnel: DaemonSet rolled out 10/10, requests cpu=35m memory=100Mi, limits cpu=250m memory=256Mi.
• Compared against current target Goldilocks/VPA recommendations: istiod 23m/~121Mi, cni 15m/100Mi, ztunnel 35m/100Mi.

Summary by CodeRabbit

• Chores
  • Enhanced configuration handling for service mesh components to apply consistent default resource allocations. Default CPU and memory requests and limits are now systematically applied to Istiod, CNI, and Ztunnel deployments when user-provided values are not explicitly configured.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR refactors three Helm chart templates to establish explicit default values with resource request/limit settings, merge them consistently with user-provided overrides using mergeOverwrite, and emit the combined result. Test expectations are updated to reflect the now-guaranteed default resource requests.

Changes

Helm Chart Defaults and Merging

Layer / File(s)	Summary
Istiod defaults and value merging functions/render/210-istiod.yaml.gotmpl	Defines chartDefaults with ambient profile, mesh config (access log, optional extension providers), pilot env, and default resource requests/limits; computes mergedValues via mergeOverwrite and renders to spec.
CNI defaults and value merging functions/render/215-istio-cni.yaml.gotmpl	Establishes CNI chartDefaults with ambient profile and resource defaults; merges with user values and emits the combined result to chart specification.
Ztunnel defaults and value merging functions/render/220-ztunnel.yaml.gotmpl	Creates chartDefaults with resource request/limit defaults; merges with state values and updates fallback rendering to use merged values instead of conditional user-provided rendering.
Test expectations for default resource requests tests/test-render/main.k	Updates ambient minimal control-plane test to expect istiod pilot.resources.requests and ztunnel resources.requests in the rendered Helm Release values.

🎯 2 (Simple) | ⏱️ ~12 minutes

🐰 Three templates now share defaults so fine,
Merging values in a pattern divine,
Istiod, CNI, Ztunnel align,
Resources requested, tests redesigned,
Helm releases with blessed baseline!

🚥 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 'fix: add istio resource defaults' accurately summarizes the main change—adding explicit resource request/limit defaults for istio components.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/default-resource

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

tests/test-render/main.k (1)

50-53: ⚡ Quick win

Add assertions for default resource limits in the minimal render test.

This test now checks default requests, but the new default limits are still unverified. Adding limits assertions here will better lock the defaults introduced by this PR.

✅ Suggested test update

                         values = {
                             profile = "ambient"
                             pilot.resources.requests = {cpu = "23m", memory = "121Mi"}
+                            pilot.resources.limits = {cpu = "1000m", memory = "512Mi"}
                         }
@@
-                        values.resources.requests = {cpu = "35m", memory = "100Mi"}
+                        values.resources.requests = {cpu = "35m", memory = "100Mi"}
+                        values.resources.limits = {cpu = "250m", memory = "256Mi"}
                     }
                 }

Also applies to: 64-64

🤖 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 `@tests/test-render/main.k` around lines 50 - 53, The minimal render test
currently asserts default resource requests but not the new default limits;
update the test to assert that pilot.resources.limits is present and equals the
expected defaults (e.g., cpu and memory) alongside the existing
pilot.resources.requests check. Locate the values block (the map containing
profile = "ambient" and pilot.resources.requests) and add assertions for
pilot.resources.limits = {cpu = "<expectedCpuLimit>", memory =
"<expectedMemoryLimit>"}; duplicate the same addition for the other occurrence
of the values block referenced in the comment (the one at the second location)
so both minimal-render assertions lock the new defaults.

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

Nitpick comments:
In `@tests/test-render/main.k`:
- Around line 50-53: The minimal render test currently asserts default resource
requests but not the new default limits; update the test to assert that
pilot.resources.limits is present and equals the expected defaults (e.g., cpu
and memory) alongside the existing pilot.resources.requests check. Locate the
values block (the map containing profile = "ambient" and
pilot.resources.requests) and add assertions for pilot.resources.limits = {cpu =
"<expectedCpuLimit>", memory = "<expectedMemoryLimit>"}; duplicate the same
addition for the other occurrence of the values block referenced in the comment
(the one at the second location) so both minimal-render assertions lock the new
defaults.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0ded51ff-283c-4725-b852-50bfdd6ff5c0

📥 Commits

Reviewing files that changed from the base of the PR and between 0ffc674 and 503fd0c.

📒 Files selected for processing (4)

• functions/render/210-istiod.yaml.gotmpl
• functions/render/215-istio-cni.yaml.gotmpl
• functions/render/220-ztunnel.yaml.gotmpl
• tests/test-render/main.k

[github-actions]

Published Crossplane Package

The following Crossplane package was published as part of this PR:

Package: ghcr.io/hops-ops/istio-stack:pr-33-b5cc4c3c066c70cbebc1cf491a54626c41a19b6b

View Package