feat(helm): S3-native Helm chart for the RFC 0019 split topology - #304
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 15 minutes and 5 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds a new Helm chart for ourios with defaults, helper templates, workload manifests, services, storage resources, and install/test documentation. Changesourios Helm Chart
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new S3-native Helm chart (deploy/helm/ourios/) that deploys ourios-server in the RFC 0019 “split topology”: receiver (StatefulSet + per-replica WAL PVC), querier (stateless Deployment), and a dedicated singleton compactor (Deployment), with values-driven configuration and operator guidance.
Changes:
- Introduces a full Helm chart scaffold (Chart metadata, defaults, helpers) for split receiver/querier/compactor workloads.
- Adds Kubernetes manifests for Services, ServiceAccount, receiver StatefulSet (WAL PVC), querier Deployment, compactor Deployment, and local-backend data PVC.
- Adds operator docs (README/NOTES) and a
helm testconnection check.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| deploy/helm/ourios/values.yaml | Default configuration surface for storage, roles, WAL PVC, AWS creds, security context. |
| deploy/helm/ourios/templates/tests/test-connection.yaml | helm test pod to validate querier (DSL POST) or receiver (TCP connect). |
| deploy/helm/ourios/templates/services.yaml | Receiver (headless + client) and querier Services. |
| deploy/helm/ourios/templates/serviceaccount.yaml | ServiceAccount with optional annotations (e.g., IRSA). |
| deploy/helm/ourios/templates/receiver-statefulset.yaml | Receiver StatefulSet with per-replica WAL volumeClaimTemplates. |
| deploy/helm/ourios/templates/querier-deployment.yaml | Stateless querier Deployment reading the shared store. |
| deploy/helm/ourios/templates/compactor-deployment.yaml | Dedicated singleton compactor Deployment. |
| deploy/helm/ourios/templates/data-pvc.yaml | Shared PVC for the local storage backend. |
| deploy/helm/ourios/templates/_helpers.tpl | Common helpers for naming/labels plus env wiring (storage, roles, AWS envFrom). |
| deploy/helm/ourios/templates/NOTES.txt | Post-install guidance and warnings for common misconfiguration. |
| deploy/helm/ourios/README.md | Chart documentation (topology, install, credentials, key values, probes). |
| deploy/helm/ourios/Chart.yaml | Chart metadata + versioning/appVersion defaults. |
| deploy/helm/ourios/.helmignore | Helm packaging ignore patterns. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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 `@deploy/helm/ourios/Chart.yaml`:
- Around line 13-15: The chart is defaulting image.tag to a nonexistent release
because Chart.yaml sets appVersion to 0.0.0. Update the ourios chart’s
appVersion to a real published ourios-server image tag, or change the image tag
defaulting logic so it no longer falls back to appVersion; use the Chart.yaml
appVersion field and the image.tag default path as the places to fix.
In `@deploy/helm/ourios/templates/_helpers.tpl`:
- Around line 79-82: The S3 storage helper currently renders OURIOS_S3_BUCKET
even when .Values.storage.s3.bucket is empty, which allows an invalid release to
install. Update the conditional block in _helpers.tpl around the storage.backend
check to enforce a render-time validation using required or fail on
.Values.storage.s3.bucket, so the chart aborts before emitting an empty bucket
value.
- Around line 125-130: Reject the all-compaction-off case at template render
time instead of silently setting OURIOS_COMPACTION_ENABLED=0 in the receiver and
querier helpers. Update the relevant helpers, especially ourios.receiverEnv and
the matching querier env helper, to call fail when compactor.enabled is false so
the chart does not render a non-compacting deployment. Keep the existing
compaction-disabled env only for valid setups where a compactor workload is
enabled.
- Around line 168-173: The Helm helper ourios.awsEnvFrom currently allows
aws.existingSecret to be used alongside IRSA, which can cause static AWS
credentials to override web-identity auth. Update the chart validation so only
one AWS auth mode is permitted, either by adding a render-time check in the
ourios.awsEnvFrom/template flow or by enforcing the “exactly one” contract in
the values schema. Make sure the validation references the aws.existingSecret
and IRSA-related values so misconfiguration fails early.
In `@deploy/helm/ourios/templates/data-pvc.yaml`:
- Around line 1-22: The shared PVC in data-pvc.yaml is created for local backend
use, but the receiver, querier, and compactor workloads can still land on
different nodes and fail with ReadWriteOnce storage. Update the local-mode
handling around the PersistentVolumeClaim and the workload templates that mount
it so local mode is only allowed with RWX-capable storage or all local-mode pods
are forced onto a single node. Use the storage.backend=local branch, the shared
data claim, and the receiver/querier/compactor deployment or statefulset
templates as the key places to fix this.
In `@deploy/helm/ourios/templates/NOTES.txt`:
- Around line 13-18: The AWS credential warning in NOTES.txt is gated by a
generic ServiceAccount annotations check, which can hide the warning when
unrelated annotations are present. Update the condition in the NOTES template to
check specifically for the IRSA annotation key eks.amazonaws.com/role-arn, and
make sure the same annotation-specific logic is used in _helpers.tpl where the
actual credential wiring is determined. Use the existing values objects around
.Values.aws.existingSecret and .Values.serviceAccount.annotations to keep the
warning aligned with the real auth behavior.
In `@deploy/helm/ourios/templates/receiver-statefulset.yaml`:
- Around line 28-36: The annotation handling in the receiver StatefulSet
template currently falls back to either shared or receiver-specific values, so
both sets are not rendered together. Update the annotations logic in the
receiver template to merge .Values.receiver.podAnnotations with
.Values.podAnnotations instead of using default, and apply the same fix in the
querier and compactor templates; use the existing podAnnotations blocks and
annotations output as the place to combine both maps.
In `@deploy/helm/ourios/templates/tests/test-connection.yaml`:
- Around line 16-26: The test pod in the test-connection template uses BusyBox
wget with `--post-data`, which is unsupported and will fail at runtime. Update
the `query` container to use an image with curl support, such as
`curlimages/curl`, and switch the request in the test command to a curl-based
POST while preserving the existing headers and endpoint. Keep the change
localized to the `query` container definition in the Helm test template.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7e241ff3-1463-40ed-b2bb-a223ddb2c636
📒 Files selected for processing (13)
deploy/helm/ourios/.helmignoredeploy/helm/ourios/Chart.yamldeploy/helm/ourios/README.mddeploy/helm/ourios/templates/NOTES.txtdeploy/helm/ourios/templates/_helpers.tpldeploy/helm/ourios/templates/compactor-deployment.yamldeploy/helm/ourios/templates/data-pvc.yamldeploy/helm/ourios/templates/querier-deployment.yamldeploy/helm/ourios/templates/receiver-statefulset.yamldeploy/helm/ourios/templates/serviceaccount.yamldeploy/helm/ourios/templates/services.yamldeploy/helm/ourios/templates/tests/test-connection.yamldeploy/helm/ourios/values.yaml
jensholdgaard
added a commit
that referenced
this pull request
Jun 28, 2026
…ons, curl test Applies the CodeRabbit/Copilot review on the s3-native chart: - merge per-role podAnnotations with the chart-level map (role wins) via a new ourios.podAnnotations helper, instead of one replacing the other; matches the podLabels behaviour (covers the duplicate receiver-line-36 thread). - fail render when storage.backend=s3 has no storage.s3.bucket (required), and flip the default backend to local so a bare helm template/lint renders zero-config; s3 is documented as the production path. - fail render when compactor.enabled=false (receiver/querier set OURIOS_COMPACTION_ENABLED=0, so the dedicated compactor is the only sweeper — hazard #4). - fail render when both aws.existingSecret and the IRSA role-arn annotation are set (static keys would shadow web-identity creds). - default image.tag to latest, not appVersion 0.0.0 (no image is published for the pre-release crate version); appVersion stays the version label. - gate the NOTES credential warning on the specific eks.amazonaws.com/role-arn key, so unrelated ServiceAccount annotations no longer suppress it. - switch the querier helm-test to curlimages/curl:8.11.1 (BusyBox wget lacks --post-data); curl -f asserts a 2xx. Receiver nc check unchanged. - document local mode as single-node/RWX dev-only (no heavy guard). Validated: helm lint; default(local) + s3 renders; s3-no-bucket, compactor-off, and mixed-auth all fail with their messages; IRSA render has no envFrom and no cred warning; podAnnotations merge confirmed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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 `@deploy/helm/ourios/templates/_helpers.tpl`:
- Around line 176-179: The mutual-exclusion check in ourios.awsEnvFrom should
not index serviceAccount.annotations directly because a null override can crash
before fail runs. Update the condition to guard the annotations map first by
defaulting serviceAccount.annotations to an empty dict before checking for the
eks.amazonaws.com/role-arn key, so the aws.existingSecret and IRSA validation
still works safely.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 050bf789-5c04-454d-a114-2cc1112e32d3
📒 Files selected for processing (9)
deploy/helm/ourios/Chart.yamldeploy/helm/ourios/README.mddeploy/helm/ourios/templates/NOTES.txtdeploy/helm/ourios/templates/_helpers.tpldeploy/helm/ourios/templates/compactor-deployment.yamldeploy/helm/ourios/templates/querier-deployment.yamldeploy/helm/ourios/templates/receiver-statefulset.yamldeploy/helm/ourios/templates/tests/test-connection.yamldeploy/helm/ourios/values.yaml
✅ Files skipped from review due to trivial changes (2)
- deploy/helm/ourios/Chart.yaml
- deploy/helm/ourios/README.md
🚧 Files skipped from review as they are similar to previous changes (5)
- deploy/helm/ourios/templates/tests/test-connection.yaml
- deploy/helm/ourios/templates/compactor-deployment.yaml
- deploy/helm/ourios/templates/querier-deployment.yaml
- deploy/helm/ourios/templates/receiver-statefulset.yaml
- deploy/helm/ourios/values.yaml
This was referenced Jun 28, 2026
Reworks the deploy/helm/ourios chart from the single local-PVC StatefulSet into the RFC 0019 S3-native topology: a receiver StatefulSet with a per-replica WAL PVC, a stateless querier Deployment that scales independently and reads S3, and a singleton compactor Deployment. Only the data/audit/manifest live on S3; the WAL is always a local durable PVC, never S3 or emptyDir (CLAUDE.md §3.4/§3.6). Credentials are never plaintext config: an existing Secret (envFrom) or IRSA via serviceAccount annotations. A `local` backend remains as a single-node/dev fallback with a shared data PVC. The binary always runs the compaction role, so the dedicated compactor is the designated sweeper while receiver/querier pods also sweep (safe via publish-CAS); flagged in the README for review. Validated with helm lint + helm template (default, s3, local, IRSA); renders confirm the WAL is a volumeClaimTemplate, the querier has no WAL, S3 env vars are set, and no plaintext AWS keys appear. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…_ENABLED The receiver and querier workloads now set OURIOS_COMPACTION_ENABLED=0 (the new binary flag), so a single dedicated compactor Deployment sweeps instead of every pod. Move the sweep interval to a compactor-only env helper; rewrite the README 'Compactor topology' note (no more N+1 sweepers; replicas>1 safe-but-redundant via publish-CAS, no leader election needed) and the NOTES warning for the all-compaction-off misconfig. helm lint + template (default and s3) green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review: run the helm-test querier probe via sh -c with single-quoted header/body so the spaces survive; fix the values-table description for compactor.intervalSecs (compactor-only, not every workload); and drop the bad NOTES advice to unset OURIOS_COMPACTION_ENABLED via extraEnv (it would duplicate the env name) — point at compactor.enabled=true instead. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ons, curl test Applies the CodeRabbit/Copilot review on the s3-native chart: - merge per-role podAnnotations with the chart-level map (role wins) via a new ourios.podAnnotations helper, instead of one replacing the other; matches the podLabels behaviour (covers the duplicate receiver-line-36 thread). - fail render when storage.backend=s3 has no storage.s3.bucket (required), and flip the default backend to local so a bare helm template/lint renders zero-config; s3 is documented as the production path. - fail render when compactor.enabled=false (receiver/querier set OURIOS_COMPACTION_ENABLED=0, so the dedicated compactor is the only sweeper — hazard #4). - fail render when both aws.existingSecret and the IRSA role-arn annotation are set (static keys would shadow web-identity creds). - default image.tag to latest, not appVersion 0.0.0 (no image is published for the pre-release crate version); appVersion stays the version label. - gate the NOTES credential warning on the specific eks.amazonaws.com/role-arn key, so unrelated ServiceAccount annotations no longer suppress it. - switch the querier helm-test to curlimages/curl:8.11.1 (BusyBox wget lacks --post-data); curl -f asserts a 2xx. Receiver nc check unchanged. - document local mode as single-node/RWX dev-only (no heavy guard). Validated: helm lint; default(local) + s3 renders; s3-no-bucket, compactor-off, and mixed-auth all fail with their messages; IRSA render has no envFrom and no cred warning; podAnnotations merge confirmed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The chart is S3-compatible, not AWS-specific. Reframe "S3-native"/"AWS" throughout to "S3-compatible object storage" (AWS S3, MinIO, Cloudflare R2, Hetzner, Ceph/RADOS, GCS S3-interop), and make non-AWS providers first-class: storage.s3.endpoint is the knob for any S3-compatible store. Restructure the credential surface to follow the backend, not the cloud: fold credentials under storage.s3.existingSecret (was top-level aws.*), collapse the duplicate region knob into storage.s3.region (drives both OURIOS_S3_REGION and the SDK chain's AWS_DEFAULT_REGION), and rename the ourios.awsEnvFrom helper to ourios.s3CredentialsEnvFrom. The AWS_* key names in the Secret are the S3 SDK convention every S3-compatible provider uses (not AWS-the-cloud-specific); IRSA stays clearly labeled as the AWS EKS-specific option. Also carry the prior review fixes: nil-safe IRSA annotation lookup (index ... | default dict), helm.sh/hook-delete-policy on the test pod, and the curl-based query helm-test (BusyBox wget lacks --post-data). helm lint + helm template verified for local, s3+endpoint+secret, the mutual-exclusion fail, and the missing-bucket fail. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ox test image Now that the binary reads explicit OURIOS_S3_* credentials (RFC 0019 §3.4 / #307), the chart's static-credential Secret documents the S3-named keys (OURIOS_S3_ACCESS_KEY_ID / OURIOS_S3_SECRET_ACCESS_KEY [/ OURIOS_S3_SESSION_TOKEN]) instead of the AWS-SDK names — injected via envFrom, working with any S3-compatible provider. IRSA stays the AWS-EKS option (AWS credential chain). Also pins the receiver-only helm-test image to busybox:1.37.0 (a floating tag could change nc behaviour and break helm test) — the last open #304 review point. helm lint + template verified (envFrom secretRef renders; busybox pinned). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jensholdgaard
force-pushed
the
helm-chart-s3-native
branch
from
June 28, 2026 21:02
b6ac877 to
cb067f4
Compare
…ed chain Address #304 review: - receiver/querier headers said the store is 'S3 by default'; the chart defaults to local, so reword to 'local by default, S3 in production'. - compactor header: it is compactor-only because it sets no receiver/querier env, not because those roles are unset (they can run in parallel). - NOTES: the credential note now mentions the AWS credential-chain fallback (shared profile / node IAM / container creds), not just existingSecret/IRSA; the compactor-disabled line no longer asserts receiver/querier state the condition doesn't check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…terval Address #304 review: OURIOS_QUERIER_DEFAULT_WINDOW_SECS and OURIOS_COMPACTION_INTERVAL_SECS must be positive integers or the server refuses to start. Validate querier.defaultWindowSecs and compactor.intervalSecs at render time (like the storage.backend guard) so a 0/negative value fails fast instead of crashlooping the pod. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Restores the Python diagrams (mingrammer) topology PNG for the chart, refreshed to the current design: receiver/querier carry OURIOS_COMPACTION_ENABLED=0 (only the dedicated compactor sweeps — no 'also sweeps' edges), the Secret holds the S3-named OURIOS_S3_* keys, and the store is labelled 'object store (S3 API)' (any S3-compatible provider). Source script committed alongside the PNG and embedded in the README, with the ASCII block kept as a text fallback. docs/ is .helmignore'd so the diagram isn't shipped in the chart package. This README is outside the docs/ mdBook tree, so the §6.7 (Mermaid/SVG) conventions don't apply. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@deploy/helm/ourios/docs/topology.py`:
- Around line 96-99: The topology diagram is showing Secret and ServiceAccount
objects as if they connect directly to S3, which misrepresents the credential
flow. Update the edges in topology.py so the credential sources are routed
through the workloads that consume them (the pods/deployment path), then from
those workloads to s3 using the existing workload→S3 links. Keep the unique
labels on the credential paths (“envFrom” and “IRSA (AWS EKS)”) but attach them
to the workload flow instead of the direct secret >> s3 and sa >> s3 edges.
In `@deploy/helm/ourios/README.md`:
- Around line 27-30: The topology regeneration command in the README uses a
repo-root-relative path, which breaks when followed from the chart directory or
from a packaged chart. Update the regeneration instructions near the
`docs/topology.py` reference to use a chart-local path such as
`docs/topology.py`, or explicitly state that the command must be run from the
repository root. Keep the surrounding guidance about Graphviz, `diagrams`, and
the text fallback unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8f9f4e30-1b80-4bde-befa-f48500e7f99a
⛔ Files ignored due to path filters (1)
deploy/helm/ourios/docs/topology.pngis excluded by!**/*.png
📒 Files selected for processing (14)
deploy/helm/ourios/.helmignoredeploy/helm/ourios/Chart.yamldeploy/helm/ourios/README.mddeploy/helm/ourios/docs/topology.pydeploy/helm/ourios/templates/NOTES.txtdeploy/helm/ourios/templates/_helpers.tpldeploy/helm/ourios/templates/compactor-deployment.yamldeploy/helm/ourios/templates/data-pvc.yamldeploy/helm/ourios/templates/querier-deployment.yamldeploy/helm/ourios/templates/receiver-statefulset.yamldeploy/helm/ourios/templates/serviceaccount.yamldeploy/helm/ourios/templates/services.yamldeploy/helm/ourios/templates/tests/test-connection.yamldeploy/helm/ourios/values.yaml
✅ Files skipped from review due to trivial changes (1)
- deploy/helm/ourios/.helmignore
🚧 Files skipped from review as they are similar to previous changes (3)
- deploy/helm/ourios/Chart.yaml
- deploy/helm/ourios/values.yaml
- deploy/helm/ourios/templates/_helpers.tpl
…SA SA Address #304 review (diagram + IRSA edge cases): - topology.py: credentials feed the workloads (envFrom/IRSA), which then read/write/sweep the store — not Secret/SA pointing at the store directly (correct trust boundary). PNG regenerated. - .helmignore: ship docs/topology.png (so the README image resolves from the chart artifact / Artifact Hub), exclude only docs/*.py (the generator). - README: chart-relative regen command; note IRSA requires serviceAccount.create=true. - _helpers: fail render when an IRSA role-arn is set with serviceAccount.create=false (the chart renders no SA, so the annotation would silently have no effect). helm lint + template verified: default + s3+IRSA render; s3+IRSA+create=false fails; package ships the PNG and excludes the script. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…y diagram source Address #304 review: - test-connection pod gets activeDeadlineSeconds: 60 so a stuck DNS/TCP connect fails helm test deterministically instead of hanging; the querier curl gets --connect-timeout 5 / --max-time 15 (the receiver nc already has -w5). - README: note the topology.py source ships in the repo checkout only (excluded from the packaged chart via .helmignore), so the link resolves on GitHub, not from a chart artifact. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address #304 review: the test pod used if/else, so a default split-topology install (receiver + querier both enabled) only checked the querier — a broken receiver Service would pass helm test. Emit one container per enabled role (independent ifs); the Pod succeeds only when every container exits 0. Verified: both→query+otlp-http, querier-only→query, receiver-only→otlp-http. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ot just S3) Address #304 review: the storageEnv header implied the region is needed for every backend (it's optional, applied only when set, and can come from standard AWS env/config), and the querier values comment said 'reads S3' though both local and s3 backends are supported. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jul 19, 2026
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
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.
What
The S3-native Helm chart (
deploy/helm/ourios/) — shipping-milestone workstream C. RFC 0019 madeourios-serverrun fully on S3; this deploys its three roles as the split topology:StatefulSet, a per-replica WAL PVC (volumeClaimTemplate); the WAL is always a local durable volume, never S3 (§3.4/§3.6). Writes data/audit/manifest to S3.Deployment, scales independently, reads S3, no PVC.Deployment(replicas: 1), the sole sweeper.Plus three Services (receiver gRPC/HTTP, querier HTTP), a ServiceAccount (IRSA-annotation passthrough), and a values-driven config surface.
Reworked from the closed-#286 local-PVC chart; kept its helpers/labels/scaffolding.
Compaction topology (uses #303's
OURIOS_COMPACTION_ENABLED)The binary compacts by default, which would mean every pod sweeps. The receiver + querier workloads now set
OURIOS_COMPACTION_ENABLED=0, so only the dedicated compactor sweeps; the interval is a compactor-only env. Scaling the compactor past 1 replica is safe (publish_casmakes concurrent sweeps correct) but redundant — leader election is intentionally not needed/added (areplicas: 1Deployment self-heals; a brief gap in background maintenance is harmless). NOTES warns loudly about the all-compaction-off misconfiguration.Credentials — never plaintext
Two paths (supply one):
storage.s3.existingSecret(a Secret with the S3-named keysOURIOS_S3_ACCESS_KEY_ID/OURIOS_S3_SECRET_ACCESS_KEY[/OURIOS_S3_SESSION_TOKEN], injected viaenvFrom; works with any S3-compatible provider) or, on AWS EKS, IRSA (serviceAccount.annotations."eks.amazonaws.com/role-arn", no static keys — uses the AWS credential chain). Mutually exclusive (setting both fails render); only mounted forbackend=s3. Defaults empty (the server then falls back to the AWS credential chain if the environment provides it).Probes
The binary exposes no HTTP health route (POST-only endpoints), so the chart uses TCP socket probes on the bound ports (documented; swap to HTTP once a
/healthzexists).Invariants
storage.s3.prefixis a store-root, not a tenant.Validation (helm v4)
helm lint→ 0 failed (only the[INFO] iconhint).helm templaterenders with defaults, with--set storage.backend=s3 --set storage.s3.bucket=… --set receiver.enabled=true --set querier.enabled=true, with IRSA (no secret), and withstorage.backend=local. Confirmed: WAL is avolumeClaimTemplate; no plaintext keys; querier has no WAL; receiver/querier carryOURIOS_COMPACTION_ENABLED=0and the compactor carries the interval.Design notes for review (kept as the agent built them, per your call)
storage.backend: local, andstorage.s3.bucketisrequiredwhenbackend=s3— a barehelm installworks zero-config for a single-node try-it, and ans3install with no bucket fails at render/install time (fast, not a crashloop).localis dev-only; S3 is the production path. (Install-time fail-fast shape; an earlier revision defaulteds3with an empty bucket.)localbackend = a single shared RWO PVC mounted by all workloads (coherent single-node / RWX only) — kept as a dev/kick-the-tires fallback; S3 is the production path.The earlier unrequested mingrammer topology PNG was dropped (off the §6.7 diagram conventions; not committing a generated binary).
🤖 Generated with Claude Code
Summary by CodeRabbit