From bb84dd5182416872fa90e5e71086983f4284709e Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Sat, 2 Nov 2024 13:08:58 -0400 Subject: [PATCH] Revert "[v2] Use health check in grpc e2e test (#6113)" This reverts commit 3c1e85d0367c524a5e9fa3667e291aee9f3b1573. --- .github/workflows/ci-lint-checks.yaml | 4 +--- Makefile | 4 ++-- .../internal/integration/e2e_integration.go | 23 +++++++++++-------- cmd/jaeger/internal/integration/kafka_test.go | 6 ++--- 4 files changed, 20 insertions(+), 17 deletions(-) diff --git a/.github/workflows/ci-lint-checks.yaml b/.github/workflows/ci-lint-checks.yaml index b665fd533ea..30523637c64 100644 --- a/.github/workflows/ci-lint-checks.yaml +++ b/.github/workflows/ci-lint-checks.yaml @@ -47,9 +47,7 @@ jobs: - uses: ./.github/actions/block-pr-from-main-branch - - run: | - git fetch origin main - make lint-nocommit + - run: make lint-nocommit dco-check: runs-on: ubuntu-latest diff --git a/Makefile b/Makefile index 50d19d2ea7d..3229e2583e6 100644 --- a/Makefile +++ b/Makefile @@ -153,9 +153,9 @@ lint-license: .PHONY: lint-nocommit lint-nocommit: - @if git diff origin/main | grep '@no''commit' ; then \ + @if git diff main | grep '@no''commit' ; then \ echo "❌ Cannot merge PR that contains @no""commit string" ; \ - GIT_PAGER=cat git diff -G '@no''commit' origin/main ; \ + GIT_PAGER=cat git diff -G '@no''commit' main ; \ false ; \ else \ echo "✅ Changes do not contain @no""commit string" ; \ diff --git a/cmd/jaeger/internal/integration/e2e_integration.go b/cmd/jaeger/internal/integration/e2e_integration.go index c8681e4ed7e..9e4843f030c 100644 --- a/cmd/jaeger/internal/integration/e2e_integration.go +++ b/cmd/jaeger/internal/integration/e2e_integration.go @@ -13,6 +13,7 @@ import ( "os" "os/exec" "path/filepath" + "strings" "testing" "time" @@ -41,10 +42,10 @@ const otlpPort = 4317 type E2EStorageIntegration struct { integration.StorageIntegration - SkipStorageCleaner bool - ConfigFile string - BinaryName string - HealthCheckPort int // overridable for Kafka tests which run two binaries and need different ports + SkipStorageCleaner bool + ConfigFile string + BinaryName string + HealthCheckEndpoint string // EnvVarOverrides contains a map of environment variables to set. // The key in the map is the environment variable to override and the value @@ -159,11 +160,10 @@ func (s *E2EStorageIntegration) e2eInitialize(t *testing.T, storage string) { } func (s *E2EStorageIntegration) doHealthCheck(t *testing.T) bool { - healthCheckPort := s.HealthCheckPort - if healthCheckPort == 0 { - healthCheckPort = ports.CollectorV2HealthChecks + healthCheckEndpoint := s.HealthCheckEndpoint + if healthCheckEndpoint == "" { + healthCheckEndpoint = "http://localhost:13133/status" } - healthCheckEndpoint := fmt.Sprintf("http://localhost:%d/status", healthCheckPort) t.Logf("Checking if %s is available on %s", s.BinaryName, healthCheckEndpoint) ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) defer cancel() @@ -187,6 +187,11 @@ func (s *E2EStorageIntegration) doHealthCheck(t *testing.T) bool { t.Logf("HTTP response not OK: %v", string(body)) return false } + // for backwards compatibility with other healthchecks + if !strings.HasSuffix(healthCheckEndpoint, "/status") { + t.Logf("OK HTTP from endpoint that is not healthcheckv2") + return true + } var healthResponse struct { Status string `json:"status"` @@ -198,7 +203,7 @@ func (s *E2EStorageIntegration) doHealthCheck(t *testing.T) bool { // Check if the status field in the JSON is "StatusOK" if healthResponse.Status != "StatusOK" { - t.Logf("Received non-OK status %s: %s", healthResponse.Status, string(body)) + t.Logf("Received non-K status %s: %s", healthResponse.Status, string(body)) return false } return true diff --git a/cmd/jaeger/internal/integration/kafka_test.go b/cmd/jaeger/internal/integration/kafka_test.go index 6e9e4d810ef..218624788f0 100644 --- a/cmd/jaeger/internal/integration/kafka_test.go +++ b/cmd/jaeger/internal/integration/kafka_test.go @@ -52,9 +52,9 @@ func TestKafkaStorage(t *testing.T) { t.Log("Collector initialized") ingester := &E2EStorageIntegration{ - BinaryName: "jaeger-v2-ingester", - ConfigFile: "../../config-kafka-ingester.yaml", - HealthCheckPort: 14133, + BinaryName: "jaeger-v2-ingester", + ConfigFile: "../../config-kafka-ingester.yaml", + HealthCheckEndpoint: "http://localhost:14133/status", StorageIntegration: integration.StorageIntegration{ CleanUp: purge, GetDependenciesReturnsSource: true,