Skip to content

enable hpa based on metrics-server#104

Closed
pratapalakshmi wants to merge 7 commits intomakeplane:developfrom
pratapalakshmi:chore/add/metrics-server/dependency
Closed

enable hpa based on metrics-server#104
pratapalakshmi wants to merge 7 commits intomakeplane:developfrom
pratapalakshmi:chore/add/metrics-server/dependency

Conversation

@pratapalakshmi
Copy link
Collaborator

@pratapalakshmi pratapalakshmi commented May 20, 2025

Description

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Improvement (change that would cause existing functionality to not work as expected)
  • Code refactoring
  • Performance improvements
  • Documentation update

Screenshots and Media (if applicable)

Test Scenarios

References

Summary by CodeRabbit

  • New Features

    • Introduced configurable autoscaling options for web, space, admin, live, api, worker, and beatworker components, allowing users to set minimum/maximum replicas and CPU/memory utilization targets.
    • Enabled automatic scaling for all major components based on resource usage, improving performance and resource efficiency.
    • Added dependency on metrics-server for autoscaling support.
  • Documentation

    • Updated configuration and setup questions to include autoscaling parameters for all key components.
  • Chores

    • Enhanced chart build and release workflows to handle new dependencies and autoscaling features.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented May 20, 2025

Walkthrough

This 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

