[amazon-cloudwatch-agent-operator] feat(target-allocator): add per-node allocation strategy by wenegiemepraise · Pull Request #398 · aws/amazon-cloudwatch-agent-operator

wenegiemepraise · 2026-07-09T11:07:33Z

Summary

Add a per-node allocation strategy to the Target Allocator so each CloudWatch Agent
(DaemonSet, one per node) scrapes only the ServiceMonitor/PodMonitor targets on its own
node — eliminating cross-node / cross-AZ scrape traffic. Targets without a resolvable node
fall back to consistent-hashing, so nothing is silently dropped.

Motivation

The fork registers only consistent-hashing, a pure hash of the target URL with no node
awareness — a pod on node A can be scraped by the agent on node B (inter-AZ data-transfer
cost + latency). This ports the upstream OpenTelemetry per-node strategy into the fork.

Changes

allocation/per_node.go — node-indexed allocator with consistent-hashing fallback and
unassigned-target tracking
allocation/strategy.go — register per-node
collector/collector.go — capture Collector.NodeName from pod.Spec.NodeName
(skip empty-NodeName pods; handle watch.Modified)
CRD allocation-strategy enum (consistent-hashing | per-node)
internal/manifests/targetallocator/configmap.go — emit allocation_strategy +
allocation_fallback_strategy

Dependencies

Depends on Fix Target Allocator startup crashes, PrometheusCR watcher, and Prometheus config pod restart #386 (TA startup fixes). Until Fix Target Allocator startup crashes, PrometheusCR watcher, and Prometheus config pod restart #386 merges, this PR's diff includes its
commits. (Note: part of Fix Target Allocator startup crashes, PrometheusCR watcher, and Prometheus config pod restart #386 — the SD-metrics registration — has already landed on main.)
Companion: helm PR [amazon-cloudwatch-observability] feat: support per-node Target Allocator allocation aws-observability/helm-charts#330 (chart per-node support +
node-enrichment transforms). The chart default per-node requires this strategy registered
in the TA build.

Testing

allocation unit tests (per-node placement, fallback, unassigned tracking)
A/B: per-node vs consistent-hashing — per-node yields node-local assignment; flipping to
consistent-hashing reproduces cross-node scrapes (validated live on EKS).

…er startup The target-allocator declared the enable-prometheus-cr-watcher flag name as a constant but never registered it on the flag set, while the operator passes --enable-prometheus-cr-watcher whenever PrometheusCR.enabled is true. Because args are parsed with pflag.ExitOnError, the unregistered flag caused the binary to print 'unknown flag' and exit(2), putting the target-allocator pod into CrashLoopBackOff. This change registers the flag and ORs it with the YAML prometheus_cr.enabled setting, then fixes three latent defects that were previously unreachable because the binary crashed first: - promOperator: set a non-empty Namespace on the synthetic Prometheus object so the prometheus-operator config generator no longer panics with 'namespace can't be empty' in store.ForNamespace. - promOperator: set EvaluationInterval so the generated config does not render an empty global.evaluation_interval, which the prometheus config parser rejects with 'empty duration string'. - main: create and register service-discovery metrics and pass them to discovery.NewManager; passing a nil sdMetrics map makes every SD provider fail to register, yielding zero discovered targets. RELEASE_NOTES updated.

Add a regression test asserting that loading a Target Allocator config whose static scrape job omits scrape_protocols still yields a non-empty ScrapeProtocols on every loaded scrape config. This is defaulted by the pinned Prometheus library during yaml.UnmarshalStrict into the prometheus Config type, so the distributed /scrape_configs payload is never empty and the agent's prometheus-receiver validation passes. The test fails fast if a future dependency or load-path change drops this defaulting.

The pod-template restart-trigger sha256 was computed from Spec.Config only, so a change to Spec.Prometheus (rendered into a separate ConfigMap) left the pod template byte-identical and the workload controller did not roll the pods. Fold the serialized Spec.Prometheus (PrometheusConfig.Yaml()) into the hash input when it is non-empty, so a Prometheus-only change bumps the pod-template annotation and triggers a rolling restart, matching agent-config behavior. When no Prometheus config is set the hash input is byte-identical to the agent config alone, leaving non-Prometheus agents unaffected.

