From a515234b34557a325f0f944ac97a7f22a9b8f9fb Mon Sep 17 00:00:00 2001 From: "Ben B." Date: Tue, 16 Jul 2024 17:49:25 +0200 Subject: [PATCH] componentParser: set correct target port (#3141) * componentParser: set correct target port Signed-off-by: Benedikt Bongartz * componentParser: Only set targetport for parsers with config Signed-off-by: Benedikt Bongartz --------- Signed-off-by: Benedikt Bongartz --- .chloggen/fix_invalid_port.yaml | 16 +++++++++++ internal/components/component.go | 8 ++++-- internal/components/multi_endpoint_test.go | 4 +-- internal/components/receivers/helpers.go | 28 +++++++++++-------- .../receivers/multi_endpoint_receiver_test.go | 26 ++++++++++------- internal/components/single_endpoint_test.go | 4 +-- internal/manifests/collector/service_test.go | 2 ++ 7 files changed, 60 insertions(+), 28 deletions(-) create mode 100755 .chloggen/fix_invalid_port.yaml diff --git a/.chloggen/fix_invalid_port.yaml b/.chloggen/fix_invalid_port.yaml new file mode 100755 index 0000000000..a28ab57b75 --- /dev/null +++ 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 index 1cdaf1dfea..5bf57a649a 100644 --- 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 index f3829d82e9..2aac06a5d1 100644 --- 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 index a6298349c5..5003e00c24 100644 --- 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 index ff6990378d..db1d1f73e3 100644 --- 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 index 11d9121f54..9e544d40b9 100644 --- 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 index bf35dc7a61..7ee7dfc4a0 100644 --- 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) }