Skip to content

Commit

Permalink
Rollback 3187
Browse files Browse the repository at this point in the history
  • Loading branch information
foolusion committed Aug 31, 2024
1 parent a1e8f92 commit d4516e2
Show file tree
Hide file tree
Showing 15 changed files with 37 additions and 24 deletions.
16 changes: 16 additions & 0 deletions .chloggen/3242-rollback-of-3187.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# 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: Rollback 3187

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

# (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: This Rollsback 3187 which breaks TargetAllocator config for clusters with custom domains.
4 changes: 2 additions & 2 deletions controllers/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1421,7 +1421,7 @@ service:
Annotations: map[string]string{},
},
Data: map[string]string{
"collector.yaml": "exporters:\n logging: null\nreceivers:\n prometheus:\n config: {}\n target_allocator:\n collector_id: ${POD_NAME}\n endpoint: http://test-targetallocator.test.svc.cluster.local:80\n interval: 30s\nservice:\n pipelines:\n metrics:\n exporters:\n - logging\n receivers:\n - prometheus\n",
"collector.yaml": "exporters:\n logging: null\nreceivers:\n prometheus:\n config: {}\n target_allocator:\n collector_id: ${POD_NAME}\n endpoint: http://test-targetallocator:80\n interval: 30s\nservice:\n pipelines:\n metrics:\n exporters:\n - logging\n receivers:\n - prometheus\n",
},
},
&corev1.ServiceAccount{
Expand Down Expand Up @@ -1878,7 +1878,7 @@ prometheus_cr:
Annotations: map[string]string{},
},
Data: map[string]string{
"collector.yaml": "exporters:\n logging: null\nreceivers:\n prometheus:\n config: {}\n target_allocator:\n collector_id: ${POD_NAME}\n endpoint: http://test-targetallocator.test.svc.cluster.local:80\n interval: 30s\nservice:\n pipelines:\n metrics:\n exporters:\n - logging\n receivers:\n - prometheus\n",
"collector.yaml": "exporters:\n logging: null\nreceivers:\n prometheus:\n config: {}\n target_allocator:\n collector_id: ${POD_NAME}\n endpoint: http://test-targetallocator:80\n interval: 30s\nservice:\n pipelines:\n metrics:\n exporters:\n - logging\n receivers:\n - prometheus\n",
},
},
&corev1.ServiceAccount{
Expand Down
2 changes: 1 addition & 1 deletion internal/manifests/collector/config_replace.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func ReplaceConfig(otelcol v1beta1.OpenTelemetryCollector, targetAllocator *v1al

// To avoid issues caused by Prometheus validation logic, which fails regex validation when it encounters
// $$ in the prom config, we update the YAML file directly without marshaling and unmarshalling.
updPromCfgMap, getCfgPromErr := ta.AddTAConfigToPromConfig(promCfgMap, naming.TAService(targetAllocator.Name), targetAllocator.Namespace)
updPromCfgMap, getCfgPromErr := ta.AddTAConfigToPromConfig(promCfgMap, naming.TAService(targetAllocator.Name))
if getCfgPromErr != nil {
return "", getCfgPromErr
}
Expand Down
4 changes: 2 additions & 2 deletions internal/manifests/collector/config_replace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestPrometheusParser(t *testing.T) {
assert.NotContains(t, prometheusConfig, "scrape_configs")

expectedTAConfig := map[interface{}]interface{}{
"endpoint": "http://test-targetallocator.default.svc.cluster.local:80",
"endpoint": "http://test-targetallocator:80",
"interval": "30s",
"collector_id": "${POD_NAME}",
}
Expand All @@ -68,7 +68,7 @@ func TestPrometheusParser(t *testing.T) {
assert.NotContains(t, prometheusConfig, "scrape_configs")

expectedTAConfig := map[interface{}]interface{}{
"endpoint": "http://test-targetallocator.default.svc.cluster.local:80",
"endpoint": "http://test-targetallocator:80",
"interval": "30s",
"collector_id": "${POD_NAME}",
}
Expand Down
2 changes: 1 addition & 1 deletion internal/manifests/collector/configmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ receivers:
config: {}
target_allocator:
collector_id: ${POD_NAME}
endpoint: http://test-targetallocator.default.svc.cluster.local:80
endpoint: http://test-targetallocator:80
interval: 30s
service:
pipelines:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ receivers:
scrape_timeout: 10s
target_allocator:
collector_id: ${POD_NAME}
endpoint: http://test-targetallocator.default.svc.cluster.local:80
endpoint: http://test-targetallocator:80
interval: 30s
service:
pipelines:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ receivers:
scrape_configs:
- honor_labels: true
http_sd_configs:
- url: http://test-targetallocator.default.svc.cluster.local:80/jobs/service-x/targets?collector_id=$POD_NAME
- url: http://test-targetallocator:80/jobs/service-x/targets?collector_id=$POD_NAME
job_name: service-x
metric_relabel_configs:
- action: keep
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ func UnescapeDollarSignsInPromConfig(cfg string) (map[interface{}]interface{}, e
// This function removes any existing service discovery configurations (e.g., `sd_configs`, `dns_sd_configs`, `file_sd_configs`, etc.)
// from the `scrape_configs` section and adds a single `http_sd_configs` configuration.
// The `http_sd_configs` points to the TA (Target Allocator) endpoint that provides the list of targets for the given job.
func AddHTTPSDConfigToPromConfig(prometheus map[interface{}]interface{}, taServiceName string, taNamespace string) (map[interface{}]interface{}, error) {
func AddHTTPSDConfigToPromConfig(prometheus map[interface{}]interface{}, taServiceName string) (map[interface{}]interface{}, error) {
prometheusConfigProperty, ok := prometheus["config"]
if !ok {
return nil, errorNoComponent("prometheusConfig")
Expand Down Expand Up @@ -249,7 +249,7 @@ func AddHTTPSDConfigToPromConfig(prometheus map[interface{}]interface{}, taServi
escapedJob := url.QueryEscape(jobName)
scrapeConfig["http_sd_configs"] = []interface{}{
map[string]interface{}{
"url": fmt.Sprintf("http://%s.%s.svc.cluster.local:80/jobs/%s/targets?collector_id=$POD_NAME", taServiceName, taNamespace, escapedJob),
"url": fmt.Sprintf("http://%s:80/jobs/%s/targets?collector_id=$POD_NAME", taServiceName, escapedJob),
},
}
}
Expand All @@ -260,7 +260,7 @@ func AddHTTPSDConfigToPromConfig(prometheus map[interface{}]interface{}, taServi
// AddTAConfigToPromConfig adds or updates the target_allocator configuration in the Prometheus configuration.
// 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[interface{}]interface{}, taServiceName string, taNamespace string) (map[interface{}]interface{}, error) {
func AddTAConfigToPromConfig(prometheus map[interface{}]interface{}, taServiceName string) (map[interface{}]interface{}, error) {
prometheusConfigProperty, ok := prometheus["config"]
if !ok {
return nil, errorNoComponent("prometheusConfig")
Expand All @@ -281,7 +281,7 @@ func AddTAConfigToPromConfig(prometheus map[interface{}]interface{}, taServiceNa
return nil, errorNotAMap("target_allocator")
}

targetAllocatorCfg["endpoint"] = fmt.Sprintf("http://%s.%s.svc.cluster.local:80", taServiceName, taNamespace)
targetAllocatorCfg["endpoint"] = fmt.Sprintf("http://%s:80", taServiceName)
targetAllocatorCfg["interval"] = "30s"
targetAllocatorCfg["collector_id"] = "${POD_NAME}"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,15 +279,15 @@ func TestAddHTTPSDConfigToPromConfig(t *testing.T) {
"job_name": "test_job",
"http_sd_configs": []interface{}{
map[string]interface{}{
"url": fmt.Sprintf("http://%s.default.svc.cluster.local:80/jobs/%s/targets?collector_id=$POD_NAME", taServiceName, url.QueryEscape("test_job")),
"url": fmt.Sprintf("http://%s:80/jobs/%s/targets?collector_id=$POD_NAME", taServiceName, url.QueryEscape("test_job")),
},
},
},
},
},
}

actualCfg, err := ta.AddHTTPSDConfigToPromConfig(cfg, taServiceName, "default")
actualCfg, err := ta.AddHTTPSDConfigToPromConfig(cfg, taServiceName)
assert.NoError(t, err)
assert.Equal(t, expectedCfg, actualCfg)
})
Expand All @@ -308,7 +308,7 @@ func TestAddHTTPSDConfigToPromConfig(t *testing.T) {

taServiceName := "test-service"

_, err := ta.AddHTTPSDConfigToPromConfig(cfg, taServiceName, "default")
_, err := ta.AddHTTPSDConfigToPromConfig(cfg, taServiceName)
assert.Error(t, err)
assert.EqualError(t, err, "no scrape_configs available as part of the configuration")
})
Expand Down Expand Up @@ -338,13 +338,13 @@ func TestAddTAConfigToPromConfig(t *testing.T) {
expectedResult := map[interface{}]interface{}{
"config": map[interface{}]interface{}{},
"target_allocator": map[interface{}]interface{}{
"endpoint": "http://test-targetallocator.default.svc.cluster.local:80",
"endpoint": "http://test-targetallocator:80",
"interval": "30s",
"collector_id": "${POD_NAME}",
},
}

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

assert.NoError(t, err)
assert.Equal(t, expectedResult, result)
Expand Down Expand Up @@ -374,7 +374,7 @@ func TestAddTAConfigToPromConfig(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
_, err := ta.AddTAConfigToPromConfig(tc.cfg, taServiceName, "default")
_, err := ta.AddTAConfigToPromConfig(tc.cfg, taServiceName)

assert.Error(t, err)
assert.EqualError(t, err, tc.errText)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ data:
config: {}
target_allocator:
collector_id: ${POD_NAME}
endpoint: http://prometheus-kubernetessd-targetallocator.chainsaw-targetallocator-kubernetessd.svc.cluster.local:80
endpoint: http://prometheus-kubernetessd-targetallocator:80
interval: 30s
service:
pipelines:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ metadata:
creationTimestamp: null
name: targetallocator-kubernetessd
spec:
namespace: chainsaw-targetallocator-kubernetessd
steps:
- name: step-00
try:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ data:
config: {}
target_allocator:
collector_id: ${POD_NAME}
endpoint: http://prometheus-cr-targetallocator.chainsaw-targetallocator-prometheuscr.svc.cluster.local:80
endpoint: http://prometheus-cr-targetallocator:80
interval: 30s
service:
pipelines:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ metadata:
creationTimestamp: null
name: targetallocator-prometheuscr
spec:
namespace: chainsaw-targetallocator-prometheuscr
steps:
- name: step-00
try:
Expand Down
2 changes: 1 addition & 1 deletion tests/e2e/smoke-targetallocator/00-assert.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ data:
- PrometheusText0.0.4
target_allocator:
collector_id: ${POD_NAME}
endpoint: http://stateful-targetallocator.chainsaw-smoke-targetallocator.svc.cluster.local:80
endpoint: http://stateful-targetallocator:80
interval: 30s
service:
pipelines:
Expand Down
1 change: 0 additions & 1 deletion tests/e2e/smoke-targetallocator/chainsaw-test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ metadata:
creationTimestamp: null
name: smoke-targetallocator
spec:
namespace: chainsaw-smoke-targetallocator
steps:
- name: step-00
try:
Expand Down

0 comments on commit d4516e2

Please sign in to comment.