startup timeout will be ignored when enabling functionsWorkerEnabled in pulsar container

[larmic]

When using

private static final PulsarContainer pulsarContainer =
        new PulsarContainer(DockerImageName.parse("apachepulsar/pulsar:latest"))
            .withStartupTimeout(Duration.of(2, ChronoUnit.MINUTES));

120 seconds will be used as waiting strategy timeout. See logs:
/pensive_antonelli: Waiting for 120 seconds for URL: http://localhost:54554/metrics

But when using

private static final PulsarContainer pulsarContainer =
        new PulsarContainer(DockerImageName.parse("apachepulsar/pulsar:latest"))
            .withFunctionsWorker()
            .withStartupTimeout(Duration.of(2, ChronoUnit.MINUTES));

startup timeout will be set to default (30 seconds). See logs:
/jolly_cerf: Waiting for 30 seconds for URL: http://localhost:54640/metrics

This is because when enabling functions worker a new log waiting strategy will be used. This fix will cache startup timeout and add (if exists) this to log waiting strategy.

[kiview]

Thanks for the PR, great find 👍
I added some comments regarding how I think we can simplify the implementation.

[kiview]

We can apply the startupTimeout to the WaitAllStrategy instead.

[larmic]

Done!

[kiview]

If instead of conditionally switching to the WaitAllStrategy we would just use a WaitAllStrategy as default and conditionally add the LogMessageWaitStrategy to it, it should simplify the implementation (no need for overriding withStartupTimeout anymore).

[larmic]

@kiview i don't know what i should do here. :( Can you explain that a little more? :)

[kiview]

I was thinking something like this in the constructor:

waitingFor(
            new WaitAllStrategy()
                .withStrategy(Wait.forHttp(METRICS_ENDPOINT).forStatusCode(200).forPort(BROKER_HTTP_PORT))
        );

And this in configure:

waitingFor(
                new WaitAllStrategy()
                    .withStrategy(waitStrategy)
                    .withStrategy(Wait.forLogMessage(".*Function worker service started.*", 1)
                        // NOTE withStartupTimeout must be called at the end, because start timeout will be
                        // applied to all strategies above
                    .withStartupTimeout(startupTimeout))
            );

But think somewhere along with the commits in this PR, we broke the shouldWaitForFunctionsWorkerStarted test. It fails on CI and locally now. Maybe I gave a wrong suggestion, we have to look into this.

[eddumelendez]

@larmic sorry for the delay and for not taking a look at this before. This was fixed via #5674