Skip to content

fix: use otel selector labels instead of regular labels for proper trace wiring #40

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.

Sign up for GitHub

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

Merged
adityachoudhari26 merged 1 commit into from
May 4, 2026
Merged

fix: use otel selector labels instead of regular labels for proper trace wiring #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions charts/ctrlplane/Chart.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ dependencies:
version: 0.1.6
- name: otel
repository: file://charts/otel
version: 0.2.0
version: 0.2.1
- name: wandb-base
repository: https://charts.wandb.ai
version: 0.11.11
Expand All @@ -14,5 +14,5 @@ dependencies:
- name: wandb-base
repository: https://charts.wandb.ai
version: 0.11.11
digest: sha256:11be9c366718847a7f76355f54f3816cd7223e2d9feb63a28d90e6397ad0cc50
generated: "2026-04-13T16:19:18.331308-05:00"
digest: sha256:03b431cdd1b20c20d969b0eacb90e33dec92599d877dde63f801f2fa926b8d86
generated: "2026-05-04T13:22:49.464936-04:00"
2 changes: 1 addition & 1 deletion charts/ctrlplane/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ apiVersion: v2
name: ctrlplane
description: Ctrlplane Helm chart for Kubernetes
type: application
version: 1.1.1
version: 1.1.2
appVersion: 1.0.0

maintainers:
Expand Down
2 changes: 1 addition & 1 deletion charts/ctrlplane/charts/otel/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ name: otel
type: application
description: A Helm chart for Kubernetes

version: 0.2.0
version: 0.2.1
appVersion: "0.109.0"
3 changes: 1 addition & 2 deletions charts/ctrlplane/charts/otel/templates/daemonset.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ metadata:
spec:
selector:
matchLabels:
{{- include "ctrlplane.selectorLabels" $ | nindent 6 }}
{{- include "otel.labels" . | nindent 6 }}
{{- include "otel.selectorLabels" . | nindent 6 }}
Comment on lines 16 to +18
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 4, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Breaking upgrade: spec.selector.matchLabels is immutable on DaemonSet.

spec.selector is an immutable field on Kubernetes DaemonSets (and Deployments) after initial creation. A helm upgrade from the previous chart version will fail with:

The DaemonSet "<name>" is invalid: spec.selector: Invalid value: ... field is immutable

This affects any cluster currently running the prior chart version. Required pre-upgrade action (one of):

  • helm upgrade --force — triggers delete + re-create (causes brief downtime/pod disruption).
  • Manual deletion — kubectl delete daemonset <name> --cascade=orphan before running helm upgrade.

This should be called out in NOTES.txt, a CHANGELOG, or upgrade documentation to prevent operators from being caught off-guard.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@charts/ctrlplane/charts/otel/templates/daemonset.yaml` around lines 16 - 18,
The DaemonSet's spec.selector.matchLabels has been changed (the selector is
rendered via the template include "otel.selectorLabels"), but spec.selector is
immutable after creation which will cause helm upgrade failures; update release
notes/NOTES.txt and CHANGELOG to clearly document this breaking change and
required pre-upgrade steps (e.g., run helm upgrade --force or kubectl delete
daemonset <name> --cascade=orphan) and include the exact error message example;
additionally, avoid changing the selector in future releases by keeping the
include "otel.selectorLabels" output stable or bumping chart major version if
selector must change.

template:
{{ include "otel.podTemplate" . }}
{{- end }}
3 changes: 1 addition & 2 deletions charts/ctrlplane/charts/otel/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ spec:
{{- end }}
selector:
matchLabels:
{{- include "ctrlplane.selectorLabels" $ | nindent 6 }}
{{- include "otel.labels" . | nindent 6 }}
{{- include "otel.selectorLabels" . | nindent 6 }}
Comment on lines 23 to +25
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 4, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Same immutable selector breaking change applies to Deployment.

spec.selector.matchLabels is immutable on Deployments post-creation. Clusters using workload.kind: Deployment will experience the same upgrade failure described for the DaemonSet. The same pre-upgrade mitigation applies (--force or manual deletion), and both cases should be covered in upgrade notes.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@charts/ctrlplane/charts/otel/templates/deployment.yaml` around lines 23 - 25,
Deployment spec.selector.matchLabels is immutable and changing the labels
generated by the otel.selectorLabels template will break upgrades; ensure the
chart does not alter existing selector labels for resources of kind Deployment
(keep the output of include "otel.selectorLabels" unchanged between releases)
and, if a selector change is unavoidable, add explicit upgrade notes documenting
the required pre-upgrade mitigation (use --force or delete the Deployment
manually) so operators know to perform the manual migration before upgrading.

template:
{{ include "otel.podTemplate" . }}
{{- end }}
2 changes: 1 addition & 1 deletion charts/ctrlplane/charts/otel/templates/service.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,4 @@ spec:
protocol: TCP
name: otlp-http
selector:
{{- include "otel.labels" . | nindent 4 }}
{{- include "otel.selectorLabels" . | nindent 4 }}
54 changes: 54 additions & 0 deletions charts/ctrlplane/tests/otel_selector_test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
suite: otel selectors are stable across chart versions
templates:
- charts/otel/templates/daemonset.yaml
- charts/otel/templates/deployment.yaml
- charts/otel/templates/service.yaml
- charts/otel/templates/configmap.yaml
release:
name: ctrlplane

tests:
- it: daemonset selector contains only stable identity labels
template: charts/otel/templates/daemonset.yaml
asserts:
- equal:
path: spec.selector.matchLabels
value:
app.kubernetes.io/name: otel
app.kubernetes.io/instance: ctrlplane

- it: daemonset pod labels still include chart-version labels
template: charts/otel/templates/daemonset.yaml
asserts:
- equal:
path: spec.template.metadata.labels["app.kubernetes.io/name"]
value: otel
- equal:
path: spec.template.metadata.labels["app.kubernetes.io/instance"]
value: ctrlplane
- exists:
path: spec.template.metadata.labels["helm.sh/chart"]
- exists:
path: spec.template.metadata.labels["ctrlplane.com/app-name"]

- it: deployment selector contains only stable identity labels
template: charts/otel/templates/deployment.yaml
set:
otel:
workload:
kind: Deployment
asserts:
- equal:
path: spec.selector.matchLabels
value:
app.kubernetes.io/name: otel
app.kubernetes.io/instance: ctrlplane

- it: service selector contains only stable identity labels
template: charts/otel/templates/service.yaml
asserts:
- equal:
path: spec.selector
value:
app.kubernetes.io/name: otel
app.kubernetes.io/instance: ctrlplane
Loading