docs(rfc): RFC-0003 Kubernetes on Kind + Gateway API platform - #185
Merged
Conversation
Plan the deployment and operability platform: run the whole polyglot stack on Kubernetes (Kind) beside the compose stack. This is a platform, not the capstone -- a future RFC-0004 (authentication) rides on it. RFC-0001 and RFC-0002 stay immutable. - RFC-0003: Kind cluster; Helm packaging via a shared library chart (the D6 contract made executable) under an umbrella with per-profile values; Gateway API via Envoy Gateway (HTTPRoute + GRPCRoute); D6 /healthz+/readyz wired to probes; resource requests/limits from the measured footprint (the JVM MaxRAMPercentage-vs-limit exhibit); Kubernetes-native observability (Prometheus Operator + ServiceMonitors); Postgres as StatefulSets + PVCs. The nginx frontend is the one documented D6 exception (opt-out probes, no ServiceMonitor) -- the same nginx-vs-Caddy contrast the platform teaches. Compose is retained as the fast-local path. A five-PR delivery plan, an honest RAM risk (~6-10 GB vs ~1.6 GB) with mitigations, and CI (per-PR helm lint + kubeconform with CRD schemas, nightly Kind e2e). - ADR-0014 (Kind), 0015 (Helm library-chart + umbrella), 0016 (Gateway API via Envoy Gateway), 0017 (Kubernetes-native observability), 0018 (StatefulSets + PVCs for stateful workloads). - README: the polyglot platform is delivered; the next arc is this deployment platform, then the authentication capstone. RFC-0003 added to the index.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe changes define RFC-0003 for running the existing platform on Kind with Helm and Envoy Gateway, add ADRs covering its Kubernetes decisions, describe delivery and validation stages, and update the README with the platform roadmap. ChangesKubernetes platform documentation
Estimated code review effort: 3 (Moderate) | ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
docs/adr/0014-kubernetes-on-kind-deployment-target.md (1)
18-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSpecify the actual reproducibility pins.
This decision says the Kind binary and node image are pinned, but does not identify their versions, digest, or the versioned file that owns those values. Record those pins here or reference the authoritative configuration so CI and local environments cannot silently drift.
🤖 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 `@docs/adr/0014-kubernetes-on-kind-deployment-target.md` around lines 18 - 20, Update ADR 0014’s Kind deployment description to specify the exact pinned Kind binary version and node-image digest, or reference the version-controlled configuration that owns those values; ensure the documented pins are authoritative for both CI and local environments.
🤖 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 `@docs/adr/0017-kubernetes-native-observability-prometheus-operator.md`:
- Around line 19-26: Update the ADR’s Alloy DaemonSet description to explicitly
require node-local pod discovery/filtering, using the pod’s node identity, so
each Alloy instance tails only logs from its own node and avoids duplicate Loki
entries. If clustering is the intended model instead, document that coordination
model and its anti-duplication guarantee.
In `@docs/rfc/0003-kubernetes-kind-platform.md`:
- Line 476: Update the range notation in the documentation entries around “PR-1”
and the corresponding entry at the additional occurrence, replacing
“ADR-0014..0018” with the mandated ASCII “ADR-0014--0018” consistently.
- Around line 25-29: Update the later wording associated with the D6 contract to
say “three endpoints and structured logs” instead of describing four endpoints.
Keep the existing endpoint list unchanged and do not treat structured logs as an
endpoint.
- Around line 311-317: Align the security-capstone references in the document:
update the mention near the Kubernetes Secret discussion and the definition
around the security-capstone section to consistently identify it as future
RFC-0004, or explicitly describe RFC-0001 Section 10 as the originating deferral
while retaining RFC-0004 as the capstone reference.
- Around line 511-520: Clarify the status of the “optional-Operator”
plain-Prometheus path in the RAM mitigation row and align it with the
observability requirements from DK6 and PR-4. Either define its profile,
acceptance criteria, and reduced guarantees as a supported fallback, or
explicitly mark it unsupported and state that kube-prometheus-stack/Prometheus
Operator remains required for RFC-compliant deployments.
---
Nitpick comments:
In `@docs/adr/0014-kubernetes-on-kind-deployment-target.md`:
- Around line 18-20: Update ADR 0014’s Kind deployment description to specify
the exact pinned Kind binary version and node-image digest, or reference the
version-controlled configuration that owns those values; ensure the documented
pins are authoritative for both CI and local environments.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 61cb1210-f8c9-4454-92da-fcfde898ead8
📒 Files selected for processing (7)
README.mddocs/adr/0014-kubernetes-on-kind-deployment-target.mddocs/adr/0015-helm-library-chart-umbrella-packaging.mddocs/adr/0016-gateway-api-envoy-gateway-edge.mddocs/adr/0017-kubernetes-native-observability-prometheus-operator.mddocs/adr/0018-stateful-workloads-statefulsets-pvcs.mddocs/rfc/0003-kubernetes-kind-platform.md
- D6 payoff names three endpoints (/healthz, /readyz, /metrics) plus structured logs, not "four endpoints". - Real secret management referenced consistently as the RFC-0004 capstone (RFC-0001 Section 10 as the originating deferral). - Plain-Prometheus (no-Operator) path marked explicitly out of scope and unsupported -- it cannot satisfy the ServiceMonitor-driven contract (DK6). - ADR-0017: Alloy DaemonSet uses node-local discovery, so it does not duplicate every log line into Loki. - ASCII range notation ADR-0014--0018.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@docs/rfc/0003-kubernetes-kind-platform.md`:
- Around line 55-58: Revise the Kubernetes mapping description in the RFC
paragraph so it explicitly separates `/healthz` and `/readyz` as probes,
`/metrics` as the Prometheus scrape target, and structured logs as independently
collected by the logging stack. Preserve the statement that this requires no
per-service special-casing apart from the documented nginx exception.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4ce4e37f-0d4b-4e91-88ff-e6a4e9365155
📒 Files selected for processing (2)
docs/adr/0017-kubernetes-native-observability-prometheus-operator.mddocs/rfc/0003-kubernetes-kind-platform.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/adr/0017-kubernetes-native-observability-prometheus-operator.md
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@docs/rfc/0003-kubernetes-kind-platform.md`:
- Around line 508-510: The RFC acceptance criterion in
docs/rfc/0003-kubernetes-kind-platform.md lines 508-510 must limit Prometheus
ServiceMonitor discovery and D6 probes to metrics-bearing/D6-compliant services
while explicitly excluding the nginx frontend. Update
docs/adr/0015-helm-library-chart-umbrella-packaging.md lines 18-27 to document
default-on opt-outs and identify nginx frontend as the sole documented
exception; preserve the existing Helm packaging contract.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fb68dbfa-9398-4c7c-bf28-5bc225e9efd0
📒 Files selected for processing (7)
README.mddocs/adr/0014-kubernetes-on-kind-deployment-target.mddocs/adr/0015-helm-library-chart-umbrella-packaging.mddocs/adr/0016-gateway-api-envoy-gateway-edge.mddocs/adr/0017-kubernetes-native-observability-prometheus-operator.mddocs/adr/0018-stateful-workloads-statefulsets-pvcs.mddocs/rfc/0003-kubernetes-kind-platform.md
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@docs/adr/0017-kubernetes-native-observability-prometheus-operator.md`:
- Around line 20-22: Update the ADR wording around “per-service ServiceMonitors”
to say “per D6-compliant service,” and explicitly identify the nginx frontend as
the exception that does not expose a metrics endpoint. Preserve the surrounding
description of label-driven discovery and Grafana ConfigMaps.
In `@docs/adr/0018-stateful-workloads-statefulsets-pvcs.md`:
- Around line 25-27: Revise the StatefulSets rationale to qualify durable
storage as surviving pod recreation only while the owning node remains
available. Explicitly state that node-local volumes do not support automatic
recovery after node loss and that node-loss recovery is out of scope.
In `@docs/rfc/0003-kubernetes-kind-platform.md`:
- Around line 274-275: Choose a single postgres_exporter topology and apply it
consistently in the DK6 text at
docs/rfc/0003-kubernetes-kind-platform.md:274-275 and the target architecture
entry at docs/rfc/0003-kubernetes-kind-platform.md:388. Align the stated
ownership, scaling, networking, Deployment/Service structure, and ServiceMonitor
wiring with that chosen topology.
- Around line 241-247: Update the JVM/container-memory wording at
docs/rfc/0003-kubernetes-kind-platform.md lines 241-247 and 525 so it
distinguishes container-aware cgroup detection from heap sizing: supported JDKs
detect container limits by default, while MaxRAMPercentage only sets the heap
percentage of that detected limit. Apply the same clarification to the README
wording referenced by the RFC, without implying MaxRAMPercentage enables
container awareness.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 68a9157a-f861-4eb9-b50e-3b2611011785
📒 Files selected for processing (7)
README.mddocs/adr/0014-kubernetes-on-kind-deployment-target.mddocs/adr/0015-helm-library-chart-umbrella-packaging.mddocs/adr/0016-gateway-api-envoy-gateway-edge.mddocs/adr/0017-kubernetes-native-observability-prometheus-operator.mddocs/adr/0018-stateful-workloads-statefulsets-pvcs.mddocs/rfc/0003-kubernetes-kind-platform.md
The bullet called the nginx frontend "the one documented exception" for ServiceMonitors. True, but only once the reader carries the "metrics-bearing (D6-compliant)" qualifier through: rendering the PR-2 charts yields six ServiceMonitors across fourteen workloads, and reconciling that against "one exception" means working out unaided that Loki, Mailpit, blackbox, loadgen and the Postgres StatefulSets were never in the set being counted. Name the six, and say why each of the others has none. An acceptance criterion should be checkable against a rendered manifest without first reverse-engineering which workloads it covers.
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
…ency gaps The RFC's JVM lesson taught a JDK 8-era model: that a container memory limit alone does not bound the heap, and that the JVM sizes against node memory unless MaxRAMPercentage tells it otherwise. JDK 21, which reports runs, reads the cgroup limit by default; MaxRAMPercentage only selects the heap's share of that detected limit. The rewritten exhibit says so and names the real failure: the non-heap regions inside the limit that a too-high percentage ignores. Three consistency fixes alongside it: - ADR-0017 claimed a ServiceMonitor per service, which ADR-0015 contradicts by opting the nginx frontend out. Scoped to workloads exposing /metrics. - ADR-0018's decision said storage survives pod rescheduling while its own consequences said local-path pins a pod to its node. Qualified to pod recreation, with node-loss recovery stated as out of scope. - The RFC called postgres_exporter a "sidecar/Deployment" in Section 5 and a standalone Deployment in Section 6. Settled on standalone, matching the chart.
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/adr/0017-kubernetes-native-observability-prometheus-operator.md (1)
27-32: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReference the exact Kubernetes pod/node filter used for node-scoped Alloy.
Line 28 says
kubernetes_sd "node" meta-label, but the effective Kubernetes discovery options are relabeling on__meta_kubernetes_pod_node_nameor filtering withdiscovery.kubernetesselectors such asspec.nodeName=<current-node>. Spell out the intended selector/filter here so the duplicated-log gap maps onto a implementable configuration.🤖 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 `@docs/adr/0017-kubernetes-native-observability-prometheus-operator.md` around lines 27 - 32, Update the node-local Alloy description near the Kubernetes SD “node” reference to state the exact filtering mechanism: constrain discovered pods by __meta_kubernetes_pod_node_name matching the current node, or use a discovery.kubernetes selector equivalent to spec.nodeName=<current-node>. Ensure the text clearly requires this filter so each DaemonSet pod ships only its own node’s logs.Source: MCP tools
🤖 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.
Outside diff comments:
In `@docs/adr/0017-kubernetes-native-observability-prometheus-operator.md`:
- Around line 27-32: Update the node-local Alloy description near the Kubernetes
SD “node” reference to state the exact filtering mechanism: constrain discovered
pods by __meta_kubernetes_pod_node_name matching the current node, or use a
discovery.kubernetes selector equivalent to spec.nodeName=<current-node>. Ensure
the text clearly requires this filter so each DaemonSet pod ships only its own
node’s logs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0918e39a-655c-43ce-aa48-465b86a55a79
📒 Files selected for processing (4)
README.mddocs/adr/0017-kubernetes-native-observability-prometheus-operator.mddocs/adr/0018-stateful-workloads-statefulsets-pvcs.mddocs/rfc/0003-kubernetes-kind-platform.md
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/adr/0018-stateful-workloads-statefulsets-pvcs.md
- README.md
- docs/rfc/0003-kubernetes-kind-platform.md
ADR-0017 attributed Alloy's node-local log discovery to a Kubernetes SD "node" meta-label pinned to spec.nodeName. No such meta-label exists; the sentence fused two separate mechanisms into a third that does not. Name the real ones: a spec.nodeName field selector on pod-role discovery, or the equivalent relabel drop on __meta_kubernetes_pod_node_name, with the node name reaching the pod through the downward API. The ADR already argues node-local scoping is required rather than incidental, so the filter it rests on has to be one PR-4 can actually configure.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Plan the deployment and operability platform: run the whole polyglot stack on Kubernetes (Kind) beside the compose stack. This is a platform, not the capstone -- a future RFC-0004 (authentication) rides on it. RFC-0001 and RFC-0002 stay immutable.
Summary by CodeRabbit