-
Notifications
You must be signed in to change notification settings - Fork 0
chore: change otel deployment to daemonset #24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughConverted the OpenTelemetry Helm chart to use a Kubernetes DaemonSet instead of a Deployment; updated Helm value references from Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer (Helm)
participant Helm as Helm chart
participant K8s as Kubernetes API
rect rgba(115,179,255,0.08)
Dev->>Helm: render templates with .Values.daemonset.*
Helm->>K8s: apply DaemonSet manifest
K8s->>Nodes: schedule one pod per node (DaemonSet)
end
note right of K8s: replicas field removed — scheduling handled by DaemonSet controller
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🔇 Additional comments (1)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
charts/ctrlplane/charts/otel/templates/daemonset.yaml (1)
7-12: Standardize whitespace in template conditionals.Line 12 has inconsistent spacing after
{{-compared to line 7-8. Helm templates typically omit extra spaces here for consistency.Apply this diff to align formatting:
- {{- toYaml .Values.daemonset.annotations | nindent 4 }} + {{- toYaml .Values.daemonset.annotations | nindent 4 }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
charts/ctrlplane/charts/otel/templates/daemonset.yaml(1 hunks)charts/ctrlplane/charts/otel/values.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/ctrlplane/charts/otel/templates/daemonset.yaml
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
[warning] 12-12: too many spaces after hyphen
(hyphens)
🔇 Additional comments (4)
charts/ctrlplane/charts/otel/templates/daemonset.yaml (3)
1-19: Verify YAMLlint syntax error doesn't cause deployment failures.Static analysis flagged an error on line 6: "syntax error: expected the node content, but found '-'". This appears to be a false positive from YAMLlint not recognizing Helm template syntax, but please verify the template renders correctly when deployed.
Can you confirm the Helm chart renders without errors using
helm templateorhelm linton this file?
32-35: Verify tolerations configuration for DaemonSet scheduling.The template conditionally applies tolerations from
.tolerations(which is empty in values.yaml:tolerations: []). For a DaemonSet to run on all nodes, you may need default tolerations for nodes with taints. Confirm that either:
- Default taints don't exist in the target cluster, or
- Tolerations will be provided at deploy-time via
--set tolerations[0]=...
1-82: Migration to DaemonSet looks correct.The conversion from Deployment to DaemonSet is properly implemented:
kindcorrectly set to DaemonSet (line 2)replicasfield appropriately removed- All
.Values.deployment.*references migrated to.Values.daemonset.*(lines 7-8, 11-12)- Pod spec structure remains compatible
charts/ctrlplane/charts/otel/values.yaml (1)
53-55: DaemonSet values configuration is correct.The top-level key rename from
deploymenttodaemonsetproperly aligns with the template updates. The structure preserves labels and annotations for customization, which is appropriate.
Summary by CodeRabbit