Fix: svc port doesnt match istio convention (#2101)

* fix https port name prefix to match istio naming convention Signed-off-by: Benedikt Bongartz <[email protected]> * verify service port names in e2e test Signed-off-by: Benedikt Bongartz <[email protected]> Signed-off-by: Benedikt Bongartz <[email protected]>
jaegertracing · Nov 3, 2022 · 3101ed1 · 3101ed1
1 parent 412329a
commit 3101ed1
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 13 deletions.
diff --git a/pkg/service/collector.go b/pkg/service/collector.go
@@ -96,36 +96,41 @@ func GetNameForHeadlessCollectorService(jaeger *v1.Jaeger) string {
 	return util.DNSName(util.Truncate("%s-collector-headless", 63, jaeger.Name))
 }
 
-// GetPortNameForGRPC returns the port name for 'grpc'. It may either be http-grpc or https-grpc, based on whether
+// GetPortNameForGRPC returns the port name for 'grpc'. It may either be
+// tls-grpc-jaeger (secure) or grpc-jaeger (insecure), based on whether
 // TLS is enabled for the agent-collector gRPC communication
 func GetPortNameForGRPC(jaeger *v1.Jaeger) string {
+	const (
+		protoSecure   = "tls-grpc-jaeger"
+		protoInsecure = "grpc-jaeger"
+	)
 	if viper.GetString("platform") == v1.FlagPlatformOpenShift {
 		// we always have TLS certs when running on OpenShift, so, TLS is always enabled
-		return "grpc-https"
+		return protoSecure
 	}
 
 	// if we don't have a jaeger provided, it's certainly not TLS...
 	if nil == jaeger {
-		return "grpc-http"
+		return protoInsecure
 	}
 
 	// perhaps the user has provisioned the certs and configured the CR manually?
 	// for that, we check whether the CLI option `collector.grpc.tls.enabled` was set for the collector
 	if val, ok := jaeger.Spec.Collector.Options.StringMap()["collector.grpc.tls.enabled"]; ok {
 		enabled, err := strconv.ParseBool(val)
 		if err != nil {
-			return "grpc-http" // not "true", defaults to false
+			return protoInsecure // not "true", defaults to false
 		}
 
 		if enabled {
-			return "grpc-https" // explicit true
+			return protoSecure // explicit true
 		}
 
-		return "grpc-http" // explicit false
+		return protoInsecure // explicit false
 	}
 
 	// doesn't look like we have TLS enabled
-	return "grpc-http"
+	return protoInsecure
 }
 
 func getTypeForCollectorService(jaeger *v1.Jaeger) corev1.ServiceType {

diff --git a/pkg/service/collector_test.go b/pkg/service/collector_test.go
@@ -66,13 +66,13 @@ func TestCollectorGRPCPortName(t *testing.T) {
 		{
 			"nil",
 			nil,
-			"grpc-http",
+			"grpc-jaeger",
 			false, // in openshift?
 		},
 		{
 			"no-tls",
 			&v1.Jaeger{},
-			"grpc-http",
+			"grpc-jaeger",
 			false, // in openshift?
 		},
 		{
@@ -84,7 +84,7 @@ func TestCollectorGRPCPortName(t *testing.T) {
 					},
 				},
 			},
-			"grpc-http",
+			"grpc-jaeger",
 			false, // in openshift?
 		},
 		{
@@ -96,7 +96,7 @@ func TestCollectorGRPCPortName(t *testing.T) {
 					},
 				},
 			},
-			"grpc-http",
+			"grpc-jaeger",
 			false, // in openshift?
 		},
 		{
@@ -108,13 +108,13 @@ func TestCollectorGRPCPortName(t *testing.T) {
 					},
 				},
 			},
-			"grpc-https",
+			"tls-grpc-jaeger",
 			false, // in openshift?
 		},
 		{
 			"in-openshift",
 			&v1.Jaeger{},
-			"grpc-https",
+			"tls-grpc-jaeger",
 			true, // in openshift?
 		},
 	} {

diff --git a/tests/e2e/miscellaneous/istio/03-assert.yaml b/tests/e2e/miscellaneous/istio/03-assert.yaml
@@ -7,3 +7,65 @@ spec:
     - name: myapp
     - name: jaeger-agent
     - name: istio-proxy
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: simplest-collector-headless
+spec:
+  ports:
+  - name: http-zipkin
+    port: 9411
+    protocol: TCP
+    targetPort: 9411
+  - name: grpc-jaeger
+    port: 14250
+    protocol: TCP
+    targetPort: 14250
+  - name: http-c-tchan-trft
+    port: 14267
+    protocol: TCP
+    targetPort: 14267
+  - name: http-c-binary-trft
+    port: 14268
+    protocol: TCP
+    targetPort: 14268
+  - name: grpc-otlp
+    port: 4317
+    protocol: TCP
+    targetPort: 4317
+  - name: http-otlp
+    port: 4318
+    protocol: TCP
+    targetPort: 4318
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: simplest-collector
+spec:
+  ports:
+  - name: http-zipkin
+    port: 9411
+    protocol: TCP
+    targetPort: 9411
+  - name: grpc-jaeger
+    port: 14250
+    protocol: TCP
+    targetPort: 14250
+  - name: http-c-tchan-trft
+    port: 14267
+    protocol: TCP
+    targetPort: 14267
+  - name: http-c-binary-trft
+    port: 14268
+    protocol: TCP
+    targetPort: 14268
+  - name: grpc-otlp
+    port: 4317
+    protocol: TCP
+    targetPort: 4317
+  - name: http-otlp
+    port: 4318
+    protocol: TCP
+    targetPort: 4318