[marcleblanc2]

Open issues

Found in review on 2026-11-13 (review thread), at 77776fc. The change works as described; this PR is a draft until the changelog covers the first two.

1. The changelog does not warn the two groups of users who lose all their targets on upgrade. "On upgrade" below does, but the changelog is what ships
   • prometheus.existingConfig set and privileged unset: reproduced on EKS with the repo's k3s example config. After the upgrade prometheus had 0 Kubernetes targets, its log had no forbidden lines, and helm upgrade printed nothing. Adding prometheus.privileged: true brought the targets back
   • prometheus.createRoleBinding: false and privileged unset, with the chart's ClusterRole bound by hand: the upgrade deletes the ClusterRole. Seen in helm template only
   • Fix: say in the changelog entry that both need prometheus.privileged: true before upgrading. The BREAKING label was removed on purpose in ddfa59a, because prometheus.existingConfig is rarely used, and stays off
2. The kubernetes-services removal has no changelog entry here, and the k3s example still contains the job. Only fix/resources: Drop unused ports, Services, Role verbs, and settings #1016 has both. Fix: merge fix/resources: Drop unused ports, Services, Role verbs, and settings #1016 first, or copy its changelog line and example change
3. Small ones:
   • the restart test passes for any 64-character hash, so it does not show the hash follows the config
   • the comment in examples/openshift/override.yaml still describes the ClusterRole without nodes/proxy
   • merged with Render sourcegraph.labels on every object, and add storage and ServiceAccount labels #1008, two unit tests fail without a textual conflict, as "Linked PRs" says

Tested in this review on another EKS test instance, by upgrading from main to this branch merged onto main, both with chart-default prometheus.privileged:

• The ClusterRole and ClusterRoleBinding were replaced by one RoleBinding to view, and the prometheus pod was replaced once
• kubernetes-apiservers and kubernetes-nodes went away, the service endpoint targets stayed at 23, and there were 0 discovery failures
• kubectl auth can-i as the ServiceAccount matched "Test results" below, and also says no to get nodes/proxy
• A second upgrade with no change did not restart the pod

Problems

Our current default Prometheus config:

1. Scrapes too many unneeded metrics, which consume additional memory and disk

• On a test instance with 2 API servers and 9 nodes, the kubernetes-apiservers and kubernetes-nodes jobs were 74% of all series (157,910 of 212,150)
• Only 2 of the 2,368 expressions in that instance's dashboards and rules read them: two Prometheus dashboard panels that list what is scraped. No alert reads them
• None of the 545 metric names that only these two jobs produced there appears in sourcegraph/sourcegraph (0936a4f)

2. Creates unnecessarily large RBAC resources:

• A ClusterRole with get/list/watch on nodes, nodes/proxy, nodes/metrics, pods, services, endpoints, and namespaces, cluster-wide
  • get on nodes/proxy also authorizes running commands in any container on the node

3. prometheus.privileged: false was broken for pod targets:

• The kubernetes-pods job lists pods in all namespaces, which the RoleBinding to view doesn't allow, so cadvisor was never scraped

4. Sounds scary: Why would Prometheus ever need to run in privileged mode? It doesn't, that's not what this config speaks to, just reusing the name, causing confusion.

Solution

• prometheus.privileged defaults to false: no cluster-scoped RBAC resource, and no kubernetes-apiservers or kubernetes-nodes job
• The kubernetes-pods job discovers pods only in the release namespace, with privileged: true too
• The kubernetes-services job is removed, as in fix/resources: Drop unused ports, Services, Role verbs, and settings #1016: it probes through a blackbox exporter the chart doesn't ship
• prometheus restarts when an upgrade changes the chart's prometheus.yml, as in Render sourcegraph.labels on every object, and add storage and ServiceAccount labels #1008, instead of keeping the old config under the new RBAC
• The prometheus.privileged install warning is removed, and the docs now say what privileged and createRoleBinding do, and that executor metrics don't need privileged: true

RBAC resources rendered for prometheus:

Values	Before	After
default	ClusterRole, ClusterRoleBinding	RoleBinding → ClusterRole view
privileged: true	ClusterRole, ClusterRoleBinding	unchanged
privileged: false	RoleBinding → view	unchanged
createRoleBinding: false	ClusterRole (unbound)	none
privileged: true, createRoleBinding: false	ClusterRole (unbound)	unchanged

On upgrade:

• Set prometheus.privileged: true to keep the ClusterRole, the ClusterRoleBinding, and the two scrape jobs. Sourcegraph's dashboards and alerts don't need it; your own do if they use kubelet or API server metrics, such as kubelet_volume_stats_available_bytes
• Pods in other namespaces annotated sourcegraph.prometheus/scrape: "true" are no longer scraped, with privileged: true too. The chart has no value that adds namespaces; scraping them takes your own prometheus.yml, through prometheus.existingConfig
  • The chart's own cadvisor and node-exporter run in the release namespace and are still scraped
• A prometheus.existingConfig that discovers targets outside the release namespace, or scrapes nodes or API servers, needs prometheus.privileged: true

Test results

• helm lint and helm unittest pass (267 tests), and helm template on main and on this branch renders the table above

On an EKS test instance, main (643f7c0) and this branch (77776fc) were installed in turn with helm upgrade, with the same values and images, cadvisor and jaeger enabled, and one annotated pod in a second namespace.

Targets found by a new prometheus pod:

Chart	Values	API servers	Nodes	cadvisor	jaeger	Pod in second namespace	Service endpoints	Discovery
main	default	2	9	9	1	1	21	ok
main	privileged: false	no job	no job	0	0	0	21	403
This PR	default	no job	no job	9	1	0	21	ok
This PR	privileged: false	no job	no job	9	1	0	21	ok
This PR	privileged: true	2	9	9	1	0	21	ok
This PR	privileged: true, createRoleBinding: false	0	0	0	0	0	0	403
This PR	createRoleBinding: false	no job	no job	0	0	0	0	403

