feat: Add new telegraf chart #101
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughAdds a new Telegraf Helm chart (Chart, values, Deployment, Service, Secret), updates shared-volume and ingress-nginx values to support Telegraf (PVC and UDP port 8094), and fixes a CI workflow dependency check to reference Chart.yaml instead of Chart.yml. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer / CI
participant K8s as Kubernetes API
participant PVC as PersistentVolume
participant Pod as telegraf Pod
participant Influx as InfluxDB
Dev->>K8s: Deploy Helm chart (Chart, values)
K8s->>PVC: Provision/Bind `telegraf-config` PVC
K8s->>Pod: Create Pod (initContainer + main)
Pod->>Influx: initContainer waits for InfluxDB:8086
Pod->>PVC: Mount shared volume (/home/telegraf/etc)
K8s->>Pod: Start telegraf container
K8s->>Pod: Service routes TCP/UDP (including UDP 8094) to Pod
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 4
🤖 Fix all issues with AI agents
In `@kubernetes/telegraf/templates/deployment.yaml`:
- Around line 88-93: The deployment template's ports block uses protocol: {{
.protocol | default "UDP" }} which conflicts with the service template default
of "TCP"; update one of them so both default to the same protocol or require
explicit protocol in values. Locate the ports loop in the deployment template
(the range over .Values.service.ports and the protocol field) and change the
default to match the service template's default ("TCP") or vice versa, or add
validation/documentation to force protocol entries in values.yaml so .protocol
is always provided.
- Around line 47-65: The initContainer telegraf-init creates UID 101 and chowns
shared volumes, but the main Telegraf container lacks a securityContext so it
may run as root; update the chart values to ensure the main container runs as
UID 101 by adding securityContext.runAsUser: 101 and securityContext.runAsGroup:
101 (or set podSecurityContext with fsGroup: 101) in the Telegraf deployment
values (values.yaml) so the primary container process uses the same UID/GID as
the chowned files.
In `@kubernetes/telegraf/templates/service.yaml`:
- Around line 26-29: The condition/usage mismatch in the Helm template: the if
checks .Values.global.externalIPs (plural) but the block uses
.Values.global.externalIP (singular); fix by choosing one convention and
updating the template accordingly — either change the condition to check
.Values.global.externalIP when you expect a single IP, or (preferably) treat
externalIPs as a list and output it consistently by referencing
.Values.global.externalIPs (or iterating over it) in the service.yaml template;
update the template block that currently references .Values.global.externalIP to
use the chosen plural variable (.Values.global.externalIPs) or iterate so the
rendered externalIPs field matches the values file.
In `@kubernetes/telegraf/values.yaml`:
- Around line 130-136: The values file currently commits real-looking InfluxDB
credentials (influxdb_user and influxdb_password) which must be removed; replace
them with obvious placeholders (e.g., CHANGE_ME) for influxdb_user and
influxdb_password, add a short inline YAML comment near config explaining these
must be changed for production, and update documentation/README to instruct
operators to override these via helm --set or a separate values file (or use
sealed/managed secrets) for secure production deployments; ensure the other
related keys (influxdb_host, influxdb_port, influxdb_database,
influxdb_skip_database_creation) remain intact but note in the comment that
credentials should come from secrets.
🧹 Nitpick comments (2)
kubernetes/telegraf/templates/service.yaml (1)
4-4: Consider usingcommon.names.fullnamefor consistency with the deployment.The service name uses
.Chart.Namedirectly, while the deployment (line 4) uses{{ include "common.names.fullname" . }}. This inconsistency could cause selector mismatches if release naming affects the fullname.♻️ Proposed fix
- name: {{ .Chart.Name }} + name: {{ include "common.names.fullname" . }}kubernetes/telegraf/templates/deployment.yaml (1)
142-143: Hardcoded imagePullSecrets reduces flexibility.The
ghcr-pull-secretis hardcoded. Consider making this configurable via values for environments using different registries or secret names.♻️ Proposed fix
imagePullSecrets: - - name: ghcr-pull-secret + {{- if .Values.imagePullSecrets }} + {{- toYaml .Values.imagePullSecrets | nindent 6 }} + {{- else }} + - name: ghcr-pull-secret + {{- end }}
duchieu2k
force-pushed
the
feature/telegraf
branch
from
e9ec282 to
6ac4e98
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@kubernetes/telegraf/templates/service.yaml`:
- Around line 4-5: The service metadata uses .Chart.Name which ignores Helm
release/nameOverride/fullnameOverride and should use the common naming helper;
replace the hardcoded name .Chart.Name in the service template with the helper
include "common.names.fullname" (same helper used by deployment.yaml and
secret.yaml) so the service honors overrides and avoids name collisions.
Comment on lines
+4
to
+5
| name: {{ .Chart.Name }} | ||
| labels: {{- include "common.labels.standard" . | nindent 4 }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Step 1: Find and examine the common chart helpers, especially the fullname helper
echo "=== Searching for common.names.fullname definition ==="
rg -n 'define "common.names.fullname"' -g '*tpl' -g '*common*'
echo -e "\n=== Searching for common.names.fullname and common.names.name usage in telegraf ==="
rg -n 'common\.names\.fullname|common\.names\.name' kubernetes/telegraf -g '*.yaml' -g '*.tpl'
echo -e "\n=== Checking .Chart.Name usage in telegraf templates ==="
rg -n '\.Chart\.Name' kubernetes/telegraf/templates -g '*.yaml'
echo -e "\n=== Examining the service.yaml file context ==="
cat -n kubernetes/telegraf/templates/service.yaml | head -30Repository: svtechnmaa/charts
Length of output: 3268
🏁 Script executed:
#!/bin/bash
# Additional verification: Examine what the common.names.fullname helper does
echo "=== Examining common.names.fullname helper implementation ==="
cat -n kubernetes/common/templates/_names.tpl | sed -n '21,35p'
echo -e "\n=== Check context: What does metadata.name use in deployment and secret? ==="
echo "--- deployment.yaml lines 1-10 ---"
cat -n kubernetes/telegraf/templates/deployment.yaml | head -10
echo -e "\n--- secret.yaml lines 1-10 ---"
cat -n kubernetes/telegraf/templates/secret.yaml | head -10Repository: svtechnmaa/charts
Length of output: 1745
Use common name helper to honor release/nameOverride/fullnameOverride.
.Chart.Name ignores Helm's naming overrides and breaks consistency with other resources in the chart. The deployment.yaml and secret.yaml both use the proper common.names.fullname helper for their metadata names, while this service uses .Chart.Name directly. This inconsistency can cause collisions when multiple releases are installed in the same namespace and prevents users from overriding the service name.
🐛 Proposed fix
metadata:
- name: {{ .Chart.Name }}
+ name: {{ include "common.names.fullname" . }}📝 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: {{ .Chart.Name }} | |
| labels: {{- include "common.labels.standard" . | nindent 4 }} | |
| name: {{ include "common.names.fullname" . }} | |
| labels: {{- include "common.labels.standard" . | nindent 4 }} |
🧰 Tools
🪛 YAMLlint (1.38.0)
[error] 5-5: syntax error: expected the node content, but found '-'
(syntax)
🤖 Prompt for AI Agents
In `@kubernetes/telegraf/templates/service.yaml` around lines 4 - 5, The service
metadata uses .Chart.Name which ignores Helm
release/nameOverride/fullnameOverride and should use the common naming helper;
replace the hardcoded name .Chart.Name in the service template with the helper
include "common.names.fullname" (same helper used by deployment.yaml and
secret.yaml) so the service honors overrides and avoids name collisions.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.