Repository navigation
fix(target-allocator): tolerate missing ServiceMonitor/PodMonitor CRDs - #414
Conversation
AshuSrv
left a comment
AshuSrv
left a comment
There was a problem hiding this comment.
The first three commits are cherry-picks of an older revision of #386, so I don't think they'll simply drop out on rebase. Cherry-picking them onto #386's current head: e8737ef drops out, but 5a2600a conflicts in config.go, flags.go and promOperator.go, and 789d60a conflicts in promOperator.go. #386 itself also currently conflicts with main in internal/manifests/collector/annotations.go.
13524bb to
ea60a40
Compare
…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, and makes three related changes the watcher needs once it starts: - 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. (cherry picked from commit 1376451)
Add a 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. (cherry picked from commit 0405f4d)
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. (cherry picked from commit d61d693)
configHashInput's error branch is unreachable from the CRD path:
Spec.Prometheus.Config is an *AnyConfig whose Object is a
map[string]interface{} populated by JSON decoding, and every type that
produces is encodable by gopkg.in/yaml.v3. Logging a warning there
implied a runtime condition operators should watch for and act on, which
is misleading.
Replace the slog.Warn with a comment that records three things a reader
cannot recover from the code: that the branch is unreachable from the
CRD path, why the sentinel is deliberately a constant rather than
derived from the error (two different unserializable specs hash equal,
so a Prometheus-only change would not roll pods while the error
persists), and that the hash's stability across operator restarts rests
on gopkg.in/yaml.v3 v3.0.1 sorting map keys -- a yaml library
preserving insertion order here would roll every agent pod on restart.
Removing the call also removes the log/slog import, which was the
package's only use of it.
The non-error path, the IsEmpty() gate and the \x00 separator are
untouched, so the pinned config-hash values asserted in
annotations_test.go are unchanged.
(cherry picked from commit ae7a53c)
The namespace fed to the Prometheus config generator was taken verbatim from OTELCOL_NAMESPACE, or from the service account namespace file when that env var was empty. Neither source was trimmed, so a trailing newline or surrounding whitespace yielded a string that is not a valid Kubernetes namespace. The file branch also gated on len(ns) > 0, which treats a whitespace-only file as a real value and so skips the defaultCollectorNamespace fallback. Both sources are now whitespace-trimmed, and a value that is blank after trimming falls through to the next source: env var, then the service account file, then defaultCollectorNamespace. The resolution moves out of NewPrometheusCRWatcher into resolveCollectorNamespace, with the service account path held in a package variable so a test can redirect the read. The log line still fires exactly once when the env var is unset, and still reports the resolved value. (cherry picked from commit c5dee74)
…fault Adds coverage for the two behaviors NewPrometheusCRWatcher depends on and that nothing else in the watcher package exercises. TestResolveCollectorNamespace pins the resolution chain across seven cases: OTELCOL_NAMESPACE wins when set, both the env var and the service account namespace file are whitespace-trimmed, a whitespace-only value in either source falls through to the next source rather than becoming the namespace, and an absent or blank file falls back to the default. This guards against a future edit dropping a TrimSpace, reverting to a length-only emptiness check, or reordering the fallback chain -- any of which yields an empty or blank namespace, and the config generator panics on a non-empty namespace requirement. TestNewPrometheusCRWatcherDefaultsEvaluationInterval pins EvaluationInterval defaulting to ScrapeInterval. Without that assignment the generated config carries an empty evaluation_interval, which fails to parse. The tests live in a new file because promOperator_test.go imports neither k8s.io/client-go/rest, the allocator config package, nor logr. The test redirects serviceAccountNamespacePath and restores it via t.Cleanup, so the package's later test files do not inherit a path into a removed temp dir. The constructor is pointed at an unroutable host to confirm it does no dialing. (cherry picked from commit 479e251)
The collector client resolves its watch namespace once at process init from OTELCOL_NAMESPACE and passes the result to Pods(ns).List and Pods(ns).Watch. A value carrying surrounding whitespace is not a valid namespace name, so those calls matched nothing, no collectors were discovered, and allocation stayed empty with no error surfaced. Trim the value so padded input resolves to the intended namespace, matching how the namespace is now resolved on the watcher path. (cherry picked from commit 02a4bb5)
LoadFromCLI ORs the CLI flag with prometheus_cr.enabled from the config file, so the flag can only turn the PrometheusCR watcher on. Passing =false has no effect when the config file enables it, which the previous help text did not convey. State the OR behavior in the flag's usage string so the asymmetry is discoverable from --help. No behavior change: only the usage string passed to flagSet.Bool changes. (cherry picked from commit ebd1629)
LoadFromCLI OR-es the --enable-prometheus-cr-watcher flag with prometheus_cr.enabled from the config file, so the flag can only enable the watcher: passing =false cannot disable one the config file turns on. Nothing covered that, and replacing the || with a plain assignment would break only the config-file-true rows while leaving every existing test in the package green. Add a table-driven test over the four (config file, CLI) combinations, plus a committed testdata kubeconfig so the test is hermetic. LoadFromCLI builds a client config before it reaches the OR, so with no --kubeconfig-path it falls back to rest.InClusterConfig() and returns "unable to load in-cluster configuration" early. That passes on a developer host that happens to have a kubeconfig and fails in CI, so the flag pointing at testdata/kubeconfig_test.yaml is required rather than decorative. The fixture's server is 127.0.0.1 and is never dialed. pflag.ContinueOnError is a test-level choice so a parse error fails the test instead of exiting the process; the production getFlagSet call keeps pflag.ExitOnError. (cherry picked from commit 95adef2)
Extract the hardcoded service account namespace path literal into defaultServiceAccountNamespacePath per review feedback. The package-level serviceAccountNamespacePath var remains as the test seam that redirects the read in unit tests. There is no behavior change. (cherry picked from commit 2d21b9b)
Makes the Target Allocator start and run healthily whether or not the community monitoring.coreos.com ServiceMonitor/PodMonitor CRDs are installed, and start or stop watching each type as its CRD appears or disappears with no restart. Previously Watch() built both informers unconditionally and blocked on WaitForNamedCacheSync, so an absent CRD meant the apiserver rejected LIST/WATCH, the informer never synced, and Watch() returned 'failed to sync cache'. Informers are now built lazily per type once their CRD is observed, and a metadata-only CustomResourceDefinition informer starts or stops each one at runtime. Requires the chart to grant get/list/watch on customresourcedefinitions (aws-observability/helm-charts#327); without it the CRD check fails and no ServiceMonitor/PodMonitor targets are discovered while the Target Allocator still reports healthy. Verified: go build ./... clean; make impi and make checklicense pass; unit tests pass across the target-allocator and manifests packages.
…denied If the Target Allocator cannot read customresourcedefinitions, the startup CRD check fails and the CRD watch fails the same way, so ServiceMonitor and PodMonitor discovery never started even when the CRDs exist. On any CRD check error other than NotFound, start the informer anyway and log a warning naming the missing permission. The start runs in the background so a CRD that is really absent cannot block startup or Close.
982565f to
d58c770
Compare
| w.logger.Info("prometheus-cr: CRD not present at startup, deferring informer until it is created", "crd", crdName) | ||
| continue | ||
| } | ||
| if startErr := w.startMonitorInformer(resourceName, notifyEvents); startErr != nil { |
There was a problem hiding this comment.
startMonitorInformer blocks in WaitForNamedCacheSync until it syncs or stopCh closes, and this call is synchronous — so a CRD that exists but cannot be listed (not yet Established, or the role is missing list/watch on monitoring.coreos.com) wedges Watch() here. Lines 437 (watchCRDs) and 440 (rateLimitedEventSender) are never reached: no CRD watch, no event pump, and the other monitor type is never evaluated.
Since this no longer returns the old failed to sync cache error, run.Group never tears the process down — the pod reports healthy and discovers nothing, indefinitely. That is a worse failure mode than the crash this PR replaces, and the trigger is a realistic startup race: installing prometheus-operator and the agent in one Helm release or GitOps apply can easily reach the TA before the CRD is Established, and it re-rolls on every TA pod start.
The Forbidden branch at line 416 already wraps the same call in go func for exactly this reason. This branch needs the same treatment; startMonitorInformer is already idempotent and concurrency-safe.
Note that no existing test can reach this — the fake clientset always syncs immediately, so a reproducing test needs a lister that blocks or returns Forbidden.
There was a problem hiding this comment.
Agreed. All starts now go through ensureMonitorInformer in dc149bc, which runs each start in the background, so startup always reaches the CRD watch and the event pump. TestWatchDoesNotBlockOnHangingList makes both monitor LISTs hang; it fails on the previous head and passes now.
| if _, err := crdInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{ | ||
| AddFunc: func(obj interface{}) { | ||
| name, ok := crdObjectName(obj) | ||
| if !ok { | ||
| return | ||
| } | ||
| resourceName, tracked := crdNameToResource[name] | ||
| if !tracked { | ||
| return | ||
| } | ||
| if err := w.startMonitorInformer(resourceName, notifyEvents); err != nil { | ||
| w.logger.Error(err, "prometheus-cr: failed to start informer after CRD became available", "crd", name) | ||
| return | ||
| } | ||
| w.logger.Info("prometheus-cr: CRD available, started informer", "crd", name, "resource", resourceName) |
There was a problem hiding this comment.
Two issues in this handler.
AddFunc calls the blocking startMonitorInformer directly (line 344). client-go delivers all events for a single listener from one goroutine, so if that call blocks (same scenario as my note on line 429), the other CRD Add and both CRD Delete events are never processed — which is precisely the per-type independence and stop-on-CRD-delete behaviour this PR adds. TestWatchPerTypeIndependence and TestWatchStopsInformerWhenCRDDeleted pass because the fake clientset always syncs immediately; neither would catch this. Every resync also re-delivers every CRD into that listener unbounded pendingNotifications ring buffer while it is stuck. Suggest go w.startMonitorInformer(resourceName, notifyEvents).
Only AddFunc/DeleteFunc are registered, so a startMonitorInformer that fails once is never retried. The CRD is already in the informer cache, so no further Add is delivered — resyncs produce Update, which has no handler. After a transient apiserver blip that monitor type stays dark until the TA restarts, which contradicts both the no-restart goal and the deferring to CRD watch log line at line 430 (that wording implies a retry that does not exist). An UpdateFunc calling the same idempotent start would make each resync a retry.
There was a problem hiding this comment.
Fixed in dc149bc. AddFunc no longer blocks, and there is now an UpdateFunc, so each resync is also a retry. Each attempt has a sync deadline and is retried with capped backoff, with at most one start in flight per type. TestWatchRetriesMonitorTypeThatFailedToStart covers a type that becomes listable later.
| w.logger.Error(startErr, "prometheus-cr: failed to start informer for present CRD, deferring to CRD watch", "crd", crdName) | ||
| continue | ||
| } | ||
| w.logger.Info("prometheus-cr: started informer for present CRD", "crd", crdName, "resource", resourceName) |
There was a problem hiding this comment.
startMonitorInformer returns nil for three outcomes where nothing actually started (shutdown-during-sync 256-261, closed-while-syncing 268-273, lost-the-race 276-280), so this line logs started informer for present CRD even when no informer was registered — misleading in exactly the startup post-mortems this PR is meant to help with.
Compounding it, Watch takes upstreamErrors chan error and main.go:208 drains it and logs Watcher error, but nothing is ever sent on it. Combined with the removal of the fatal failed to sync cache, there is now no channel by which a failed start reaches the operator — only log lines. Suggest returning (started bool, err error) and pushing real failures to upstreamErrors.
There was a problem hiding this comment.
startMonitorInformer now returns (started bool, err error), and "started informer" is logged only when started is true. Start failures are sent to upstreamErrors without blocking. Covered by TestStartMonitorInformerReportsStarted and TestWatchReportsStartFailureUpstream.
| func configHashInput(instance v1alpha1.AmazonCloudWatchAgent) string { | ||
| config := instance.Spec.Config | ||
| if !instance.Spec.Prometheus.IsEmpty() { | ||
| promYaml, err := instance.Spec.Prometheus.Yaml() | ||
| if err != nil { | ||
| // Unreachable from the CRD path: Config.Object only ever holds JSON-decoded | ||
| // types, all of which gopkg.in/yaml.v3 encodes. Deliberately a constant: two | ||
| // different unserializable specs hash equal, so a Prometheus-only change would | ||
| // not roll pods while the error persists. Hash stability across operator | ||
| // restarts depends on gopkg.in/yaml.v3 (v3.0.1) sorting map keys | ||
| // (encode.go:189, and :242 for the inline-map branch). | ||
| config += "\x00prometheus-serialize-error" | ||
| } else { | ||
| config += "\x00" + promYaml // null byte prevents collision between config suffix and promYAML prefix | ||
| } | ||
| } | ||
| return config |
There was a problem hiding this comment.
This hashes a spec-derived proxy for the mounted prometheus.yaml rather than the rendered ConfigMap, but ReplacePrometheusConfig (config_replace.go:111) derives that file from Spec.TargetAllocator.Enabled, instance.Name (via naming.TAService) and featuregate.EnableTargetAllocatorRewrite in addition to Spec.Prometheus.
Flip spec.targetAllocator.enabled from false to true with Spec.Config and Spec.Prometheus unchanged: the ConfigMap is rewritten from static scrape configs to TA http_sd, the hash is byte-identical, the pod-template annotation does not change, and the agent pods are never rolled — they keep serving the old config. Renaming the instance or flipping the rewrite feature gate are the same class. That is the exact bug the roll-on-config-change commit is meant to fix, just on a different input, and every signal stays green while it happens.
internal/manifests/targetallocator/annotations.go:36 already hashes the rendered ConfigMap data; doing the same here would cover all of these inputs by construction. Worth a test that asserts the hash moves when targetAllocator.enabled flips — nothing currently does.
There was a problem hiding this comment.
Agreed. af7c1cc moves the Prometheus rendering into renderPrometheusConfig(), and the hash is now taken over its output, the same data the ConfigMap gets. Agents without Prometheus keep their current hash. New tests cover flipping targetAllocator.enabled, the instance name and the rewrite gate. On a test cluster, flipping enabled changed the hash and rolled the pods.
|
|
||
| var ( | ||
| ns = os.Getenv("OTELCOL_NAMESPACE") | ||
| ns = strings.TrimSpace(os.Getenv("OTELCOL_NAMESPACE")) |
There was a problem hiding this comment.
This now diverges from resolveCollectorNamespace in the watcher package: that one falls back to the service-account namespace file and then amazon-cloudwatch, while this path leaves ns as "".
With OTELCOL_NAMESPACE unset or whitespace-only — reachable when Spec.TargetAllocator.Env is overridden, since container.go:54 only injects the fieldRef when no entry of that name already exists — Pods("").List/Watch runs cluster-wide, and the allocator can enrol label-matching agent pods from other namespaces as shards.
The added TrimSpace also turns the whitespace case from a hard os.Exit(1) into that silent cluster-wide watch. Exporting resolveCollectorNamespace (or moving it to a package both can import) and using the one helper in both places would keep them in step.
There was a problem hiding this comment.
Fixed in e2bbc1c. The resolution moved to a shared collectornamespace package that both the pod watch and the CR watcher use, so the namespace is never "". TestWatchIsScopedToNamespace checks that a pod in another namespace isn't picked up.
|
Reviewed the Target Allocator CRD-resilience change plus the folded-in base fixes. The event-driven per-type approach is the right shape, and the Two process items, independent of the code: The checklist notes no GitHub Actions have run on this PR — the green runs linked are on the fork, on a previous revision of the branch. CI needs to run on The |
…atcher startMonitorInformer waits for the informer cache to sync. It was called synchronously from the Watch startup loop and from the CRD watch's add handler, so a CRD that exists but cannot be listed yet (not Established, or missing list/watch on monitoring.coreos.com) kept Watch from reaching the CRD watch and the event pump, and kept the CRD watch from processing any other event. The Target Allocator stayed up but discovered nothing. All starts now go through ensureMonitorInformer, which runs the start in the background with at most one start in flight per monitor type. - Each attempt is bounded by a sync timeout and retried with capped backoff, so a type that fails to start once is picked up without a restart. - CRD resyncs and updates are handled too, so each one is also a retry. - Failures are sent on upstreamErrors instead of only being logged. - When the CRD check itself is denied, starts are attempted a bounded number of times, so an absent CRD does not keep a reflector retrying for the pod lifetime. startMonitorInformer now returns whether it registered an informer, so "started informer" is only logged when one was started.
…rt annotation The pod restart hash used Spec.Prometheus as written by the user. The Prometheus ConfigMap the agent mounts is rendered from more than that: Spec.TargetAllocator.Enabled switches it between the scrape configs and the Target Allocator http_sd form, and the instance name and the EnableTargetAllocatorRewrite feature gate change it too. Changing any of those rewrote the ConfigMap without rolling the agent pods. Move the Prometheus data rendering out of ConfigMaps() into renderPrometheusConfig() and hash its output, so the hash covers exactly what the agent receives. Agents without a Prometheus config keep the same hash as before.
…ods and CRs The agent pod watch used OTELCOL_NAMESPACE directly and fell back to "" when it was unset, which client-go treats as all namespaces. The Prometheus CR watcher already fell back to the service account namespace and then amazon-cloudwatch, so the two could disagree. Move that resolution into a collectornamespace package and use it in both places, so the pod watch is never cluster-wide and always matches the namespace used for CR discovery.
d58c770 to
e2bbc1c
Compare
|
Thanks for the thorough review; all five points are addressed in the last three commits. On the fallback: when the CRD check is denied, starts are now capped (3 attempts, each bounded by the sync timeout), so with the CRDs absent nothing keeps retrying. Merging helm #376 first is still preferable. CI on the fork needs a maintainer to approve the workflow runs on the new head. |
5 participants
Summary
Make the Target Allocator (TA) start and run healthily whether or not the community
monitoring.coreos.comServiceMonitor/PodMonitor CRDs are installed, and start or stop watching each type automatically as its CRD appears or disappears, with no restart.Supersedes #394 and #386, rebased onto current
main. Includes all of #386, the Target Allocator startup fixes this change depends on (see "Included base fixes").Problem
The TA consumes the SM/PM CRDs but does not require them to exist. Today
Watch()builds the ServiceMonitor and PodMonitor informers unconditionally at startup and blocks onWaitForNamedCacheSync. If a CRD is absent, the apiserver rejects LIST/WATCH for that GVR, the informer never syncs, andWatch()returnsfailed to sync cache. Because setup runs once, installing the CRD later requires a TA restart.Fix
Make the watcher event-driven and per-type instead of one-shot and all-or-nothing:
crdExists- check whether a given CRD is installed before building its informer.startMonitorInformer/stopMonitorInformer- build or tear down a single monitor informer on demand (idempotent); stopping drops its targets and signals a reload.watchCRDs- an informer overCustomResourceDefinitionobjects; on Add it starts the matching monitor informer, on Delete it stops it.Watch()starts informers only for CRDs present at startup; absent ones are skipped and started later by the CRD watch.Every informer start runs in the background (
ensureMonitorInformer), with at most one start in flight per type. Each attempt must sync within a deadline. A failed attempt is reported onupstreamErrorsand retried with capped backoff. CRD resyncs are also treated as retries. So a type that cannot be listed yet never stops the other type, the CRD watch or the event pump.The TA needs
get/list/watchonapiextensions.k8s.io/customresourcedefinitions; the chart PR grants it (aws-observability/helm-charts#376). If that permission is missing, the CRD check fails with Forbidden; the TA then starts both informers anyway and logs a warning naming the permission, so discovery still works when the CRDs exist. Those starts are capped at 3 attempts, so a CRD that is really absent does not leave anything retrying.Review follow-up
The last three commits address the review:
dc149bcf: starts no longer block the watcher, failed starts are retried,startMonitorInformerreturns(started, err), errors reachupstreamErrors, and the denied-check fallback is bounded.af7c1cce: the pod restart hash covers the Prometheus config as rendered for the agent, so changingtargetAllocator.enabled, the instance name or the rewrite feature gate rolls the pods. Agents without Prometheus keep the same hash.e2bbc1cb: the agent pod watch and the CR watcher resolve the namespace through onecollectornamespacepackage, so the pod watch is never cluster-wide.Included base fixes (from #386)
The first ten commits are all of #386 by @musa-asad, at its current head (
2d21b9b6), replayed onto currentmainwith authorship kept. #386 is no longer being driven, so it is folded in here instead of landing separately. They cover:--enable-prometheus-cr-watcherflag (the TA otherwise exits with "unknown flag" when the operator passes it), ORed withprometheus_cr.enabled, plus the Prometheus namespace and evaluation-interval settings the watcher needsscrape_protocolson config load (test)OTELCOL_NAMESPACE, with testsWhile replaying onto
main:RELEASE_NOTESedits were dropped, since that file is updated at release time (Fix Target Allocator startup crashes, PrometheusCR watcher, and Prometheus config pod restart #386 adds nothing to it on net).cmd/amazon-cloudwatch-agent-target-allocator/main.gois identical tomain's, which already has the service-discovery metrics registration.internal/manifests/collector/annotations.gokeeps bothmain'sPodLabelsand Fix Target Allocator startup crashes, PrometheusCR watcher, and Prometheus config pod restart #386'sconfigHashInput.Testing
Unit tests, added in this PR:
TestWatchStartsHealthyWithoutCRDs- no CRDs:Watch()healthy, no error, no jobsTestWatchStartsInformerWhenCRDCreated- CRD created at runtime: informer starts, no restartTestWatchStopsInformerWhenCRDDeleted,TestStopMonitorInformerDropsType- CRD removed: informer stopped, targets dropped, reload signalledTestWatchPerTypeIndependence- SM present, PM absent: only the SM informer startsTestWatchStartsInformerWhenCRDCheckForbidden,TestWatchForbiddenCRDCheckDoesNotBlockWhenCRDAbsent- CRD check denied: informers still start; with the CRD absent, Watch keeps running and Close returnsTestWatchToleratesCRDCheckError,TestCrdExistsSurfacesNonNotFoundError,TestWatchIgnoresUntrackedCRD,TestCloseInterruptsInFlightSync,TestCloseIdempotent,TestStartMonitorInformerIdempotent,TestCRDObjectNameTestWatchDoesNotBlockOnHangingList,TestWatchRetriesMonitorTypeThatFailedToStart,TestWatchReportsStartFailureUpstream,TestTargetAllocatorEnabledChangeBumpsHash,TestInstanceNameChangeBumpsHashWithTargetAllocator,TestTargetAllocatorRewriteGateChangeBumpsHash,TestConfigHashMatchesRenderedPrometheusConfigMap; plusTestStartMonitorInformerReportsStarted,TestForbiddenFallbackIsBoundedWhenCRDAbsent,TestNewClientNamespaceWhenEnvUnset,TestNewClientNamespaceFromEnv,TestWatchIsScopedToNamespace,TestResolveTestScrapeProtocolsDefaultedOnLoad,TestPrometheusConfigChangeBumpsHash,TestEmptyPrometheusHashUnchangedOn an EKS 1.34 test cluster, with images built from this branch plus #415, and kube-prometheus-stack providing the CRDs:
targetAllocator.enabledchanges the pod hash and rolls the agent pods; flipping it back restores the original hash.PR Checklist
makepasses locally -go build ./...,go vetand the full unit test suite pass (with envtest), including-raceon the Target Allocator packages;golangci-lintreports 0 issues.e2bbc1cb): PR Build and Operator Integration Test.TestServiceMonitorPodMonitorCRDsBundled, which fails as expected on that cluster because kube-prometheus-stack owns the CRDs there.TestTargetAllocatorHealthyOnBundledInstallandTestTargetAllocatorDiscoversMonitors. Those cover a healthy TA and monitor discovery with the CRDs present. The missing-CRD path itself is covered only by the unit tests above, because the E2E harness installs the CRDs before the TA starts.TestWatchStartsHealthyWithoutCRDsreproduces thefailed to sync cachefailure; the review follow-up tests fail on the previous head.