Add a per-node allocation strategy so each CloudWatch agent (DaemonSet, one per node) scrapes only the ServiceMonitor/PodMonitor targets on its own node, eliminating cross-node/cross-AZ scrape traffic. Targets that cannot be matched to a node-local agent (node-less endpoints, nodes without a Ready agent) fall back to consistent-hashing so they are never silently dropped. - allocation/per_node.go: perNodeAllocator (node index, consistent-hashing fallback ring, unassigned tracking, descriptive logging) + registration. - allocation/strategy.go: Collector.NodeName, NewCollector(name, node), WithFallbackStrategy option, targets_unassigned gauge. - collector/collector.go: capture pod.Spec.NodeName, skip empty-NodeName pods, handle watch.Modified so a collector's node is picked up once scheduled (fixes targets being stuck on the fallback after a DaemonSet rollout). - target/target.go: GetNodeName() from __meta_kubernetes_*_node_name labels. - config + main: FallbackAllocationStrategy wiring. - apis/v1alpha1 + CRD: allocationStrategy enum gains "per-node". - internal/manifests/targetallocator/configmap.go: emit per-node strategy and the consistent-hashing fallback from the CR.

# Conflicts: # cmd/amazon-cloudwatch-agent-target-allocator/main.go

Cover the remaining branches of the per-node strategy and its supporting code: filter application, unsupported/seeded/empty fallback, target and collector removal, unassigned and reassignment paths, collector-less bootstrapping, the already-tracked guard, GetNodeName label resolution, and the collector watcher skipping unscheduled pods then picking up the node on Modified.

Re-align the Config struct fields after adding FallbackAllocationStrategy so gofmt/CI passes, and add unit coverage for GetAllocationFallbackStrategy.

musa-asad · 2026-07-21T15:42:20Z

+	filter Filter
+}
+
+func newPerNodeAllocator(log logr.Logger, opts ...AllocationOption) Allocator {


newPerNodeAllocator leaves fallbackHasher nil unless SetFallbackStrategy is called, so a hand written per-node config with no fallback keeps targets that have no node label but never scrapes them. Safe today because configmap.go emits consistent-hashing, but could we default it or warn when per-node has no fallback?

musa-asad · 2026-07-21T15:42:21Z

+	}
+
+	// Rebuild the node index from the current collector set.
+	pn.collectorByNode = make(map[string]*Collector)


collectorByNode[c.NodeName] = c has no tie break, so two collectors on the same NodeName means last one wins and its targets flap. Can't happen at one pod per node, but a maxSurge DaemonSet rollout could hit it. Could we add a tie break (say the smaller pod name) or note the one pod per node assumption?

musa-asad · 2026-07-21T15:42:22Z