File(s) Change Summary
.gitignore Ignores charts/plane-ce/charts directory.
.github/workflows/chart-preview.yml, .github/workflows/chart-releaser.yml Adds steps to build Helm chart dependencies before packaging or releasing charts.
charts/plane-ce/Chart.yaml Declares metrics-server as a Helm chart dependency.
charts/plane-ce/values.yaml Adds autoscaling configuration blocks for all major components.
charts/plane-ce/questions.yml Introduces autoscaling-related variables for each component in the questions UI.
charts/plane-ce/templates/_helpers.tpl Adds enable.hpa template to detect metrics-server; fixes template block ending.
charts/plane-ce/templates/workloads/*.deployment.yaml Conditionally adds HPA manifests for web, space, admin, live, api, worker, and beat-worker workloads.

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
Loading

Suggested reviewers

  • mguptahub

Poem

In the meadow of charts, the pods now dance,
Scaling up or down as workloads enhance.
The metrics-server hums, keeping watchful eyes,
While rabbits with laptops cheer the autoscale rise.
Helm hops along, dependencies in tow—
Kubernetes magic, let the workloads grow!
🐇✨


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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 named charts elsewhere:

-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: 80
charts/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.tpl partial 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

📥 Commits

Reviewing files that changed from the base of the PR and between c76b1a7 and c465658.

⛔ Files ignored due to path filters (1)
  • charts/plane-ce/Chart.lock is 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)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (6)
charts/plane-ce/values.yaml (6)

93-98: Apply autoscaling refactor to space component

This autoscaling block is a direct duplicate of the one in web (lines 79–85). Apply the same anchor/helper refactor here to avoid repetition.


107-112: Apply autoscaling refactor to admin component

Duplicate autoscaling settings detected—reuse the common definition via YAML anchors or a shared Helm template.


121-126: Apply autoscaling refactor to live component

Identical block as above; extract into a shared anchor/helper to simplify updates.


135-140: Apply autoscaling refactor to api component

Same autoscaling parameters—consider consolidating under a single definition for consistency.


148-153: Apply autoscaling refactor to worker component

This block repeats the common autoscaling settings—extract and reference a shared anchor or template.


161-166: Apply autoscaling refactor to beatworker component

Again, 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 whitespace

The autoscaling configuration under web is identical across all service components. To improve maintainability and adhere to DRY, consider defining a YAML anchor (or a Helm helper) for the common autoscaling block and reusing it.
Also, line 83 has trailing spaces after 80—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 file

YAML 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

📥 Commits

Reviewing files that changed from the base of the PR and between c465658 and 69c26c8.

📒 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)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 new Build Dependencies step correctly invokes helm dependency build charts/${{ env.CHART_NAME_CE }} to pull in the metrics-server chart before releasing.

Recommendation:

  • Move the conditional inside the step to a step-level if: so the step is skipped entirely when plane-ce is 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

📥 Commits

Reviewing files that changed from the base of the PR and between 69c26c8 and da385ae.

📒 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 added helm dependency build charts/$CHART_REPO ensures Plane-CE pulls in its new metrics-server dependency. If the plane-enterprise chart also declares metrics-server in its Chart.yaml, you’ll need to insert a similar helm dependency build charts/plane-enterprise under the build-plane-enterprise run block to prevent missing-dependency errors.

@pratapalakshmi pratapalakshmi changed the title chore add metrics-server dependency enable hpa based on metrics-server May 22, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
charts/plane-ce/values.yaml (2)

204-204: Fix trailing whitespace and excessive blank lines

There is trailing whitespace on line 82 and excessive blank lines around line 204, as indicated by the static analysis tool.

-    targetCPUUtilizationPercentage: 80  
+    targetCPUUtilizationPercentage: 80

And 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 README

While 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:

  1. The chart now supports horizontal pod autoscaling
  2. The metrics-server must be installed in the cluster for autoscaling to function
  3. 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

📥 Commits

Reviewing files that changed from the base of the PR and between da385ae and 98702e1.

📒 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 closure

The missing closing tag for the "imagePullSecret" template has been added, properly closing the block definition.


5-12: Well-implemented helper function for HPA support

This 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:

  1. Looks up the "system:metrics-server" ClusterRole in the Kubernetes API
  2. 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 component

The 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 component

The 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 component

The 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 component

The autoscaling configuration for the live component follows the consistent pattern established for other components.


131-135: Well-structured autoscaling configuration for api component

The autoscaling configuration for the api component maintains consistency with other components while providing appropriate scaling parameters.


143-147: Well-structured autoscaling configuration for worker component

The 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 component

The autoscaling configuration for the beatworker component maintains the consistent pattern used for all components.

@akshat5302 akshat5302 changed the base branch from develop to add-hpa-ce May 29, 2025 07:40
akshat5302 and others added 2 commits May 29, 2025 13:11
…ing HPA with specific resource utilization targets in values.yaml. Update deployment templates to reference individual autoscaling settings for each component.
@akshat5302 akshat5302 changed the base branch from add-hpa-ce to develop May 29, 2025 07:42
@pratapalakshmi pratapalakshmi force-pushed the chore/add/metrics-server/dependency branch from 83004c8 to 217ade5 Compare May 29, 2025 07:43
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 nc in the container image may lead to probe failures if netcat isn'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: 3

This removes the dependency on nc and 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 deprecated serviceAccount field.

In Pod specs, only serviceAccountName is needed; serviceAccount is deprecated since Kubernetes v1.8. You can safely remove:

-      serviceAccount: {{ .Release.Name }}-srv-account

and retain:

      serviceAccountName: {{ .Release.Name }}-srv-account

75-88: Fix indentation for envFrom and volumes.

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.0 could 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

📥 Commits

Reviewing files that changed from the base of the PR and between 98702e1 and 217ade5.

📒 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 new autoscaling block 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 new

INTAKE_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 Certificate is only created when email is enabled and a valid SMTP domain is provided. All references to .Release.Name and .Release.Namespace are 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 block

email_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 LoadBalancer with externalTrafficPolicy: Local, which is appropriate for preserving client IP in SMTP. Ensure the label selector app.name matches 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.image variable under the Plane Version group looks correct and aligns with other services.

Comment on lines +154 to +161
email_service:
enabled: false
replicas: 1
memory_limit: 1000Mi
cpu_limit: 500m
image: artifacts.plane.so/makeplane/email-commercial
pullPolicy: Always

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Comment on lines +43 to +45
labels:
app.name: {{ .Release.Namespace }}-{{ .Release.Name }}-email-app
annotations:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
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.

Comment on lines +431 to +476
- 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"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.token

Also 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.

Suggested change
- 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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (7)
charts/plane-ce/Chart.yaml (1)

19-22: Add newline at end-of-file

YAML 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 whitespace

Line 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 spaces

The value line for targetCPUUtilizationPercentage: 80 has 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 EOF

There 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 anchors

The 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 entries

Several 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 anchors

The 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

📥 Commits

Reviewing files that changed from the base of the PR and between 217ade5 and 1687134.

⛔ Files ignored due to path filters (1)
  • charts/plane-ce/Chart.lock is 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)

Comment on lines +101 to +106
- name: Build Dependencies
run: |
if [ "${{ github.event.inputs.plane-ce }}" = "true" ]; then
helm dependency build charts/${{ env.CHART_NAME_CE }}
fi

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
- 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.

@pratapalakshmi
Copy link
Collaborator Author

These changes are picked in a different branch add-hpa-ce. Therefore closing it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants