Skip to content

Fix flaky integration/unit tests and broken TSan/e2e CI jobs - #1044

Open
liusc28 wants to merge 1 commit into
GoogleCloudPlatform:masterfrom
liusc28:fix-flaky-tests
Open

liusc28 wants to merge 1 commit into
GoogleCloudPlatform:masterfrom
liusc28:fix-flaky-tests

Conversation

@liusc28

@liusc28 liusc28 commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

Presubmits on open PRs (e.g. #1041, #1042) keep failing on tests and jobs unrelated to the change under review. This PR fixes those failures. It only changes tests, test scripts and CI config; no product code.

Flaky tests

TestStatisticsServiceControlCallStatus ("quota call is 403") (example: expected value: 3, got value: 2)

  • Cause: after the 403, the service control client re-checks the denied quota cache entry with the server on every cache flush (every 1-2s), so allocate_quota.PERMISSION_DENIED keeps growing. The test read it once, 3s after the request, so the value depended on timing.
  • Fix: assert that it reaches at least 3, using the new StatsVerifier.CheckMinimumCounters. CheckExpectedCounters now also keeps polling (up to 10s) while a counter is below its expected value, and still fails as soon as a counter goes above it.

TestAccessLog (example)

  • Cause: Envoy writes the access log asynchronously and flushes it to the file every 10s, so the entry may not be in the file yet when the test reads it.
  • Fix: poll the file for up to 15s until it has the expected content.

TestDownstreamMTLS (example: write: broken pipe instead of tls: unknown certificate authority)

  • Cause: with TLS 1.3, the client finishes the handshake before Envoy verifies the client cert, so it can write the request after Envoy has already rejected the cert and closed the connection. The write error then hides the TLS alert.
  • Fix: when an error is expected and the connection broke (EPIPE/ECONNRESET), retry on a new connection, up to 5 attempts.

TestRetryCallServiceManagement (integration) (example)

  • Cause: after the 429, ConfigManager waits 10s before retrying and only serves ADS once it has the service config. Meanwhile Envoy retries the ADS connection with jittered exponential backoff (up to 30s), so the listener came up 32-37s after Envoy started, after the 30s Envoy health check (10 x 3s) had given up.
  • Fix: add TestEnv.SetEnvoyHealthCheckRetries; this test uses 20 retries (60s).

TestRetryCallServiceManagement (unit, src/go/configmanager) (example)

  • Cause: the "retryInterval is too short" case retries after 100ms and expects to land inside the mock server's 150ms silent window. The 50ms margin is too small for slow runs such as -race.
  • Fix: 1s silent window and 1s retry interval for the success case. The "too short" case keeps 100ms, which leaves a 900ms margin.

Broken CI jobs

ESPv2-presubmit-tsan (example: FATAL: ThreadSanitizer: unexpected memory mapping)

  • Cause: clang-14 TSan requires PIE binaries to be loaded within a fixed address range. That fails when the kernel places the binary outside it, which is common on hosts with a high vm.mmap_rnd_bits. Only the 2 tests that weren't remote-cache hits actually ran, and both failed.
  • Fix: test:clang-tsan --run_under="setarch --addr-no-randomize", as the integration tests already do for Envoy. This changes the test action keys, so the first run re-executes all TSan tests.

ESPv2-cloud-run-e2e-cloud-function-http-bookstore (example: nodejs12 is not a supported runtime on GCF 2nd gen)

  • Cause: since Cloud SDK 492.0.0, gcloud functions deploy creates 2nd gen functions by default, and nodejs12 is not available there (it is also decommissioned on 1st gen).
  • Fix: --no-gen2 --runtime nodejs22. The script relies on 1st gen behavior (httpsTrigger.url, roles/cloudfunctions.invoker).

ESPv2-cloud-run-e2e-app-engine-http-bookstore (example: Resources is not supported for App Engine Standard Environment)

  • Fix: remove the resources section, which only applies to the flexible environment, and move from the deprecated nodejs18 to nodejs22.

Not fixed

These are infra issues and are not addressed here:

  • ESPv2-anthos-cloud-run-e2e-anthos-cloud-run-http-bookstore: GKE cluster creation fails with INTERNAL (example).
  • ESPv2-presubmit-asan: hits the 2h job timeout while still linking test binaries (example).

Testing

  • gofmt, goimports, go vet and misspell pass on the changed files.
  • go test ./src/go/configmanager -run TestRetryCallServiceManagement -race -count=5 -cpu=1 passes.
  • go test ./tests/env/components/ passes.
  • The integration test, TSan and e2e changes can only be verified by CI; see this PR's presubmits.

- statistics_test: the denied quota cache entry is re-checked with the
  server on every cache flush, so allocate_quota.PERMISSION_DENIED keeps
  growing and its value at a fixed fetch time varies. Assert a minimum
  instead. StatsVerifier.CheckExpectedCounters now also polls for up to
  10s while a counter is still below its expected value.
- access_log_test: Envoy writes the access log asynchronously. Poll the
  file for up to 15s instead of reading it once.
- transport_security_test: with TLS 1.3, the client can write the
  request after Envoy has rejected the client cert and closed the
  connection, so the error is EPIPE/ECONNRESET instead of the TLS alert.
  Retry on a new connection in that case.
- managed_service_config_test: after the 429, ConfigManager only serves
  ADS after its 10s retry, and Envoy's ADS reconnect backoff can add up
  to 30s more. Allow 60s for the Envoy health check.
- config_manager_test: widen the timing margin of
  TestRetryCallServiceManagement from 50ms to 900ms. 50ms was too tight
  for -race runs.
- TSan: run the tests with ASLR disabled. clang-14 TSan aborts with
  "unexpected memory mapping" on hosts with a high vm.mmap_rnd_bits.
- e2e: deploy the Cloud Function as 1st gen on nodejs22 (nodejs12 is
  decommissioned and gcloud now defaults to 2nd gen). Move the App
  Engine bookstore to nodejs22 and remove the resources section, which
  App Engine standard now rejects.
@google-oss-prow

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: liusc28
Once this PR has been reviewed and has the lgtm label, please assign guoyilin42 for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@liusc28

liusc28 commented Sep 29, 2026

Copy link
Copy Markdown
Contributor Author

/cc @TigerSunWork

@google-oss-prow

Copy link
Copy Markdown

@liusc28: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ESPv2-anthos-cloud-run-e2e-anthos-cloud-run-http-bookstore bd9f4b7 link true /test ESPv2-anthos-cloud-run-e2e-anthos-cloud-run-http-bookstore
ESPv2-cloud-run-e2e-app-engine-http-bookstore bd9f4b7 link true /test ESPv2-cloud-run-e2e-app-engine-http-bookstore
ESPv2-presubmit-asan bd9f4b7 link true /test ESPv2-presubmit-asan
Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant