Repository navigation
test(otel/pernode): per-node, CRD-bundling and scraper-routing E2E suites - #773
Aakash-Dantre wants to merge 1 commit into
Conversation
6a4a17f to
41fb73b
Compare
| } | ||
|
|
||
| // Core per-node invariant: scraped by the agent on the pod's own node. | ||
| if targetNode != agentNode { |
There was a problem hiding this comment.
I don't think this can detect cross-node scraping. The Prometheus receiver sets resource k8s.node.name from the scraped target's discovery labels (__meta_kubernetes_pod_node_name; see addKubernetesResource in prometheusreceiver/internal/prom_to_otlp.go), and nothing in the prometheuscr pipeline overwrites it. target_node comes from the same label via the relabel in workload.yaml, so the two are always equal, wherever the series was scraped. The comment on agentNodeResourceKey in metrics_test.go (that it comes from ${env:K8S_NODE_NAME}) doesn't match the pipeline either.
Could we assert allocation at the TA instead? /jobs/<job>/targets?collector_id=<agent pod> returns each collector's targets. Comparing each target's __meta_kubernetes_pod_node_name with that agent pod's spec.nodeName from the K8s API tests the allocator directly, and the /jobs probe pod you already have can do it.
There was a problem hiding this comment.
You're right, thanks. In a432c10 it checks allocation at the TA as you suggested: each target's pod node against the assigned agent pod's spec.nodeName. Verified on a live cluster: passes with per-node, fails when switched to consistent-hashing.
| done | ||
| kubectl -n amazon-cloudwatch patch AmazonCloudWatchAgent cloudwatch-agent --type='json' \ | ||
| -p='[{"op": "replace", "path": "/spec/image", "value": "${var.cwagent_image_repo}:${var.cwagent_image_tag}"}]' | ||
| kubectl -n amazon-cloudwatch patch AmazonCloudWatchAgent cloudwatch-agent --type=merge \ |
There was a problem hiding this comment.
The harness forces allocationStrategy: per-node and the TA image by patching the CR after install, so the suite would still pass if the chart's own per-node wiring regressed. aws-observability/helm-charts#376 already renders per-node by default, and the TA image is a chart value (agent.prometheus.targetAllocator.image.repositoryDomainMap.public / .repository / .tag). The comment at line 196 saying it isn't one is out of date. Could the harness set these through helm_release set entries and drop the CR patch, so the E2E covers the chart as shipped?
There was a problem hiding this comment.
Done in a432c10: images and prometheusScrape are set through helm_release values, no CR patch. I'll run the Terraform harness end to end before asking for re-review.
17d6403 to
a432c10
Compare
…E suites
Adds the EKS end-to-end coverage for the Target Allocator work: per-node
allocation (asserted at the Target Allocator: every assigned target is on its
agent's node), zero-step CRD bundling, Target Allocator health and monitor
discovery with the bundled CRDs, and annotation-based routing of monitors
between the per-node and cluster-scraper agents.
test/otel/pernode/ per_node, crd_bundling, ta_resilience,
scraper_routing, metrics + k8s helpers
terraform/eks/daemon/otel-pernode/ EKS harness for the suite
Supersedes the earlier per-node-only suite: this branch contains all of it
plus the routing coverage.
The suites are tagged `integration` and are not yet registered in the
generated test matrix, so they do not run in CI until a matrix entry is
added.
The routing /jobs probe presents the agents' Target Allocator client
certificate, because the Target Allocator's HTTPS server requires and
verifies one.
The harness configures the operator and Target Allocator images and
prometheusScrape through chart values rather than patching the agent CR after
install, so the suite exercises the chart's own wiring.
a432c10 to
f18f0d3
Compare
2 participants
Summary
End-to-end coverage for the Target Allocator ServiceMonitor/PodMonitor work: per-node allocation, zero-step CRD bundling, Target Allocator resilience when the SM/PM CRDs are absent, and annotation-based routing of monitors between the per-node and cluster-scraper agents.
Supersedes #720 and #724, combined and re-applied onto current
main. #724 as it stands was committed from an older tree, so it also deletes 77 files that exist onmain(including the Azure VM/AKS suites andtest/e2e/containerinsights) and rolls back later changes to others. This PR carries only the new files and is purely additive: 14 files, +1685/-0.What
test/otel/pernode/per_node,crd_bundling,ta_resilience,scraper_routing,metricstests + k8s helpers and workload manifeststerraform/eks/daemon/otel-pernode/prometheusScrapeare set through chart values, so the suite exercises the chart as shipped (no post-install CR patch).All suites carry the
integrationbuild tag.TestPerNodeAllocation,TestPerNodeCoverageAcrossNodesTestServiceMonitorPodMonitorCRDsBundledTestTargetAllocatorHealthyOnBundledInstall,TestTargetAllocatorDiscoversMonitorsTestScraperRoleWiringscraperRole=cluster-scraper; the per-node agent carries the default roleTestClusterScraperTargetAllocatorHealthyTestAnnotationRoutingPartition/jobs, the annotated monitor is owned only by the cluster-scraper TA and the unannotated one only by the per-node TARunning
Against a cluster with the operator and chart changes below deployed:
Not yet wired into CI
The suites are not registered in
generator/or any workflow, so they do not run in CI as submitted. An earlier revision editedgenerator/resources/ec2_linux_test_matrix.jsonandgenerator/test_case_generator.go, but those edits were reverts of latermainchanges rather than a new matrix entry, so they are not carried here. A matrix entry for an EKS target is the remaining follow-up.Dependencies
Exercises these, which must be deployed first:
PR Checklist
makepasses locally - this repo has nomaketarget for test packages.go vet -tags integration ./test/otel/pernode/...is clean on this commit. Repo-widego vet ./...fails identically onmain(unkeyed struct literals intest/metric_dimension); that is not introduced here.TestServiceMonitorPodMonitorCRDsBundleddoes not apply to that cluster, because kube-prometheus-stack already owns the SM/PM CRDs and the chart correctly leaves them alone. Switching the per-node TA toconsistent-hashingmakesTestPerNodeAllocationfail on a cross-node assignment, so the test detects the case it is meant to. Not yet run through the Terraform harness or CI (not in the test matrix, see above).