-
Notifications
You must be signed in to change notification settings - Fork 23
[INFRA-177] Plane-EE: Add resource requests for Plane Enterprise Helm Chart #117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…m chart - Updated `questions.yml` to include `memoryRequest` and `cpuRequest` for services: web, space, admin, live, silo, monitor, api, worker, beatworker, and email_service. - Modified `values.yaml` to set default values for `memoryRequest` and `cpuRequest` for the aforementioned services. - Adjusted deployment templates to utilize the new request configurations for resource management.
|
Warning Rate limit exceeded@akshat5302 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 18 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
WalkthroughThe changes introduce configurable resource requests for memory and CPU across multiple services in the plane-enterprise Helm chart. This involves updating deployment templates to use templated values, extending the configuration schema in Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HelmChart
participant Kubernetes
User->>HelmChart: Set resource requests (memory/cpu) via values or defaults
HelmChart->>Kubernetes: Rendered manifests with configured resource requests
Kubernetes->>Kubernetes: Deploy workloads with specified resource requests/limits
Suggested reviewers
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (4)
charts/plane-enterprise/templates/workloads/email.deployment.yaml (1)
69-70: Inconsistent resource key naming
This file uses snake_case (memory_request,cpu_request), whereas other templates (e.g., worker, monitor, space) use camelCase (memoryRequest,cpuRequest). Standardizing on one convention will improve maintainability and reduce confusion.charts/plane-enterprise/templates/workloads/live.deployment.yaml (1)
50-51: Resource requests templated correctly
Dynamic resource requests for the live service follow the same pattern. Please verify the default values are documented in the chart README or upgrade notes for end users.charts/plane-enterprise/values.yaml (1)
75-76: Verify default request values and consistency across services
You've addedmemoryRequestandcpuRequestkeys (defaulting to50Mi/50m) for core services (web, monitor, space, admin, live, api, worker, beatworker, silo). Confirm these defaults align with your performance/availability guidelines and Chart documentation. Also ensure the camelCase naming matches the references in your templates andquestions.yml.Also applies to: 84-85, 95-96, 105-106, 115-116, 125-126, 135-136, 141-142, 148-149
charts/plane-enterprise/questions.yml (1)
105-112: Ensure question schema aligns with values.yaml
Newservices.*.memoryRequestandservices.*.cpuRequestentries correctly prompt for resource requests. Verify that each label, default, and grouping matches yourvalues.yaml, and that the order inquestions.ymlmirrors the YAML hierarchy so users have a consistent experience.Also applies to: 140-147, 175-182, 210-217, 257-264, 384-391, 423-430, 471-478, 493-500
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
charts/plane-enterprise/Chart.yaml(1 hunks)charts/plane-enterprise/questions.yml(10 hunks)charts/plane-enterprise/templates/workloads/admin.deployment.yaml(1 hunks)charts/plane-enterprise/templates/workloads/api.deployment.yaml(1 hunks)charts/plane-enterprise/templates/workloads/beat-worker.deployment.yaml(1 hunks)charts/plane-enterprise/templates/workloads/email.deployment.yaml(1 hunks)charts/plane-enterprise/templates/workloads/live.deployment.yaml(1 hunks)charts/plane-enterprise/templates/workloads/monitor.stateful.yaml(1 hunks)charts/plane-enterprise/templates/workloads/silo.deployment.yaml(1 hunks)charts/plane-enterprise/templates/workloads/space.deployment.yaml(1 hunks)charts/plane-enterprise/templates/workloads/web.deployment.yaml(1 hunks)charts/plane-enterprise/templates/workloads/worker.deployment.yaml(1 hunks)charts/plane-enterprise/values.yaml(7 hunks)
🔇 Additional comments (9)
charts/plane-enterprise/Chart.yaml (1)
8-8: Chart version bump
Version updated from1.2.3to1.2.4to reflect the introduction of configurable resource requests.charts/plane-enterprise/templates/workloads/worker.deployment.yaml (1)
27-28: Templatize worker resource requests and validate definitions
Memory and CPU requests now reference.Values.services.worker.memoryRequestand.Values.services.worker.cpuRequestwith default fallbacks, improving configurability while preserving behavior. Ensure these keys are defined in bothvalues.yamlandquestions.ymlto avoid Helm rendering errors.charts/plane-enterprise/templates/workloads/email.deployment.yaml (1)
69-70: Templatize email service resource requests and validate definitions
Requests now pull from.Values.services.email_service.memory_requestand.Values.services.email_service.cpu_requestwith defaults, enhancing flexibility. Verify corresponding entries exist invalues.yamlandquestions.ymlunder theemail_servicesection.charts/plane-enterprise/templates/workloads/monitor.stateful.yaml (1)
47-48: Templatize monitor resource requests and validate definitions
Memory and CPU requests now reference.Values.services.monitor.memoryRequestand.Values.services.monitor.cpuRequestwith default fallbacks, improving configurability. Ensure these keys are defined invalues.yamlandquestions.ymlto prevent missing-value template failures.charts/plane-enterprise/templates/workloads/space.deployment.yaml (1)
50-51: Templatize space service resource requests and validate definitions
Memory and CPU requests now pull from.Values.services.space.memoryRequestand.Values.services.space.cpuRequestwith default fallbacks, enabling dynamic configuration. Confirm these variables exist invalues.yamlandquestions.yml.charts/plane-enterprise/templates/workloads/api.deployment.yaml (1)
50-51: Well-structured templating for resource requests
The CPU and memory requests are now parametrized with sensible fallbacks ("50Mi"and"50m"), enabling dynamic configuration. Ensure that the corresponding defaults invalues.yamland prompts inquestions.ymlmatch these keys and types.charts/plane-enterprise/templates/workloads/admin.deployment.yaml (1)
50-51: Parametrized resource requests are consistent
The memory and CPU requests for the admin service are now configurable with the same fallback values. Confirm that the default values defined inquestions.ymlandvalues.yamlalign (both"50Mi"and"50m").charts/plane-enterprise/templates/workloads/web.deployment.yaml (1)
50-51: Templated resource requests align with chart schema
The web service now supports configurable resource requests with defaults. Ensure these keys are defined inquestions.ymland maintain consistency in documentation and examples.charts/plane-enterprise/templates/workloads/beat-worker.deployment.yaml (1)
27-28: Good use of templated resource requests with default fallbacks
Thememoryandcpurequests now reference.Values.services.beatworker.memoryRequestand.Values.services.beatworker.cpuRequestwith sensible defaults (50Mi,50m) for backward compatibility.
| memory: {{ .Values.services.silo.memory_request | default "50Mi" | quote }} | ||
| cpu: {{ .Values.services.silo.cpu_request | default "50m" | quote }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Inconsistent Helm value key naming
The silo service uses snake_case keys (memory_request/cpu_request), while other services use memoryRequest/cpuRequest. For consistency across the chart, standardize the key names (preferably lowerCamelCase as used elsewhere).
🤖 Prompt for AI Agents
In charts/plane-enterprise/templates/workloads/silo.deployment.yaml at lines
73-74, the Helm values use snake_case keys memory_request and cpu_request, which
is inconsistent with other services using lowerCamelCase keys memoryRequest and
cpuRequest. Rename these keys to memoryRequest and cpuRequest in both the
template and the values file to maintain consistent naming conventions across
the chart.
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
- Updated `questions.yml` to change variable names from `memory_limit` and `cpu_limit` to `memoryLimit` and `cpuLimit`. - Modified `values.yaml` to reflect the new variable names for memory and CPU limits and requests. - Adjusted `email.deployment.yaml` to utilize the updated variable names for resource requests and limits.
- Updated `questions.yml` to change variable names from `memory_request`, `cpu_request`, `memory_limit`, and `cpu_limit` to `memoryRequest` and `cpuRequest`, `memoryLimit` and `cpuLimit`. - Modified `silo.deployment.yaml` to utilize the updated variable names for resource requests and limits.
…README.md - Updated the README.md to include `memoryRequest` and `cpuRequest` settings for services: web, space, admin, live, monitor, api, silo, worker, and beatworker. - Ensured consistency in documentation for resource management across all relevant services.
…urce configurations - Changed variable names for memory and CPU limits and requests from `memory_limit`, `cpu_limit`, `memory_request`, and `cpu_request` to `memoryLimit`, `cpuLimit`, `memoryRequest`, and `cpuRequest` for the email service. - Ensured consistency with previous refactoring of resource variable names across the Helm chart.
Description
Type of Change