Added Helm Chart for deployment of Capture #145
Conversation
WalkthroughThis pull request introduces a complete Helm chart for the Capture hardware monitoring agent, including chart metadata, Kubernetes resource templates (Deployment, Service, ConfigMap, Secret), installation documentation, default configuration values, and Helm template helpers for standardized naming and labeling conventions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
charts/helm/capture/Chart.yaml (1)
6-6: Consider using a semantic version forappVersion.Using a branch name like
"develop"forappVersionmakes it difficult to track which version of the application is deployed. Consider using semantic versioning (e.g.,"0.1.0") or commit SHA for better version tracking and rollback capabilities.charts/helm/capture/values.yaml (1)
10-10: Inconsistent port type definition.The service port is defined as a number (line 10:
59232) while the config port is defined as a string (line 13:"59232"). For consistency and clarity, both should use the same type. Since environment variables are typically strings, consider makingservice.portan integer and keepingconfig.portas a string, or reference the same value.🔎 Suggested approach
Option 1 - Keep current types but add a comment:
service: type: ClusterIP port: 59232 # Integer for Service port config: port: "59232" # String for environment variableOption 2 - Reference the same value:
service: type: ClusterIP port: 59232 config: port: "{{ .Values.service.port }}" # Reference service portAlso applies to: 13-13
charts/helm/capture/INSTALLATION.md (1)
22-22: Enhance documentation for the required API secret.The documentation mentions setting
secret.apiSecretbut doesn't emphasize that it's a required value or provide guidance on generating a secure secret.🔎 Suggested enhancement
### 2. Customize values.yaml -Edit `values.yaml` and set the required `secret.apiSecret`. Adjust image, service, and other settings as needed. +Edit `values.yaml` and configure the following: + +**Required:** +- `secret.apiSecret`: Generate a strong random secret (minimum 32 characters recommended) + ```shell + # Generate a secure random secret + openssl rand -base64 32 + ``` + +**Optional:** +- Adjust `image.repository` and `image.tag` for specific versions +- Configure `service.type` and `service.port` as needed +- Set `resources`, `nodeSelector`, `tolerations`, or `affinity` for schedulingcharts/helm/capture/templates/deployment.yaml (1)
35-46: Consider adding explicit timeout and failure thresholds to health probes.The health probes configuration is functional but could be more robust with explicit
timeoutSecondsandfailureThresholdvalues. This is especially important if the application might have variable response times under load.🔎 Suggested enhancement
livenessProbe: httpGet: path: /health port: http initialDelaySeconds: 5 periodSeconds: 10 + timeoutSeconds: 3 + failureThreshold: 3 readinessProbe: httpGet: path: /health port: http initialDelaySeconds: 5 periodSeconds: 10 + timeoutSeconds: 3 + failureThreshold: 3This provides clearer behavior:
timeoutSeconds: 3allows up to 3 seconds for the health check to respondfailureThreshold: 3requires 3 consecutive failures before marking unhealthycharts/helm/capture/templates/_helpers.tpl (1)
14-20: Labels follow Kubernetes conventions correctly.The standard label set is well-implemented. Consider sanitizing
Chart.Versionon line 19 to handle semantic version build metadata (e.g.,1.0.0+build), as the+character is invalid in Kubernetes label values.🔎 Optional: Sanitize Chart.Version for label compatibility
-helm.sh/chart: {{ printf "%s-%s" .Chart.Name .Chart.Version | quote }} +helm.sh/chart: {{ printf "%s-%s" .Chart.Name (.Chart.Version | replace "+" "_") | quote }}
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
charts/helm/capture/.helmignorecharts/helm/capture/Chart.yamlcharts/helm/capture/INSTALLATION.mdcharts/helm/capture/templates/_helpers.tplcharts/helm/capture/templates/configmap.yamlcharts/helm/capture/templates/deployment.yamlcharts/helm/capture/templates/secret.yamlcharts/helm/capture/templates/service.yamlcharts/helm/capture/values.yaml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-02-01T14:10:43.142Z
Learnt from: mertssmnoglu
Repo: bluewave-labs/capture PR: 45
File: cmd/capture/main.go:34-38
Timestamp: 2025-02-01T14:10:43.142Z
Learning: In the Capture project, environment variable validation for PORT and API_SECRET is handled in internal/config/config.go's NewConfig function. PORT gets a default value if not set, and API_SECRET is required with a fatal error if missing.
Applied to files:
charts/helm/capture/templates/configmap.yaml
🪛 YAMLlint (1.37.1)
charts/helm/capture/templates/configmap.yaml
[error] 4-4: syntax error: expected , but found ''
(syntax)
charts/helm/capture/templates/service.yaml
[error] 7-7: syntax error: could not find expected ':'
(syntax)
charts/helm/capture/templates/deployment.yaml
[error] 7-7: syntax error: could not find expected ':'
(syntax)
charts/helm/capture/templates/secret.yaml
[error] 4-4: syntax error: expected , but found ''
(syntax)
🔇 Additional comments (2)
charts/helm/capture/templates/_helpers.tpl (2)
1-3: LGTM!The
capture.namehelper follows the standard Helm pattern correctly, usingnameOverridewith proper 63-character truncation and hyphen trimming.
22-25: LGTM!The
capture.selectorLabelshelper correctly defines a stable, minimal subset of labels suitable for immutable selectors in Services and Deployments.
|
@mertssmnoglu Could you please review this PR? |
1 participant
This PR introduces a Helm chart for deploying Capture on Kubernetes. It provides templated resources (Deployment, Service, Secret, ConfigMap), default values, and installation instructions to make installs and upgrades repeatable.
Solves Issue #72
Changes: