You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/02/19 07:28:28 UTC
[GitHub] [pulsar] lhotari opened a new pull request #9626: [Testing] Add debugging feature which leaves integration test containers running after test completes
lhotari opened a new pull request #9626:
URL: https://github.com/apache/pulsar/pull/9626
### Motivation
For debugging purposes, it is useful to have the ability to leave the integration test containers running after the test completes. For example, this feature was necessary in investigating the issue #9622 . It was possible to view the log files and find out the issue. If the containers are killed, this options is lost.
### Modifications
[Testcontainers has a feature to disable "Ryuk"](https://www.testcontainers.org/features/configuration/#disabling-ryuk), the automatic container cleanup which usually kills all containers started by Testcontainers after the JVM shuts down.
This mode can be activated by setting environment variable `TESTCONTAINERS_RYUK_DISABLED=true`.
This Testcontainers feature doesn't make a difference when the test code itself stops the containers. Therefore there should be handling in the test code itself to skip stopping of containers for debugging purposes.
The modifications in this PR skip stopping PulsarContainer and PulsarCluster instances if environment variable `TESTCONTAINERS_RYUK_DISABLED=true` .
### Usage example
In unix shells, one can pass environment variables by prepending the command with the variables:
```
TESTCONTAINERS_RYUK_DISABLED=true mvn -B -f tests/pom.xml test -DintegrationTests -DredirectTestOutputToFile=false -DtestRetryCount=0 -DfailIfNoTests=false -Dtest=CLITest
```
After the test run, one can use `docker ps` and `docker exec -it [container_name] bash` to get a shell in the running container that was left behind the test run when this feature introduced by this PR is active.
When container cleanup is disabled, one can use this command after debugging to kill docker containers started by Testcontainers:
```
docker ps -q --filter "label=org.testcontainers=true" | xargs -r docker kill
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie merged pull request #9626: [Testing] Add debugging feature which leaves integration test containers running after test completes
Posted by GitBox <gi...@apache.org>.
sijie merged pull request #9626:
URL: https://github.com/apache/pulsar/pull/9626
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #9626: [Testing] Add debugging feature which leaves integration test containers running after test completes
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #9626:
URL: https://github.com/apache/pulsar/pull/9626#issuecomment-783254517
@bsideup I have revisited the solution in this PR to use the Testcontainers reuse mode instead of depending on the usage of `TESTCONTAINERS_RYUK_DISABLED=true`. Would you mind reviewing the changes?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #9626: [Testing] Add debugging feature which leaves integration test containers running after test completes
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #9626:
URL: https://github.com/apache/pulsar/pull/9626#issuecomment-781986394
/pulsar run-failure-checks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] bsideup commented on pull request #9626: [Testing] Add debugging feature which leaves integration test containers running after test completes
Posted by GitBox <gi...@apache.org>.
bsideup commented on pull request #9626:
URL: https://github.com/apache/pulsar/pull/9626#issuecomment-782133879
@lhotari TBH I would advice against this approach. There is no guarantee that the JVM hook will be using `stop()` (we thought about terminating multiple containers by the session label filter). Not to mention that Ryuk generally should not be disabled, unless your CI does not support it (the reason why the flag was added on a first place).
If you need to the container to remain running, consider using [the reusable containers mode](https://github.com/testcontainers/testcontainers-java/pull/1781).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #9626: [Testing] Add debugging feature which leaves integration test containers running after test completes
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #9626:
URL: https://github.com/apache/pulsar/pull/9626#issuecomment-781985894
> It would be wonderful if we can add the description into the test README.md, that would be helpful for others who want to debug the tests.
@zymap Makes sense. I was thinking of updating the README later. I have plans to add features for enabling attaching a debugger to a container as well as enabling the use of JConsole / Java Mission Control / Java Flight Recorder by enabling JMX for the JVMs in a configurable way. The README could be updated at that time to also cover the feature of this current PR.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #9626: [Testing] Add debugging feature which leaves integration test containers running after test completes
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #9626:
URL: https://github.com/apache/pulsar/pull/9626#issuecomment-782087052
/pulsarbot run-failure-checks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari edited a comment on pull request #9626: [Testing] Add debugging feature which leaves integration test containers running after test completes
Posted by GitBox <gi...@apache.org>.
lhotari edited a comment on pull request #9626:
URL: https://github.com/apache/pulsar/pull/9626#issuecomment-782144592
@bsideup Thanks for your comments. In this case, the changes in this PR aren't meant to be used in CI at all. The reason to leave the container running is to debug an issue locally. I've had the impression that it's the reason why `TESTCONTAINERS_RYUK_DISABLED=true` exists. At least, that's how I've been using it in the past.
Would you also recommend using the reusable container mode for also for the debugging use case that I have described? I guess that would require also enabling `testcontainers.reuse.enable` in `~/.testcontainers.properties`?
I have one concern. The problem with reusable container mode is that it impacts the execution when a container gets reused. I'd simply like to leave the containers after the test and the test JVM completes so that I could open a shell to the container and inspect the state. Perhaps this would be a feature request for Testcontainers?
Is there a workaround with reusable containers by adding a random label to the container etc so that the container would never get reused but just left behind?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #9626: [Testing] Add debugging feature which leaves integration test containers running after test completes
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #9626:
URL: https://github.com/apache/pulsar/pull/9626#issuecomment-783431069
/pulsarbot run-failure-checks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari edited a comment on pull request #9626: [Testing] Add debugging feature which leaves integration test containers running after test completes
Posted by GitBox <gi...@apache.org>.
lhotari edited a comment on pull request #9626:
URL: https://github.com/apache/pulsar/pull/9626#issuecomment-782144592
@bsideup Thanks for your comments. In this case, the changes in this PR aren't meant to be used in CI at all. The reason to leave the container running is to debug an issue locally. I've had the impression that it's the reason why `TESTCONTAINERS_RYUK_DISABLED=true` exists. At least, that's how I've been using it in the past.
Would you also recommend using the reusable container mode for also for the debugging use case that I have described? I guess that would require also enabling `testcontainers.reuse.enable` in `~/.testcontainers.properties`?
I have one concern. The problem with reusable container mode is that it impacts the execution when a container gets reused. I'd simply like to leave the containers after the test and the test JVM completes so that I could open a shell to the container and inspect the state. Perhaps this would be a feature request for Testcontainers?
Is there a workaround with reusable containers by adding a random label to the container etc so that the container would never get reused?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #9626: [Testing] Add debugging feature which leaves integration test containers running after test completes
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #9626:
URL: https://github.com/apache/pulsar/pull/9626#issuecomment-782170505
Thanks for the advice @bsideup . I'll revisit this PR so that it doesn't depend on `TESTCONTAINERS_RYUK_DISABLED=true` and uses the reuse containers feature as part of the solution. That's cleaner.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] bsideup commented on pull request #9626: [Testing] Add debugging feature which leaves integration test containers running after test completes
Posted by GitBox <gi...@apache.org>.
bsideup commented on pull request #9626:
URL: https://github.com/apache/pulsar/pull/9626#issuecomment-783379953
@lhotari it is okay :) I had a feeling that the `stop()` override is unnecessary but you know your usage better 👍
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari removed a comment on pull request #9626: [Testing] Add debugging feature which leaves integration test containers running after test completes
Posted by GitBox <gi...@apache.org>.
lhotari removed a comment on pull request #9626:
URL: https://github.com/apache/pulsar/pull/9626#issuecomment-781986394
/pulsar run-failure-checks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #9626: [Testing] Add debugging feature which leaves integration test containers running after test completes
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #9626:
URL: https://github.com/apache/pulsar/pull/9626#issuecomment-782144592
@bsideup Thanks for your comments. In this case, the changes in this PR aren't meant to be used in CI at all. The reason to leave the container running is to debug an issue locally. I've had the impression that it's the reason why `TESTCONTAINERS_RYUK_DISABLED=true` exists. At least, that's how I've been using it in the past.
Would you also recommend using the reusable container mode for also for the debugging use case that I have described? I guess that would require also enabling `testcontainers.reuse.enable` in `~/.testcontainers.properties`?
I have one concern. The problem with reusable container mode is that it impacts the execution. I'd simply like to leave the containers after the test and the test JVM completes so that I could open a shell to the container and inspect the state. Perhaps this would be a feature request for Testcontainers?
Is there a workaround with reusable containers by adding a random label to the container etc so that the container would never get reused?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #9626: [Testing] Add debugging feature which leaves integration test containers running after test completes
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #9626:
URL: https://github.com/apache/pulsar/pull/9626#issuecomment-783368920
/pulsarbot run-failure-checks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] bsideup commented on pull request #9626: [Testing] Add debugging feature which leaves integration test containers running after test completes
Posted by GitBox <gi...@apache.org>.
bsideup commented on pull request #9626:
URL: https://github.com/apache/pulsar/pull/9626#issuecomment-782162662
@lhotari
> I've had the impression that it's the reason why TESTCONTAINERS_RYUK_DISABLED=true exists
We added the flag because BitBucket did not support mounting Docker socket a few years ago, but, since the builds were ephemeral, Ryuk could be omitted.
> Is there a workaround with reusable containers by adding a random label to the container etc so that the container would never get reused but just left behind?
Yes. The reusable feature works by hashing the container's definition. If you make the hash unique (e.g. random label / env variable / network alias / network id) then a new container will be started each execution and old containers won't be terminated (one can see it as a disadvantage but in your case this is exactly what you need :D)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org