Rollback 3187 (#3248)

foolusion · web-flow · commit dfbf0b897255 · 2024-09-05T11:03:22.000-04:00
# Conflicts:
#	controllers/builder_test.go
diff --git a/.chloggen/3242-rollback-of-3187.yaml b/.chloggen/3242-rollback-of-3187.yaml
@@ -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.
diff --git a/controllers/builder_test.go b/controllers/builder_test.go
@@ -1421,7 +1421,7 @@ service:
 						Annotations: map[string]string{},
 					},
 					Data: map[string]string{
-						"collector.yaml": "exporters:\n    debug: 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                - debug\n            receivers:\n                - prometheus\n",
+						"collector.yaml": "exporters:\n    debug: 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                - debug\n            receivers:\n                - prometheus\n",
 					},
 				},
 				&corev1.ServiceAccount{
@@ -1878,7 +1878,7 @@ prometheus_cr:
 						Annotations: map[string]string{},
 					},
 					Data: map[string]string{
-						"collector.yaml": "exporters:\n    debug: 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                - debug\n            receivers:\n                - prometheus\n",
+						"collector.yaml": "exporters:\n    debug: 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                - debug\n            receivers:\n                - prometheus\n",
 					},
 				},
 				&corev1.ServiceAccount{
diff --git a/internal/manifests/collector/config_replace.go b/internal/manifests/collector/config_replace.go
@@ -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
 	}
diff --git a/internal/manifests/collector/config_replace_test.go b/internal/manifests/collector/config_replace_test.go
@@ -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}",
 		}
@@ -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}",
 		}
diff --git a/internal/manifests/collector/configmap_test.go b/internal/manifests/collector/configmap_test.go
@@ -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:
diff --git a/internal/manifests/collector/testdata/config_expected_targetallocator.yaml b/internal/manifests/collector/testdata/config_expected_targetallocator.yaml
@@ -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:
diff --git a/internal/manifests/collector/testdata/relabel_config_expected_with_sd_config.yaml b/internal/manifests/collector/testdata/relabel_config_expected_with_sd_config.yaml
@@ -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
diff --git a/internal/manifests/targetallocator/adapters/config_to_prom_config.go b/internal/manifests/targetallocator/adapters/config_to_prom_config.go
@@ -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")
@@ -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),
 			},
 		}
 	}
@@ -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")
@@ -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}"
 
diff --git a/internal/manifests/targetallocator/adapters/config_to_prom_config_test.go b/internal/manifests/targetallocator/adapters/config_to_prom_config_test.go
@@ -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)
 	})
@@ -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")
 	})
@@ -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)
@@ -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)
diff --git a/tests/e2e-targetallocator/targetallocator-kubernetessd/00-assert.yaml b/tests/e2e-targetallocator/targetallocator-kubernetessd/00-assert.yaml
@@ -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:
diff --git a/tests/e2e-targetallocator/targetallocator-kubernetessd/chainsaw-test.yaml b/tests/e2e-targetallocator/targetallocator-kubernetessd/chainsaw-test.yaml
@@ -5,7 +5,6 @@ metadata:
   creationTimestamp: null
   name: targetallocator-kubernetessd
 spec:
-  namespace: chainsaw-targetallocator-kubernetessd
   steps:
   - name: step-00
     try:
diff --git a/tests/e2e-targetallocator/targetallocator-prometheuscr/00-assert.yaml b/tests/e2e-targetallocator/targetallocator-prometheuscr/00-assert.yaml
@@ -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:
diff --git a/tests/e2e-targetallocator/targetallocator-prometheuscr/chainsaw-test.yaml b/tests/e2e-targetallocator/targetallocator-prometheuscr/chainsaw-test.yaml
@@ -5,7 +5,6 @@ metadata:
   creationTimestamp: null
   name: targetallocator-prometheuscr
 spec:
-  namespace: chainsaw-targetallocator-prometheuscr
   steps:
   - name: step-00
     try:
diff --git a/tests/e2e/smoke-targetallocator/00-assert.yaml b/tests/e2e/smoke-targetallocator/00-assert.yaml
@@ -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:
diff --git a/tests/e2e/smoke-targetallocator/chainsaw-test.yaml b/tests/e2e/smoke-targetallocator/chainsaw-test.yaml
@@ -5,7 +5,6 @@ metadata:
   creationTimestamp: null
   name: smoke-targetallocator
 spec:
-  namespace: chainsaw-smoke-targetallocator
   steps:
   - name: step-00
     try:

Original file line number	Diff line number	Diff line change
`@@ -71,7 +71,7 @@ func ReplaceConfig(otelcol v1beta1.OpenTelemetryCollector, targetAllocator *v1al`
`71`	`71`
`72`	`72`	`// To avoid issues caused by Prometheus validation logic, which fails regex validation when it encounters`
`73`	`73`	`// $$ in the prom config, we update the YAML file directly without marshaling and unmarshalling.`
`74`		`- updPromCfgMap, getCfgPromErr := ta.AddTAConfigToPromConfig(promCfgMap, naming.TAService(targetAllocator.Name), targetAllocator.Namespace)`
	`74`	`+ updPromCfgMap, getCfgPromErr := ta.AddTAConfigToPromConfig(promCfgMap, naming.TAService(targetAllocator.Name))`
`75`	`75`	`if getCfgPromErr != nil {`
`76`	`76`	`return "", getCfgPromErr`
`77`	`77`	`}`