Fix

Signed-off-by: Pavol Loffay <[email protected]>

Author: pavolloffay

apis/v1beta1/collector_webhook_test.go

@@ -17,13 +17,13 @@ package v1beta1
 import (
 	"context"
 	"fmt"
-	"gopkg.in/yaml.v3"
 	"os"
 	"testing"
 
 	"github.com/go-logr/logr"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"gopkg.in/yaml.v3"
 	appsv1 "k8s.io/api/apps/v1"
 	authv1 "k8s.io/api/authorization/v1"
 	autoscalingv2 "k8s.io/api/autoscaling/v2"

apis/v1beta1/config.go

@@ -18,8 +18,11 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
+	"net"
 	"reflect"
 	"sort"
+	"strconv"
+	"strings"
 
 	"gopkg.in/yaml.v3"
 )
@@ -135,6 +138,26 @@ type Service struct {
 	Pipelines AnyConfig `json:"pipelines" yaml:"pipelines"`
 }
 
+// MetricsPort gets the port number for the metrics endpoint from the collector config if it has been set.
+func (s *Service) MetricsPort() (int32, error) {
+	if s.GetTelemetry() == nil {
+		// telemetry isn't set, use the default
+		return 8888, nil
+	}
+	_, port, netErr := net.SplitHostPort(s.GetTelemetry().Metrics.Address)
+	if netErr != nil && strings.Contains(netErr.Error(), "missing port in address") {
+		return 8888, nil
+	} else if netErr != nil {
+		return 0, netErr
+	}
+	i64, err := strconv.ParseInt(port, 10, 32)
+	if err != nil {
+		return 0, err
+	}
+
+	return int32(i64), nil
+}
+
 // MetricsConfig comes from the collector.
 type MetricsConfig struct {
 	// Level is the level of telemetry metrics, the possible values are:

apis/v1beta1/config_test.go

@@ -210,3 +210,71 @@ func TestGetTelemetryFromYAMLIsNil(t *testing.T) {
 	require.NoError(t, err)
 	assert.Nil(t, cfg.Service.GetTelemetry())
 }
+
+func TestConfigToMetricsPort(t *testing.T) {
+
+	for _, tt := range []struct {
+		desc         string
+		expectedPort int32
+		config       Service
+	}{
+		{
+			"custom port",
+			9090,
+			Service{
+				Telemetry: &AnyConfig{
+					Object: map[string]interface{}{
+						"metrics": map[string]interface{}{
+							"address": "0.0.0.0:9090",
+						},
+					},
+				},
+			},
+		},
+		{
+			"bad address",
+			8888,
+			Service{
+				Telemetry: &AnyConfig{
+					Object: map[string]interface{}{
+						"metrics": map[string]interface{}{
+							"address": "0.0.0.0",
+						},
+					},
+				},
+			},
+		},
+		{
+			"missing address",
+			8888,
+			Service{
+				Telemetry: &AnyConfig{
+					Object: map[string]interface{}{
+						"metrics": map[string]interface{}{
+							"level": "detailed",
+						},
+					},
+				},
+			},
+		},
+		{
+			"missing metrics",
+			8888,
+			Service{
+				Telemetry: &AnyConfig{},
+			},
+		},
+		{
+			"missing telemetry",
+			8888,
+			Service{},
+		},
+	} {
+		t.Run(tt.desc, func(t *testing.T) {
+			// these are acceptable failures, we return to the collector's default metric port
+			port, err := tt.config.MetricsPort()
+			assert.NoError(t, err)
+			assert.Equal(t, tt.expectedPort, port)
+		})
+	}
+}

internal/manifests/collector/adapters/config_to_ports.go

@@ -16,10 +16,7 @@ package adapters
 
 import (
 	"fmt"
-	"net"
 	"sort"
-	"strconv"
-	"strings"
 
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
@@ -142,23 +139,3 @@ func ConfigToPorts(logger logr.Logger, config map[interface{}]interface{}) ([]co
 
 	return ports, nil
 }
-
-// ConfigToMetricsPort gets the port number for the metrics endpoint from the collector config if it has been set.
-func ConfigToMetricsPort(config v1beta1.Service) (int32, error) {
-	if config.GetTelemetry() == nil {
-		// telemetry isn't set, use the default
-		return 8888, nil
-	}
-	_, port, netErr := net.SplitHostPort(config.GetTelemetry().Metrics.Address)
-	if netErr != nil && strings.Contains(netErr.Error(), "missing port in address") {
-		return 8888, nil
-	} else if netErr != nil {
-		return 0, netErr
-	}
-	i64, err := strconv.ParseInt(port, 10, 32)
-	if err != nil {
-		return 0, err
-	}
-
-	return int32(i64), nil
-}

internal/manifests/collector/adapters/config_to_ports_test.go

@@ -25,7 +25,6 @@ import (
 	"k8s.io/apimachinery/pkg/util/intstr"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
-	"github.com/open-telemetry/opentelemetry-operator/apis/v1beta1"
 	"github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters"
 	"github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/parser"
 	"github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/parser/receiver"
@@ -96,11 +95,6 @@ func TestExtractPortsFromConfig(t *testing.T) {
 	targetPort4317 := intstr.IntOrString{Type: 0, IntVal: 4317, StrVal: ""}
 	targetPort4318 := intstr.IntOrString{Type: 0, IntVal: 4318, StrVal: ""}
 
-	svcPorts := []corev1.ServicePort{}
-	for _, p := range ports {
-		svcPorts = append(svcPorts, p)
-	}
-
 	expectedPorts := []corev1.ServicePort{
 		{Name: "examplereceiver", Port: 12345},
 		{Name: "port-12346", Port: 12346},
@@ -113,7 +107,7 @@ func TestExtractPortsFromConfig(t *testing.T) {
 		{Name: "otlp-http", AppProtocol: &httpAppProtocol, Port: 4318, TargetPort: targetPort4318},
 		{Name: "zipkin", AppProtocol: &httpAppProtocol, Protocol: "TCP", Port: 9411},
 	}
-	assert.ElementsMatch(t, expectedPorts, svcPorts)
+	assert.ElementsMatch(t, expectedPorts, ports)
 }
 
 func TestNoPortsParsed(t *testing.T) {
@@ -211,73 +205,6 @@ func TestParserFailed(t *testing.T) {
 	assert.NoError(t, err)
 	assert.True(t, mockParserCalled)
 }
-func TestConfigToMetricsPort(t *testing.T) {
-
-	for _, tt := range []struct {
-		desc         string
-		expectedPort int32
-		config       v1beta1.Service
-	}{
-		{
-			"custom port",
-			9090,
-			v1beta1.Service{
-				Telemetry: &v1beta1.AnyConfig{
-					Object: map[string]interface{}{
-						"metrics": map[string]interface{}{
-							"address": "0.0.0.0:9090",
-						},
-					},
-				},
-			},
-		},
-		{
-			"bad address",
-			8888,
-			v1beta1.Service{
-				Telemetry: &v1beta1.AnyConfig{
-					Object: map[string]interface{}{
-						"metrics": map[string]interface{}{
-							"address": "0.0.0.0",
-						},
-					},
-				},
-			},
-		},
-		{
-			"missing address",
-			8888,
-			v1beta1.Service{
-				Telemetry: &v1beta1.AnyConfig{
-					Object: map[string]interface{}{
-						"metrics": map[string]interface{}{
-							"level": "detailed",
-						},
-					},
-				},
-			},
-		},
-		{
-			"missing metrics",
-			8888,
-			v1beta1.Service{
-				Telemetry: &v1beta1.AnyConfig{},
-			},
-		},
-		{
-			"missing telemetry",
-			8888,
-			v1beta1.Service{},
-		},
-	} {
-		t.Run(tt.desc, func(t *testing.T) {
-			// these are acceptable failures, we return to the collector's default metric port
-			port, err := adapters.ConfigToMetricsPort(tt.config)
-			assert.NoError(t, err)
-			assert.Equal(t, tt.expectedPort, port)
-		})
-	}
-}
 
 type mockParser struct {
 	portsFunc func() ([]corev1.ServicePort, error)

internal/manifests/collector/container.go

@@ -202,7 +202,7 @@ func getConfigContainerPorts(logger logr.Logger, cfgYaml string, conf v1beta1.Co
 		}
 	}
 
-	metricsPort, err := adapters.ConfigToMetricsPort(conf.Service)
+	metricsPort, err := conf.Service.MetricsPort()
 	if err != nil {
 		logger.Info("couldn't determine metrics port from configuration, using 8888 default value", "error", err)
 		metricsPort = 8888

internal/manifests/collector/ingress.go

@@ -171,12 +171,7 @@ func servicePortsFromCfg(logger logr.Logger, otelcol v1beta1.OpenTelemetryCollec
 		ports = append(toServicePorts(otelcol.Spec.Ports), resultingInferredPorts...)
 	}
 
-	svcPorts := []corev1.ServicePort{}
-	for _, p := range ports {
-		svcPorts = append(svcPorts, p)
-	}
-
-	return svcPorts, err
+	return ports, nil
 }
 
 func toServicePorts(spec []v1beta1.PortsSpec) []corev1.ServicePort {

internal/manifests/collector/service.go

@@ -64,7 +64,7 @@ func MonitoringService(params manifests.Params) (*corev1.Service, error) {
 	labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, []string{})
 	labels[monitoringLabel] = valueExists
 
-	metricsPort, err := adapters.ConfigToMetricsPort(params.OtelCol.Spec.Config.Service)
+	metricsPort, err := params.OtelCol.Spec.Config.Service.MetricsPort()
 	if err != nil {
 		return nil, err
 	}
@@ -147,11 +147,6 @@ func Service(params manifests.Params) (*corev1.Service, error) {
 		trafficPolicy = corev1.ServiceInternalTrafficPolicyLocal
 	}
 
-	svcPorts := []corev1.ServicePort{}
-	for _, p := range ports {
-		svcPorts = append(svcPorts, p)
-	}
-
 	return &corev1.Service{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:        naming.Service(params.OtelCol.Name),
@@ -163,7 +158,7 @@ func Service(params manifests.Params) (*corev1.Service, error) {
 			InternalTrafficPolicy: &trafficPolicy,
 			Selector:              manifestutils.SelectorLabels(params.OtelCol.ObjectMeta, ComponentOpenTelemetryCollector),
 			ClusterIP:             "",
-			Ports:                 svcPorts,
+			Ports:                 ports,
 		},
 	}, nil
 }