[log4j] New broken tests in master: LogstashIT

[Matt Sicker <bo...@gmail.com>]

These tests are failing on macOS with a connection error of some sort.
Not sure if it fails on other operating systems, but it reliably fails
early enough in the macOS builds to cancel the others, and it fails
for me locally as well. Possible cause could be related to JUnit
dependency updates if there was a race condition involving finding
available ports or something, though I haven't looked too closely at
what's the issue.

Re: [log4j] New broken tests in master: LogstashIT

[Volkan Yazıcı <vo...@yazi.ci>]

I use the official Elasticsearch and Logstash Docker images to test if
JTL+SocketAppender plays nicely with the ELK stack. These images are pretty
big and require OS-specific configuration (e.g., upping the `nofile`) to
run. I failed to convince Elastic to release more lightweight images to
work around this testing problem.
<https://github.com/elastic/elasticsearch-docker/issues/172>

Testcontainers don't provide container images, they use the official images
provided by Elastic
<https://github.com/testcontainers/testcontainers-java/blob/a9c2e9e148097f4996dc7a96f85fced1ee857ddc/modules/elasticsearch/src/main/java/org/testcontainers/elasticsearch/ElasticsearchContainer.java#L51>,
just like us. Hence, TC doesn't address the problem I have mentioned above.
TC is simply a convenience to talk to Docker daemon through a
developer-friendly Java API. I am not a big fan of this due to its security
implications. With TC, your container bootstrapping code becomes a part of
the application and in build environments which don't provide access to a
Docker daemon, it becomes difficult to disable that bootstrapping code and
opt for something else. For instance, consider a GitLab CI environment
where neither Docker, nor Docker-in-Docker is supported due to security
reasons – I personally think this is the sanest approach. There you are
better off having your container bootstrapping code in two places,
`pom.xml` and `.gitlab-ci.yml`, and not in Java.

I had a talk about this with Sergei Egorov, the Testcontainers fame, and I
had the impression that the ideal approach would be to add multiple
containerization platform API (e.g. Kubernetes) support next to Docker. But
TC is not there yet.

I can indeed add a `@DisabledIfCondition` annotation, but I think it is
still the cleanest to keep the infra code out of the sources.

On Wed, Jun 22, 2022 at 7:36 PM Matt Sicker <bo...@gmail.com> wrote:

> What are you using for Docker integration tests? I've used
> Testcontainers for that for the past couple years, and it usually
> works fine on my machine. I do have Docker, though if the images are
> x86-only or something, I'm not able to run those without some
> finagling I think. You can use assumeTrue() or similar on a check for
> ability to run the tests, though, so that they do run where possible
> (simplest approach sometimes is to just add
> `@DisabledIfEnvironmentVariable(named = "CI", matches = "true")` to
> tests that won't work in CI without further setup).
>
> On Wed, Jun 22, 2022 at 1:57 AM Volkan Yazıcı <vo...@yazi.ci> wrote:
> >
> > Docker-dependent ITs need to be disabled by default. I forgot to copy the
> > skip=true snippet for maven-failsafe-plugin while merging LOG4J2-3502,
> > i.e., JPMS support for JTL. I have also added notes to not forget it next
> > time:
> >
> > Disable ITs, which are Docker-dependent, by default.
> > Running Docker on all expected environments (OSes, Docker-disabled CI
> > hosts, etc.) still needs to be worked out.
> > Next to that, certain container images (e.g., ELK stack) require
> > environment-specific limits (e.g., `nofile`).
> >
> > On Tue, Jun 21, 2022 at 6:39 PM Matt Sicker <bo...@gmail.com> wrote:
> >
> > > These tests are failing on macOS with a connection error of some sort.
> > > Not sure if it fails on other operating systems, but it reliably fails
> > > early enough in the macOS builds to cancel the others, and it fails
> > > for me locally as well. Possible cause could be related to JUnit
> > > dependency updates if there was a race condition involving finding
> > > available ports or something, though I haven't looked too closely at
> > > what's the issue.
> > >
>

Re: [log4j] New broken tests in master: LogstashIT

[Matt Sicker <bo...@gmail.com>]

What are you using for Docker integration tests? I've used
Testcontainers for that for the past couple years, and it usually
works fine on my machine. I do have Docker, though if the images are
x86-only or something, I'm not able to run those without some
finagling I think. You can use assumeTrue() or similar on a check for
ability to run the tests, though, so that they do run where possible
(simplest approach sometimes is to just add
`@DisabledIfEnvironmentVariable(named = "CI", matches = "true")` to
tests that won't work in CI without further setup).

On Wed, Jun 22, 2022 at 1:57 AM Volkan Yazıcı <vo...@yazi.ci> wrote:
>
> Docker-dependent ITs need to be disabled by default. I forgot to copy the
> skip=true snippet for maven-failsafe-plugin while merging LOG4J2-3502,
> i.e., JPMS support for JTL. I have also added notes to not forget it next
> time:
>
> Disable ITs, which are Docker-dependent, by default.
> Running Docker on all expected environments (OSes, Docker-disabled CI
> hosts, etc.) still needs to be worked out.
> Next to that, certain container images (e.g., ELK stack) require
> environment-specific limits (e.g., `nofile`).
>
> On Tue, Jun 21, 2022 at 6:39 PM Matt Sicker <bo...@gmail.com> wrote:
>
> > These tests are failing on macOS with a connection error of some sort.
> > Not sure if it fails on other operating systems, but it reliably fails
> > early enough in the macOS builds to cancel the others, and it fails
> > for me locally as well. Possible cause could be related to JUnit
> > dependency updates if there was a race condition involving finding
> > available ports or something, though I haven't looked too closely at
> > what's the issue.
> >

Re: [log4j] New broken tests in master: LogstashIT

[Volkan Yazıcı <vo...@yazi.ci>]

Docker-dependent ITs need to be disabled by default. I forgot to copy the
skip=true snippet for maven-failsafe-plugin while merging LOG4J2-3502,
i.e., JPMS support for JTL. I have also added notes to not forget it next
time:

Disable ITs, which are Docker-dependent, by default.
Running Docker on all expected environments (OSes, Docker-disabled CI
hosts, etc.) still needs to be worked out.
Next to that, certain container images (e.g., ELK stack) require
environment-specific limits (e.g., `nofile`).

On Tue, Jun 21, 2022 at 6:39 PM Matt Sicker <bo...@gmail.com> wrote:

> These tests are failing on macOS with a connection error of some sort.
> Not sure if it fails on other operating systems, but it reliably fails
> early enough in the macOS builds to cancel the others, and it fails
> for me locally as well. Possible cause could be related to JUnit
> dependency updates if there was a race condition involving finding
> available ports or something, though I haven't looked too closely at
> what's the issue.
>