goutamadwant opened a new pull request, #12481:
URL: https://github.com/apache/seatunnel/pull/12481
### Purpose of this pull request
`AbstractTestFlinkContainer#tearDown` runs `execInContainer("rm", "-rf",
...)` on the TaskManager
before it stops anything. If the TaskManager is not running, that call
throws:
- `IllegalStateException: execInContainer can only be used while the
Container is running` when
the TaskManager container never started (for example, its wait strategy
timed out on a slow
runner);
- `ConflictException: Status 409 ... container ... is not running` when it
started and then died.
`tearDown` then exits, and the JobManager is never stopped.
The leaked JobManager keeps the `jobmanager` network alias on the shared
test network until the
test JVM exits. The next Flink test case starts a new JobManager under the
same alias. Its
TaskManager can resolve `jobmanager` to the leaked one and register there.
The new JobManager
then logs `Missing resources ... numberOfRequiredSlots=1` / `Current
resources: (none)`, and the
job waits until the workflow timeout. A local `DatabendIT` run on Flink 1.18
hung exactly this way
after `testFakeToDatabend`'s TaskManager exited during startup.
This PR:
- skips the volume cleanup for a container that is not running;
- always runs every teardown step (TaskManager, JobManager, host mount
directory), so a failure
in one of them no longer skips the others;
- rethrows the first failure once all steps have run, with any later
failures attached as
suppressed exceptions.
The host directory deletion moves into a small package-private method so the
unit test can run
`tearDown` without touching the real `/tmp/seatunnel_mnt`. What it deletes
is unchanged.
`AbstractTestSparkContainer` and `SeaTunnelContainer` use a similar
exec-then-stop teardown and
may deserve the same treatment in a follow-up. They are not changed here.
### Does this PR introduce _any_ user-facing change?
No. Test infrastructure only.
### How was this patch tested?
**Unit tests (red → green).** Added `AbstractTestFlinkContainerTest`. It
uses mocked
containers and replaces the host directory deletion with a flag:
- `shouldStopJobManagerWhenTaskManagerIsNotRunning`: on `dev`, `tearDown`
throws
`IllegalStateException: execInContainer can only be used while the
Container is running` and
never calls `jobManager.stop()`. With this PR it passes.
- `shouldStopJobManagerWhenCleaningTaskManagerVolumeFails`: on `dev`, fails
with
`Wanted but not invoked: genericContainer.stop()`. With this PR, both
containers are stopped and
the original exception is rethrown.
- `shouldKeepFirstFailureAndSuppressLaterOnes`: the TaskManager cleanup
fails and the JobManager
stop also fails; the first failure is thrown and the second is attached as
suppressed.
All three fail against the `dev` teardown logic and pass with this PR on JDK
8 and JDK 11,
together with the existing `seatunnel-e2e-common` unit tests (37 tests). A
marker file in
`/tmp/seatunnel_mnt` survived the test run.
```
./mvnw -B verify -DskipUT=false -DskipIT=true -pl
seatunnel-e2e/seatunnel-e2e-common -am \
-Dtest='AbstractTestFlinkContainerTest,SharedTestContainerResourceTest,SeaTunnelContainer*Test'
-DfailIfNoTests=false
```
**E2E (local, harness not included in this PR).** Start a Flink 1.18
cluster, kill its
TaskManager, and call `tearDown()`. Then start a new cluster and run
`fake_to_assert.conf`:
- `dev`: `tearDown()` threw in 11 of 11 runs (`ConflictException ... is not
running`), and the
first JobManager was left running every time. In 6 of those 11 runs, the
new TaskManager
registered with the leaked JobManager. The next job then hung for more
than 120 s with
`Missing resources ... Current resources: (none)`.
- this PR: in every run (8 on an earlier revision of this change, 4 on the
final one),
`tearDown()` completed, no JobManager was left running, the new
TaskManager registered with its
own JobManager, and the job passed.
**Not verified:** how often a TaskManager fails to start on GitHub runners.
I did not find this
sequence in the `kudu-connector-it` timeouts I checked, which have a
different cause (#12132).
--
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]