• The cluster had 9 nodes, and cadvisor runs one pod per node
• 403: the Kubernetes API refused the list requests of every job that shows 0 in that row
• Every target found was up, except jaeger on both charts: its annotated port 16686 returns 404 for /metrics, and fix/resources: Drop unused ports, Services, Role verbs, and settings #1016 removes the annotation
• In each row, the cluster had the RBAC resources the table above lists
• In the two createRoleBinding: false rows, binding the ServiceAccount by hand brought the targets back without a restart: a ClusterRoleBinding to the chart's ClusterRole gave the privileged: true row's targets, and a RoleBinding to view gave the default row's
  • A new pod under a hand-made Role with get, list, and watch on pods, services, and endpoints also found the default row's targets
• kubectl auth can-i as the ServiceAccount, with this PR's default:
  • Yes in the release namespace: list pods, list services, watch endpoints, and, through view, get pods/log and list configmaps
  • No: secrets, pods in the second namespace, pods, services, and endpoints in all namespaces, nodes, nodes/proxy, nodes/metrics, namespaces, and /metrics
  • With privileged: true it answers as it does on main's default: yes to each of those cluster-wide requests, and no to secrets, pods/log, and list configmaps

Upgrading from main to this PR, both with default values:

• helm diff listed the prometheus ConfigMap and Deployment as changed, the ClusterRole and ClusterRoleBinding as removed, the RoleBinding as added, and nothing else
• 1 of 26 pods was replaced, prometheus, and it loaded the new config 24 seconds after helm upgrade started
• All 589 rules evaluated without error before and after, with the same 3 alerts firing and 2 pending
• Series went from 212,150 to 53,648, each counted 16 minutes after prometheus started
  • kubernetes-apiservers (134,489) and kubernetes-nodes (23,421) were removed
  • The 13 Sourcegraph service jobs, builtin-alertmanager, and the 383 series without a job label kept the same set of series
  • builtin-prometheus lost 90 series, about the removed jobs and their discovery, and gained 46, about API requests the test made
  • kubernetes-pods lost the 528 series of the pod in the second namespace
  • Its other differences (241 lost, 221 gained) are cadvisor series about things that changed on the nodes during the test: the replaced prometheus pod, host network interfaces that came and went (22 of the 23 new ones belong to pods another release started during the test), and 4 filesystem series for one disk, whose cause was not proven
• Of the 2,368 expressions in the instance's dashboards and rules, 1,302 returned data on main. On this PR none gained gaps, and 21 returned a different set of series:
  • 2 Prometheus dashboard panels read the removed jobs: "Samples scraped by job" and "Metrics with highest cardinalities"
  • 4 Prometheus dashboard panels lost the pod in the second namespace, which was itself a Prometheus
  • 14 Zoekt panels gained the gRPC method List, which Zoekt first reported at the end of the main measurement
  • 1 topk panel also differed between two prometheus pods with the same main config
• Re-running the 2,368 expressions on main's data without the two removed jobs changed those 2 panels and no others

Other upgrades:

• On this PR, setting privileged: false gave an empty helm diff and kept the pod; setting privileged: true replaced it
• Adding createRoleBinding: false to privileged: true removed only the ClusterRoleBinding and kept the pod. Its API server and node scrapes then returned 403, and its cadvisor and service endpoint targets stayed up
• From main with privileged: false to this PR with privileged: false, the pod was replaced, and cadvisor went from 0 to 9 targets
• On main, changing privileged changed the ConfigMap and the RBAC resources but not the Deployment, so prometheus kept running the old config

Also seen on main: at one moment before these measurements, its default config was scraping the 10 cadvisor pods of a second Sourcegraph release in the same cluster, 17,318 more series. Leaving those series out changed 17 expressions; for example, the gitserver CPU panel read 2.0 times as high with them. This PR found no pod outside the release namespace in any row.

Executor metrics, checked on two other test instances, where prometheus has only the RoleBinding to view and its kubernetes-service-endpoints job lists only the release namespace:

• Each has a Kubernetes-native executor pod with no scrape annotation and nothing listening on its declared port 6060, and prometheus has no target at its address
• Their metrics arrive through the worker-executors Service in the release namespace, and so do those of an executor on a machine outside the cluster

Not tested

• An executor pod in another namespace of the cluster, and the dind executor chart. By the source, both executor charts run the same executor, which sends its metrics with each heartbeat
• The chart's node-exporter, which was disabled on the test instance
• OpenShift
• The local cluster and the frontend health check of the manual testing process; its other steps were run, with EKS as the cluster
• main with createRoleBinding: false
• How Grafana draws the panels: the comparison ran each panel's and rule's query against Prometheus
• prometheus.existingConfig, in this test; "Open issues" has the review's result

Linked PRs

• fix/resources: Drop unused ports, Services, Role verbs, and settings #1016 merges cleanly with this PR
• Render sourcegraph.labels on every object, and add storage and ServiceAccount labels #1008 does too, but whichever merges second must:
  • Set prometheus.privileged: true in the two tests that expect the prometheus ClusterRole and ClusterRoleBinding in the default render, in allObjectLabels_test.yaml and objectLabels_test.yaml
  • Remove the duplicate changelog entry for the restart

Amp threads

• Design, implementation, and the first EKS test
• Review, fixes, and the EKS test of each combination

Changelog

• prometheus.privileged now defaults to false
• The kubernetes-pods scrape job now discovers pods only in the release namespace
• The prometheus pod now restarts when an upgrade changes the chart's prometheus.yml

[github-actions]

Open the Diff Tour for this PR in Sourcegraph