Sort order of ports returned to fix flaky tests (#1003)

* Sort order of ports returned to fix flaky tests Signed-off-by: Kevin Earls <kearls@redhat.com> * Verify type of receiver name Signed-off-by: Kevin Earls <kearls@redhat.com> * Sort ports too before returning Signed-off-by: Kevin Earls <kearls@redhat.com> * Add unit test Signed-off-by: Kevin Earls <kearls@redhat.com> * respond to comments Signed-off-by: Kevin Earls <kearls@redhat.com> * fix nits Signed-off-by: Kevin Earls <kearls@redhat.com>
open-telemetry · Aug 3, 2022 · fc3cae5 · fc3cae5
1 parent d91ecc2
commit fc3cae5
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 54 deletions.
diff --git a/pkg/collector/adapters/config_to_ports.go b/pkg/collector/adapters/config_to_ports.go
@@ -16,6 +16,7 @@ package adapters
 
 import (
 	"errors"
+	"sort"
 
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
@@ -89,5 +90,10 @@ func ConfigToReceiverPorts(logger logr.Logger, config map[interface{}]interface{
 			ports = append(ports, rcvrPorts...)
 		}
 	}
+
+	sort.Slice(ports, func(i, j int) bool {
+		return ports[i].Name < ports[j].Name
+	})
+
 	return ports, nil
 }
diff --git a/pkg/collector/adapters/config_to_ports_test.go b/pkg/collector/adapters/config_to_ports_test.go
@@ -22,6 +22,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
 	"github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters"
@@ -87,61 +88,24 @@ service:
 	assert.Len(t, ports, 11)
 
 	// verify
-	expectedPorts := map[int32]bool{}
-	expectedPorts[int32(12345)] = false
-	expectedPorts[int32(12346)] = false
-	expectedPorts[int32(14250)] = false
-	expectedPorts[int32(6831)] = false
-	expectedPorts[int32(6833)] = false
-	expectedPorts[int32(15268)] = false
-	expectedPorts[int32(4318)] = false
-	expectedPorts[int32(55681)] = false
-	expectedPorts[int32(55555)] = false
-	expectedPorts[int32(9411)] = false
+	httpAppProtocol := "http"
+	grpcAppProtocol := "grpc"
+	targetPortZero := intstr.IntOrString{Type: 0, IntVal: 0, StrVal: ""}
+	targetPort4317 := intstr.IntOrString{Type: 0, IntVal: 4317, StrVal: ""}
+	targetPort4318 := intstr.IntOrString{Type: 0, IntVal: 4318, StrVal: ""}
 
-	expectedNames := map[string]bool{}
-	expectedNames["examplereceiver"] = false
-	expectedNames["examplereceiver-settings"] = false
-	expectedNames["jaeger-grpc"] = false
-	expectedNames["jaeger-thrift-compact"] = false
-	expectedNames["jaeger-thrift-binary"] = false
-	expectedNames["jaeger-custom-thrift-http"] = false
-	expectedNames["otlp-grpc"] = false
-	expectedNames["otlp-http"] = false
-	expectedNames["otlp-http-legacy"] = false
-	expectedNames["otlp-2-grpc"] = false
-	expectedNames["zipkin"] = false
-
-	expectedAppProtocols := map[string]string{}
-	expectedAppProtocols["otlp-grpc"] = "grpc"
-	expectedAppProtocols["otlp-http"] = "http"
-	expectedAppProtocols["otlp-http-legacy"] = "http"
-	expectedAppProtocols["jaeger-custom-thrift-http"] = "http"
-	expectedAppProtocols["jaeger-grpc"] = "grpc"
-	expectedAppProtocols["otlp-2-grpc"] = "grpc"
-	expectedAppProtocols["zipkin"] = "http"
-
-	// make sure we only have the ports in the set
-	for _, port := range ports {
-		assert.NotNil(t, expectedPorts[port.Port])
-		assert.NotNil(t, expectedNames[port.Name])
-		expectedPorts[port.Port] = true
-		expectedNames[port.Name] = true
-
-		if appProtocol, ok := expectedAppProtocols[port.Name]; ok {
-			assert.Equal(t, appProtocol, *port.AppProtocol)
-		}
-	}
-
-	// and make sure all the ports from the set are there
-	for _, val := range expectedPorts {
-		assert.True(t, val)
-	}
-
-	// make sure we only have the ports names in the set
-	for _, val := range expectedNames {
-		assert.True(t, val)
-	}
+	assert.Len(t, ports, 11)
+	assert.Equal(t, corev1.ServicePort{Name: "examplereceiver", Port: int32(12345)}, ports[0])
+	assert.Equal(t, corev1.ServicePort{Name: "examplereceiver-settings", Port: int32(12346)}, ports[1])
+	assert.Equal(t, corev1.ServicePort{Name: "jaeger-custom-thrift-http", AppProtocol: &httpAppProtocol, Protocol: "TCP", Port: int32(15268), TargetPort: targetPortZero}, ports[2])
+	assert.Equal(t, corev1.ServicePort{Name: "jaeger-grpc", AppProtocol: &grpcAppProtocol, Protocol: "TCP", Port: int32(14250)}, ports[3])
+	assert.Equal(t, corev1.ServicePort{Name: "jaeger-thrift-binary", Protocol: "UDP", Port: int32(6833)}, ports[4])
+	assert.Equal(t, corev1.ServicePort{Name: "jaeger-thrift-compact", Protocol: "UDP", Port: int32(6831)}, ports[5])
+	assert.Equal(t, corev1.ServicePort{Name: "otlp-2-grpc", AppProtocol: &grpcAppProtocol, Protocol: "TCP", Port: int32(55555)}, ports[6])
+	assert.Equal(t, corev1.ServicePort{Name: "otlp-grpc", AppProtocol: &grpcAppProtocol, Port: int32(4317), TargetPort: targetPort4317}, ports[7])
+	assert.Equal(t, corev1.ServicePort{Name: "otlp-http", AppProtocol: &httpAppProtocol, Port: int32(4318), TargetPort: targetPort4318}, ports[8])
+	assert.Equal(t, corev1.ServicePort{Name: "otlp-http-legacy", AppProtocol: &httpAppProtocol, Port: int32(55681), TargetPort: targetPort4318}, ports[9])
+	assert.Equal(t, corev1.ServicePort{Name: "zipkin", AppProtocol: &httpAppProtocol, Protocol: "TCP", Port: int32(9411)}, ports[10])
 }
 
 func TestNoPortsParsed(t *testing.T) {