Infra/106 advanced devops - #123
Merged
Merged
Conversation
api-docs was the only service without a health: path, so it fell back to a TCP-only readiness probe with no liveness/startup probe at all -- a hung-but-alive container never got restarted. swagger-ui serves an HTML root, so / works the same way every other service's health check does.
Enables CPU-based autoscaling (min 1 / max 2) for all 9 app services -- metrics-server is already live on the cluster and every service already declares resources.requests, so HPA is viable with no other prerequisites. Postgres, Keycloak, Ollama and the monitoring stack keep their own templates and stay fixed-replica; they're stateful/singleton and shouldn't scale out. deployment.yaml now omits spec.replicas for autoscaled services -- otherwise every helm upgrade (which runs on every push to main) would reset it back to 1 and fight the HPA's own scaling decisions. The namespace ResourceQuota is nearly fully committed already (verified live: ~90%+ of both limits.cpu and limits.memory used just running one replica of everything), so in practice most scale-up attempts will sit Pending rather than actually schedule -- documented in the helm README along with how to tell the difference between Pending-on-quota and metrics-server not being reachable.
strategy: is now overridable per service (same pattern as resources:). web-client and api-docs (80Mi/80m each) get maxSurge: 1 / maxUnavailable: 0 -- cheap enough to fit the namespace's remaining quota slack, so these two get a genuine zero-downtime rolling deploy. Every other service keeps the existing maxSurge: 0 default, since surging a second replica of any of them would exceed the namespace's CPU quota and break the rollout. Also adds a preStop sleep hook + explicit terminationGracePeriodSeconds to every service's pod template, at no extra resource cost: it gives the endpoints controller time to drain a pod out of Service routing before SIGTERM, shrinking the request-drop window that maxSurge: 0 rollouts otherwise have for the rest of the services (old pod is killed before readiness on the new one even starts).
The frontend only ever saw mock data because .env.development.example defaulted VITE_USE_MOCKS to true -- every deployed environment already runs live (the var is never set in web-client/Dockerfile, in either docker-compose build config, or in the CD workflow's build_args, and .dockerignore excludes .env* from the build context entirely), so this was purely a local pnpm dev default, not a deployment gap. Verified end-to-end against a fresh docker compose stack: every feature (organization, members, events, payments, letters, feedback, helper, profile) renders real backend data through Traefik with zero console errors. The mock path (VITE_USE_MOCKS=true, persona switching) still works unchanged -- nothing in src/mocks/** touched. Also documents a known limitation: pnpm dev's dev-proxy strips cookies and never routes the initial page load through Traefik, so it can't satisfy the forward-auth session cookie every backend route requires -- live-mode API calls from pnpm dev itself will fail. This doesn't affect any real deployment (docker-compose/VM/Kubernetes all go through Traefik end-to-end already); for local iteration against real data, browse the docker-compose stack directly at http://localhost/ instead of pnpm dev.
|
Warning Review limit reached
Next review available in: 32 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?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 reviews. How do review 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 refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds per-service Kubernetes HorizontalPodAutoscaler support via a new Helm template, updates the Deployment template to conditionally omit replicas under autoscaling and add a preStop lifecycle hook, configures autoscaling/strategy values for several services, documents these changes in the infra README, and updates web-client environment defaults and documentation for mock data usage. ChangesHelm Autoscaling and Self-Healing
Web-client Dev Environment and Mock Data Docs
Estimated code review effort: 2 (Simple) | ~12 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
infra/helm/team-devoops/templates/hpa.yaml (1)
16-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding a
behaviorblock for scale-down stabilization.The HPA template omits the optional
behaviorfield, so Kubernetes defaults apply (300s scale-down stabilization, 0s scale-up). This is acceptable formaxReplicas: 2, but explicitly defining abehaviorblock makes the scaling policy self-documenting and prevents surprises if cluster defaults change.♻️ Optional: add behavior block
minReplicas: {{ $svc.autoscaling.minReplicas }} maxReplicas: {{ $svc.autoscaling.maxReplicas }} + behavior: + scaleDown: + stabilizationWindowSeconds: 300 + policies: + - type: Percent + value: 100 + periodSeconds: 15 + scaleUp: + stabilizationWindowSeconds: 0 + policies: + - type: Percent + value: 100 + periodSeconds: 15 metrics:🤖 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 `@infra/helm/team-devoops/templates/hpa.yaml` around lines 16 - 22, The HPA template in the autoscaling manifest is missing an explicit behavior policy, so add a `behavior` block to the same template that renders the `metrics` section and `targetCPUUtilizationPercentage`. Keep the existing scaling logic intact, and define scale-down stabilization (and any scale-up policy you want to preserve) directly in the HPA spec so the behavior is explicit and stable if cluster defaults change.
🤖 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 `@infra/helm/team-devoops/values.yaml`:
- Around line 399-403: The py-genai-helper autoscaling settings conflict with
its RWO persistence, since the service’s PVC is only safe with a single replica.
Update the autoscaling block for this service in values.yaml so it cannot scale
beyond one pod, or disable autoscaling entirely if that is the intended
behavior. Keep the fix aligned with the existing persistence configuration
around the py-genai-helper service and its autoscaling.enabled/maxReplicas
settings.
---
Nitpick comments:
In `@infra/helm/team-devoops/templates/hpa.yaml`:
- Around line 16-22: The HPA template in the autoscaling manifest is missing an
explicit behavior policy, so add a `behavior` block to the same template that
renders the `metrics` section and `targetCPUUtilizationPercentage`. Keep the
existing scaling logic intact, and define scale-down stabilization (and any
scale-up policy you want to preserve) directly in the HPA spec so the behavior
is explicit and stable if cluster defaults change.
🪄 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: 10f326c4-000d-4709-a145-887f2db6cdc2
📒 Files selected for processing (6)
infra/helm/README.mdinfra/helm/team-devoops/templates/deployment.yamlinfra/helm/team-devoops/templates/hpa.yamlinfra/helm/team-devoops/values.yamlweb-client/.env.development.exampleweb-client/README.md
Its vector-store PVC is ReadWriteOnce (see the persistence block just below), which can only mount to one node at a time -- the chart's own service catalogue doc already says persistence is "only safe with a single replica." A second HPA-spawned replica wouldn't fail cleanly, it'd sit stuck on a Multi-Attach error indefinitely. Dropping the autoscaling block entirely rather than pinning maxReplicas: 1, since a min=max=1 HPA is a no-op object with nothing to show for it -- this service now joins Postgres/Keycloak/Ollama/monitoring as intentionally not autoscaled. Found via CodeRabbit review.
The manual CD run on 3df1b0c failed: api-docs' new pod's startup probe got a 404 on GET / and was killed/restarted in a loop until progressDeadlineSeconds, so helm --rollback-on-failure reverted the release. Confirmed live via kubectl describe rs (Warning Unhealthy: "HTTP probe failed with statuscode: 404") and a port-forward straight to the pod: GET / -> 404, GET /docs/ -> 200. Root cause: api/Dockerfile bakes BASE_URL=/docs into the image, so swagger-ui serves its UI under /docs, not root -- I assumed root when adding this service's health path in df9ddb4. The namespace's tight ResourceQuota was a red herring here; this pod scheduled, pulled, and started fine, it just never passed its own health check.
Collaborator
Author
|
CD runs through without problems now. |
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
2 participants
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.
This adds advanced devops steategies like horizontal pod scaling and autohealing.
Closes #106
Summary by CodeRabbit
New Features
Bug Fixes
Documentation