Conversation
Add a docker integration test for the cAdvisor exporter. The test scrapes a single prometheus.exporter.cadvisor instance into Mimir and asserts that cAdvisor metrics appear. cAdvisor uses its raw cgroup driver, so the test needs a privileged container but no container runtime. This POC includes a temporary discovery test. It lists every cAdvisor metric that reaches Mimir so the assertion list can be pinned from a real Linux run. The discovery test fails on purpose to surface its output. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the temporary discovery test with the real assertion list, pinned from a CI run. The list covers every cAdvisor collector family: build and version, cpu, memory, filesystem, network, and blkio. Leave out container_pressure_* (needs kernel CONFIG_PSI) and container_health_state (needs a Docker HEALTHCHECK on the target container), since neither is guaranteed on every host. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dehaansa
marked this pull request as ready for review
September 9, 2026 16:07
dehaansa
changed the title
dehaansa
changed the title
dehaansa
enabled auto-merge (squash)
September 9, 2026 16:14
kgeckhart
reviewed
Sep 9, 2026
Comment on lines
+1
to
+3
| // No container runtime sockets are configured. cAdvisor uses its raw cgroup | ||
| // driver, which reads /sys/fs/cgroup. This needs a privileged container but no | ||
| // docker or containerd runtime. See test.yaml. |
Contributor
There was a problem hiding this comment.
Maybe I missed it but I don't see anything here about raw cgroup drivers, probably best to nix this one and keep the other.
If it was AI, I had to add a rule which forces AI to only put why comments next to the piece which needs the "why" explained.
Contributor
Author
There was a problem hiding this comment.
Ope yeah, AI, I'll re-review that 😞
Comment on lines
+13
to
+14
| // cAdvisor only runs on Linux. The test exercises the Grafana cAdvisor fork's | ||
| // collectors, so it must run against a real Linux cgroup tree. |
Contributor
There was a problem hiding this comment.
Is this truly a limitation of our fork or is it true of cadvisor in general?
Contributor
Author
There was a problem hiding this comment.
Not a limitation of our fork, but I see how the comment reads that way. Will rewrite.
…afana/alloy into poc/cadvisor-integration-test
… blocked pod start A read-only host /var/run mount stops Kubernetes from mounting the service account token, so the Alloy pod failed with RunContainerError. The raw cgroup driver does not need /var/run. Only the docker and containerd plugins use it, and this test does not use them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
thampiotr
reviewed
Sep 10, 2026
Comment on lines
+1
to
+6
| # cAdvisor reads the node's cgroup tree and machine stats. The Alloy pod runs | ||
| # privileged and mounts the host paths read-only. This follows the canonical | ||
| # cAdvisor DaemonSet, but does not mount /var/run. A read-only /var/run blocks | ||
| # Kubernetes from mounting the service account token and the pod fails to start. | ||
| # The raw driver does not need /var/run; only the docker and containerd plugins | ||
| # use it, and this test does not use them. |
Contributor
There was a problem hiding this comment.
Suggested change
| # cAdvisor reads the node's cgroup tree and machine stats. The Alloy pod runs | |
| # privileged and mounts the host paths read-only. This follows the canonical | |
| # cAdvisor DaemonSet, but does not mount /var/run. A read-only /var/run blocks | |
| # Kubernetes from mounting the service account token and the pod fails to start. | |
| # The raw driver does not need /var/run; only the docker and containerd plugins | |
| # use it, and this test does not use them. | |
| # This follows the canonical | |
| # cAdvisor DaemonSet, but does not mount /var/run. A read-only /var/run blocks | |
| # Kubernetes from mounting the service account token and the pod fails to start. | |
| # The raw driver does not need /var/run; only the docker and containerd plugins | |
| # use it, and this test does not use them. |
Nit: the first two sentences are obvious in code. The rest is useful, but maybe should be closer to the mount line?
| # The raw cgroup driver needs a privileged container to read /sys/fs/cgroup. | ||
| securityContext: | ||
| privileged: true | ||
| mounts: |
Contributor
There was a problem hiding this comment.
I did some digging with an agent and I think that the current test returns metrics without container labels. We could also add container labels by adding the mount config:
controller:
volumes:
extra:
- name: containerd-sock
hostPath:
path: /run/containerd/containerd.sock
type: Socket
alloy:
mounts:
extra:
- name: containerd-sock
mountPath: /run/containerd/containerd.sock
readOnly: true
And we would also need to add some extra assertion methods to verify labels exist (or match a value) on metrics, not just that metrics exist.
Here's a little table of what I think we can expect to see:
| label | current PR | with socket mounted |
|---|---|---|
name |
(empty) | container IDs |
image |
(empty) | docker.io/grafana/alloy:latest, docker.io/grafana/mimir:2.17.9, docker.io/kindest/kindnetd:v20240813-c6f155d6, … |
container_label_io_kubernetes_pod_name |
(empty) | coredns-6f6b679f8f-pj72q, etcd-…-control-plane, kindnet-hhs62, … |
container_label_io_kubernetes_pod_namespace |
(empty) | kube-system, local-path-storage, test-prometheus-exporter-cadvisor |
container_label_io_kubernetes_container_name |
(empty) | alloy, coredns, etcd, kube-apiserver, … (12) |
This was referenced Sep 10, 2026
…iner labels Strengthen the k8s cAdvisor test beyond metric presence: - Assert container_fs_usage_bytes and container_fs_limit_bytes report values greater than zero, via a new Mimir.QueryPositive helper. - Mount the node containerd socket so the containerd plugin resolves pod metadata, and assert container_label_io_kubernetes_pod_name is attached, via a new Mimir.QueryLabelPresent helper. Docker-runtime-specific behaviours (container_health_state, application metrics) are not covered here: kind uses containerd, not Docker, so they cannot be exercised in this harness. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nerd socket read-only Address review feedback: drop the comment sentences that restate the config, keeping only the non-obvious containerd-socket and /var/run rationale, and mount the containerd socket read-only (cAdvisor only connects to it). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Assert the pod namespace and container name labels alongside the pod name, per the reviewer's suggested set, to more fully cover the containerd metadata path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… query Make the label helper variadic (QueryLabelsPresent) and combine the label matchers into a single series query, so the three io.kubernetes.* labels are verified in one check that also requires them on the same series. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Brief description of Pull Request
Adds a k8s integration test for
prometheus.exporter.cadvisorunderintegration-tests/k8s/tests/prometheus-exporter-cadvisor. The test scrapes a single cAdvisor exporter into Mimir and asserts that cAdvisor metrics appear. cAdvisor uses its raw cgroup driver, so the test needs a privileged container but no container runtime.Pull Request Details
Purpose: add end-to-end coverage for the cAdvisor exporter so future bumps of the
grafana/cadvisorfork are guarded against regressions in the exporter's collectors.PR Checklist