fix: don't bind Jetty if server.port is -1

[Dogacel]

Motivation:

Make behavior in Tomcat and Jetty consistent when server.port is set to -1.

Modifications:

• Configure JettyServlet to remove the binding to a random port if server.port is -1 and an armeria connector is added.

Result:

• Closes Spring integration using Jetty opens a port event when server.port: -1 #5039.
• Jetty no longer spawns the servlet on a random port if server.port is assigned to -1.

[ikhoon]

Question) I was wondering what would happen if server.port: 0.

[Dogacel]

Good question, I don't think we can differentiate. Do you think we should open a separate issue to upstream?

[ikhoon]

It would be a cleaner idea.
As a workaround, I'd like to add an option to close and remove ServerConnector in JettyService. It does not automatically work, though.
For example:

class JettyService {
    JettyService(..., boolean disableJettyServer) { ... }

    void start() throws Exception {
        if (disableJettyServer) {
            ...
            serverConnector.stop();
        }
    }
}

@Bean
public JettyService jettyService(ServletWebServerApplicationContext applicationContext,
                                 @Value("${server.port}") Integer port) throws Exception {
    final JettyWebServer jettyWebServer = jettyServer(applicationContext);
    boolean disableJettyServer == port != null && port == -1;
    return JettyService.of(jettyWebServer.getServer(), null, disableJettyServer);
}

[Dogacel]

I am trying to understand how we handle the -1 case for tomcat,

https://github.com/spring-projects/spring-boot/blob/3e4a9f5204ee2fe989583b17b77d8dab8558282a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactory.java#L326-L327

It seems like default behavior for both Jetty and Tomcat is the same, round it up to 0 and assign a random port.

Besides customizing the JettyService, what options do we have? Initial recommendation is to

We should override the getWebServer and fix it for consistent behavior with the Spring Tomcat integration.

However it would require us to copy-paste bunch of stuff, I am not sure if this is a pattern Armeria is willing to do.

[trustin]

It looks like it will take some time until the Spring folks figure out what to do on the upstream. Shall we proceed to introducing the workaround suggested by @ikhoon? That way, we could avoid unnecessary behavior changes when Spring is not involved.

[Dogacel]

Sorry it has been a busy couple weeks, travel, moving etc.

I can try to implement it 👍

[github-actions]

🔍 Build Scan® (commit: 13bbc65)

Job name	Status	Build Scan®
build-ubicloud-standard-16-jdk-8	✅	https://ge.armeria.dev/s/qqyyrxqifpaxc
build-ubicloud-standard-16-jdk-21-snapshot-blockhound	✅	https://ge.armeria.dev/s/3dduxdb72k46a
build-ubicloud-standard-16-jdk-17-min-java-17-coverage	✅	https://ge.armeria.dev/s/mvkdvlrwchfyw
build-ubicloud-standard-16-jdk-17-min-java-11	✅	https://ge.armeria.dev/s/il6bb5mhi6cxc
build-ubicloud-standard-16-jdk-17-leak	✅	https://ge.armeria.dev/s/22rxwpe5ry57o
build-ubicloud-standard-16-jdk-11	✅	https://ge.armeria.dev/s/kkf4o2rcdk52c
build-macos-latest-jdk-21	✅	https://ge.armeria.dev/s/xw4y4hlvugjmq

[mergify]

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

• Queue this pull request