Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions .chloggen/fix_target-allocator-only-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
component: target allocator

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Accept a prometheus receiver that only declares `target_allocator:` without a `config:` block.

# One or more tracking issues related to the change
issues: [2998]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext: |
When the prometheus receiver is configured with only a `target_allocator:` block and no `config:`,
reconciliation previously failed with `no prometheusConfig available as part of the configuration`.
The target allocator supplies scrape configuration externally in this mode (e.g. via discovered
PrometheusCR objects), so the operator now skips the scrape_configs cleanup when no `config:`
block is present. The webhook validator likewise permits this shape.
Original file line number Diff line number Diff line change
Expand Up @@ -282,14 +282,16 @@ func WithCollectorTargetReloadInterval(interval string) TAOption {
// If the `EnableTargetAllocatorRewrite` feature flag for the target allocator is enabled, this function
// removes the existing scrape_configs from the collector's Prometheus configuration as it's not required.
func AddTAConfigToPromConfig(prometheus map[any]any, taServiceName string, taOpts ...TAOption) (map[any]any, error) {
prometheusConfigProperty, ok := prometheus["config"]
if !ok {
return nil, errorNoComponent("prometheusConfig")
}

prometheusCfg, ok := prometheusConfigProperty.(map[any]any)
if !ok {
return nil, errorNotAMap("prometheusConfig")
// When the receiver has a `config:` block, strip any user-provided scrape_configs:
// they will be supplied by the target allocator at runtime. When the block is absent
// (TA-only mode: scrape configs come from the TA itself, e.g. a mounted configmap or
// PrometheusCR objects), there is nothing to strip.
if prometheusConfigProperty, ok := prometheus["config"]; ok {
prometheusCfg, ok := prometheusConfigProperty.(map[any]any)
if !ok {
return nil, errorNotAMap("prometheusConfig")
}
delete(prometheusCfg, "scrape_configs")
}

// Create the TargetAllocConfig dynamically if it doesn't exist
Expand All @@ -313,9 +315,6 @@ func AddTAConfigToPromConfig(prometheus map[any]any, taServiceName string, taOpt
}
}

// Remove the scrape_configs key from the map
delete(prometheusCfg, "scrape_configs")

return prometheus, nil
}

Expand Down Expand Up @@ -344,6 +343,12 @@ func ValidateTargetAllocatorConfig(targetAllocatorPrometheusCR bool, promReceive
if targetAllocatorPrometheusCR {
return nil
}
// TA-only mode: when the receiver has no `config:` block, the user is delegating
// scrape configuration to the target allocator itself (loaded from an external
// source at runtime). Permit this — we have nothing to validate locally.
if _, hasConfig := promReceiverConfig["config"]; !hasConfig {
return nil
}
// if PrometheusCR isn't enabled, we need at least one scrape config
scrapeConfigs, err := GetScrapeConfigsFromPromConfig(promReceiverConfig)
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -339,17 +339,12 @@ func TestAddTAConfigToPromConfig(t *testing.T) {
assert.Equal(t, expectedResult, result)
})

t.Run("missing or invalid prometheusConfig property, returns error", func(t *testing.T) {
t.Run("invalid prometheusConfig property, returns error", func(t *testing.T) {
testCases := []struct {
name string
cfg map[any]any
errText string
}{
{
name: "missing config property",
cfg: map[any]any{},
errText: "no prometheusConfig available as part of the configuration",
},
{
name: "invalid config property",
cfg: map[any]any{
Expand All @@ -370,6 +365,47 @@ func TestAddTAConfigToPromConfig(t *testing.T) {
})
}
})

t.Run("TA-only mode: no config block, only target_allocator set", func(t *testing.T) {
// Regression test for #2998. The user supplied only a `target_allocator:` block
// on the prometheus receiver (no `config:`). Reconciliation must not fail, and
// no spurious `config:` block should be inserted — the prometheus receiver itself
// does not require one.
cfg := map[any]any{
"target_allocator": map[any]any{
"endpoint": "user-supplied-endpoint",
},
}
taServiceName := "test-targetallocator"

result, err := ta.AddTAConfigToPromConfig(cfg, taServiceName)

assert.NoError(t, err)
assert.NotNil(t, result)
taSection, ok := result["target_allocator"].(map[any]any)
assert.True(t, ok)
// Operator-managed endpoint replaces the user-supplied one.
assert.Equal(t, "http://test-targetallocator:80", taSection["endpoint"])
// No `config:` block should be added when the user didn't supply one.
_, hasConfig := result["config"]
assert.False(t, hasConfig)
})

t.Run("TA-only mode: no config block, no target_allocator block", func(t *testing.T) {
// Edge case for #2998: an entirely empty receiver succeeds once TA is enabled.
// The operator inserts the target_allocator block but leaves `config:` absent.
cfg := map[any]any{}
taServiceName := "test-targetallocator"

result, err := ta.AddTAConfigToPromConfig(cfg, taServiceName)

assert.NoError(t, err)
assert.NotNil(t, result)
_, hasConfig := result["config"]
assert.False(t, hasConfig)
_, hasTA := result["target_allocator"]
assert.True(t, hasTA)
})
}

func TestValidatePromConfig(t *testing.T) {
Expand Down Expand Up @@ -481,10 +517,24 @@ func TestValidateTargetAllocatorConfig(t *testing.T) {
expectedError: nil,
},
{
// TA-only mode (#2998): receiver has no `config:` block, so the user is
// delegating scrape configuration to the target allocator. Validator permits.
description: "receiver config empty and PrometheusCR disabled",
config: map[any]any{},
targetAllocatorPrometheusCR: false,
expectedError: fmt.Errorf("no %s available as part of the configuration", "prometheusConfig"),
expectedError: nil,
},
{
// TA-only mode with explicit target_allocator block — same as the empty case,
// permitted because no `config:` means scrape configs come from the TA itself.
description: "no config block but target_allocator set, PrometheusCR disabled",
config: map[any]any{
"target_allocator": map[any]any{
"endpoint": "http://my-ta:80",
},
},
targetAllocatorPrometheusCR: false,
expectedError: nil,
},
{
description: "scrape configs empty and PrometheusCR disabled",
Expand Down
7 changes: 7 additions & 0 deletions internal/manifests/targetallocator/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,13 @@ func getGlobalConfigFromOtelConfig(otelConfig v1beta1.Config) (v1beta1.AnyConfig
func getScrapeConfigsFromOtelConfig(otelcolConfig string) ([]v1beta1.AnyConfig, error) {
// Collector supports environment variable substitution, but the TA does not.
// TA Scrape Configs should have a single "$", as it does not support env var substitution
promConfig, err := adapters.ConfigToPromConfig(otelcolConfig)
if err != nil {
return nil, err
}
if _, hasConfig := promConfig["config"]; !hasConfig {
return []v1beta1.AnyConfig{}, nil
}
prometheusReceiverConfig, err := adapters.UnescapeDollarSignsInPromConfig(otelcolConfig)
if err != nil {
return nil, err
Expand Down
11 changes: 11 additions & 0 deletions internal/manifests/targetallocator/configmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,17 @@ func TestGetScrapeConfigsFromOtelConfig(t *testing.T) {
},
wantErr: errors.New("no scrape_configs available as part of the configuration"),
},
{
name: "no prom config key",
input: v1beta1.Config{
Receivers: v1beta1.AnyConfig{
Object: map[string]any{
"prometheus": map[string]any{},
},
},
},
want: []v1beta1.AnyConfig{},
},
{
name: "one scrape config",
input: v1beta1.Config{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
apiVersion: apps/v1
kind: StatefulSet
metadata:
name: no-promconfig-collector
status:
readyReplicas: 1
replicas: 1
---
apiVersion: apps/v1
kind: Deployment
metadata:
name: no-promconfig-targetallocator
status:
observedGeneration: 1
readyReplicas: 1
replicas: 1
---
apiVersion: v1
kind: ConfigMap
metadata:
name: no-promconfig-targetallocator
---
# The rendered collector config must contain the operator-injected
# target_allocator block but no `config:` key under prometheus (the user
# didn't supply one and the operator should not synthesise one).
apiVersion: v1
data:
collector.yaml: |
exporters:
prometheus:
endpoint: 0.0.0.0:9090
receivers:
prometheus:
target_allocator:
collector_id: ${POD_NAME}
endpoint: http://no-promconfig-targetallocator:80
interval: 30s
service:
pipelines:
metrics:
exporters:
- prometheus
receivers:
- prometheus
telemetry:
metrics:
readers:
- pull:
exporter:
prometheus:
host: 0.0.0.0
port: 8888
without_scope_info: false
without_type_suffix: false
without_units: false
kind: ConfigMap
metadata:
labels:
app.kubernetes.io/component: opentelemetry-collector
app.kubernetes.io/managed-by: opentelemetry-operator
app.kubernetes.io/name: no-promconfig-collector
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
apiVersion: v1
automountServiceAccountToken: true
kind: ServiceAccount
metadata:
name: collector
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: collector-no-promconfig
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's safer to make the name the same as the ClusterRoleBinding, reduce the chance of collisions with other tests. Those have historically been a source of strange bugs.

rules:
- apiGroups:
- ""
resources:
- pods
- nodes
- nodes/metrics
- services
- endpoints
- namespaces
verbs:
- get
- watch
- list
- apiGroups:
- networking.k8s.io
resources:
- ingresses
verbs:
- get
- watch
- list
- nonResourceURLs:
- /metrics
- /metrics/cadvisor
verbs:
- get
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: (join('-', ['collector', $namespace]))
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: collector-no-promconfig
subjects:
- kind: ServiceAccount
name: collector
namespace: ($namespace)
---
# Regression manifest for #2998: prometheus receiver has no `config:` block.
# Scrape configuration is delegated entirely to the target allocator, which
# loads it from PrometheusCR objects discovered in-cluster. The webhook used
# to reject this shape; reconciliation used to fail on it.
apiVersion: opentelemetry.io/v1beta1
kind: OpenTelemetryCollector
metadata:
name: no-promconfig
spec:
config:
receivers:
prometheus: {}
exporters:
prometheus:
endpoint: 0.0.0.0:9090
Comment on lines +65 to +66
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I don't think there's a reason to use a more complex exporter here. nopexporter should do just fine and be faster.

service:
pipelines:
metrics:
receivers: [prometheus]
exporters: [prometheus]
mode: statefulset
serviceAccount: collector
targetAllocator:
enabled: true
prometheusCR:
enabled: true
scrapeInterval: 1s
serviceMonitorSelector: {}
podMonitorSelector: {}
serviceAccount: ta
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# yaml-language-server: $schema=https://raw.githubusercontent.com/kyverno/chainsaw/main/.schemas/json/test-chainsaw-v1alpha1.json
apiVersion: chainsaw.kyverno.io/v1alpha1
kind: Test
metadata:
creationTimestamp: null
name: targetallocator-no-promconfig
spec:
steps:
- name: Setup Target Allocator RBAC
use:
template: ../../step-templates/target-allocator-rbac-comprehensive.yaml
with:
bindings:
- name: clusterRoleName
value: targetallocator-no-promconfig
- name: clusterRoleBindingName
value: (join('-', ['ta', $namespace]))
- name: step-00
try:
- apply:
template: true
file: 00-install.yaml
- assert:
file: 00-assert.yaml
catch:
- podLogs:
selector: app.kubernetes.io/managed-by=opentelemetry-operator
Loading