From 9cc07234866dad9c455a89af08effe736416f5bb Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Mon, 18 Jan 2021 18:27:27 +0800 Subject: [PATCH 01/22] add istio test case and makefile Signed-off-by: Megrez Lu --- Makefile | 32 +++++++-- test/e2e/istio_test.go | 151 +++++++++++++++++++++++++++++++++++++++++ test/e2e/utils.go | 2 +- 3 files changed, 179 insertions(+), 6 deletions(-) create mode 100644 test/e2e/istio_test.go diff --git a/Makefile b/Makefile index 3ee00f63b..4f55889b5 100644 --- a/Makefile +++ b/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 diff --git a/test/e2e/istio_test.go b/test/e2e/istio_test.go new file mode 100644 index 000000000..add8288e2 --- /dev/null +++ b/test/e2e/istio_test.go @@ -0,0 +1,151 @@ +// +build istio + +package e2e + +import ( + "context" + "encoding/json" + "errors" + "io/ioutil" + "net/http" + "os" + "os/exec" + "strconv" + "strings" + "testing" + "time" + + framework "github.com/operator-framework/operator-sdk/pkg/test" + log "github.com/sirupsen/logrus" + 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" + + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" +) + +type IstioTestSuite struct { + suite.Suite +} + +// LIFECYCLE - Suite +func (suite *IstioTestSuite) SetupSuite() { + t = suite.T() + var err error + ctx, err = prepare(t) + if err != nil { + if ctx != nil { + ctx.Cleanup() + } + require.FailNow(t, "Failed in prepare") + } + fw = framework.Global + namespace = ctx.GetID() + require.NotNil(t, namespace, "GetID failed") + + addToFrameworkSchemeForSmokeTests(t) +} + +func (suite *IstioTestSuite) TearDownSuite() { + handleSuiteTearDown() +} + +// LIFECYCLE - Test + +func (suite *IstioTestSuite) SetupTest() { + t = suite.T() +} + +func (suite *IstioTestSuite) AfterTest(suiteName, testName string) { + handleTestFailure() +} + +func TestIstioSuite(t *testing.T) { + suite.Run(t, new(IstioTestSuite)) +} + +func (suite *IstioTestSuite) TestEnvoySidecar() { + // First deploy a Jaeger instance + jaegerInstanceName := "simplest" + jaegerInstance := createJaegerInstanceFromFile(jaegerInstanceName, "../../examples/simplest.yaml") + defer undeployJaegerInstance(jaegerInstance) + err := WaitForDeployment(t, fw.KubeClient, namespace, jaegerInstanceName, 1, retryInterval, timeout+(1*time.Minute)) + require.NoError(t, err) + + // Now deploy examples/business-application-injected-sidecar.yaml + businessAppCR := getBusinessAppCR(err) + defer os.Remove(businessAppCR.Name()) + cmd := exec.Command("kubectl", "create", "--namespace", namespace, "--filename", businessAppCR.Name()) + output, err := cmd.CombinedOutput() + if err != nil && !strings.Contains(string(output), "AlreadyExists") { + require.NoError(t, err, "Failed creating Jaeger instance with: [%s]\n", string(output)) + } + const vertxDeploymentName = "myapp" + err = WaitForDeployment(t, fw.KubeClient, namespace, vertxDeploymentName, 1, retryInterval, timeout) + require.NoError(t, err, "Failed waiting for myapp deployment") + + // Add a liveliness probe to create some traces + vertxPort := intstr.IntOrString{IntVal: 8080} + livelinessHandler := &corev1.HTTPGetAction{Path: "/", Port: vertxPort, Scheme: corev1.URISchemeHTTP} + 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") + }) + require.NoError(t, err) + + // Confirm that we've created some traces + ports := []string{"0:16686"} + portForward, closeChan := CreatePortForward(namespace, jaegerInstanceName, "all-in-one", ports, fw.KubeConfig) + defer portForward.Close() + defer close(closeChan) + forwardedPorts, err := portForward.GetPorts() + require.NoError(t, err) + queryPort := strconv.Itoa(int(forwardedPorts[0].Local)) + + url := "http://localhost:" + queryPort + "/api/traces?service=order" + err = WaitAndPollForHTTPResponse(url, func(response *http.Response) (bool, error) { + body, err := ioutil.ReadAll(response.Body) + if err != nil { + return false, err + } + + resp := &resp{} + err = json.Unmarshal(body, &resp) + if err != nil { + return false, err + } + + return len(resp.Data) > 0 && strings.Contains(string(body), "traceID"), nil + }) + 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 +} diff --git a/test/e2e/utils.go b/test/e2e/utils.go index a3dcf5c88..8b8d3fd4f 100644 --- a/test/e2e/utils.go +++ b/test/e2e/utils.go @@ -440,7 +440,7 @@ type services struct { Data []string `json:"data"` total int `json:"total"` limit int `json:"limit"` - offset int `json:offset` + offset int `json:"offset"` errors interface{} `json:"errors"` } From e691c9b9c08d34e8387ee9eb85a53f8b685d488c Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Mon, 18 Jan 2021 18:29:22 +0800 Subject: [PATCH 02/22] add istio smoke test to github ci Signed-off-by: Megrez Lu --- .ci/run-e2e-tests.sh | 4 ++++ .github/workflows/e2e-kubernetes.yaml | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/.ci/run-e2e-tests.sh b/.ci/run-e2e-tests.sh index 99173e1a6..f552d5c5c 100755 --- a/.ci/run-e2e-tests.sh +++ b/.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 diff --git a/.github/workflows/e2e-kubernetes.yaml b/.github/workflows/e2e-kubernetes.yaml index e37caf610..3d4ec04f7 100644 --- a/.github/workflows/e2e-kubernetes.yaml +++ b/.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: From 2a77d9d8d740f8505d091b406e4ccca83f41d466 Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Mon, 18 Jan 2021 19:38:10 +0800 Subject: [PATCH 03/22] label ns to enable istio sidecar injection Signed-off-by: Megrez Lu --- test/e2e/istio_test.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/e2e/istio_test.go b/test/e2e/istio_test.go index add8288e2..97c9610b7 100644 --- a/test/e2e/istio_test.go +++ b/test/e2e/istio_test.go @@ -45,6 +45,27 @@ func (suite *IstioTestSuite) SetupSuite() { namespace = ctx.GetID() require.NotNil(t, namespace, "GetID failed") + // label namespace + ns, err := framework.Global.KubeClient.CoreV1().Namespaces().Get(context.Background(), namespace, metav1.GetOptions{}) + if err != nil { + t.Errorf("failed to get the namespaces details: %v", err) + } + labels := ns.GetLabels() + + if labels == nil { + labels = make(map[string]string) + } + + labels["istio-injection"] = "enabled" + + ns.SetLabels(labels) + + ns, err = framework.Global.KubeClient.CoreV1().Namespaces().Update(context.Background(), ns, metav1.UpdateOptions{}) + + if err != nil { + t.Errorf("failed to update labels of the namespace details: %v", err) + } + addToFrameworkSchemeForSmokeTests(t) } From 7a51f100029d716f3d8dc8c476601ccc97030914 Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Tue, 19 Jan 2021 10:46:16 +0800 Subject: [PATCH 04/22] check istio-proxy injection in pod Signed-off-by: Megrez Lu --- test/e2e/istio_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/test/e2e/istio_test.go b/test/e2e/istio_test.go index 97c9610b7..44dfbff8b 100644 --- a/test/e2e/istio_test.go +++ b/test/e2e/istio_test.go @@ -15,6 +15,8 @@ import ( "testing" "time" + "k8s.io/apimachinery/pkg/labels" + framework "github.com/operator-framework/operator-sdk/pkg/test" log "github.com/sirupsen/logrus" corev1 "k8s.io/api/core/v1" @@ -133,6 +135,34 @@ func (suite *IstioTestSuite) TestEnvoySidecar() { }) require.NoError(t, err) + err = wait.Poll(retryInterval, timeout, func() (done bool, err error) { + pods, err := fw.KubeClient.CoreV1().Pods(namespace).List(context.Background(), metav1.ListOptions{ + LabelSelector: labels.FormatLabels(map[string]string{ + "app": vertxDeploymentName, + }), + Limit: 10, + }) + require.NoError(t, err) + + if pods.Size() == 0 { + return false, errors.New("Vertx Pods not found") + } + + for _, pod := range pods.Items { + exist := containerExistsInPod(pod.Spec.Containers, "istio-proxy") + if exist { + log.Infof("Istio-proxy found in pod %s", pod.Name) + return true, nil + } else { + return false, nil + } + } + + return false, errors.New("Unexpected error while checking pods") + }) + + require.NoError(t, err, "Fail to wait for istio-proxy injection") + // Confirm that we've created some traces ports := []string{"0:16686"} portForward, closeChan := CreatePortForward(namespace, jaegerInstanceName, "all-in-one", ports, fw.KubeConfig) @@ -170,3 +200,13 @@ func getBusinessAppCR(err error) *os.File { require.NoError(t, err) return file } + +func containerExistsInPod(containers []corev1.Container, expectedContainerName string) (exist bool) { + exist = false + for _, c := range containers { + if c.Name == expectedContainerName { + exist = true + } + } + return +} From 796102f433c5951c33a36734ae916c70edace915 Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Tue, 19 Jan 2021 21:22:03 +0800 Subject: [PATCH 05/22] change service port name with grpc- prefix to fix istio protocol selection Signed-off-by: Megrez Lu --- pkg/service/collector.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/service/collector.go b/pkg/service/collector.go index a7adc6833..145954c13 100644 --- a/pkg/service/collector.go +++ b/pkg/service/collector.go @@ -102,7 +102,7 @@ func GetPortNameForGRPC(jaeger *v1.Jaeger) string { // 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? @@ -110,18 +110,18 @@ func GetPortNameForGRPC(jaeger *v1.Jaeger) string { 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 { From 3749df73593df9bfcad655c73b5bb2cd6d64cc67 Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Tue, 19 Jan 2021 21:47:39 +0800 Subject: [PATCH 06/22] fix collector test Signed-off-by: Megrez Lu --- pkg/service/collector.go | 2 +- pkg/service/collector_test.go | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/service/collector.go b/pkg/service/collector.go index 145954c13..146407b58 100644 --- a/pkg/service/collector.go +++ b/pkg/service/collector.go @@ -97,7 +97,7 @@ 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... diff --git a/pkg/service/collector_test.go b/pkg/service/collector_test.go index f5f2cd041..02f914ae4 100644 --- a/pkg/service/collector_test.go +++ b/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? }, } { From 551e7e103e781070686ca9447d6864a48ab94899 Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Thu, 21 Jan 2021 12:21:30 +0800 Subject: [PATCH 07/22] format import block Signed-off-by: Megrez Lu --- test/e2e/istio_test.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/test/e2e/istio_test.go b/test/e2e/istio_test.go index 44dfbff8b..c27e38e74 100644 --- a/test/e2e/istio_test.go +++ b/test/e2e/istio_test.go @@ -15,17 +15,15 @@ import ( "testing" "time" - "k8s.io/apimachinery/pkg/labels" - 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/labels" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/wait" - - "github.com/stretchr/testify/require" - "github.com/stretchr/testify/suite" ) type IstioTestSuite struct { From 4df97eefd9e3c1345d1ff6059442a3193d2adbf3 Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Thu, 21 Jan 2021 12:27:39 +0800 Subject: [PATCH 08/22] use require to assert error Signed-off-by: Megrez Lu --- test/e2e/istio_test.go | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/test/e2e/istio_test.go b/test/e2e/istio_test.go index c27e38e74..ad3e67342 100644 --- a/test/e2e/istio_test.go +++ b/test/e2e/istio_test.go @@ -47,24 +47,20 @@ func (suite *IstioTestSuite) SetupSuite() { // label namespace ns, err := framework.Global.KubeClient.CoreV1().Namespaces().Get(context.Background(), namespace, metav1.GetOptions{}) - if err != nil { - t.Errorf("failed to get the namespaces details: %v", err) - } - labels := ns.GetLabels() + require.NoError(t, err, "failed to get the namespaces details: %v", err) + + nsLabels := ns.GetLabels() - if labels == nil { - labels = make(map[string]string) + if nsLabels == nil { + nsLabels = make(map[string]string) } - labels["istio-injection"] = "enabled" + nsLabels["istio-injection"] = "enabled" - ns.SetLabels(labels) + ns.SetLabels(nsLabels) ns, err = framework.Global.KubeClient.CoreV1().Namespaces().Update(context.Background(), ns, metav1.UpdateOptions{}) - - if err != nil { - t.Errorf("failed to update labels of the namespace details: %v", err) - } + require.NoError(t, err, "failed to update labels of the namespace %s", namespace) addToFrameworkSchemeForSmokeTests(t) } From 8bbe66edf66bb61b68e0434ab2223ae0b3d1b816 Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Thu, 21 Jan 2021 12:28:36 +0800 Subject: [PATCH 09/22] format code Signed-off-by: Megrez Lu --- test/e2e/istio_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/e2e/istio_test.go b/test/e2e/istio_test.go index ad3e67342..68e0ae973 100644 --- a/test/e2e/istio_test.go +++ b/test/e2e/istio_test.go @@ -50,13 +50,10 @@ func (suite *IstioTestSuite) SetupSuite() { require.NoError(t, err, "failed to get the namespaces details: %v", err) nsLabels := ns.GetLabels() - if nsLabels == nil { nsLabels = make(map[string]string) } - nsLabels["istio-injection"] = "enabled" - ns.SetLabels(nsLabels) ns, err = framework.Global.KubeClient.CoreV1().Namespaces().Update(context.Background(), ns, metav1.UpdateOptions{}) From 60f49304ec2cded2964dd431bfb99777864842bc Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Thu, 21 Jan 2021 12:34:37 +0800 Subject: [PATCH 10/22] change conditional checks to functions Signed-off-by: Megrez Lu --- test/e2e/istio_test.go | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/test/e2e/istio_test.go b/test/e2e/istio_test.go index 68e0ae973..1d612d532 100644 --- a/test/e2e/istio_test.go +++ b/test/e2e/istio_test.go @@ -134,10 +134,7 @@ func (suite *IstioTestSuite) TestEnvoySidecar() { Limit: 10, }) require.NoError(t, err) - - if pods.Size() == 0 { - return false, errors.New("Vertx Pods not found") - } + require.NotEqual(t, 0, pods.Size(), "Vertx Pods not found") for _, pod := range pods.Items { exist := containerExistsInPod(pod.Spec.Containers, "istio-proxy") @@ -166,15 +163,11 @@ func (suite *IstioTestSuite) TestEnvoySidecar() { url := "http://localhost:" + queryPort + "/api/traces?service=order" err = WaitAndPollForHTTPResponse(url, func(response *http.Response) (bool, error) { body, err := ioutil.ReadAll(response.Body) - if err != nil { - return false, err - } + require.NoError(t, err) resp := &resp{} err = json.Unmarshal(body, &resp) - if err != nil { - return false, err - } + require.NoError(t, err) return len(resp.Data) > 0 && strings.Contains(string(body), "traceID"), nil }) From ebd750f21f7facae5cc0899c99f0265ef54cc932 Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Thu, 21 Jan 2021 12:46:09 +0800 Subject: [PATCH 11/22] move common container check to utils.go Signed-off-by: Megrez Lu --- test/e2e/istio_test.go | 36 +----------------------------------- test/e2e/utils.go | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 35 deletions(-) diff --git a/test/e2e/istio_test.go b/test/e2e/istio_test.go index 1d612d532..59db24021 100644 --- a/test/e2e/istio_test.go +++ b/test/e2e/istio_test.go @@ -21,7 +21,6 @@ import ( "github.com/stretchr/testify/suite" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/wait" ) @@ -126,30 +125,7 @@ func (suite *IstioTestSuite) TestEnvoySidecar() { }) require.NoError(t, err) - err = wait.Poll(retryInterval, timeout, func() (done bool, err error) { - pods, err := fw.KubeClient.CoreV1().Pods(namespace).List(context.Background(), metav1.ListOptions{ - LabelSelector: labels.FormatLabels(map[string]string{ - "app": vertxDeploymentName, - }), - Limit: 10, - }) - require.NoError(t, err) - require.NotEqual(t, 0, pods.Size(), "Vertx Pods not found") - - for _, pod := range pods.Items { - exist := containerExistsInPod(pod.Spec.Containers, "istio-proxy") - if exist { - log.Infof("Istio-proxy found in pod %s", pod.Name) - return true, nil - } else { - return false, nil - } - } - - return false, errors.New("Unexpected error while checking pods") - }) - - require.NoError(t, err, "Fail to wait for istio-proxy injection") + waitForSpecificContainerWithinDeployment(vertxDeploymentName, "istio-proxy") // Confirm that we've created some traces ports := []string{"0:16686"} @@ -184,13 +160,3 @@ func getBusinessAppCR(err error) *os.File { require.NoError(t, err) return file } - -func containerExistsInPod(containers []corev1.Container, expectedContainerName string) (exist bool) { - exist = false - for _, c := range containers { - if c.Name == expectedContainerName { - exist = true - } - } - return -} diff --git a/test/e2e/utils.go b/test/e2e/utils.go index 8b8d3fd4f..f6c49d1fd 100644 --- a/test/e2e/utils.go +++ b/test/e2e/utils.go @@ -3,9 +3,11 @@ package e2e import ( "context" "crypto/tls" + "errors" "fmt" "io" "io/ioutil" + "k8s.io/apimachinery/pkg/labels" "net/http" "os" "os/exec" @@ -861,3 +863,40 @@ func getTracingClientWithCollectorEndpoint(serviceName, collectorEndpoint string } return cfg.NewTracer() } + +func waitForSpecificContainerWithinDeployment(deployment, container string) { + err := wait.Poll(retryInterval, timeout, func() (done bool, err error) { + pods, err := fw.KubeClient.CoreV1().Pods(namespace).List(context.Background(), metav1.ListOptions{ + LabelSelector: labels.FormatLabels(map[string]string{ + "app": deployment, + }), + Limit: 10, + }) + require.NoError(t, err) + require.NotEqual(t, 0, pods.Size(), "%s pods not found", deployment) + + for _, pod := range pods.Items { + exist := containerExistsInPod(pod.Spec.Containers, container) + if exist { + logrus.Infof("%s found in pod %s", container, pod.Name) + return true, nil + } else { + return false, nil + } + } + + return false, errors.New("unexpected error while checking pods") + }) + + require.NoError(t, err, "Fail to wait for istio-proxy injection") +} + +func containerExistsInPod(containers []corev1.Container, expectedContainerName string) (exist bool) { + exist = false + for _, c := range containers { + if c.Name == expectedContainerName { + exist = true + } + } + return +} From aea644a2e954ef8cdc2cc5357420ce5ec976c1d4 Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Thu, 21 Jan 2021 13:42:38 +0800 Subject: [PATCH 12/22] fix lint Signed-off-by: Megrez Lu --- test/e2e/utils.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/utils.go b/test/e2e/utils.go index f6c49d1fd..e67a68eeb 100644 --- a/test/e2e/utils.go +++ b/test/e2e/utils.go @@ -880,9 +880,9 @@ func waitForSpecificContainerWithinDeployment(deployment, container string) { if exist { logrus.Infof("%s found in pod %s", container, pod.Name) return true, nil - } else { - return false, nil } + + return false, nil } return false, errors.New("unexpected error while checking pods") From fd1d059841cd978cf5cd7c68277548c06f43c9a6 Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Thu, 21 Jan 2021 13:44:05 +0800 Subject: [PATCH 13/22] format utils.go Signed-off-by: Megrez Lu --- test/e2e/utils.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/test/e2e/utils.go b/test/e2e/utils.go index e67a68eeb..2db79c09f 100644 --- a/test/e2e/utils.go +++ b/test/e2e/utils.go @@ -7,7 +7,6 @@ import ( "fmt" "io" "io/ioutil" - "k8s.io/apimachinery/pkg/labels" "net/http" "os" "os/exec" @@ -17,23 +16,21 @@ import ( "testing" "time" - "github.com/opentracing/opentracing-go" - "github.com/uber/jaeger-client-go/config" - - "github.com/jaegertracing/jaeger-operator/pkg/apis/kafka/v1beta1" - osv1 "github.com/openshift/api/route/v1" osv1sec "github.com/openshift/api/security/v1" + "github.com/opentracing/opentracing-go" framework "github.com/operator-framework/operator-sdk/pkg/test" "github.com/operator-framework/operator-sdk/pkg/test/e2eutil" "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/uber/jaeger-client-go/config" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" rbac "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/discovery" @@ -42,6 +39,7 @@ import ( "github.com/jaegertracing/jaeger-operator/pkg/apis" v1 "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1" + "github.com/jaegertracing/jaeger-operator/pkg/apis/kafka/v1beta1" "github.com/jaegertracing/jaeger-operator/pkg/util" ) From 250c572855b6be0ef87501dca3241ec8ca24014f Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Thu, 21 Jan 2021 14:03:37 +0800 Subject: [PATCH 14/22] merge pod/container test function Signed-off-by: Megrez Lu --- test/e2e/istio_test.go | 3 ++- test/e2e/utils.go | 57 ++++++++++-------------------------------- 2 files changed, 15 insertions(+), 45 deletions(-) diff --git a/test/e2e/istio_test.go b/test/e2e/istio_test.go index 59db24021..833f04c45 100644 --- a/test/e2e/istio_test.go +++ b/test/e2e/istio_test.go @@ -125,7 +125,8 @@ func (suite *IstioTestSuite) TestEnvoySidecar() { }) require.NoError(t, err) - waitForSpecificContainerWithinDeployment(vertxDeploymentName, "istio-proxy") + exists := testContainerInPod(vertxDeploymentName, "istio-proxy", nil) + require.True(suite.T(), exists) // Confirm that we've created some traces ports := []string{"0:16686"} diff --git a/test/e2e/utils.go b/test/e2e/utils.go index 2db79c09f..a3ba97d44 100644 --- a/test/e2e/utils.go +++ b/test/e2e/utils.go @@ -3,7 +3,6 @@ package e2e import ( "context" "crypto/tls" - "errors" "fmt" "io" "io/ioutil" @@ -30,7 +29,6 @@ import ( rbac "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/discovery" @@ -640,12 +638,18 @@ func wasUsingOtelAllInOne(jaegerInstanceName, namespace string) bool { return false } +// verifyAgentImage test if this Jaeger Instance is using the OTEL agent? func verifyAgentImage(appName, namespace string, expected bool) { - require.Equal(t, expected, wasUsingOtelAgent(appName, namespace)) + require.Equal(t, expected, testContainerInPod(appName, namespace, func(container *corev1.Container) bool { + logrus.Infof("Test %s is using agent image %s", t.Name(), container.Image) + return strings.Contains(container.Image, "jaeger-opentelemetry-agent") + })) } -// Was this Jaeger Instance using the OTEL agent? -func wasUsingOtelAgent(appName, namespace string) bool { +// testContainerInPod is a general function to test if the container exists in the pod +// provided that the pod has `app` label. Return true if and only if the container exists and +// the user-defined function `predicate` returns true if given. +func testContainerInPod(appName, namespace string, predicate func(*corev1.Container) bool) bool { var pods *corev1.PodList var pod corev1.Pod @@ -677,8 +681,10 @@ func wasUsingOtelAgent(appName, namespace string) bool { containers := pod.Spec.Containers for _, container := range containers { if container.Name == "jaeger-agent" { - logrus.Infof("Test %s is using agent image %s", t.Name(), container.Image) - return strings.Contains(container.Image, "jaeger-opentelemetry-agent") + if predicate != nil { + return predicate(&container) + } + return true } } @@ -861,40 +867,3 @@ func getTracingClientWithCollectorEndpoint(serviceName, collectorEndpoint string } return cfg.NewTracer() } - -func waitForSpecificContainerWithinDeployment(deployment, container string) { - err := wait.Poll(retryInterval, timeout, func() (done bool, err error) { - pods, err := fw.KubeClient.CoreV1().Pods(namespace).List(context.Background(), metav1.ListOptions{ - LabelSelector: labels.FormatLabels(map[string]string{ - "app": deployment, - }), - Limit: 10, - }) - require.NoError(t, err) - require.NotEqual(t, 0, pods.Size(), "%s pods not found", deployment) - - for _, pod := range pods.Items { - exist := containerExistsInPod(pod.Spec.Containers, container) - if exist { - logrus.Infof("%s found in pod %s", container, pod.Name) - return true, nil - } - - return false, nil - } - - return false, errors.New("unexpected error while checking pods") - }) - - require.NoError(t, err, "Fail to wait for istio-proxy injection") -} - -func containerExistsInPod(containers []corev1.Container, expectedContainerName string) (exist bool) { - exist = false - for _, c := range containers { - if c.Name == expectedContainerName { - exist = true - } - } - return -} From 40aa30062da8f31caaca2bf713c6aed3842f17a8 Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Thu, 21 Jan 2021 14:07:13 +0800 Subject: [PATCH 15/22] it should be podName instead of namespace Signed-off-by: Megrez Lu --- test/e2e/utils.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/utils.go b/test/e2e/utils.go index a3ba97d44..7c6f4c65d 100644 --- a/test/e2e/utils.go +++ b/test/e2e/utils.go @@ -649,13 +649,13 @@ func verifyAgentImage(appName, namespace string, expected bool) { // testContainerInPod is a general function to test if the container exists in the pod // provided that the pod has `app` label. Return true if and only if the container exists and // the user-defined function `predicate` returns true if given. -func testContainerInPod(appName, namespace string, predicate func(*corev1.Container) bool) bool { +func testContainerInPod(appName, podName string, predicate func(*corev1.Container) bool) bool { var pods *corev1.PodList var pod corev1.Pod // Sometimes the app gets redeployed twice and we can get three pods, wait till there are either 1 or 2 err := wait.Poll(retryInterval, timeout, func() (done bool, err error) { - pods, err = fw.KubeClient.CoreV1().Pods(namespace).List(context.Background(), metav1.ListOptions{LabelSelector: "app=" + appName}) + pods, err = fw.KubeClient.CoreV1().Pods(podName).List(context.Background(), metav1.ListOptions{LabelSelector: "app=" + appName}) require.NoError(t, err) if len(pods.Items) > 0 && len(pods.Items) < 3 { return true, nil From 675074581637816160ee9bd08b39bd43dff0abd6 Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Thu, 21 Jan 2021 14:12:21 +0800 Subject: [PATCH 16/22] fix Signed-off-by: Megrez Lu --- test/e2e/istio_test.go | 2 +- test/e2e/utils.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/e2e/istio_test.go b/test/e2e/istio_test.go index 833f04c45..e36945019 100644 --- a/test/e2e/istio_test.go +++ b/test/e2e/istio_test.go @@ -125,7 +125,7 @@ func (suite *IstioTestSuite) TestEnvoySidecar() { }) require.NoError(t, err) - exists := testContainerInPod(vertxDeploymentName, "istio-proxy", nil) + exists := testContainerInPod(namespace, vertxDeploymentName, "istio-proxy", nil) require.True(suite.T(), exists) // Confirm that we've created some traces diff --git a/test/e2e/utils.go b/test/e2e/utils.go index 7c6f4c65d..449643fa1 100644 --- a/test/e2e/utils.go +++ b/test/e2e/utils.go @@ -640,7 +640,7 @@ func wasUsingOtelAllInOne(jaegerInstanceName, namespace string) bool { // verifyAgentImage test if this Jaeger Instance is using the OTEL agent? func verifyAgentImage(appName, namespace string, expected bool) { - require.Equal(t, expected, testContainerInPod(appName, namespace, func(container *corev1.Container) bool { + require.Equal(t, expected, testContainerInPod(namespace, appName, "jaeger-agent", func(container *corev1.Container) bool { logrus.Infof("Test %s is using agent image %s", t.Name(), container.Image) return strings.Contains(container.Image, "jaeger-opentelemetry-agent") })) @@ -649,13 +649,13 @@ func verifyAgentImage(appName, namespace string, expected bool) { // testContainerInPod is a general function to test if the container exists in the pod // provided that the pod has `app` label. Return true if and only if the container exists and // the user-defined function `predicate` returns true if given. -func testContainerInPod(appName, podName string, predicate func(*corev1.Container) bool) bool { +func testContainerInPod(namespace, appName, containerName string, predicate func(*corev1.Container) bool) bool { var pods *corev1.PodList var pod corev1.Pod // Sometimes the app gets redeployed twice and we can get three pods, wait till there are either 1 or 2 err := wait.Poll(retryInterval, timeout, func() (done bool, err error) { - pods, err = fw.KubeClient.CoreV1().Pods(podName).List(context.Background(), metav1.ListOptions{LabelSelector: "app=" + appName}) + pods, err = fw.KubeClient.CoreV1().Pods(namespace).List(context.Background(), metav1.ListOptions{LabelSelector: "app=" + appName}) require.NoError(t, err) if len(pods.Items) > 0 && len(pods.Items) < 3 { return true, nil @@ -680,7 +680,7 @@ func testContainerInPod(appName, podName string, predicate func(*corev1.Containe containers := pod.Spec.Containers for _, container := range containers { - if container.Name == "jaeger-agent" { + if container.Name == containerName { if predicate != nil { return predicate(&container) } @@ -688,7 +688,7 @@ func testContainerInPod(appName, podName string, predicate func(*corev1.Containe } } - require.Failf(t, "Did not find an agent image for %s in namespace %s", appName, namespace) + require.Failf(t, "Did not find container %s for pod with label{app=%s} in namespace %s", containerName, appName, namespace) return false } From d38c69426ee1371758b2506ce53587967847761d Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Thu, 21 Jan 2021 14:23:30 +0800 Subject: [PATCH 17/22] fix t Signed-off-by: Megrez Lu --- test/e2e/istio_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/istio_test.go b/test/e2e/istio_test.go index e36945019..7bdd25ec0 100644 --- a/test/e2e/istio_test.go +++ b/test/e2e/istio_test.go @@ -126,7 +126,7 @@ func (suite *IstioTestSuite) TestEnvoySidecar() { require.NoError(t, err) exists := testContainerInPod(namespace, vertxDeploymentName, "istio-proxy", nil) - require.True(suite.T(), exists) + require.True(t, exists) // Confirm that we've created some traces ports := []string{"0:16686"} From 0413f3454125b6c6da33bc545de461cf8aa1e61d Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Thu, 21 Jan 2021 14:37:59 +0800 Subject: [PATCH 18/22] refactor deployUpdate function Signed-off-by: Megrez Lu --- test/e2e/examples1_test.go | 24 ++---------------------- test/e2e/istio_test.go | 22 ++-------------------- test/e2e/utils.go | 29 ++++++++++++++++++++++++++--- 3 files changed, 30 insertions(+), 45 deletions(-) diff --git a/test/e2e/examples1_test.go b/test/e2e/examples1_test.go index 9ddacb0ce..e23eebd95 100644 --- a/test/e2e/examples1_test.go +++ b/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 { @@ -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) diff --git a/test/e2e/istio_test.go b/test/e2e/istio_test.go index 7bdd25ec0..57127cee9 100644 --- a/test/e2e/istio_test.go +++ b/test/e2e/istio_test.go @@ -5,7 +5,6 @@ package e2e import ( "context" "encoding/json" - "errors" "io/ioutil" "net/http" "os" @@ -16,13 +15,11 @@ 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 IstioTestSuite struct { @@ -105,23 +102,8 @@ func (suite *IstioTestSuite) TestEnvoySidecar() { 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) diff --git a/test/e2e/utils.go b/test/e2e/utils.go index 449643fa1..621d9a737 100644 --- a/test/e2e/utils.go +++ b/test/e2e/utils.go @@ -3,7 +3,9 @@ package e2e import ( "context" "crypto/tls" + "errors" "fmt" + "github.com/prometheus/common/log" "io" "io/ioutil" "net/http" @@ -640,7 +642,7 @@ func wasUsingOtelAllInOne(jaegerInstanceName, namespace string) bool { // verifyAgentImage test if this Jaeger Instance is using the OTEL agent? func verifyAgentImage(appName, namespace string, expected bool) { - require.Equal(t, expected, testContainerInPod(namespace, appName, "jaeger-agent", func(container *corev1.Container) bool { + require.Equal(t, expected, testContainerInPod(namespace, appName, "jaeger-agent", func(container corev1.Container) bool { logrus.Infof("Test %s is using agent image %s", t.Name(), container.Image) return strings.Contains(container.Image, "jaeger-opentelemetry-agent") })) @@ -649,7 +651,7 @@ func verifyAgentImage(appName, namespace string, expected bool) { // testContainerInPod is a general function to test if the container exists in the pod // provided that the pod has `app` label. Return true if and only if the container exists and // the user-defined function `predicate` returns true if given. -func testContainerInPod(namespace, appName, containerName string, predicate func(*corev1.Container) bool) bool { +func testContainerInPod(namespace, appName, containerName string, predicate func(corev1.Container) bool) bool { var pods *corev1.PodList var pod corev1.Pod @@ -682,7 +684,7 @@ func testContainerInPod(namespace, appName, containerName string, predicate func for _, container := range containers { if container.Name == containerName { if predicate != nil { - return predicate(&container) + return predicate(container) } return true } @@ -867,3 +869,24 @@ func getTracingClientWithCollectorEndpoint(serviceName, collectorEndpoint string } return cfg.NewTracer() } + +func waitForDeploymentAndUpdate(deploymentName, containerName string, update func(container *corev1.Container)) error { + return wait.Poll(retryInterval, timeout, func() (done bool, err error) { + deployment, err := fw.KubeClient.AppsV1().Deployments(namespace).Get(context.Background(), deploymentName, metav1.GetOptions{}) + require.NoError(t, err) + containers := deployment.Spec.Template.Spec.Containers + for index, container := range containers { + if container.Name == containerName { + update(&deployment.Spec.Template.Spec.Containers[index]) + updatedDeployment, err := fw.KubeClient.AppsV1().Deployments(namespace).Update(context.Background(), deployment, metav1.UpdateOptions{}) + if err != nil { + log.Warnf("Error %v updating container, retrying", err) + return false, nil + } + log.Infof("Updated deployment %v", updatedDeployment.Name) + return true, nil + } + } + return false, errors.New(fmt.Sprintf("container %s in deployment %s not found", containerName, deploymentName)) + }) +} From d2e39c4381e2b26840db835a254a8bd71bfb1b7d Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Thu, 21 Jan 2021 14:41:10 +0800 Subject: [PATCH 19/22] make lint happy Signed-off-by: Megrez Lu --- test/e2e/utils.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/e2e/utils.go b/test/e2e/utils.go index 621d9a737..ed5874875 100644 --- a/test/e2e/utils.go +++ b/test/e2e/utils.go @@ -3,7 +3,6 @@ package e2e import ( "context" "crypto/tls" - "errors" "fmt" "github.com/prometheus/common/log" "io" @@ -887,6 +886,6 @@ func waitForDeploymentAndUpdate(deploymentName, containerName string, update fun return true, nil } } - return false, errors.New(fmt.Sprintf("container %s in deployment %s not found", containerName, deploymentName)) + return false, fmt.Errorf("container %s in deployment %s not found", containerName, deploymentName) }) } From 0bc7672f5a85562eb70b2f367bbcdb1c14ac11d2 Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Thu, 21 Jan 2021 16:40:55 +0800 Subject: [PATCH 20/22] move getBusinessAppCR to utils.go Signed-off-by: Megrez Lu --- test/e2e/examples1_test.go | 11 ----------- test/e2e/istio_test.go | 11 ----------- test/e2e/utils.go | 11 +++++++++++ 3 files changed, 11 insertions(+), 22 deletions(-) diff --git a/test/e2e/examples1_test.go b/test/e2e/examples1_test.go index e23eebd95..664469d9b 100644 --- a/test/e2e/examples1_test.go +++ b/test/e2e/examples1_test.go @@ -153,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() diff --git a/test/e2e/istio_test.go b/test/e2e/istio_test.go index 57127cee9..71d4e15a2 100644 --- a/test/e2e/istio_test.go +++ b/test/e2e/istio_test.go @@ -132,14 +132,3 @@ func (suite *IstioTestSuite) TestEnvoySidecar() { }) 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 -} diff --git a/test/e2e/utils.go b/test/e2e/utils.go index ed5874875..78e784ea5 100644 --- a/test/e2e/utils.go +++ b/test/e2e/utils.go @@ -889,3 +889,14 @@ func waitForDeploymentAndUpdate(deploymentName, containerName string, update fun return false, fmt.Errorf("container %s in deployment %s not found", containerName, deploymentName) }) } + +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 +} From 5854c809be50151a209c4c74efdb00d3371736bc Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Thu, 21 Jan 2021 16:46:37 +0800 Subject: [PATCH 21/22] format utils.go Signed-off-by: Megrez Lu --- test/e2e/utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/utils.go b/test/e2e/utils.go index 78e784ea5..92c478405 100644 --- a/test/e2e/utils.go +++ b/test/e2e/utils.go @@ -4,7 +4,6 @@ import ( "context" "crypto/tls" "fmt" - "github.com/prometheus/common/log" "io" "io/ioutil" "net/http" @@ -21,6 +20,7 @@ import ( "github.com/opentracing/opentracing-go" framework "github.com/operator-framework/operator-sdk/pkg/test" "github.com/operator-framework/operator-sdk/pkg/test/e2eutil" + "github.com/prometheus/common/log" "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" From 6e7c287c80ac63af11a7cd22199aa5ca81ecbdac Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Fri, 22 Jan 2021 19:02:25 +0800 Subject: [PATCH 22/22] remove error parameter in getBusinessAppCR Signed-off-by: Megrez Lu --- test/e2e/examples1_test.go | 2 +- test/e2e/istio_test.go | 2 +- test/e2e/utils.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/e2e/examples1_test.go b/test/e2e/examples1_test.go index 664469d9b..d282dd036 100644 --- a/test/e2e/examples1_test.go +++ b/test/e2e/examples1_test.go @@ -104,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() diff --git a/test/e2e/istio_test.go b/test/e2e/istio_test.go index 71d4e15a2..bfee20ad3 100644 --- a/test/e2e/istio_test.go +++ b/test/e2e/istio_test.go @@ -85,7 +85,7 @@ func (suite *IstioTestSuite) TestEnvoySidecar() { 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() diff --git a/test/e2e/utils.go b/test/e2e/utils.go index 92c478405..494e90df7 100644 --- a/test/e2e/utils.go +++ b/test/e2e/utils.go @@ -890,7 +890,7 @@ func waitForDeploymentAndUpdate(deploymentName, containerName string, update fun }) } -func getBusinessAppCR(err error) *os.File { +func getBusinessAppCR() *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)