Skip to content

Commit 9cbc534

Browse files
committed
fix(helm): derive metrics targetPort from bindAddress
The earlier commits exposed both `controller.metrics.bindAddress` and `controller.metrics.service.targetPort` as independent values, so a user could shift the runtime bind without moving the Service target and end up with a Service that points at a port the controller binary is not listening on. Drop the separate `service.targetPort` knob and derive both the container port and the Service targetPort from `bindAddress` via `regexFind`, matching the `kmcp` sub-chart's pattern. Add a test that asserts the derived ports stay in sync when `bindAddress` is customised, plus a regression test that documents the `controller.env` escape hatch (user-supplied env vars are emitted after chart defaults, so the user's value wins at runtime). The escape hatch still cannot move the Service target; values.yaml now flags this. Addresses review feedback on kagent-dev#1803. Signed-off-by: Daniel Orbach <ddorbach@gmail.com>
1 parent 698c8c8 commit 9cbc534

5 files changed

Lines changed: 30 additions & 8 deletions

File tree

helm/kagent/templates/controller-deployment.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ spec:
105105
protocol: TCP
106106
{{- if .Values.controller.metrics.enabled }}
107107
- name: metrics
108-
containerPort: {{ .Values.controller.metrics.service.targetPort }}
108+
containerPort: {{ .Values.controller.metrics.bindAddress | regexFind "[0-9]+" | int }}
109109
protocol: TCP
110110
{{- end }}
111111
resources:

helm/kagent/templates/controller-metrics-service.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ spec:
1111
ports:
1212
- name: {{ ternary "https" "http-metrics" .Values.controller.metrics.secureServing }}
1313
port: {{ .Values.controller.metrics.service.port }}
14-
targetPort: {{ .Values.controller.metrics.service.targetPort }}
14+
targetPort: {{ .Values.controller.metrics.bindAddress | regexFind "[0-9]+" | int }}
1515
protocol: TCP
1616
selector:
1717
{{- include "kagent.controller.selectorLabels" . | nindent 4 }}

helm/kagent/tests/controller-deployment_test.yaml

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -562,12 +562,11 @@ tests:
562562
name: METRICS_SECURE
563563
value: "false"
564564

565-
- it: should reflect custom bind address and target port
565+
- it: should derive metrics container port from bindAddress
566566
template: controller-deployment.yaml
567567
set:
568568
controller.metrics.enabled: true
569569
controller.metrics.bindAddress: ":9443"
570-
controller.metrics.service.targetPort: 9443
571570
asserts:
572571
- contains:
573572
path: spec.template.spec.containers[0].env
@@ -580,3 +579,22 @@ tests:
580579
name: metrics
581580
containerPort: 9443
582581
protocol: TCP
582+
583+
- it: should let controller.env override the chart-supplied metrics env
584+
template: controller-deployment.yaml
585+
set:
586+
controller.metrics.enabled: true
587+
controller.env:
588+
- name: METRICS_BIND_ADDRESS
589+
value: "0"
590+
asserts:
591+
- contains:
592+
path: spec.template.spec.containers[0].env
593+
content:
594+
name: METRICS_BIND_ADDRESS
595+
value: ":8443"
596+
- contains:
597+
path: spec.template.spec.containers[0].env
598+
content:
599+
name: METRICS_BIND_ADDRESS
600+
value: "0"

helm/kagent/tests/controller-metrics-service_test.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,11 @@ tests:
3939
path: spec.ports[0].protocol
4040
value: TCP
4141

42-
- it: should respect custom port and targetPort
42+
- it: should derive targetPort from bindAddress
4343
set:
4444
controller.metrics.enabled: true
45+
controller.metrics.bindAddress: ":9443"
4546
controller.metrics.service.port: 9443
46-
controller.metrics.service.targetPort: 9443
4747
asserts:
4848
- equal:
4949
path: spec.ports[0].port

helm/kagent/values.yaml

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,12 @@ controller:
230230
# -- Prometheus-style /metrics endpoint for the controller manager.
231231
# When enabled, provisions a dedicated metrics Service plus the ClusterRoles
232232
# required for authenticated scrapes. Bind `<fullname>-metrics-reader` to
233-
# your Prometheus ServiceAccount to grant scrape access.
233+
# your Prometheus ServiceAccount to grant scrape access. The Service
234+
# targetPort and pod containerPort are derived from `bindAddress` so the
235+
# two cannot drift; only the cluster-facing `service.port` is independently
236+
# configurable. Overriding `METRICS_BIND_ADDRESS` via `controller.env`
237+
# changes the runtime bind but not the Service — keep `service.port`
238+
# aligned manually if you take that escape hatch.
234239
# @default -- disabled
235240
metrics:
236241
enabled: false
@@ -239,7 +244,6 @@ controller:
239244
service:
240245
type: ClusterIP
241246
port: 8443
242-
targetPort: 8443
243247
env: []
244248
envFrom: []
245249

0 commit comments

Comments
 (0)