enable hpa based on metrics-server#104
enable hpa based on metrics-server#104pratapalakshmi wants to merge 7 commits intomakeplane:developfrom
Conversation
WalkthroughThis update introduces Kubernetes HorizontalPodAutoscaler (HPA) support to multiple Plane-CE Helm chart workloads. Autoscaling configuration blocks are added to values and question files, new HPA manifests are conditionally rendered in deployment templates, and the metrics-server dependency is declared and managed. Workflow scripts are adjusted to ensure chart dependencies are built. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Helm Chart
participant Kubernetes
participant Metrics Server
User->>Helm Chart: Install/Upgrade with autoscaling enabled
Helm Chart->>Kubernetes: Deploy workloads
Helm Chart->>Kubernetes: Conditionally deploy HPA resources (if metrics-server exists)
Kubernetes->>Metrics Server: Query resource metrics (CPU/Memory)
HPA->>Kubernetes: Scale workloads based on metrics
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (7)
.gitignore (1)
7-8: Consider explicit directory ignore patterns.It’s clearer to append a trailing slash (
/) to indicate directory exclusion and avoid accidentally ignoring files namedchartselsewhere:-charts/plane-ce/charts -charts/plane-enterprise/charts +charts/plane-ce/charts/ +charts/plane-enterprise/charts/charts/plane-ce/Chart.yaml (1)
18-22: Add newline at end of file.YAMLlint flags a missing newline at EOF. Please ensure there is exactly one blank line (newline character) at the end to comply with YAML standards.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 22-22: no new line character at the end of file
(new-line-at-end-of-file)
charts/plane-enterprise/Chart.yaml (1)
20-20: Add newline at end of file.YAMLlint reports a missing newline at EOF. Ensure one newline is present at the end.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 20-20: no new line character at the end of file
(new-line-at-end-of-file)
charts/plane-ce/values.yaml (1)
169-174: Define memory utilization threshold for autoscaling.The HPA templates conditionally include a memory metric (
targetMemoryUtilizationPercentage), but no default is declared here. To enable memory-based scaling, consider extending the block:autoscaling: enabled: true minReplicas: 1 maxReplicas: 3 targetCPUUtilizationPercentage: 80 targetMemoryUtilizationPercentage: 80charts/plane-ce/templates/workloads/api.deployment.yaml (1)
79-119: Extract HPA templating into a shared helper.Multiple workloads use identical HPA definitions. To reduce duplication and simplify maintenance, move the HPA spec into a template helper (e.g.,
_hpa.tpl) and include it with parameters:{{- define "plane.hpa" -}} apiVersion: {{ ternary "autoscaling/v2" "autoscaling/v2beta2" ... }} kind: HorizontalPodAutoscaler metadata: name: {{ .name }}-hpa namespace: {{ .namespace }} spec: scaleTargetRef: ... minReplicas: {{ .minReplicas }} maxReplicas: {{ .maxReplicas }} metrics: {{ include "plane.hpa.metrics" .metrics | indent 2 }} {{- end }}Then invoke:
{{- if .Values.autoscaling.enabled }} {{- include "plane.hpa" (dict "name" .Release.Name "-api" "namespace" .Release.Namespace "minReplicas" .Values.autoscaling.minReplicas "maxReplicas" .Values.autoscaling.maxReplicas "metrics" .Values.autoscaling ) | indent 0 }} {{- end }}🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 119-119: no new line character at the end of file
(new-line-at-end-of-file)
charts/plane-ce/templates/workloads/live.deployment.yaml (1)
69-76: DRY: Extract shared HPA definition into a helper
The HPA block is duplicated across multiple workload templates. Consider moving it into a single_hpa.tplpartial and passing in the release name, target deployment, and metrics so you’re not repeating the same code in every file.charts/plane-ce/templates/workloads/space.deployment.yaml (1)
78-101: DRY up CPU/memory metric blocks
The CPU and memory resource metrics share the same structure with only the name and value differing. Consider looping over a map to eliminate duplication and simplify future extensions:metrics: {{- $m := dict "cpu" .Values.autoscaling.targetCPUUtilizationPercentage "memory" .Values.autoscaling.targetMemoryUtilizationPercentage }} {{- range $name, $pct := $m }} {{- if $pct }} - type: Resource resource: name: {{ $name }} {{- if $.Capabilities.APIVersions.Has "autoscaling/v2" }} target: type: Utilization averageUtilization: {{ $pct }} {{- else }} targetAverageUtilization: {{ $pct }} {{- end }} {{- end }} {{- end }}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
charts/plane-ce/Chart.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
.gitignore(1 hunks)charts/plane-ce/Chart.yaml(1 hunks)charts/plane-ce/templates/workloads/admin.deployment.yaml(1 hunks)charts/plane-ce/templates/workloads/api.deployment.yaml(1 hunks)charts/plane-ce/templates/workloads/beat-worker.deployment.yaml(1 hunks)charts/plane-ce/templates/workloads/live.deployment.yaml(1 hunks)charts/plane-ce/templates/workloads/space.deployment.yaml(1 hunks)charts/plane-ce/templates/workloads/web.deployment.yaml(1 hunks)charts/plane-ce/templates/workloads/worker.deployment.yaml(1 hunks)charts/plane-ce/values.yaml(1 hunks)charts/plane-enterprise/Chart.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/plane-enterprise/Chart.yaml
[error] 20-20: no new line character at the end of file
(new-line-at-end-of-file)
charts/plane-ce/Chart.yaml
[error] 22-22: no new line character at the end of file
(new-line-at-end-of-file)
charts/plane-ce/templates/workloads/beat-worker.deployment.yaml
[warning] 66-66: wrong indentation: expected 2 but found 4
(indentation)
charts/plane-ce/templates/workloads/worker.deployment.yaml
[warning] 65-65: wrong indentation: expected 2 but found 4
(indentation)
🔇 Additional comments (1)
charts/plane-enterprise/Chart.yaml (1)
17-20: Confirm necessity of metrics-server dependency for enterprise chart.The enterprise chart now depends on
metrics-server, but no HPA resources are added here. If autoscaling isn’t required for enterprise workloads, this dependency may be unnecessary overhead. Please verify whether enterprise templates leverage metrics; otherwise consider removing it.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 20-20: no new line character at the end of file
(new-line-at-end-of-file)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
charts/plane-ce/values.yaml (6)
93-98: Apply autoscaling refactor tospacecomponentThis
autoscalingblock is a direct duplicate of the one inweb(lines 79–85). Apply the same anchor/helper refactor here to avoid repetition.
107-112: Apply autoscaling refactor toadmincomponentDuplicate
autoscalingsettings detected—reuse the common definition via YAML anchors or a shared Helm template.
121-126: Apply autoscaling refactor tolivecomponentIdentical block as above; extract into a shared anchor/helper to simplify updates.
135-140: Apply autoscaling refactor toapicomponentSame autoscaling parameters—consider consolidating under a single definition for consistency.
148-153: Apply autoscaling refactor toworkercomponentThis block repeats the common
autoscalingsettings—extract and reference a shared anchor or template.
161-166: Apply autoscaling refactor tobeatworkercomponentAgain, identical to other components. Leverage a YAML anchor or Helm helper to DRY this configuration.
🧹 Nitpick comments (2)
charts/plane-ce/values.yaml (2)
79-85: Refactor repeated autoscaling blocks and remove trailing whitespaceThe
autoscalingconfiguration underwebis identical across all service components. To improve maintainability and adhere to DRY, consider defining a YAML anchor (or a Helm helper) for the commonautoscalingblock and reusing it.
Also, line 83 has trailing spaces after80—please remove them to satisfy lint rules.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 83-83: trailing spaces
(trailing-spaces)
211-211: Remove extra blank line at end of fileYAML lint warns about too many blank lines. Please delete the extraneous blank line to comply with the lint rule.
🧰 Tools
🪛 YAMLlint (1.37.1)
[warning] 211-211: too many blank lines
(1 > 0) (empty-lines)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (9)
.gitignore(1 hunks)charts/plane-ce/templates/workloads/admin.deployment.yaml(1 hunks)charts/plane-ce/templates/workloads/api.deployment.yaml(1 hunks)charts/plane-ce/templates/workloads/beat-worker.deployment.yaml(1 hunks)charts/plane-ce/templates/workloads/live.deployment.yaml(1 hunks)charts/plane-ce/templates/workloads/space.deployment.yaml(1 hunks)charts/plane-ce/templates/workloads/web.deployment.yaml(1 hunks)charts/plane-ce/templates/workloads/worker.deployment.yaml(1 hunks)charts/plane-ce/values.yaml(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- .gitignore
- charts/plane-ce/templates/workloads/live.deployment.yaml
- charts/plane-ce/templates/workloads/api.deployment.yaml
- charts/plane-ce/templates/workloads/admin.deployment.yaml
- charts/plane-ce/templates/workloads/space.deployment.yaml
- charts/plane-ce/templates/workloads/beat-worker.deployment.yaml
- charts/plane-ce/templates/workloads/web.deployment.yaml
- charts/plane-ce/templates/workloads/worker.deployment.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/plane-ce/values.yaml
[error] 83-83: trailing spaces
(trailing-spaces)
[warning] 211-211: too many blank lines
(1 > 0) (empty-lines)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/chart-releaser.yml (1)
101-106: Add a dedicated dependency build step for Plane-CE
The newBuild Dependenciesstep correctly invokeshelm dependency build charts/${{ env.CHART_NAME_CE }}to pull in themetrics-serverchart before releasing.Recommendation:
- Move the conditional inside the step to a step-level
if:so the step is skipped entirely whenplane-ceis false.- Add an
id:field for clearer logging and potential downstream references.Example refactor:
- - name: Build Dependencies - run: | - if [ "${{ github.event.inputs.plane-ce }}" = "true" ]; then - helm dependency build charts/${{ env.CHART_NAME_CE }} - fi + - name: Build Plane-CE Dependencies + id: build-deps-ce + if: ${{ github.event.inputs.plane-ce == 'true' }} + run: helm dependency build charts/${{ env.CHART_NAME_CE }}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
.github/workflows/chart-preview.yml(1 hunks).github/workflows/chart-releaser.yml(1 hunks)
🔇 Additional comments (1)
.github/workflows/chart-preview.yml (1)
95-95: Verify dependency build for Plane-Enterprise preview
The addedhelm dependency build charts/$CHART_REPOensures Plane-CE pulls in its newmetrics-serverdependency. If theplane-enterprisechart also declaresmetrics-serverin itsChart.yaml, you’ll need to insert a similarhelm dependency build charts/plane-enterpriseunder thebuild-plane-enterpriserun block to prevent missing-dependency errors.
…i, worker, and beatworker in questions.yml
…ptions across web, space, admin, live, api, worker, and beatworker sections.
pratapalakshmi
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
charts/plane-ce/values.yaml (2)
204-204: Fix trailing whitespace and excessive blank linesThere is trailing whitespace on line 82 and excessive blank lines around line 204, as indicated by the static analysis tool.
- targetCPUUtilizationPercentage: 80 + targetCPUUtilizationPercentage: 80And for line 204, remove the excessive blank line:
api_key_rate_limit: "60/minute" -🧰 Tools
🪛 YAMLlint (1.37.1)
[warning] 204-204: too many blank lines
(1 > 0) (empty-lines)
79-159: Document HPA requirements in chart's READMEWhile the implementation correctly checks for the metrics-server's presence, users may not be aware they need the metrics-server installed for autoscaling to work.
Consider adding documentation in the chart's README to inform users that:
- The chart now supports horizontal pod autoscaling
- The metrics-server must be installed in the cluster for autoscaling to function
- How to customize the autoscaling parameters for each component
This will help users understand the new functionality and requirements.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 82-82: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
charts/plane-ce/templates/_helpers.tpl(1 hunks)charts/plane-ce/templates/workloads/admin.deployment.yaml(1 hunks)charts/plane-ce/templates/workloads/api.deployment.yaml(1 hunks)charts/plane-ce/templates/workloads/beat-worker.deployment.yaml(1 hunks)charts/plane-ce/templates/workloads/live.deployment.yaml(1 hunks)charts/plane-ce/templates/workloads/space.deployment.yaml(1 hunks)charts/plane-ce/templates/workloads/web.deployment.yaml(1 hunks)charts/plane-ce/templates/workloads/worker.deployment.yaml(1 hunks)charts/plane-ce/values.yaml(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- charts/plane-ce/templates/workloads/beat-worker.deployment.yaml
- charts/plane-ce/templates/workloads/live.deployment.yaml
- charts/plane-ce/templates/workloads/api.deployment.yaml
- charts/plane-ce/templates/workloads/web.deployment.yaml
- charts/plane-ce/templates/workloads/admin.deployment.yaml
- charts/plane-ce/templates/workloads/space.deployment.yaml
- charts/plane-ce/templates/workloads/worker.deployment.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/plane-ce/values.yaml
[error] 82-82: trailing spaces
(trailing-spaces)
[warning] 204-204: too many blank lines
(1 > 0) (empty-lines)
🔇 Additional comments (9)
charts/plane-ce/templates/_helpers.tpl (2)
3-3: Fixed template closureThe missing closing tag for the "imagePullSecret" template has been added, properly closing the block definition.
5-12: Well-implemented helper function for HPA supportThis new "enable.hpa" template provides a clean way to conditionally enable Horizontal Pod Autoscaler resources based on the presence of the metrics-server in the cluster. The implementation correctly:
- Looks up the "system:metrics-server" ClusterRole in the Kubernetes API
- Returns a boolean value that can be used in conditional statements throughout the chart templates
This approach ensures that HPA resources are only created when the metrics-server is actually available, preventing deployment errors in clusters without it.
charts/plane-ce/values.yaml (7)
79-83: Well-structured autoscaling configuration for web componentThe autoscaling configuration uses standard HPA parameters with reasonable default values. The chosen utilization thresholds (80% CPU, 70% memory) provide a good balance between resource efficiency and scaling responsiveness.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 82-82: trailing spaces
(trailing-spaces)
92-96: Well-structured autoscaling configuration for space componentThe autoscaling configuration for the space component follows the same pattern as other components, maintaining consistency throughout the values file.
105-109: Well-structured autoscaling configuration for admin componentThe autoscaling configuration for the admin component uses the same standardized structure and threshold values as other components.
118-122: Well-structured autoscaling configuration for live componentThe autoscaling configuration for the live component follows the consistent pattern established for other components.
131-135: Well-structured autoscaling configuration for api componentThe autoscaling configuration for the api component maintains consistency with other components while providing appropriate scaling parameters.
143-147: Well-structured autoscaling configuration for worker componentThe autoscaling configuration for the worker component follows the same pattern as other components, ensuring a consistent approach to resource management.
155-159: Well-structured autoscaling configuration for beatworker componentThe autoscaling configuration for the beatworker component maintains the consistent pattern used for all components.
…ing HPA with specific resource utilization targets in values.yaml. Update deployment templates to reference individual autoscaling settings for each component.
pratapalakshmi
force-pushed
the
chore/add/metrics-server/dependency
branch
from
83004c8 to
217ade5
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (10)
charts/plane-ce/values.yaml (2)
82-82: Remove trailing whitespace.
Line 82 has unintended trailing spaces that should be removed to satisfy YAML lint rules.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 82-82: trailing spaces
(trailing-spaces)
204-204: Remove excessive blank line.
The file ends with an extra blank line; remove it to adhere to the configured empty-lines rule.🧰 Tools
🪛 YAMLlint (1.37.1)
[warning] 204-204: too many blank lines
(1 > 0) (empty-lines)
charts/plane-enterprise/templates/config-secrets/email-env.yaml (1)
1-22: Cleanup template linting errors
YAMLlint is flagging syntax errors due to Helm directives ({{-) and there’s a trailing space on line 20 and no newline at end-of-file. Consider:
- Removing trailing spaces on line 20.
- Ensuring a final newline.
- Adding a top-of-file comment to disable yamllint for Helm templates.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 20-20: trailing spaces
(trailing-spaces)
[error] 22-22: no new line character at the end of file
(new-line-at-end-of-file)
charts/plane-enterprise/templates/certs/cert-issuers.yaml (1)
1-1: Suppress YAMLlint false positives and ensure newline
Linter errors on line 1 are false positives due to Helm templating. Add a disable comment for YAMLlint or adjust your linter config, and verify there’s a newline at the end of the file.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/plane-enterprise/templates/certs/email-certs.yaml (1)
10-10: Remove trailing spaces and add newline
There’s trailing whitespace on line 10 and no newline at the end of the file (line 15). Please trim the trailing spaces and ensure the file ends with a newline.Also applies to: 15-15
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 10-10: trailing spaces
(trailing-spaces)
charts/plane-enterprise/templates/workloads/email.deployment.yaml (4)
1-2: Ensure conditional block syntax and whitespace trimming are correct.Using
{{- if … }}at the top without a trailing-on the end tag may lead to unexpected whitespace or blank lines in the rendered manifest. Consider changing:- {{- if .Values.services.email_service.enabled }} + {{- if .Values.services.email_service.enabled -}} ... - {{- end }} + {{- end -}}This ensures no extra empty lines appear before or after the block.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
28-67: Replace exec‐based readinessProbe with tcpSocket.Relying on
ncin the container image may lead to probe failures ifnetcatisn't installed. A TCP socket probe is more declarative:readinessProbe: - exec: - command: - - nc - - -zv - - localhost - - "10025" + tcpSocket: + port: 10025 initialDelaySeconds: 30 periodSeconds: 10 timeoutSeconds: 5 failureThreshold: 3This removes the dependency on
ncand is lighter weight.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 44-44: trailing spaces
(trailing-spaces)
[error] 45-45: trailing spaces
(trailing-spaces)
68-74: Remove deprecatedserviceAccountfield.In Pod specs, only
serviceAccountNameis needed;serviceAccountis deprecated since Kubernetes v1.8. You can safely remove:- serviceAccount: {{ .Release.Name }}-srv-accountand retain:
serviceAccountName: {{ .Release.Name }}-srv-account
75-88: Fix indentation forenvFromandvolumes.YAML lint warns about inconsistent indentation. For example,
envFrom:should be indented 8 spaces from the root of the document, not 10. Align these blocks with your chart’s conventions:- envFrom: - - configMapRef: + envFrom: + - configMapRef: name: {{ .Release.Name }}-email-vars optional: false ... - volumes: - - name: spam-blacklist + volumes: + - name: spam-blacklist configMap: name: {{ .Release.Name }}-email-vars🧰 Tools
🪛 YAMLlint (1.37.1)
[warning] 75-75: wrong indentation: expected 8 but found 10
(indentation)
[warning] 79-79: wrong indentation: expected 8 but found 10
(indentation)
[warning] 88-88: wrong indentation: expected 6 but found 8
(indentation)
charts/plane-enterprise/questions.yml (1)
20-24: Quote version strings to avoid YAML parsing quirks.Unquoted
v1.10.0could be misinterpreted by some parsers. Wrap it in quotes:- default: v1.10.0 + default: "v1.10.0"This ensures it’s always treated as a string.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
.gitignore(1 hunks)charts/plane-ce/templates/_helpers.tpl(1 hunks)charts/plane-ce/templates/workloads/admin.deployment.yaml(1 hunks)charts/plane-ce/templates/workloads/api.deployment.yaml(1 hunks)charts/plane-ce/templates/workloads/beat-worker.deployment.yaml(1 hunks)charts/plane-ce/templates/workloads/live.deployment.yaml(1 hunks)charts/plane-ce/templates/workloads/space.deployment.yaml(1 hunks)charts/plane-ce/templates/workloads/web.deployment.yaml(1 hunks)charts/plane-ce/templates/workloads/worker.deployment.yaml(1 hunks)charts/plane-ce/values.yaml(6 hunks)charts/plane-enterprise/Chart.yaml(1 hunks)charts/plane-enterprise/README.md(5 hunks)charts/plane-enterprise/questions.yml(3 hunks)charts/plane-enterprise/templates/certs/cert-issuers.yaml(1 hunks)charts/plane-enterprise/templates/certs/email-certs.yaml(1 hunks)charts/plane-enterprise/templates/config-secrets/app-env.yaml(1 hunks)charts/plane-enterprise/templates/config-secrets/email-env.yaml(1 hunks)charts/plane-enterprise/templates/workloads/email.deployment.yaml(1 hunks)charts/plane-enterprise/values.yaml(3 hunks)
✅ Files skipped from review due to trivial changes (2)
- charts/plane-enterprise/Chart.yaml
- charts/plane-enterprise/README.md
🚧 Files skipped from review as they are similar to previous changes (9)
- .gitignore
- charts/plane-ce/templates/_helpers.tpl
- charts/plane-ce/templates/workloads/admin.deployment.yaml
- charts/plane-ce/templates/workloads/live.deployment.yaml
- charts/plane-ce/templates/workloads/api.deployment.yaml
- charts/plane-ce/templates/workloads/worker.deployment.yaml
- charts/plane-ce/templates/workloads/beat-worker.deployment.yaml
- charts/plane-ce/templates/workloads/web.deployment.yaml
- charts/plane-ce/templates/workloads/space.deployment.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/plane-enterprise/templates/certs/cert-issuers.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/plane-enterprise/templates/certs/email-certs.yaml
[error] 10-10: trailing spaces
(trailing-spaces)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 15-15: no new line character at the end of file
(new-line-at-end-of-file)
charts/plane-enterprise/templates/config-secrets/email-env.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 20-20: trailing spaces
(trailing-spaces)
[error] 22-22: no new line character at the end of file
(new-line-at-end-of-file)
charts/plane-enterprise/templates/workloads/email.deployment.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 44-44: trailing spaces
(trailing-spaces)
[error] 45-45: trailing spaces
(trailing-spaces)
[warning] 75-75: wrong indentation: expected 8 but found 10
(indentation)
[warning] 79-79: wrong indentation: expected 8 but found 10
(indentation)
[warning] 88-88: wrong indentation: expected 6 but found 8
(indentation)
[error] 103-103: no new line character at the end of file
(new-line-at-end-of-file)
charts/plane-ce/values.yaml
[error] 82-82: trailing spaces
(trailing-spaces)
[warning] 204-204: too many blank lines
(1 > 0) (empty-lines)
🔇 Additional comments (14)
charts/plane-ce/values.yaml (7)
79-83: Add autoscaling configuration for the WEB component.
The newautoscalingblock defines sensible default HPA settings (minReplicas, maxReplicas, CPU and memory targets). Ensure these values align with your expected workload profiles.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 82-82: trailing spaces
(trailing-spaces)
92-96: Add autoscaling configuration for the SPACE component.
The added HPA settings mirror the WEB component defaults. Confirm these targets fit the SPACE service’s resource usage patterns.
105-109: Add autoscaling configuration for the ADMIN component.
Configuration is consistent with other services. Verify that these thresholds suit the ADMIN workload’s behavior under load.
118-122: Add autoscaling configuration for the LIVE component.
Settings follow the established pattern. Ensure the LIVE service can scale appropriately based on these CPU/memory thresholds.
131-135: Add autoscaling configuration for the API component.
Defaults look good. Double-check that the API’s resource footprint matches these auto-scaling targets.
143-147: Add autoscaling configuration for the WORKER component.
Values are in line with other back-end services. Validate against typical worker job profiles.
155-159: Add autoscaling configuration for the BEATWORKER component.
Consistent HPA defaults; confirm these thresholds reflect the beat worker’s load characteristics.charts/plane-enterprise/templates/config-secrets/email-env.yaml (1)
1-2: Conditional rendering is correct
The{{- if .Values.services.email_service.enabled }}guard ensures this ConfigMap is only rendered when the email service is enabled.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/plane-enterprise/templates/certs/cert-issuers.yaml (1)
1-45: Issuer conditional logic expanded correctly
The updated condition{{- if and .Values.ingress.enabled (or .Values.services.email_service.enabled .Values.ssl.createIssuer) (empty .Values.ssl.tls_secret_name) }}ensures the cert-issuer resources are created for both ingress and the new email service when no TLS secret exists.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 11-11: trailing spaces
(trailing-spaces)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/plane-enterprise/templates/config-secrets/app-env.yaml (1)
52-52: Integrate SMTP domain into application config
The newINTAKE_EMAIL_DOMAIN: {{ .Values.env.email_service_envs.smtp_domain | default "" | quote }}correctly surfaces the SMTP domain to your main application environment, defaulting to an empty string when unset.
charts/plane-enterprise/templates/certs/email-certs.yaml (1)
1-15: Certificate template logic is sound
The guard{{- if and .Values.services.email_service.enabled .Values.env.email_service_envs.smtp_domain }}ensures a cert-manager
Certificateis only created when email is enabled and a valid SMTP domain is provided. All references to.Release.Nameand.Release.Namespaceare correct.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 10-10: trailing spaces
(trailing-spaces)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 15-15: no new line character at the end of file
(new-line-at-end-of-file)
charts/plane-enterprise/values.yaml (1)
218-220: Define SMTP domain env defaults
The blockemail_service_envs: smtp_domain: ''correctly defaults the SMTP domain to an empty string. Ensure documentation is updated to note that setting this value is required to enable TLS certificate creation and ConfigMap rendering.
charts/plane-enterprise/templates/workloads/email.deployment.yaml (1)
3-26: Validate Service resource and selectors.The Service is of type
LoadBalancerwithexternalTrafficPolicy: Local, which is appropriate for preserving client IP in SMTP. Ensure the label selectorapp.namematches exactly the labels on the Pods (case‐sensitive). Consider standardizing on a well-known label key (e.g.,app.kubernetes.io/name) if the rest of the chart has adopted that convention.charts/plane-enterprise/questions.yml (1)
62-68: Approve new email image question.The addition of the
services.email_service.imagevariable under the Plane Version group looks correct and aligns with other services.
| email_service: | ||
| enabled: false | ||
| replicas: 1 | ||
| memory_limit: 1000Mi | ||
| cpu_limit: 500m | ||
| image: artifacts.plane.so/makeplane/email-commercial | ||
| pullPolicy: Always | ||
|
|
There was a problem hiding this comment.
Fix naming consistency for resource limits
Other services use memoryLimit/cpuLimit, but here the keys are memory_limit/cpu_limit. This mismatch can break your templates. Please rename to match the rest:
email_service:
enabled: false
replicas: 1
- memory_limit: 1000Mi
- cpu_limit: 500m
+ memoryLimit: 1000Mi
+ cpuLimit: 500m
image: artifacts.plane.so/makeplane/email-commercial
pullPolicy: Always📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| email_service: | |
| enabled: false | |
| replicas: 1 | |
| memory_limit: 1000Mi | |
| cpu_limit: 500m | |
| image: artifacts.plane.so/makeplane/email-commercial | |
| pullPolicy: Always | |
| email_service: | |
| enabled: false | |
| replicas: 1 | |
| memoryLimit: 1000Mi | |
| cpuLimit: 500m | |
| image: artifacts.plane.so/makeplane/email-commercial | |
| pullPolicy: Always |
🤖 Prompt for AI Agents
In charts/plane-enterprise/values.yaml around lines 154 to 161, the resource
limit keys for email_service use snake_case (memory_limit, cpu_limit) instead of
camelCase (memoryLimit, cpuLimit) like other services. Rename memory_limit to
memoryLimit and cpu_limit to cpuLimit to maintain naming consistency and avoid
template breakage.
| serviceAccount: {{ .Release.Name }}-srv-account | ||
| serviceAccountName: {{ .Release.Name }}-srv-account | ||
| --- | ||
| {{- end }} No newline at end of file |
There was a problem hiding this comment.
Add newline at end of file.
YAML lint flagged a missing newline at EOF. Please ensure there is a trailing newline after the closing {{- end }} to avoid syntax errors.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 103-103: no new line character at the end of file
(new-line-at-end-of-file)
🤖 Prompt for AI Agents
In charts/plane-enterprise/templates/workloads/email.deployment.yaml at line
103, add a newline character immediately after the closing {{- end }} statement
to ensure the file ends with a newline, which is required by YAML syntax and
linting tools.
| labels: | ||
| app.name: {{ .Release.Namespace }}-{{ .Release.Name }}-email-app | ||
| annotations: |
There was a problem hiding this comment.
Eliminate trailing spaces.
Lines 44 and 45 contain trailing whitespace. Please remove the extra spaces to satisfy YAML linting:
- labels:
- app.name: {{ .Release.Namespace }}-{{ .Release.Name }}-email-app
- annotations:
+ labels:
+ app.name: {{ .Release.Namespace }}-{{ .Release.Name }}-email-app
+ annotations:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| labels: | |
| app.name: {{ .Release.Namespace }}-{{ .Release.Name }}-email-app | |
| annotations: | |
| labels: | |
| app.name: {{ .Release.Namespace }}-{{ .Release.Name }}-email-app | |
| annotations: |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 44-44: trailing spaces
(trailing-spaces)
[error] 45-45: trailing spaces
(trailing-spaces)
🤖 Prompt for AI Agents
In charts/plane-enterprise/templates/workloads/email.deployment.yaml at lines 43
to 45, remove the trailing spaces at the end of lines 44 and 45 to comply with
YAML linting rules. Ensure no extra whitespace exists after the text on these
lines.
| - variable: services.email_service.enabled | ||
| label: "Enable Email Service" | ||
| type: boolean | ||
| default: false | ||
| group: "Email Service Setup" | ||
| show_subquestion_if: true | ||
| subquestions: | ||
| - variable: services.email_service.replicas | ||
| label: "Default Replica Count" | ||
| type: int | ||
| default: 1 | ||
| - variable: services.email_service.memory_limit | ||
| label: "Memory Limit" | ||
| type: string | ||
| default: 1000Mi | ||
| - variable: services.email_service.cpu_limit | ||
| label: "CPU Limit" | ||
| type: string | ||
| default: 500m | ||
| - variable: env.email_service_envs.smtp_domain | ||
| label: "SMTP Domain" | ||
| type: string | ||
| default: "" | ||
| - variable: ssl.issuer | ||
| label: "SSL Issuer" | ||
| type: enum | ||
| options: | ||
| - "http" | ||
| - "cloudflare" | ||
| - "digitalocean" | ||
| default: "http" | ||
| - variable: ssl.server | ||
| label: "Let's Encrypt Server URL" | ||
| type: string | ||
| default: "https://acme-v02.services.api.letsencrypt.org/directory" | ||
| - variable: ssl.email | ||
| label: "Let's Encrypt Reg. Email" | ||
| type: string | ||
| default: "plane@example.com" | ||
| - variable: ssl.token | ||
| label: "Provider API Token" | ||
| type: password | ||
| default: "" | ||
| description: "Not required for 'http' issuer" | ||
| show_if: "ssl.issuer=cloudflare || ssl.issuer=digitalocean" | ||
|
|
There was a problem hiding this comment.
Fix SSL question variable paths to map to the email service.
You’ve added SSL controls within the Email Service Setup group, but using top‐level keys (ssl.issuer, ssl.server, etc.) will collide with the main ingress SSL settings and won’t populate Values.services.email_service.ssl. Change them to nested paths:
- - variable: ssl.issuer
+ - variable: services.email_service.ssl.issuer
...
- - variable: ssl.server
+ - variable: services.email_service.ssl.server
...
- - variable: ssl.email
+ - variable: services.email_service.ssl.email
...
- - variable: ssl.token
+ - variable: services.email_service.ssl.tokenAlso update the show_if logic to reference services.email_service.ssl.issuer. This ensures the values end up in the correct location and don’t override global SSL settings.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - variable: services.email_service.enabled | |
| label: "Enable Email Service" | |
| type: boolean | |
| default: false | |
| group: "Email Service Setup" | |
| show_subquestion_if: true | |
| subquestions: | |
| - variable: services.email_service.replicas | |
| label: "Default Replica Count" | |
| type: int | |
| default: 1 | |
| - variable: services.email_service.memory_limit | |
| label: "Memory Limit" | |
| type: string | |
| default: 1000Mi | |
| - variable: services.email_service.cpu_limit | |
| label: "CPU Limit" | |
| type: string | |
| default: 500m | |
| - variable: env.email_service_envs.smtp_domain | |
| label: "SMTP Domain" | |
| type: string | |
| default: "" | |
| - variable: ssl.issuer | |
| label: "SSL Issuer" | |
| type: enum | |
| options: | |
| - "http" | |
| - "cloudflare" | |
| - "digitalocean" | |
| default: "http" | |
| - variable: ssl.server | |
| label: "Let's Encrypt Server URL" | |
| type: string | |
| default: "https://acme-v02.services.api.letsencrypt.org/directory" | |
| - variable: ssl.email | |
| label: "Let's Encrypt Reg. Email" | |
| type: string | |
| default: "plane@example.com" | |
| - variable: ssl.token | |
| label: "Provider API Token" | |
| type: password | |
| default: "" | |
| description: "Not required for 'http' issuer" | |
| show_if: "ssl.issuer=cloudflare || ssl.issuer=digitalocean" | |
| - variable: services.email_service.replicas | |
| label: "Default Replica Count" | |
| type: int | |
| default: 1 | |
| - variable: services.email_service.memory_limit | |
| label: "Memory Limit" | |
| type: string | |
| default: 1000Mi | |
| - variable: services.email_service.cpu_limit | |
| label: "CPU Limit" | |
| type: string | |
| default: 500m | |
| - variable: env.email_service_envs.smtp_domain | |
| label: "SMTP Domain" | |
| type: string | |
| default: "" | |
| - - variable: ssl.issuer | |
| + - variable: services.email_service.ssl.issuer | |
| label: "SSL Issuer" | |
| type: enum | |
| options: | |
| - "http" | |
| - "cloudflare" | |
| - "digitalocean" | |
| default: "http" | |
| - - variable: ssl.server | |
| + - variable: services.email_service.ssl.server | |
| label: "Let's Encrypt Server URL" | |
| type: string | |
| default: "https://acme-v02.services.api.letsencrypt.org/directory" | |
| - - variable: ssl.email | |
| + - variable: services.email_service.ssl.email | |
| label: "Let's Encrypt Reg. Email" | |
| type: string | |
| default: "plane@example.com" | |
| - - variable: ssl.token | |
| + - variable: services.email_service.ssl.token | |
| label: "Provider API Token" | |
| type: password | |
| default: "" | |
| description: "Not required for 'http' issuer" | |
| - show_if: "ssl.issuer=cloudflare || ssl.issuer=digitalocean" | |
| + show_if: "services.email_service.ssl.issuer=cloudflare || services.email_service.ssl.issuer=digitalocean" |
🤖 Prompt for AI Agents
In charts/plane-enterprise/questions.yml between lines 431 and 476, the
SSL-related variables under the Email Service Setup group use top-level keys
like ssl.issuer, ssl.server, ssl.email, and ssl.token, which conflict with
global SSL settings. To fix this, nest these variables under
services.email_service.ssl (e.g., services.email_service.ssl.issuer) so their
values map correctly to the email service configuration. Also, update the
show_if condition to reference services.email_service.ssl.issuer instead of
ssl.issuer to maintain proper conditional display logic.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
charts/plane-ce/Chart.yaml (1)
19-22: Add newline at end-of-fileYAML parsers and linters expect a terminating newline. Please ensure there's a blank line after the last dependency entry.
Example diff:
dependencies: - name: metrics-server version: 3.12.2 repository: https://kubernetes-sigs.github.io/metrics-server/ +🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 22-22: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/chart-preview.yml (1)
95-95: Remove trailing whitespaceLine 95 contains trailing spaces, causing a YAML lint error. Please delete any extra spaces on blank lines.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 95-95: trailing spaces
(trailing-spaces)
charts/plane-ce/values.yaml (3)
83-83: Remove trailing spacesThe value line for
targetCPUUtilizationPercentage: 80has trailing spaces that trigger a YAML lint warning. Please trim the extra spaces.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 83-83: trailing spaces
(trailing-spaces)
211-211: Remove extra blank line at EOFThere is an unnecessary blank line after the last value, which violates the linter rule on empty lines. Please remove it.
🧰 Tools
🪛 YAMLlint (1.37.1)
[warning] 211-211: too many blank lines
(1 > 0) (empty-lines)
79-85: DRY repeated autoscaling blocks with YAML anchorsThe autoscaling configuration for each component is identical. To improve maintainability, consider declaring an anchor for the defaults and reusing it:
autoscalingDefaults: &autoscalingDefaults enabled: true minReplicas: 1 maxReplicas: 3 targetCPUUtilizationPercentage: 80 targetMemoryUtilizationPercentage: 70 web: # ... autoscaling: <<: *autoscalingDefaults # repeat for other components...This reduces duplication and simplifies future changes.
Also applies to: 93-99, 107-113, 121-127, 135-141, 148-154, 161-167
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 83-83: trailing spaces
(trailing-spaces)
charts/plane-ce/questions.yml (2)
58-81: Remove trailing whitespace from autoscaling entriesSeveral lines in the new autoscaling question blocks include trailing spaces, leading to YAML lint errors. Please strip trailing whitespace from all these lines.
Also applies to: 113-136, 168-191, 236-259, 313-336, 351-374, 389-412
58-81: Consolidate repeated autoscaling questions using YAML anchorsThe autoscaling variables are repeated verbatim across multiple components. You can introduce a YAML anchor to define the set once and reference it for each component:
autoscalingBase: &autoscalingBase - variable: AUTOSCALING.enabled label: "Enable Autoscaling" type: boolean default: true # ... other anchors for min/max/targets questions: - <<: *autoscalingBase variable: web.autoscaling.enabled # ...This DRY approach will make updates across all components much easier.
Also applies to: 113-136, 168-191, 236-259, 313-336, 351-374, 389-412
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
charts/plane-ce/Chart.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.github/workflows/chart-preview.yml(1 hunks).github/workflows/chart-releaser.yml(1 hunks)charts/plane-ce/Chart.yaml(1 hunks)charts/plane-ce/questions.yml(7 hunks)charts/plane-ce/templates/workloads/admin.deployment.yaml(1 hunks)charts/plane-ce/templates/workloads/api.deployment.yaml(1 hunks)charts/plane-ce/templates/workloads/live.deployment.yaml(1 hunks)charts/plane-ce/values.yaml(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- charts/plane-ce/templates/workloads/api.deployment.yaml
- charts/plane-ce/templates/workloads/admin.deployment.yaml
- charts/plane-ce/templates/workloads/live.deployment.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/chart-preview.yml
[error] 95-95: trailing spaces
(trailing-spaces)
charts/plane-ce/questions.yml
[error] 125-125: trailing spaces
(trailing-spaces)
[error] 175-175: trailing spaces
(trailing-spaces)
[error] 181-181: trailing spaces
(trailing-spaces)
[error] 248-248: trailing spaces
(trailing-spaces)
[error] 257-257: trailing spaces
(trailing-spaces)
[error] 320-320: trailing spaces
(trailing-spaces)
[error] 321-321: trailing spaces
(trailing-spaces)
[error] 329-329: trailing spaces
(trailing-spaces)
[error] 334-334: trailing spaces
(trailing-spaces)
[error] 358-358: trailing spaces
(trailing-spaces)
[error] 368-368: trailing spaces
(trailing-spaces)
[error] 372-372: trailing spaces
(trailing-spaces)
[error] 401-401: trailing spaces
(trailing-spaces)
[error] 410-410: trailing spaces
(trailing-spaces)
charts/plane-ce/Chart.yaml
[error] 22-22: no new line character at the end of file
(new-line-at-end-of-file)
charts/plane-ce/values.yaml
[error] 83-83: trailing spaces
(trailing-spaces)
[warning] 211-211: too many blank lines
(1 > 0) (empty-lines)
| - name: Build Dependencies | ||
| run: | | ||
| if [ "${{ github.event.inputs.plane-ce }}" = "true" ]; then | ||
| helm dependency build charts/${{ env.CHART_NAME_CE }} | ||
| fi | ||
|
|
There was a problem hiding this comment.
Missing Helm repo addition before dependency build
The new Build Dependencies step invokes helm dependency build but does not add the metrics-server repository first, which will cause the dependency fetch to fail. You need to add and update the repo before building.
Apply this diff:
- name: Build Dependencies
run: |
- if [ "${{ github.event.inputs.plane-ce }}" = "true" ]; then
- helm dependency build charts/${{ env.CHART_NAME_CE }}
- fi
+ helm repo add metrics-server https://kubernetes-sigs.github.io/metrics-server/
+ helm repo update
+ if [ "${{ github.event.inputs.plane-ce }}" = "true" ]; then
+ helm dependency build charts/${{ env.CHART_NAME_CE }}
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Build Dependencies | |
| run: | | |
| if [ "${{ github.event.inputs.plane-ce }}" = "true" ]; then | |
| helm dependency build charts/${{ env.CHART_NAME_CE }} | |
| fi | |
| - name: Build Dependencies | |
| run: | | |
| helm repo add metrics-server https://kubernetes-sigs.github.io/metrics-server/ | |
| helm repo update | |
| if [ "${{ github.event.inputs.plane-ce }}" = "true" ]; then | |
| helm dependency build charts/${{ env.CHART_NAME_CE }} | |
| fi |
🤖 Prompt for AI Agents
In .github/workflows/chart-releaser.yml around lines 101 to 106, before running
`helm dependency build`, add a step to add the `metrics-server` Helm repository
using `helm repo add metrics-server <repo-url>` and then run `helm repo update`
to ensure dependencies can be fetched successfully. Insert these commands before
the dependency build command inside the conditional block.
|
These changes are picked in a different branch add-hpa-ce. Therefore closing it |
2 participants
Description
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
New Features
Documentation
Chores