+	defer timer.ObserveDuration()
+
+	CollectorsAllocatable.WithLabelValues(perNodeStrategyName).Set(float64(len(collectors)))
+	if len(collectors) == 0 {


SetCollectors returns early on an empty set without clearing its maps, so GetTargetsForCollectorAndJob keeps serving dead collectors until the next non empty call. consistentHashingAllocator does the same, so I read it as intentional parity, but if you want per-node strictly correct we could clear the maps at zero.

musa-asad · 2026-07-21T15:42:23Z

+// NodeName is the Kubernetes node the collector pod runs on; it is used by the
+// per-node allocation strategy to match targets to the collector on their node.
 // This struct can be extended with information like annotations and labels in the future.
 type Collector struct {


Collector.Hash() only returns Name, so a Modified event that changes NodeName on the same pod produces no diff and collectorByNode never rebuilds. It's safe because spec.NodeName is immutable and unscheduled pods are skipped, but that's subtle, so could we add a comment or hash Name plus NodeName?

musa-asad · 2026-07-21T15:42:23Z


 	allocatorPrehook = prehook.New(cfg.GetTargetsFilterStrategy(), log)
-	allocator, err = allocation.New(cfg.GetAllocationStrategy(), log, allocation.WithFilter(allocatorPrehook))
+	allocator, err = allocation.New(cfg.GetAllocationStrategy(), log,


Since this PR is what first registers per-node, an older allocator image without it hits allocation.New erroring and os.Exit(1), so it just CrashLoopBackOffs instead of degrading. Could the companion chart pin the Target Allocator image at this commit or newer when it sets the strategy?

newPerNodeAllocator leaves fallbackHasher nil until SetFallbackStrategy runs, so a hand-written per-node config with no fallback silently keeps node-less targets in the pool but never scrapes them (the operator's configmap always emits consistent-hashing, so production is unaffected). Emit a one-time warning when a target is left unassigned because no fallback is configured, pointing at the consistent-hashing fallback. Add TestPerNodeWarnsOnceWhenNoFallback.

collectorByNode was rebuilt with last-write-wins over a map, so if two collectors reported the same node (e.g. a transient maxSurge DaemonSet rollout with two pods on a node) ownership — and target placement — flapped with map iteration order. Keep the collector with the smaller pod name deterministically. Add TestPerNodeTwoCollectorsSameNodeTieBreak.

Document that SetCollectors intentionally does not clear the node index/target mappings when the collector set is momentarily empty (parity with consistentHashingAllocator); clearing would drop every target during a transient zero-collector window, and state is corrected on the next non-empty call.

A NodeName change on the same pod would produce no collector diff (so per-node's collectorByNode would not rebuild), but that cannot happen: the collector watch skips unscheduled pods (empty NodeName) until scheduled, and a scheduled pod's spec.NodeName is immutable. Document the invariant and note to hash Name+NodeName if it ever changes.

movence · 2026-09-01T19:22:07Z

 			switch event.Type { //nolint:exhaustive
-			case watch.Added:
-				collectorMap[pod.Name] = allocation.NewCollector(pod.Name)
+			case watch.Added, watch.Modified:


The List path above skips pods with a DeletionTimestamp but this watch branch does not, so a terminating pod keeps its node ownership until the Deleted event lands. Could we add the same check here?

movence · 2026-09-01T19:22:09Z

+	}
+	if pn.fallbackHasher == nil && !pn.warnedNoFallback {
+		pn.warnedNoFallback = true
+		pn.log.Info("per-node: no fallback strategy configured; targets that cannot be matched to a " +


A hand-written per-node config without allocation_fallback_strategy leaves node-less targets unassigned behind this one-time Info log. Would defaulting the fallback here, or logging at Warn, be safer?

we can default the fallback strategy 👍

movence · 2026-09-01T19:22:10Z

                    description: |-
                      AllocationStrategy determines which strategy the target allocator should use for allocation.
-                      The current option is consistent-hashing.
+                      The options are consistent-hashing and per-node.


The Go doc comment on this field in amazoncloudwatchagent_types.go still mentions only consistent-hashing, so the next make manifests will revert this text. Worth updating the comment too?

The initial pod List skips pods with a DeletionTimestamp, but the watch's Added/Modified branch did not, so a terminating agent kept ownership of its node until the Deleted event landed. Under per-node that leaves the node's targets pinned to a collector that is shutting down, so they go unscraped for the rest of the grace period instead of being re-placed on the node's replacement agent or via the fallback. Apply the same DeletionTimestamp check on Added/Modified and drop the collector as soon as the pod is marked for deletion. Add Test_runWatch_TerminatingPodReleased.

A hand-written per-node config with no allocation_fallback_strategy left targets that carry no node label retained but assigned to no collector, so they were never scraped behind a one-time log line. The operator-generated configmap always emits consistent-hashing, so only hand-written configs were exposed, but silently-unscraped targets is not a safe default. Default the fallback to consistent-hashing in Config.GetAllocationFallbackStrategy whenever the per-node strategy is selected and no fallback is set, following the existing defaulting pattern in that file (GetAllocationStrategy). Setting allocation_fallback_strategy to an empty string remains an explicit opt-out, which is now the only way to reach the no-fallback log; reword it to point at the default and document the invariant on perNodeAllocator. Extend TestGetAllocationFallbackStrategy with the per-node default and opt-out cases.

The generated CRD lists both consistent-hashing and per-node, but the Go doc comment on TargetAllocatorSpec.AllocationStrategy still said the current option is consistent-hashing, so the next make manifests would have reverted the CRD description. Update the comment to match; make manifests now produces no diff for this field.

Pull in the three review fixes from aws#398 (terminating-collector release, per-node fallback default, allocationStrategy doc sync) so this stacked branch shows the same aws#398 code reviewers approved there. Conflict in config/config.go was the Config struct: both branches realigned it after aws#398 added FallbackAllocationStrategy, and this branch additionally adds ScraperRole. Kept ScraperRole with the shared alignment.

musa-asad and others added 5 commits June 16, 2026 09:21

Merge remote-tracking branch 'origin/main' into ta-per-node-allocation

87d1331

# Conflicts: # cmd/amazon-cloudwatch-agent-target-allocator/main.go

wenegiemepraise mentioned this pull request Jul 15, 2026

[amazon-cloudwatch-agent-operator] Route SM/PM by cloudwatch.aws/scraper annotation #399

Open

wenegiemepraise added 2 commits July 15, 2026 15:06

fix(target-allocator): gofmt config struct and test fallback getter

0bc5971

Re-align the Config struct fields after adding FallbackAllocationStrategy so gofmt/CI passes, and add unit coverage for GetAllocationFallbackStrategy.

musa-asad reviewed Jul 21, 2026

View reviewed changes

wenegiemepraise added 4 commits July 24, 2026 10:53

musa-asad previously approved these changes Jul 24, 2026

View reviewed changes

movence reviewed Sep 1, 2026

View reviewed changes

wenegiemepraise added 3 commits September 1, 2026 20:54

wenegiemepraise dismissed musa-asad’s stale review via 56881c0 September 1, 2026 19:57

wenegiemepraise requested review from movence and musa-asad September 1, 2026 19:59

Navigation Menu

Uh oh!

[amazon-cloudwatch-agent-operator] feat(target-allocator): add per-node allocation strategy - #398

[amazon-cloudwatch-agent-operator] feat(target-allocator): add per-node allocation strategy#398
wenegiemepraise wants to merge 14 commits into
aws:mainfrom
spanaik:ta-per-node-allocation

wenegiemepraise commented Jul 9, 2026

Uh oh!

musa-asad Jul 21, 2026 •

edited

Loading

Uh oh!

musa-asad Jul 21, 2026 •

edited

Loading

Uh oh!

musa-asad Jul 21, 2026 •

edited

Loading

Uh oh!

musa-asad Jul 21, 2026 •

edited

Loading

Uh oh!

musa-asad Jul 21, 2026 •

edited

Loading

Uh oh!

movence Sep 1, 2026

Uh oh!

movence Sep 1, 2026

Uh oh!

wenegiemepraise Sep 1, 2026

Uh oh!

movence Sep 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

wenegiemepraise commented Jul 9, 2026

Summary

Motivation

Changes

Dependencies

Testing

Uh oh!

musa-asad Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

musa-asad Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

musa-asad Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

musa-asad Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

musa-asad Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

movence Sep 1, 2026

Choose a reason for hiding this comment

Uh oh!

movence Sep 1, 2026

Choose a reason for hiding this comment

Uh oh!

wenegiemepraise Sep 1, 2026

Choose a reason for hiding this comment

Uh oh!

movence Sep 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

musa-asad Jul 21, 2026 •

edited

Loading

musa-asad Jul 21, 2026 •

edited

Loading

musa-asad Jul 21, 2026 •

edited

Loading

musa-asad Jul 21, 2026 •

edited

Loading

musa-asad Jul 21, 2026 •

edited

Loading