componentParser: set correct target port (#3141)

* componentParser: set correct target port Signed-off-by: Benedikt Bongartz <[email protected]> * componentParser: Only set targetport for parsers with config Signed-off-by: Benedikt Bongartz <[email protected]> --------- Signed-off-by: Benedikt Bongartz <[email protected]>
open-telemetry · Jul 16, 2024 · a515234 · a515234
1 parent d2a9337
commit a515234
Show file tree

Hide file tree

Showing 7 changed files with 60 additions and 28 deletions.
diff --git a/.chloggen/fix_invalid_port.yaml b/.chloggen/fix_invalid_port.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: operator
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: set correct target port in services created based on the given otel config.
+
+# One or more tracking issues related to the change
+issues: [3139]
+
+# (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:
diff --git a/internal/components/component.go b/internal/components/component.go
@@ -107,14 +107,18 @@ type ComponentPortParser interface {
 }
 
 func ConstructServicePort(current *corev1.ServicePort, port int32) corev1.ServicePort {
-	return corev1.ServicePort{
+	svc := corev1.ServicePort{
 		Name:        current.Name,
 		Port:        port,
-		TargetPort:  current.TargetPort,
 		NodePort:    current.NodePort,
 		AppProtocol: current.AppProtocol,
 		Protocol:    current.Protocol,
 	}
+
+	if port > 0 && current.TargetPort.IntValue() > 0 {
+		svc.TargetPort = intstr.FromInt32(port)
+	}
+	return svc
 }
 
 func GetPortsForConfig(logger logr.Logger, config map[string]interface{}, retriever ParserRetriever) ([]corev1.ServicePort, error) {

diff --git a/internal/components/multi_endpoint_test.go b/internal/components/multi_endpoint_test.go
@@ -175,7 +175,7 @@ func TestMultiPortReceiver_Ports(t *testing.T) {
 				{
 					Name:       "receiver3-http",
 					Port:       80,
-					TargetPort: intstr.FromInt32(8080),
+					TargetPort: intstr.FromInt(80),
 				},
 			},
 			wantErr: assert.NoError,
@@ -239,7 +239,7 @@ func TestMultiPortReceiver_Ports(t *testing.T) {
 				{
 					Name:        "receiver6-grpc",
 					Port:        4317,
-					TargetPort:  intstr.FromInt32(4317),
+					TargetPort:  intstr.FromInt(4317),
 					Protocol:    corev1.ProtocolTCP,
 					AppProtocol: &components.GrpcProtocol,
 				},

diff --git a/internal/components/receivers/helpers.go b/internal/components/receivers/helpers.go
@@ -68,17 +68,21 @@ var (
 			)),
 		components.NewMultiPortReceiver("jaeger",
 			components.WithPortMapping(components.GrpcProtocol, 14250,
+				components.WithTargetPort(14250),
 				components.WithProtocol(corev1.ProtocolTCP),
 				components.WithAppProtocol(&components.GrpcProtocol),
 			),
 			components.WithPortMapping("thrift_http", 14268,
+				components.WithTargetPort(14268),
 				components.WithProtocol(corev1.ProtocolTCP),
 				components.WithAppProtocol(&components.HttpProtocol),
 			),
 			components.WithPortMapping("thrift_compact", 6831,
+				components.WithTargetPort(6831),
 				components.WithProtocol(corev1.ProtocolUDP),
 			),
 			components.WithPortMapping("thrift_binary", 6832,
+				components.WithTargetPort(6832),
 				components.WithProtocol(corev1.ProtocolUDP),
 			),
 		),
@@ -92,20 +96,20 @@ var (
 				components.WithAppProtocol(&components.HttpProtocol),
 			),
 		),
-		components.NewSinglePortParser("awsxray", 2000),
-		components.NewSinglePortParser("carbon", 2003),
-		components.NewSinglePortParser("collectd", 8081),
-		components.NewSinglePortParser("fluentforward", 8006),
-		components.NewSinglePortParser("influxdb", 8086),
-		components.NewSinglePortParser("opencensus", 55678, components.WithAppProtocol(nil)),
-		components.NewSinglePortParser("sapm", 7276),
-		components.NewSinglePortParser("signalfx", 9943),
-		components.NewSinglePortParser("splunk_hec", 8088),
-		components.NewSinglePortParser("statsd", 8125, components.WithProtocol(corev1.ProtocolUDP)),
+		components.NewSinglePortParser("awsxray", 2000, components.WithTargetPort(2000)),
+		components.NewSinglePortParser("carbon", 2003, components.WithTargetPort(2003)),
+		components.NewSinglePortParser("collectd", 8081, components.WithTargetPort(8081)),
+		components.NewSinglePortParser("fluentforward", 8006, components.WithTargetPort(8006)),
+		components.NewSinglePortParser("influxdb", 8086, components.WithTargetPort(8086)),
+		components.NewSinglePortParser("opencensus", 55678, components.WithAppProtocol(nil), components.WithTargetPort(55678)),
+		components.NewSinglePortParser("sapm", 7276, components.WithTargetPort(7276)),
+		components.NewSinglePortParser("signalfx", 9943, components.WithTargetPort(9943)),
+		components.NewSinglePortParser("splunk_hec", 8088, components.WithTargetPort(8088)),
+		components.NewSinglePortParser("statsd", 8125, components.WithProtocol(corev1.ProtocolUDP), components.WithTargetPort(8125)),
 		components.NewSinglePortParser("tcplog", components.UnsetPort, components.WithProtocol(corev1.ProtocolTCP)),
 		components.NewSinglePortParser("udplog", components.UnsetPort, components.WithProtocol(corev1.ProtocolUDP)),
-		components.NewSinglePortParser("wavefront", 2003),
-		components.NewSinglePortParser("zipkin", 9411, components.WithAppProtocol(&components.HttpProtocol), components.WithProtocol(corev1.ProtocolTCP)),
+		components.NewSinglePortParser("wavefront", 2003, components.WithTargetPort(2003)),
+		components.NewSinglePortParser("zipkin", 9411, components.WithAppProtocol(&components.HttpProtocol), components.WithProtocol(corev1.ProtocolTCP), components.WithTargetPort(3100)),
 		NewScraperParser("prometheus"),
 		NewScraperParser("kubeletstats"),
 		NewScraperParser("sshcheck"),

diff --git a/internal/components/receivers/multi_endpoint_receiver_test.go b/internal/components/receivers/multi_endpoint_receiver_test.go
@@ -58,6 +58,7 @@ func TestMultiEndpointReceiverParsers(t *testing.T) {
 						{
 							Name:        "jaeger-grpc",
 							Port:        14250,
+							TargetPort:  intstr.FromInt(14250),
 							Protocol:    corev1.ProtocolTCP,
 							AppProtocol: &grpc,
 						},
@@ -77,6 +78,7 @@ func TestMultiEndpointReceiverParsers(t *testing.T) {
 						{
 							Name:        "jaeger-grpc",
 							Port:        1234,
+							TargetPort:  intstr.FromInt(1234),
 							Protocol:    corev1.ProtocolTCP,
 							AppProtocol: &grpc,
 						},
@@ -97,24 +99,28 @@ func TestMultiEndpointReceiverParsers(t *testing.T) {
 						{
 							Name:        "jaeger-grpc",
 							Port:        14250,
+							TargetPort:  intstr.FromInt(14250),
 							Protocol:    corev1.ProtocolTCP,
 							AppProtocol: &grpc,
 						},
 						{
 							Name:        "port-14268",
 							Port:        14268,
+							TargetPort:  intstr.FromInt(14268),
 							Protocol:    corev1.ProtocolTCP,
 							AppProtocol: &http,
 						},
 						{
-							Name:     "port-6831",
-							Port:     6831,
-							Protocol: corev1.ProtocolUDP,
+							Name:       "port-6831",
+							Port:       6831,
+							TargetPort: intstr.FromInt(6831),
+							Protocol:   corev1.ProtocolUDP,
 						},
 						{
-							Name:     "port-6832",
-							Port:     6832,
-							Protocol: corev1.ProtocolUDP,
+							Name:       "port-6832",
+							Port:       6832,
+							TargetPort: intstr.FromInt(6832),
+							Protocol:   corev1.ProtocolUDP,
 						},
 					},
 				},
@@ -155,7 +161,7 @@ func TestMultiEndpointReceiverParsers(t *testing.T) {
 						{
 							Name:        "otlp-grpc",
 							Port:        1234,
-							TargetPort:  intstr.FromInt32(4317),
+							TargetPort:  intstr.FromInt(1234),
 							AppProtocol: &grpc,
 						},
 					},
@@ -221,7 +227,7 @@ func TestMultiEndpointReceiverParsers(t *testing.T) {
 						{
 							Name:        "otlp-test-grpc",
 							Port:        1234,
-							TargetPort:  intstr.FromInt32(4317),
+							TargetPort:  intstr.FromInt32(1234),
 							AppProtocol: &grpc,
 						},
 					},
@@ -287,7 +293,7 @@ func TestMultiEndpointReceiverParsers(t *testing.T) {
 						{
 							Name:        "loki-grpc",
 							Port:        1234,
-							TargetPort:  intstr.FromInt32(9095),
+							TargetPort:  intstr.FromInt(1234),
 							AppProtocol: &grpc,
 						},
 					},
@@ -353,7 +359,7 @@ func TestMultiEndpointReceiverParsers(t *testing.T) {
 						{
 							Name:        "skywalking-grpc",
 							Port:        1234,
-							TargetPort:  intstr.FromInt32(11800),
+							TargetPort:  intstr.FromInt(1234),
 							AppProtocol: &grpc,
 						},
 					},

diff --git a/internal/components/single_endpoint_test.go b/internal/components/single_endpoint_test.go
@@ -247,7 +247,7 @@ func TestSingleEndpointParser_Ports(t *testing.T) {
 				{
 					Name:        "testparser",
 					Port:        8080,
-					TargetPort:  intstr.FromInt32(4317),
+					TargetPort:  intstr.FromInt32(8080),
 					Protocol:    corev1.ProtocolTCP,
 					AppProtocol: &components.GrpcProtocol,
 				},
@@ -357,7 +357,7 @@ func TestNewSilentSinglePortParser_Ports(t *testing.T) {
 				{
 					Name:        "testparser",
 					Port:        8080,
-					TargetPort:  intstr.FromInt32(4317),
+					TargetPort:  intstr.FromInt32(8080),
 					Protocol:    corev1.ProtocolTCP,
 					AppProtocol: &components.GrpcProtocol,
 				},

diff --git a/internal/manifests/collector/service_test.go b/internal/manifests/collector/service_test.go
@@ -20,6 +20,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
 
 	"github.com/open-telemetry/opentelemetry-operator/apis/v1beta1"
 	"github.com/open-telemetry/opentelemetry-operator/internal/config"
@@ -331,6 +332,7 @@ func serviceWithInternalTrafficPolicy(name string, ports []v1beta1.PortsSpec, in
 
 	svcPorts := []v1.ServicePort{}
 	for _, p := range ports {
+		p.ServicePort.TargetPort = intstr.FromInt32(p.Port)
 		svcPorts = append(svcPorts, p.ServicePort)
 	}