damccorm opened a new pull request, #41302:
URL: https://github.com/apache/beam/pull/41302
Note: this PR was auto-generated by tooling. Before moving it out of draft,
the author has done a full review of the PR, specifically evaluating it on the
following criteria:
- The PR is an appropriate fix for the issue (e.g. we're not changing a test
to avoid a bug)
- The PR is a simple fix to the issue (e.g. we're not adding infra when we
could just modify a test)
- The PR is not covering for other problems (e.g. if we're bumping timeouts,
it is not because we're covering for a performance problem)
Full details of the methodology of generation and review can be found here -
https://docs.google.com/document/d/14lj7ECVov4T4EdwmIzNlWrRGE3lLDMOwX5xUMGzaLoc/edit?usp=sharing
If you have concerns with the PR or with the underlying methodology used to
generate this, please comment below - they will be addressed by the author not
the tooling :)
## Summary
Fixes container startup retry failures in `cloudsql_it_test.py` observed in
`PreCommit Python Transforms` ([run
43436215988](https://github.com/apache/beam/actions/runs/43436215988/job/131921903357))
and `PostCommit Python Versions` ([run
43434755479](https://github.com/apache/beam/actions/runs/43434755479/job/131916863225)):
```
Retry 1/3: Failed to start POSTGRESQL container. Reason:
UnixHTTPConnectionPool(host='localhost', port=None): Read timed out. (read
timeout=60).
Retry 2/3: Failed to start POSTGRESQL container. Reason: 409 Client Error
for
http+docker://localhost/v1.43/containers/create?name=testcontainers-ryuk-e17c496e-44ab-402b-8eba-85e15f2f3713:
Conflict ("Conflict. The container name
"/testcontainers-ryuk-e17c496e-44ab-402b-8eba-85e15f2f3713" is already in use
by container
"8e614888ff54b4b3d63e0522109eba84bfc2ea00c00c90d4ff96e6347b24a2d5".")
Retry 3/3: Failed to start POSTGRESQL container. Reason: 409 Client Error
for
http+docker://localhost/v1.43/containers/create?name=testcontainers-ryuk-e17c496e-44ab-402b-8eba-85e15f2f3713:
Conflict (...)
```
### Root Cause
When the first container startup in an xdist worker calls
`Reaper.get_instance()` -> `Reaper._create_instance()` and the Docker HTTP
client times out at 60 seconds during `POST
/containers/create?name=testcontainers-ryuk-<SESSION_ID>`:
- The Docker daemon reserves the container name
`/testcontainers-ryuk-<SESSION_ID>` at the start of `containerCreate` before
finishing layer creation and registering the container in `daemon.containers`.
- When `cleanup_testcontainer_on_retry` immediately runs,
`docker_client.client.containers.get(f"testcontainers-ryuk-{session_id}")`
fails with `404 NotFound` (or cannot remove the in-progress creation), leaving
`testcontainers.core.labels.SESSION_ID` and
`testcontainers.core.container.SESSION_ID` unchanged.
- Subsequent immediate retries (and subsequent test classes in the same
xdist worker) attempt to create `testcontainers-ryuk-<SESSION_ID>` with the
same `SESSION_ID` and fail with `409 Conflict`.
### Fix
- In `cleanup_testcontainer_on_retry`
(`sdks/python/apache_beam/testing/test_utils.py`), when `Reaper._instance` is
`None`, call `Reaper.delete_instance()`, perform best-effort removal of
`testcontainers-ryuk-<session_id>`, and rotate `SESSION_ID` across
`testcontainers.core.labels`, `testcontainers.core.container`, and
`testcontainers.core.docker_client` so the next retry uses a fresh Ryuk
container name instead of colliding with an in-flight Docker daemon creation.
- Add a unit test in `sdks/python/apache_beam/testing/test_utils_test.py`.
### Verification
- Triggered `beam_PreCommit_Python_Transforms.yml` workflow run on branch:
[`43457310793`](https://github.com/apache/beam/actions/runs/43457310793)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
