Fix service port naming convention (#1368)

Signed-off-by: Megrez Lu <lujiajing1126@gmail.com>

.ci/run-e2e-tests.sh

@@ -70,6 +70,10 @@ then
     export SPECIFY_OTEL_IMAGES=true
     export SPECIFY_OTEL_CONFIG=true
     make e2e-tests-smoke
+elif [ "${TEST_GROUP}" = "istio" ]
+then
+    echo "Running Smoke Tests with istio"
+    make e2e-tests-istio
 else
     echo "Unknown TEST_GROUP [${TEST_GROUP}]"; exit 1
 fi

.github/workflows/e2e-kubernetes.yaml

@@ -6,7 +6,7 @@ jobs:
     runs-on: ubuntu-16.04
     strategy:
       matrix:
-        TEST_GROUP: [smoke, es, cassandra, streaming, examples1, examples2, generate, es-otel, streaming-otel, smoke-otel, upgrade]
+        TEST_GROUP: [smoke, es, cassandra, streaming, examples1, examples2, generate, es-otel, streaming-otel, smoke-otel, upgrade, istio]
     steps:
     - uses: actions/setup-go@v1
       with:

Makefile

@@ -25,9 +25,13 @@ ES_OPERATOR_NAMESPACE ?= openshift-logging
 ES_OPERATOR_BRANCH ?= release-4.4
 ES_OPERATOR_IMAGE ?= quay.io/openshift/origin-elasticsearch-operator:4.4
 SDK_VERSION=v0.18.2
+ISTIO_VERSION ?= 1.8.2
+ISTIOCTL="./deploy/test/istio/bin/istioctl"
 GOPATH ?= "$(HOME)/go"
 GOROOT ?= "$(shell go env GOROOT)"
 
+SED ?= "sed"
+
 PROMETHEUS_OPERATOR_TAG ?= v0.39.0
 PROMETHEUS_BUNDLE ?= https://raw.githubusercontent.com/prometheus-operator/prometheus-operator/${PROMETHEUS_OPERATOR_TAG}/bundle.yaml
 
@@ -115,7 +119,7 @@ prepare-e2e-tests: build docker push
 	@cat deploy/role_binding.yaml >> deploy/test/namespace-manifests.yaml
 	@echo "---" >> deploy/test/namespace-manifests.yaml
 
-	@sed "s~image: jaegertracing\/jaeger-operator\:.*~image: $(BUILD_IMAGE)~gi" test/operator.yaml >> deploy/test/namespace-manifests.yaml
+	@${SED} "s~image: jaegertracing\/jaeger-operator\:.*~image: $(BUILD_IMAGE)~gi" test/operator.yaml >> deploy/test/namespace-manifests.yaml
 
 	@cp deploy/crds/jaegertracing.io_jaegers_crd.yaml deploy/test/global-manifests.yaml
 	@echo "---" >> deploy/test/global-manifests.yaml
@@ -194,6 +198,11 @@ e2e-tests-upgrade: prepare-e2e-tests
 	@echo Running Upgrade end-to-end tests...
 	UPGRADE_TEST_VERSION=$(shell .ci/get_test_upgrade_version.sh ${JAEGER_VERSION}) go test -tags=upgrade  ./test/e2e/... $(TEST_OPTIONS)
 
+.PHONY: e2e-tests-istio
+e2e-tests-istio: prepare-e2e-tests istio
+	@echo Running Istio end-to-end tests...
+	@STORAGE_NAMESPACE=$(STORAGE_NAMESPACE) KAFKA_NAMESPACE=$(KAFKA_NAMESPACE) go test -tags=istio ./test/e2e/... $(TEST_OPTIONS)
+
 .PHONY: run
 run: crd
 	@rm -rf /tmp/_cert*
@@ -250,6 +259,19 @@ else
 	@kubectl create -f ./test/elasticsearch.yml --namespace $(STORAGE_NAMESPACE) 2>&1 | grep -v "already exists" || true
 endif
 
+.PHONY: istio
+istio:
+	@echo Install istio with minimal profile
+	@mkdir -p deploy/test
+	@[ -f "${ISTIOCTL}" ] || (curl -L https://istio.io/downloadIstio | ISTIO_VERSION=${ISTIO_VERSION} TARGET_ARCH=x86_64 sh - && mv ./istio-${ISTIO_VERSION} ./deploy/test/istio)
+	@${ISTIOCTL} install --set profile=minimal -y
+
+.PHONY: undeploy-istio
+undeploy-istio:
+	@[ -f "${ISTIOCTL}" ] && (${ISTIOCTL} manifest generate --set profile=demo | kubectl delete --ignore-not-found=true -f -) || true
+	@kubectl delete namespace istio-system --ignore-not-found=true || true
+	@rm -rf deploy/test/istio
+
 .PHONY: cassandra
 cassandra: storage
 	@kubectl create -f ./test/cassandra.yml --namespace $(STORAGE_NAMESPACE) 2>&1 | grep -v "already exists" || true
@@ -270,7 +292,7 @@ else
 	@kubectl create clusterrolebinding strimzi-cluster-operator-entity-operator-delegation --clusterrole=strimzi-entity-operator --serviceaccount ${KAFKA_NAMESPACE}:strimzi-cluster-operator 2>&1 | grep -v "already exists" || true
 	@kubectl create clusterrolebinding strimzi-cluster-operator-topic-operator-delegation --clusterrole=strimzi-topic-operator --serviceaccount ${KAFKA_NAMESPACE}:strimzi-cluster-operator 2>&1 | grep -v "already exists" || true
 	@curl --fail --location $(KAFKA_YAML) --output deploy/test/kafka-operator.yaml --create-dirs
-	@sed 's/namespace: .*/namespace: $(KAFKA_NAMESPACE)/' deploy/test/kafka-operator.yaml | kubectl -n $(KAFKA_NAMESPACE) apply -f - 2>&1 | grep -v "already exists" || true
+	@${SED} 's/namespace: .*/namespace: $(KAFKA_NAMESPACE)/' deploy/test/kafka-operator.yaml | kubectl -n $(KAFKA_NAMESPACE) apply -f - 2>&1 | grep -v "already exists" || true
 	@kubectl set env deployment strimzi-cluster-operator -n ${KAFKA_NAMESPACE} STRIMZI_NAMESPACE="*"
 endif
 
@@ -294,7 +316,7 @@ else
 	@echo Creating namespace $(KAFKA_NAMESPACE)
 	@kubectl create namespace $(KAFKA_NAMESPACE) 2>&1 | grep -v "already exists" || true
 	@curl --fail --location $(KAFKA_EXAMPLE) --output deploy/test/kafka-example.yaml --create-dirs
-	@sed -i 's/size: 100Gi/size: 10Gi/g' deploy/test/kafka-example.yaml
+	@${SED} -i 's/size: 100Gi/size: 10Gi/g' deploy/test/kafka-example.yaml
 	@kubectl -n $(KAFKA_NAMESPACE) apply --dry-run=true -f deploy/test/kafka-example.yaml
 	@kubectl -n $(KAFKA_NAMESPACE) apply -f deploy/test/kafka-example.yaml 2>&1 | grep -v "already exists" || true
 endif
@@ -321,7 +343,7 @@ else
 endif
 
 .PHONY: clean
-clean: undeploy-kafka undeploy-es-operator undeploy-prometheus-operator
+clean: undeploy-kafka undeploy-es-operator undeploy-prometheus-operator undeploy-istio
 	@rm -f deploy/test/*.yaml
 	@if [ -d deploy/test ]; then rmdir deploy/test ; fi
 	@kubectl delete -f ./test/cassandra.yml --ignore-not-found=true -n $(STORAGE_NAMESPACE) || true
@@ -383,7 +405,7 @@ deploy: ingress crd
 	@kubectl apply -f deploy/service_account.yaml
 	@kubectl apply -f deploy/cluster_role.yaml
 	@kubectl apply -f deploy/cluster_role_binding.yaml
-	@sed "s~image: jaegertracing\/jaeger-operator\:.*~image: $(BUILD_IMAGE)~gi" deploy/operator.yaml | kubectl apply -f -
+	@${SED} "s~image: jaegertracing\/jaeger-operator\:.*~image: $(BUILD_IMAGE)~gi" deploy/operator.yaml | kubectl apply -f -
 
 .PHONY: operatorhub
 operatorhub: check-operatorhub-pr-template

pkg/service/collector.go

@@ -97,31 +97,31 @@ func GetNameForHeadlessCollectorService(jaeger *v1.Jaeger) string {
 func GetPortNameForGRPC(jaeger *v1.Jaeger) string {
 	if viper.GetString("platform") == v1.FlagPlatformOpenShift {
 		// we always have TLS certs when running on OpenShift, so, TLS is always enabled
-		return "https-grpc"
+		return "grpc-https"
 	}
 
 	// if we don't have a jaeger provided, it's certainly not TLS...
 	if nil == jaeger {
-		return "http-grpc"
+		return "grpc-http"
 	}
 
 	// 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.Map()["collector.grpc.tls.enabled"]; ok {
 		enabled, err := strconv.ParseBool(val)
 		if err != nil {
-			return "http-grpc" // not "true", defaults to false
+			return "grpc-http" // not "true", defaults to false
 		}
 
 		if enabled {
-			return "https-grpc" // explicit true
+			return "grpc-https" // explicit true
 		}
 
-		return "http-grpc" // explicit false
+		return "grpc-http" // explicit false
 	}
 
 	// doesn't look like we have TLS enabled
-	return "http-grpc"
+	return "grpc-http"
 }
 
 func getTypeForCollectorService(jaeger *v1.Jaeger) corev1.ServiceType {

pkg/service/collector_test.go

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

test/e2e/examples1_test.go

@@ -3,9 +3,7 @@
 package e2e
 
 import (
-	"context"
 	"encoding/json"
-	"errors"
 	"io/ioutil"
 	"net/http"
 	"os"
@@ -16,13 +14,10 @@ import (
 	"time"
 
 	framework "github.com/operator-framework/operator-sdk/pkg/test"
-	log "github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
-	"k8s.io/apimachinery/pkg/util/wait"
 )
 
 type ExamplesTestSuite struct {
@@ -109,7 +104,7 @@ func (suite *ExamplesTestSuite) TestBusinessApp() {
 	require.NoError(t, err)
 
 	// Now deploy examples/business-application-injected-sidecar.yaml
-	businessAppCR := getBusinessAppCR(err)
+	businessAppCR := getBusinessAppCR()
 	defer os.Remove(businessAppCR.Name())
 	cmd := exec.Command("kubectl", "create", "--namespace", namespace, "--filename", businessAppCR.Name())
 	output, err := cmd.CombinedOutput()
@@ -126,23 +121,8 @@ func (suite *ExamplesTestSuite) TestBusinessApp() {
 	handler := &corev1.Handler{HTTPGet: livelinessHandler}
 	livelinessProbe := &corev1.Probe{Handler: *handler, InitialDelaySeconds: 1, FailureThreshold: 3, PeriodSeconds: 10, SuccessThreshold: 1, TimeoutSeconds: 1}
 
-	err = wait.Poll(retryInterval, timeout, func() (done bool, err error) {
-		vertxDeployment, err := fw.KubeClient.AppsV1().Deployments(namespace).Get(context.Background(), vertxDeploymentName, metav1.GetOptions{})
-		require.NoError(t, err)
-		containers := vertxDeployment.Spec.Template.Spec.Containers
-		for index, container := range containers {
-			if container.Name == vertxDeploymentName {
-				vertxDeployment.Spec.Template.Spec.Containers[index].LivenessProbe = livelinessProbe
-				updatedVertxDeployment, err := fw.KubeClient.AppsV1().Deployments(namespace).Update(context.Background(), vertxDeployment, metav1.UpdateOptions{})
-				if err != nil {
-					log.Warnf("Error %v updating vertx app, retrying", err)
-					return false, nil
-				}
-				log.Infof("Updated deployment %v", updatedVertxDeployment.Name)
-				return true, nil
-			}
-		}
-		return false, errors.New("Vertx deployment not found")
+	err = waitForDeploymentAndUpdate(vertxDeploymentName, vertxDeploymentName, func(container *corev1.Container) {
+		container.LivenessProbe = livelinessProbe
 	})
 	require.NoError(t, err)
 
@@ -173,17 +153,6 @@ func (suite *ExamplesTestSuite) TestBusinessApp() {
 	require.NoError(t, err, "SmokeTest failed")
 }
 
-func getBusinessAppCR(err error) *os.File {
-	content, err := ioutil.ReadFile("../../examples/business-application-injected-sidecar.yaml")
-	require.NoError(t, err)
-	newContent := strings.Replace(string(content), "image: jaegertracing/vertx-create-span:operator-e2e-tests", "image: "+vertxExampleImage, 1)
-	file, err := ioutil.TempFile("", "vertx-example")
-	require.NoError(t, err)
-	err = ioutil.WriteFile(file.Name(), []byte(newContent), 0666)
-	require.NoError(t, err)
-	return file
-}
-
 func execOcCommand(args ...string) {
 	cmd := exec.Command("oc", args...)
 	output, err := cmd.CombinedOutput()