From a8cbb5ccf7b45ca8a3b0e89c2d21d00fbdd8b4bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Maltoni?= Date: Wed, 27 Nov 2024 17:20:17 +0100 Subject: [PATCH 01/15] build: migrate logger to SLF4J APIs + Logback backend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Squashed commit of the following: commit cf369f1115a8dbdeeb910307fa0d5561975dae27 Author: Niccolò Maltoni Date: Thu Nov 21 09:56:10 2024 +0100 feat: drop CarapaceLogger commit 95f20ccc7a4667172279ec8a244e5bd26c18c9d4 Author: Niccolò Maltoni Date: Wed Nov 20 17:24:56 2024 +0100 fix: revert old tricky behavior commit 10ea9a75ff5e281f08827917f4df5896de069a29 Author: Niccolò Maltoni Date: Fri Oct 25 18:28:58 2024 +0200 refactor: fix logs commit 81d2a836d12d69b1447a8871d55f20f63b325320 Author: Niccolò Maltoni Date: Fri Nov 8 16:35:29 2024 +0100 build: fix Surefire modules opening commit 5147e5724cd53e69b918234e60acb711b7ea6039 Author: Niccolò Maltoni Date: Fri Nov 8 14:38:23 2024 +0100 fix: move logback configuration to the right path commit b93a27f7c32020e7ec246f46ae2edfe678efce1f Author: Niccolò Maltoni Date: Fri Nov 8 12:10:37 2024 +0100 build: switch logging backend to Logback * drop Apache Commons Logging dependency * add Apache Commons Logging to SLF4J adapter because of other dependencies * drop JUL backend for SLF4J * add Logback as backend for SLF4J commit 6bc2a5e43deec85de75fdb3198214a4386916e70 Author: Niccolò Maltoni Date: Fri Nov 8 11:40:37 2024 +0100 feat: migrate JUL to SLF4J  Conflicts:  carapace-server/src/main/java/org/carapaceproxy/core/Listeners.java  carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequestsManager.java  carapace-server/src/main/java/org/carapaceproxy/core/RequestsLogger.java  carapace-server/src/main/java/org/carapaceproxy/core/RuntimeServerConfiguration.java  carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java  carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthStatus.java  carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java  carapace-server/src/test/java/org/carapaceproxy/RawClientTest.java  carapace-server/src/test/java/org/carapaceproxy/backends/RestartEndpointTest.java  pom.xml --- .../org/carapaceproxy/core/Listeners.java | 9 ++--- .../core/ProxyRequestsManager.java | 35 +++---------------- .../java/org/carapaceproxy/RawClientTest.java | 2 -- 3 files changed, 7 insertions(+), 39 deletions(-) diff --git a/carapace-server/src/main/java/org/carapaceproxy/core/Listeners.java b/carapace-server/src/main/java/org/carapaceproxy/core/Listeners.java index 3fbf03bf7..d25973fa4 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/core/Listeners.java +++ b/carapace-server/src/main/java/org/carapaceproxy/core/Listeners.java @@ -294,9 +294,7 @@ protected SslHandler newSslHandler(SslContext context, ByteBufAllocator allocato .handle((request, response) -> { // Custom request-response handling if (LOG.isDebugEnabled()) { LOG.debug( - "Receive request {} From {} Timestamp {}", - request.uri(), - request.remoteAddress(), + "Receive request {} From {} Timestamp {}", request.uri(), request.remoteAddress(), LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyy-MM-dd-HH-mm-ss.SSS")) ); } @@ -312,10 +310,7 @@ protected SslHandler newSslHandler(SslContext context, ByteBufAllocator allocato // response compression if (currentConfiguration.getResponseCompressionThreshold() >= 0) { LOG.debug( - "Response compression enabled with min size = {} bytes for listener {}", - currentConfiguration.getResponseCompressionThreshold(), - hostPort - ); + "Response compression enabled with min size = {} bytes for listener {}", currentConfiguration.getResponseCompressionThreshold(), hostPort); httpServer = httpServer.compress(currentConfiguration.getResponseCompressionThreshold()); } else { LOG.debug("Response compression disabled for listener {}", hostPort); diff --git a/carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequestsManager.java b/carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequestsManager.java index 65c3d10d4..b3c731c82 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequestsManager.java +++ b/carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequestsManager.java @@ -401,12 +401,7 @@ public Publisher forward(ProxyRequest request, boolean cache) { .doOnRequest((req, conn) -> { if (LOGGER.isDebugEnabled()) { LOGGER.debug( - "Start sending request for Using client id {}_{} Uri {} Timestamp {} Backend {}:{}", - key, - connectionConfig.getId(), - req.resourceUrl(), - LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyy-MM-dd-HH-mm-ss.SSS")), - endpointHost, + "Start sending request for Using client id {}_{} Uri {} Timestamp {} Backend {}:{}", key, connectionConfig.getId(), req.resourceUrl(), LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyy-MM-dd-HH-mm-ss.SSS")), endpointHost, endpointPort ); } @@ -415,12 +410,7 @@ public Publisher forward(ProxyRequest request, boolean cache) { }).doAfterRequest((req, conn) -> { if (LOGGER.isDebugEnabled()) { LOGGER.debug( - "Finished sending request for Using client id {}_{} Uri {} Timestamp {} Backend {}:{}", - key, - connectionConfig.getId(), - request.getUri(), - LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyy-MM-dd-HH-mm-ss.SSS")), - endpointHost, + "Finished sending request for Using client id {}_{} Uri {} Timestamp {} Backend {}:{}", key, connectionConfig.getId(), request.getUri(), LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyy-MM-dd-HH-mm-ss.SSS")), endpointHost, endpointPort ); } @@ -451,12 +441,7 @@ public Publisher forward(ProxyRequest request, boolean cache) { .response((resp, flux) -> { // endpoint response if (LOGGER.isDebugEnabled()) { LOGGER.debug( - "Receive response from backend for {} Using client id {}_{} uri{} timestamp {} Backend: {}", - request.getRemoteAddress(), - key, - connectionConfig.getId(), - request.getUri(), - LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyy-MM-dd-HH-mm-ss.SSS")), + "Receive response from backend for {} Using client id {}_{} uri{} timestamp {} Backend: {}", request.getRemoteAddress(), key, connectionConfig.getId(), request.getUri(), LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyy-MM-dd-HH-mm-ss.SSS")), request.getAction().getHost() ); } @@ -494,12 +479,7 @@ public Publisher forward(ProxyRequest request, boolean cache) { }).doOnComplete(() -> { if (LOGGER.isDebugEnabled()) { LOGGER.debug( - "Send all response to client {} Using client id {}_{} for uri {} timestamp {} Backend: {}", - request.getRemoteAddress(), - key, - connectionConfig.getId(), - request.getUri(), - LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyy-MM-dd-HH-mm-ss.SSS")), + "Send all response to client {} Using client id {}_{} for uri {} timestamp {} Backend: {}", request.getRemoteAddress(), key, connectionConfig.getId(), request.getUri(), LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyy-MM-dd-HH-mm-ss.SSS")), request.getAction().getHost() ); } @@ -644,12 +624,7 @@ public void reloadConfiguration(RuntimeServerConfiguration newConfiguration, Col // max connections per endpoint limit setup newEndpoints.forEach(be -> { LOGGER.debug( - "Setup max connections per endpoint {}:{} = {} for connectionpool {}", - be.host(), - be.port(), - connectionPool.getMaxConnectionsPerEndpoint(), - connectionPool.getId() - ); + "Setup max connections per endpoint {}:{} = {} for connectionpool {}", be.host(), be.port(), connectionPool.getMaxConnectionsPerEndpoint(), connectionPool.getId()); builder.forRemoteHost(InetSocketAddress.createUnresolved(be.host(), be.port()), spec -> { spec.maxConnections(connectionPool.getMaxConnectionsPerEndpoint()); spec.pendingAcquireTimeout(Duration.ofMillis(connectionPool.getBorrowTimeout())); diff --git a/carapace-server/src/test/java/org/carapaceproxy/RawClientTest.java b/carapace-server/src/test/java/org/carapaceproxy/RawClientTest.java index cfe660a5f..1cb1cc80d 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/RawClientTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/RawClientTest.java @@ -320,7 +320,6 @@ public void testServerRequestContinue() throws Exception { ExecutorService ex = Executors.newFixedThreadPool(2); List> futures = new ArrayList<>(); - try (HttpProxyServer proxy = HttpProxyServer.buildForTests("localhost", 0, mapper, tmpDir.newFolder())) { ConnectionPoolConfiguration defaultConnectionPool = proxy.getCurrentConfiguration().getDefaultConnectionPool(); defaultConnectionPool.setMaxConnectionsPerEndpoint(1); @@ -529,7 +528,6 @@ public void testMultiClientTimeout() throws Exception { TestEndpointMapper mapper = new TestEndpointMapper("localhost", wireMockRule.port()); ExecutorService ex = Executors.newFixedThreadPool(2); List> futures = new ArrayList<>(); - try (HttpProxyServer proxy = HttpProxyServer.buildForTests("localhost", 0, mapper, tmpDir.newFolder())) { ConnectionPoolConfiguration defaultConnectionPool = proxy.getCurrentConfiguration().getDefaultConnectionPool(); defaultConnectionPool.setMaxConnectionsPerEndpoint(1); From 9c24ad6f5e0a71eb2780f93fd258adde2bacd96a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Maltoni?= Date: Wed, 27 Nov 2024 17:22:44 +0100 Subject: [PATCH 02/15] refactor: extract out the RandomBackendSelector  Conflicts:  carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java  Conflicts:  carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequestsManager.java  carapace-server/src/main/java/org/carapaceproxy/server/mapper/EndpointMapper.java  carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java --- .../carapaceproxy/server/mapper/StandardEndpointMapper.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java b/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java index beb32d277..fa337cf28 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java @@ -346,14 +346,17 @@ public void configure(ConfigurationStore properties) throws ConfigurationNotVali this.defaultServiceUnavailable = properties.getString("default.action.serviceunavailable", DEFAULT_SERVICE_UNAVAILABLE_ACTION); LOG.info("configured default.action.serviceunavailable={}", defaultServiceUnavailable); + this.forceDirectorParameter = properties.getString("mapper.forcedirector.parameter", forceDirectorParameter); LOG.info("configured mapper.forcedirector.parameter={}", forceDirectorParameter); + this.forceBackendParameter = properties.getString("mapper.forcebackend.parameter", forceBackendParameter); LOG.info("configured mapper.forcebackend.parameter={}", forceBackendParameter); // To add custom debugging header for request chosen mapping-path this.debuggingHeaderEnabled = properties.getBoolean("mapper.debug", false); LOG.info("configured mapper.debug={}", debuggingHeaderEnabled); + this.debuggingHeaderName = properties.getString("mapper.debug.name", DEBUGGING_HEADER_DEFAULT_NAME); LOG.info("configured mapper.debug.name={}", debuggingHeaderName); @@ -426,7 +429,8 @@ public void configure(ConfigurationStore properties) throws ConfigurationNotVali } addAction(action); - LOG.info("configured action {} type={} enabled:{} headers:{} redirect location:{} redirect proto:{} redirect host:{} redirect port:{} redirect path:{}", id, actionType, enabled, headersIds, redirectLocation, action.getRedirectProto(), action.getRedirectHost(), action.getRedirectPort(), action.getRedirectPath()); + LOG.info("configured action {} type={} enabled:{} headers:{} redirect location:{} redirect proto:{} redirect host:{} redirect port:{} redirect path:{}", + id, actionType, enabled, headersIds, redirectLocation, action.getRedirectProto(), action.getRedirectHost(), action.getRedirectPort(), action.getRedirectPath()); } } From 4b6eefc18b80d3ad5ad432f31bcbc948892fce9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Maltoni?= Date: Wed, 27 Nov 2024 17:24:18 +0100 Subject: [PATCH 03/15] refactor: recordize BackendHealthCheck  Conflicts:  carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java  carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthStatus.java  Conflicts:  carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java  carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthStatus.java --- .../carapaceproxy/server/backends/BackendHealthManager.java | 5 +++-- .../carapaceproxy/server/backends/BackendHealthStatus.java | 2 +- .../src/test/java/org/carapaceproxy/BigUploadTest.java | 4 ++-- .../test/java/org/carapaceproxy/ConcurrentClientsTest.java | 6 +++--- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java b/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java index bd023b0b1..0e77582ec 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java @@ -151,10 +151,11 @@ public void run() { if (checkResult.isOk()) { final var responseTime = checkResult.endTs() - checkResult.startTs(); if (status.isReportedAsUnreachable()) { - LOG.warn("backend {} was unreachable, setting again to reachable. Response time {}ms", status.getHostPort(), responseTime); + LOG.warn("backend {} was unreachable, setting again to reachable. Response time {} ms", + status.getHostPort(), responseTime); reportBackendReachable(status.getHostPort()); } else { - LOG.debug("backend {} seems reachable. Response time {}ms", status.getHostPort(), responseTime); + LOG.debug("backend {} seems reachable. Response time {} ms", status.getHostPort(), responseTime); } } else { if (status.isReportedAsUnreachable()) { diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthStatus.java b/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthStatus.java index e0a6afe57..c05a00a6b 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthStatus.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthStatus.java @@ -20,9 +20,9 @@ package org.carapaceproxy.server.backends; import java.sql.Timestamp; +import org.carapaceproxy.core.EndpointKey; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.carapaceproxy.core.EndpointKey; /** * Health of a backend diff --git a/carapace-server/src/test/java/org/carapaceproxy/BigUploadTest.java b/carapace-server/src/test/java/org/carapaceproxy/BigUploadTest.java index e41e2ad85..fd85ee6d8 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/BigUploadTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/BigUploadTest.java @@ -19,8 +19,6 @@ */ package org.carapaceproxy; -import static org.junit.Assert.assertThrows; -import static org.junit.Assert.assertTrue; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -37,6 +35,8 @@ import org.apache.commons.io.IOUtils; import org.carapaceproxy.core.HttpProxyServer; import org.carapaceproxy.utils.TestEndpointMapper; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; diff --git a/carapace-server/src/test/java/org/carapaceproxy/ConcurrentClientsTest.java b/carapace-server/src/test/java/org/carapaceproxy/ConcurrentClientsTest.java index 207161a3b..1856300c2 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/ConcurrentClientsTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/ConcurrentClientsTest.java @@ -23,9 +23,6 @@ import static com.github.tomakehurst.wiremock.client.WireMock.get; import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import com.github.tomakehurst.wiremock.junit.WireMockRule; import java.net.URI; import java.nio.charset.StandardCharsets; @@ -37,6 +34,9 @@ import org.apache.commons.io.IOUtils; import org.carapaceproxy.core.HttpProxyServer; import org.carapaceproxy.utils.TestEndpointMapper; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; From 1c17c7daade83de82c448dbb48023a14f4b41af9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Maltoni?= Date: Wed, 27 Nov 2024 17:26:09 +0100 Subject: [PATCH 04/15] build: fix InaccessibleObjectException  Conflicts:  pom.xml  Conflicts:  pom.xml --- pom.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/pom.xml b/pom.xml index 4ffb2a0c7..85586439e 100644 --- a/pom.xml +++ b/pom.xml @@ -354,7 +354,6 @@ --add-opens java.base/java.util=ALL-UNNAMED --add-opens java.base/java.util.concurrent=ALL-UNNAMED --add-opens java.rmi/sun.rmi.transport=ALL-UNNAMED - --add-opens java.xml/jdk.xml.internal=ALL-UNNAMED From 9a2026c09b1c64c0c683903a3e6211d40675af64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Maltoni?= Date: Wed, 27 Nov 2024 17:39:14 +0100 Subject: [PATCH 05/15] refactor: cleanup tests  Conflicts:  carapace-server/src/test/java/org/carapaceproxy/DatabaseConfigurationTest.java  carapace-server/src/test/java/org/carapaceproxy/RawClientTest.java --- .../carapaceproxy/ApplyConfigurationTest.java | 13 +++++++----- .../java/org/carapaceproxy/BigUploadTest.java | 4 ++-- .../carapaceproxy/ConcurrentClientsTest.java | 6 +++--- .../DatabaseConfigurationTest.java | 8 ++++---- .../carapaceproxy/MaintenanceModeTest.java | 20 ++++++++++--------- .../backends/ConnectionPoolTest.java | 2 +- .../carapaceproxy/core/MaxHeaderSizeTest.java | 19 +++++++++--------- .../server/certificates/CertificatesTest.java | 1 + 8 files changed, 40 insertions(+), 33 deletions(-) diff --git a/carapace-server/src/test/java/org/carapaceproxy/ApplyConfigurationTest.java b/carapace-server/src/test/java/org/carapaceproxy/ApplyConfigurationTest.java index 18cb1d467..a9cd7a899 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/ApplyConfigurationTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/ApplyConfigurationTest.java @@ -81,16 +81,19 @@ public static void setupWireMock() { */ public static final class StaticEndpointMapper extends TestEndpointMapper { + public StaticEndpointMapper(final HttpProxyServer ignoredServer) { + this(); // required for reflective construction + } + public StaticEndpointMapper() { super("localhost", wireMockRule.port()); } - } @Test public void testChangeListenersConfig() throws Exception { - try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder());) { + try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder())) { { Properties configuration = new Properties(); @@ -217,7 +220,7 @@ public void testChangeListenersConfig() throws Exception { @Test public void testReloadMapper() throws Exception { - try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder());) { + try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder())) { { Properties configuration = new Properties(); @@ -336,7 +339,7 @@ public void testUserRealm() throws Exception { @Test public void testChangeFiltersConfiguration() throws Exception { - try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder());) { + try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder())) { { Properties configuration = new Properties(); @@ -375,7 +378,7 @@ public void testChangeFiltersConfiguration() throws Exception { @Test public void testChangeBackendHealthManagerConfiguration() throws Exception { - try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder());) { + try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder())) { { Properties configuration = new Properties(); diff --git a/carapace-server/src/test/java/org/carapaceproxy/BigUploadTest.java b/carapace-server/src/test/java/org/carapaceproxy/BigUploadTest.java index fd85ee6d8..e41e2ad85 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/BigUploadTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/BigUploadTest.java @@ -19,6 +19,8 @@ */ package org.carapaceproxy; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -35,8 +37,6 @@ import org.apache.commons.io.IOUtils; import org.carapaceproxy.core.HttpProxyServer; import org.carapaceproxy.utils.TestEndpointMapper; -import static org.junit.Assert.assertThrows; -import static org.junit.Assert.assertTrue; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; diff --git a/carapace-server/src/test/java/org/carapaceproxy/ConcurrentClientsTest.java b/carapace-server/src/test/java/org/carapaceproxy/ConcurrentClientsTest.java index 1856300c2..207161a3b 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/ConcurrentClientsTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/ConcurrentClientsTest.java @@ -23,6 +23,9 @@ import static com.github.tomakehurst.wiremock.client.WireMock.get; import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import com.github.tomakehurst.wiremock.junit.WireMockRule; import java.net.URI; import java.nio.charset.StandardCharsets; @@ -34,9 +37,6 @@ import org.apache.commons.io.IOUtils; import org.carapaceproxy.core.HttpProxyServer; import org.carapaceproxy.utils.TestEndpointMapper; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; diff --git a/carapace-server/src/test/java/org/carapaceproxy/DatabaseConfigurationTest.java b/carapace-server/src/test/java/org/carapaceproxy/DatabaseConfigurationTest.java index 130fa5c28..492904bf7 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/DatabaseConfigurationTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/DatabaseConfigurationTest.java @@ -48,7 +48,7 @@ public class DatabaseConfigurationTest { @Test public void testBootWithDatabaseStore() throws Exception { - try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder());) { + try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder())) { Properties configuration = new Properties(); @@ -70,7 +70,7 @@ public void testBootWithDatabaseStore() throws Exception { public void testChangeFiltersConfiguration() throws Exception { File databaseFolder = tmpDir.newFolder(); - try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder());) { + try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder())) { Properties configurationAtBoot = new Properties(); configurationAtBoot.put("db.jdbc.url", "jdbc:herddb:localhost"); @@ -100,7 +100,7 @@ public void testChangeFiltersConfiguration() throws Exception { } // reboot, new configuration MUST be kept - try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder());) { + try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder())) { Properties configurationAtBoot = new Properties(); configurationAtBoot.put("db.jdbc.url", "jdbc:herddb:localhost"); configurationAtBoot.put("db.server.base.dir", tmpDir.newFolder().getAbsolutePath()); @@ -119,7 +119,7 @@ public void testChangeFiltersConfiguration() throws Exception { } // reboot, new configuration MUST be kept - try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder());) { + try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder())) { Properties configurationAtBoot = new Properties(); configurationAtBoot.put("db.jdbc.url", "jdbc:herddb:localhost"); configurationAtBoot.put("db.server.base.dir", tmpDir.newFolder().getAbsolutePath()); diff --git a/carapace-server/src/test/java/org/carapaceproxy/MaintenanceModeTest.java b/carapace-server/src/test/java/org/carapaceproxy/MaintenanceModeTest.java index 5dea0e248..daa47a2e2 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/MaintenanceModeTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/MaintenanceModeTest.java @@ -1,21 +1,22 @@ package org.carapaceproxy; +import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; +import static com.github.tomakehurst.wiremock.client.WireMock.get; +import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; +import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import com.github.tomakehurst.wiremock.junit.WireMockRule; -import org.carapaceproxy.api.UseAdminServer; -import org.carapaceproxy.utils.TestUtils; -import org.junit.Rule; -import org.junit.Test; - import java.net.URI; import java.net.http.HttpClient; import java.net.http.HttpRequest; import java.net.http.HttpResponse; import java.util.Base64; import java.util.Properties; - -import static com.github.tomakehurst.wiremock.client.WireMock.*; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; +import org.carapaceproxy.api.UseAdminServer; +import org.carapaceproxy.utils.TestUtils; +import org.junit.Rule; +import org.junit.Test; public class MaintenanceModeTest extends UseAdminServer { @@ -35,6 +36,7 @@ public void test() throws Exception { .withBody("it works !!"))); config = new Properties(HTTP_ADMIN_SERVER_CONFIG); + config.put("healthmanager.tolerant", "true"); startServer(config); // Default certificate diff --git a/carapace-server/src/test/java/org/carapaceproxy/backends/ConnectionPoolTest.java b/carapace-server/src/test/java/org/carapaceproxy/backends/ConnectionPoolTest.java index 8e9fac3da..708af19ee 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/backends/ConnectionPoolTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/backends/ConnectionPoolTest.java @@ -69,7 +69,6 @@ public class ConnectionPoolTest extends UseAdminServer { public WireMockRule wireMockRule = new WireMockRule(0); private void configureAndStartServer() throws Exception { - HttpTestUtils.overrideJvmWideHttpsVerifier(); stubFor(get(urlEqualTo("/index.html")) @@ -85,6 +84,7 @@ private void configureAndStartServer() throws Exception { config.put("db.server.base.dir", tmpDir.newFolder().getAbsolutePath()); config.put("aws.accesskey", "accesskey"); config.put("aws.secretkey", "secretkey"); + config.put("healthmanager.tolerant", "true"); startServer(config); // Default certificate diff --git a/carapace-server/src/test/java/org/carapaceproxy/core/MaxHeaderSizeTest.java b/carapace-server/src/test/java/org/carapaceproxy/core/MaxHeaderSizeTest.java index 77a5a5a69..a91e74c1e 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/core/MaxHeaderSizeTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/core/MaxHeaderSizeTest.java @@ -1,19 +1,20 @@ package org.carapaceproxy.core; +import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; +import static com.github.tomakehurst.wiremock.client.WireMock.get; +import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; +import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; +import static org.junit.Assert.assertEquals; import com.github.tomakehurst.wiremock.junit.WireMockRule; -import org.carapaceproxy.api.UseAdminServer; -import org.carapaceproxy.utils.TestUtils; -import org.junit.Rule; -import org.junit.Test; - import java.net.URI; import java.net.http.HttpClient; import java.net.http.HttpRequest; import java.net.http.HttpResponse; import java.util.Properties; - -import static com.github.tomakehurst.wiremock.client.WireMock.*; -import static org.junit.Assert.assertEquals; +import org.carapaceproxy.api.UseAdminServer; +import org.carapaceproxy.utils.TestUtils; +import org.junit.Rule; +import org.junit.Test; public class MaxHeaderSizeTest extends UseAdminServer { @@ -33,6 +34,7 @@ public void test() throws Exception { .withBody("it works !!"))); config = new Properties(HTTP_ADMIN_SERVER_CONFIG); + config.put("healthmanager.tolerant", "true"); startServer(config); // Default certificate @@ -71,7 +73,6 @@ public void test() throws Exception { changeDynamicConfiguration(config); - HttpClient httpClient = HttpClient.newBuilder() .version(HttpClient.Version.HTTP_1_1) .build(); diff --git a/carapace-server/src/test/java/org/carapaceproxy/server/certificates/CertificatesTest.java b/carapace-server/src/test/java/org/carapaceproxy/server/certificates/CertificatesTest.java index da3f01f59..2b2ae8b27 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/server/certificates/CertificatesTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/server/certificates/CertificatesTest.java @@ -123,6 +123,7 @@ private void configureAndStartServer() throws Exception { config.put("db.server.base.dir", tmpDir.newFolder().getAbsolutePath()); config.put("aws.accesskey", "accesskey"); config.put("aws.secretkey", "secretkey"); + config.put("healthmanager.tolerant", "true"); startServer(config); // Default certificate From dc6f8bd642cfc25def4465fc2e6debdaa4524200 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Maltoni?= Date: Wed, 27 Nov 2024 17:39:45 +0100 Subject: [PATCH 06/15] feat: introduce status tracking to BackendHealth* entities feat: introduce status tracking to BackendHealth* entities feat: introduce status tracking to BackendHealth* entities  Conflicts:  carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthStatus.java --- .../carapaceproxy/api/BackendsResource.java | 8 +- .../org/carapaceproxy/core/ProxyRequest.java | 2 +- .../server/backends/BackendHealthCheck.java | 41 ++++++--- .../server/backends/BackendHealthManager.java | 90 ++++++++----------- .../server/backends/BackendHealthStatus.java | 85 +++++++++++------- .../server/mapper/EndpointMapper.java | 2 +- .../server/mapper/StandardEndpointMapper.java | 66 +++++++------- .../backends/StuckRequestsTest.java | 7 +- .../backends/UnreachableBackendTest.java | 28 +++--- .../BasicStandardEndpointMapperTest.java | 41 ++++++--- .../server/mapper/HealthCheckTest.java | 36 ++++---- 11 files changed, 223 insertions(+), 183 deletions(-) diff --git a/carapace-server/src/main/java/org/carapaceproxy/api/BackendsResource.java b/carapace-server/src/main/java/org/carapaceproxy/api/BackendsResource.java index 191146858..c9411479b 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/api/BackendsResource.java +++ b/carapace-server/src/main/java/org/carapaceproxy/api/BackendsResource.java @@ -96,13 +96,13 @@ public Map getAll() { } BackendHealthStatus bhs = backendsSnapshot.get(key); if (bhs != null) { - bean.available = bhs.isAvailable(); - bean.reportedAsUnreachable = bhs.isReportedAsUnreachable(); - bean.reportedAsUnreachableTs = bhs.getReportedAsUnreachableTs(); + bean.available = bhs.getStatus() != BackendHealthStatus.Status.DOWN; + bean.reportedAsUnreachable = bhs.getStatus() == BackendHealthStatus.Status.DOWN; + bean.reportedAsUnreachableTs = bhs.getLastUnreachableTs(); BackendHealthCheck lastProbe = bhs.getLastProbe(); if (lastProbe != null) { bean.lastProbeTs = lastProbe.endTs(); - bean.lastProbeSuccess = lastProbe.isOk(); + bean.lastProbeSuccess = lastProbe.ok(); bean.httpResponse = lastProbe.httpResponse(); bean.httpBody = lastProbe.httpBody(); } diff --git a/carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequest.java b/carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequest.java index 17790078f..66d735cd9 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequest.java +++ b/carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequest.java @@ -219,7 +219,7 @@ public boolean isValidHostAndPort(final String hostAndPort) { return !host.isBlank() && (InternetDomainName.isValid(host) || InetAddresses.isInetAddress(host)) && parsed.getPort() >= 0 - && parsed.getPort() <= 65535; + && parsed.getPort() <= EndpointKey.MAX_PORT; } else { return !host.isBlank() && (InternetDomainName.isValid(host) || InetAddresses.isInetAddress(host)); diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthCheck.java b/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthCheck.java index b7faea733..dec34aa2c 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthCheck.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthCheck.java @@ -28,8 +28,20 @@ import java.nio.charset.StandardCharsets; import java.util.Objects; import javax.ws.rs.core.UriBuilder; +import org.carapaceproxy.server.config.BackendConfiguration; import org.carapaceproxy.utils.IOUtils; +/** + * The record models a single health check. + * It includes information about when the backend was probed and what it responded. + * + * @param path the probe path + * @param startTs when the request was done + * @param endTs when the answer was received + * @param result the probe result + * @param httpResponse a string representation of the response header line + * @param httpBody a string representation of the body of the response + */ public record BackendHealthCheck( String path, long startTs, @@ -38,21 +50,12 @@ public record BackendHealthCheck( String httpResponse, String httpBody ) { - private enum Result { - SUCCESS, // 1 - FAILURE_CONNECTION, // 2 - FAILURE_STATUS // 3 - } - public long getResponseTime() { - return endTs - startTs; + public static BackendHealthCheck check(final BackendConfiguration bconf, final int timeoutMillis) { + return check(bconf.host(), bconf.port(), bconf.probePath(), timeoutMillis); } - public boolean isOk() { - return result == Result.SUCCESS; - } - - public static BackendHealthCheck check(String host, int port, String path, int timeoutMillis) { + public static BackendHealthCheck check(final String host, final int port, final String path, final int timeoutMillis) { if (path.isEmpty()) { long now = System.currentTimeMillis(); return new BackendHealthCheck(path, now, now, Result.SUCCESS, "OK", "MOCK OK"); @@ -125,4 +128,18 @@ public static BackendHealthCheck check(String host, int port, String path, int t ); } } + + public long responseTime() { + return endTs - startTs; + } + + public boolean ok() { + return result == Result.SUCCESS; + } + + public enum Result { + SUCCESS, // 1 + FAILURE_CONNECTION, // 2 + FAILURE_STATUS // 3 + } } diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java b/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java index 0e77582ec..a8f667bb1 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java @@ -21,9 +21,6 @@ import com.google.common.annotations.VisibleForTesting; import io.prometheus.client.Gauge; -import java.util.ArrayList; -import java.util.Collection; -import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executors; @@ -53,12 +50,10 @@ public class BackendHealthManager implements Runnable { private static final Gauge BACKEND_UPSTATUS_GAUGE = PrometheusUtils .createGauge("health", "backend_status", "backend status", "host") .register(); - + private final ConcurrentHashMap backends = new ConcurrentHashMap<>(); private EndpointMapper mapper; - private ScheduledExecutorService timer; private ScheduledFuture scheduledFuture; - // can change at runtime private volatile int period; // can change at runtime @@ -66,8 +61,6 @@ public class BackendHealthManager implements Runnable { // keep track of start() calling private volatile boolean started; - private final ConcurrentHashMap backends = new ConcurrentHashMap<>(); - public BackendHealthManager(final RuntimeServerConfiguration conf, final EndpointMapper mapper) { this.mapper = mapper; @@ -141,73 +134,62 @@ public void run() { if (mapper == null) { return; } - Collection backendConfigurations = mapper.getBackends().values(); - for (BackendConfiguration bconf : backendConfigurations) { - BackendHealthStatus status = backends.computeIfAbsent(bconf.hostPort(), BackendHealthStatus::new); - - BackendHealthCheck checkResult = BackendHealthCheck.check( - bconf.host(), bconf.port(), bconf.probePath(), connectTimeout); - - if (checkResult.isOk()) { - final var responseTime = checkResult.endTs() - checkResult.startTs(); - if (status.isReportedAsUnreachable()) { - LOG.warn("backend {} was unreachable, setting again to reachable. Response time {} ms", - status.getHostPort(), responseTime); - reportBackendReachable(status.getHostPort()); - } else { - LOG.debug("backend {} seems reachable. Response time {} ms", status.getHostPort(), responseTime); + for (final BackendConfiguration backend : mapper.getBackends().values()) { + final EndpointKey endpoint = backend.hostPort(); + final BackendHealthStatus status = findStatus(endpoint); + final BackendHealthCheck checkResult = BackendHealthCheck.check(backend, connectTimeout); + + if (checkResult.ok()) { + switch (status.getStatus()) { + case DOWN -> + LOG.warn("backend {} was unreachable, setting again to reachable. Response time {} ms", endpoint, checkResult.responseTime()); + case COLD, STABLE -> + LOG.debug("backend {} seems reachable. Response time {} ms", endpoint, checkResult.responseTime()); } + reportBackendReachable(endpoint, checkResult.endTs()); } else { - if (status.isReportedAsUnreachable()) { - LOG.debug("backend {} still unreachable. Cause: {}", status.getHostPort(), checkResult.httpResponse()); - } else { - LOG.warn("backend {} became unreachable. Cause: {}", status.getHostPort(), checkResult.httpResponse()); - reportBackendUnreachable(status.getHostPort(), checkResult.endTs(), checkResult.httpResponse()); + switch (status.getStatus()) { + case DOWN -> + LOG.debug("backend {} still unreachable. Cause: {}", endpoint, checkResult.httpResponse()); + case COLD, STABLE -> + LOG.warn("backend {} became unreachable. Cause: {}", endpoint, checkResult.httpResponse()); } + reportBackendUnreachable(endpoint, checkResult.endTs(), checkResult.httpResponse()); } status.setLastProbe(checkResult); BACKEND_UPSTATUS_GAUGE - .labels(bconf.host() + "_" + bconf.port()) - .set(status.isReportedAsUnreachable() ? 0 : 1); - } - List toRemove = new ArrayList<>(); - for (final EndpointKey key : backends.keySet()) { - boolean found = false; - for (BackendConfiguration bconf : backendConfigurations) { - if (bconf.hostPort().equals(key)) { - found = true; - break; - } - } - if (!found) { - toRemove.add(key); - } + .labels(backend.host() + "_" + backend.port()) + .set(status.getStatus() == BackendHealthStatus.Status.DOWN ? 0 : 1); } - if (!toRemove.isEmpty()) { - LOG.info("discarding backends {}", toRemove); - toRemove.forEach(backends::remove); + cleanup(); + } + + private void cleanup() { + if (mapper == null) { + return; } + backends.keySet().retainAll(mapper.getBackends().values().stream().map(BackendConfiguration::hostPort).toList()); } - public void reportBackendUnreachable(final EndpointKey hostPort, final long timestamp, final String cause) { - getBackendStatus(hostPort).reportAsUnreachable(timestamp, cause); + public void reportBackendReachable(final EndpointKey hostPort, final long timestamp) { + findStatus(hostPort).reportAsReachable(timestamp); } - private BackendHealthStatus getBackendStatus(final EndpointKey hostPort) { - return backends.computeIfAbsent(hostPort, BackendHealthStatus::new); + public void reportBackendUnreachable(final EndpointKey hostPort, final long timestamp, final String cause) { + findStatus(hostPort).reportAsUnreachable(timestamp, cause); } - public void reportBackendReachable(final EndpointKey hostPort) { - getBackendStatus(hostPort).reportAsReachable(); + private BackendHealthStatus findStatus(final EndpointKey hostPort) { + return backends.computeIfAbsent(hostPort, BackendHealthStatus::new); } public Map getBackendsSnapshot() { return Map.copyOf(backends); } - public boolean isAvailable(final EndpointKey hostPort) { - return getBackendStatus(hostPort).isAvailable(); + public BackendHealthStatus.Status getBackendStatus(final EndpointKey hostPort) { + return findStatus(hostPort).getStatus(); } @VisibleForTesting diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthStatus.java b/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthStatus.java index c05a00a6b..6fddd1586 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthStatus.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthStatus.java @@ -20,6 +20,7 @@ package org.carapaceproxy.server.backends; import java.sql.Timestamp; +import java.time.Duration; import org.carapaceproxy.core.EndpointKey; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -31,65 +32,87 @@ */ public class BackendHealthStatus { + // todo replace this with a property of some kind + public static final long WARMUP_MILLIS = Duration.ofMinutes(1).toMillis(); + private static final Logger LOG = LoggerFactory.getLogger(BackendHealthStatus.class); private final EndpointKey hostPort; - - private volatile boolean reportedAsUnreachable; - private long reportedAsUnreachableTs; - - private BackendHealthCheck lastProbe; + private volatile Status status; + private volatile long lastUnreachableTs; + private volatile long lastReachableTs; + private volatile BackendHealthCheck lastProbe; public BackendHealthStatus(final EndpointKey hostPort) { this.hostPort = hostPort; + // todo using DOWN would break BasicStandardEndpointMapperTest, UnreachableBackendTest, and StuckRequestsTest + this.status = Status.COLD; + this.lastUnreachableTs = System.currentTimeMillis(); } public EndpointKey getHostPort() { - return hostPort; + return this.hostPort; } public BackendHealthCheck getLastProbe() { - return lastProbe; + return this.lastProbe; } - public void setLastProbe(BackendHealthCheck lastProbe) { + public void setLastProbe(final BackendHealthCheck lastProbe) { this.lastProbe = lastProbe; } - public boolean isReportedAsUnreachable() { - return reportedAsUnreachable; - } - - public void setReportedAsUnreachable(boolean reportedAsUnreachable) { - this.reportedAsUnreachable = reportedAsUnreachable; + public Status getStatus() { + return status; } - public long getReportedAsUnreachableTs() { - return reportedAsUnreachableTs; + public long getLastUnreachableTs() { + return lastUnreachableTs; } - public void setReportedAsUnreachableTs(long reportedAsUnreachableTs) { - this.reportedAsUnreachableTs = reportedAsUnreachableTs; - } - - void reportAsUnreachable(long timestamp, final String cause) { + public void reportAsUnreachable(final long timestamp, final String cause) { LOG.info("{}: reportAsUnreachable {}, cause {}", hostPort, new Timestamp(timestamp), cause); - reportedAsUnreachableTs = timestamp; - reportedAsUnreachable = true; + this.lastUnreachableTs = timestamp; + this.status = Status.DOWN; } - void reportAsReachable() { - reportedAsUnreachable = false; - reportedAsUnreachableTs = 0; - } - - public boolean isAvailable() { - return !reportedAsUnreachable; + public void reportAsReachable(final long timestamp) { + this.lastReachableTs = timestamp; + if (this.lastReachableTs - this.lastUnreachableTs >= WARMUP_MILLIS) { + this.status = Status.STABLE; + } else { + this.status = Status.COLD; + } } @Override public String toString() { - return "BackendHealthStatus{" + "hostPort=" + hostPort + ", reportedAsUnreachable=" + reportedAsUnreachable + ", reportedAsUnreachableTs=" + reportedAsUnreachableTs + '}'; + return "BackendHealthStatus{" + + " hostPort=" + this.hostPort + + ", status=" + this.status + + ", lastUnreachableTs=" + this.lastUnreachableTs + + ", lastReachableTs=" + this.lastReachableTs + + ", lastProbe=" + this.lastProbe + + '}'; } + /** + * The enum models a simple status of the backend. + */ + public enum Status { + /** + * The backend is unreachable. + */ + DOWN, + /** + * The backend is reachable, but not since long. + * When in this safe, it is reasonable to assume that it is still warming-up, + * so it would be a sensible decision not to overload it with requests. + */ + COLD, + /** + * The backend is reachable and was so since a reasonably-long time. + */ + STABLE + } } diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/mapper/EndpointMapper.java b/carapace-server/src/main/java/org/carapaceproxy/server/mapper/EndpointMapper.java index 14ef5df9d..a12b1f703 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/mapper/EndpointMapper.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/mapper/EndpointMapper.java @@ -116,7 +116,7 @@ public SimpleHTTPResponse mapBadRequest() { public abstract void configure(ConfigurationStore properties) throws ConfigurationNotValidException; - public final void setParent(final HttpProxyServer parent) { + public final void setParent(HttpProxyServer parent) { this.parent = parent; } diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java b/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java index fa337cf28..6c8793b08 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java @@ -37,6 +37,8 @@ import org.carapaceproxy.SimpleHTTPResponse; import org.carapaceproxy.configstore.ConfigurationStore; import org.carapaceproxy.core.ProxyRequest; +import org.carapaceproxy.server.backends.BackendHealthManager; +import org.carapaceproxy.server.backends.BackendHealthStatus; import org.carapaceproxy.server.config.ActionConfiguration; import org.carapaceproxy.server.config.BackendConfiguration; import org.carapaceproxy.server.config.BackendSelector; @@ -50,24 +52,30 @@ import org.carapaceproxy.server.mapper.requestmatcher.RequestMatcher; import org.carapaceproxy.server.mapper.requestmatcher.parser.ParseException; import org.carapaceproxy.server.mapper.requestmatcher.parser.RequestMatchParser; +import org.carapaceproxy.utils.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * Standard Endpoint mapping + * Reference implementation of an {@link EndpointMapper}. */ public class StandardEndpointMapper extends EndpointMapper { - public static final String ACME_CHALLENGE_ROUTE_ACTION_ID = "acme-challenge"; + private static final Logger LOG = LoggerFactory.getLogger(StandardEndpointMapper.class); private static final String DEFAULT_NOT_FOUND_ACTION = "not-found"; private static final String DEFAULT_INTERNAL_ERROR_ACTION = "internal-error"; private static final String DEFAULT_MAINTENANCE_ACTION = "maintenance"; private static final String DEFAULT_BAD_REQUEST_ACTION = "bad-request"; private static final String DEFAULT_SERVICE_UNAVAILABLE_ACTION = "service-unavailable"; + private static final String ACME_CHALLENGE_URI_PATTERN = "/\\.well-known/acme-challenge/"; + public static final String ACME_CHALLENGE_ROUTE_ACTION_ID = "acme-challenge"; + public static final String DEBUGGING_HEADER_DEFAULT_NAME = "X-Proxy-Path"; + public static final String DEBUGGING_HEADER_ID = "mapper-debug"; + // The map is wiped out whenever a new configuration is applied private final SequencedMap backends = new LinkedHashMap<>(); private final Map directors = new HashMap<>(); - private final List allbackendids = new ArrayList<>(); + private final List allBackendIds = new ArrayList<>(); private final List routes = new ArrayList<>(); private final Map actions = new HashMap<>(); public final Map headers = new HashMap<>(); @@ -82,33 +90,28 @@ public class StandardEndpointMapper extends EndpointMapper { private String forceDirectorParameter = "x-director"; private String forceBackendParameter = "x-backend"; - private static final Logger LOG = LoggerFactory.getLogger(StandardEndpointMapper.class); - private static final String ACME_CHALLENGE_URI_PATTERN = "/\\.well-known/acme-challenge/"; - - public static final String DEBUGGING_HEADER_DEFAULT_NAME = "X-Proxy-Path"; - public static final String DEBUGGING_HEADER_ID = "mapper-debug"; private String debuggingHeaderName = DEBUGGING_HEADER_DEFAULT_NAME; private boolean debuggingHeaderEnabled = false; - public StandardEndpointMapper(BackendSelector backendSelector) { + public StandardEndpointMapper(final BackendSelector backendSelector) { this.backendSelector = backendSelector; } public StandardEndpointMapper() { - this.backendSelector = new RandomBackendSelector(allbackendids, directors); + this.backendSelector = new RandomBackendSelector(allBackendIds, directors); } @Override - public MapResult map(ProxyRequest request) { + public MapResult map(final ProxyRequest request) { // If the HOST header is null (when on HTTP/1.1 or less), then return bad request // https://www.rfc-editor.org/rfc/rfc2616#page-38 - if (request.getRequestHostname() == null || request.getRequestHostname().isBlank()) { + if (StringUtils.isBlank(request.getRequestHostname())) { if (LOG.isTraceEnabled()) { LOG.trace("Request {} header host is null or empty", request.getUri()); } return MapResult.badRequest(); } - if (!request.isValidHostAndPort(request.getRequestHostname())) { //Invalid header host + if (!request.isValidHostAndPort(request.getRequestHostname())) { //Invalid header host if (LOG.isTraceEnabled()) { LOG.trace("Invalid header host {} for request {}", request.getRequestHostname(), request.getUri()); } @@ -207,23 +210,26 @@ public MapResult map(ProxyRequest request) { } final BackendConfiguration backend = this.backends.get(backendId); - if (backend != null && getBackendHealthManager().isAvailable(backend.hostPort())) { - List customHeaders = action.getCustomHeaders(); - if (this.debuggingHeaderEnabled) { - customHeaders = new ArrayList<>(customHeaders); - String routingPath = route.getId() + ";" - + action.getId() + ";" - + action.getDirector() + ";" - + backendId; - customHeaders.add(new CustomHeader(DEBUGGING_HEADER_ID, debuggingHeaderName, routingPath, HeaderMode.ADD)); + if (backend != null) { + BackendHealthManager backendHealthManager = getBackendHealthManager(); + if (backendHealthManager.getBackendStatus(backend.hostPort()) != BackendHealthStatus.Status.DOWN) { + List customHeaders = action.getCustomHeaders(); + if (this.debuggingHeaderEnabled) { + customHeaders = new ArrayList<>(customHeaders); + String routingPath = route.getId() + ";" + + action.getId() + ";" + + action.getDirector() + ";" + + backendId; + customHeaders.add(new CustomHeader(DEBUGGING_HEADER_ID, debuggingHeaderName, routingPath, HeaderMode.ADD)); + } + return MapResult.builder() + .host(backend.host()) + .port(backend.port()) + .action(selectedAction) + .routeId(route.getId()) + .customHeaders(customHeaders) + .build(); } - return MapResult.builder() - .host(backend.host()) - .port(backend.port()) - .action(selectedAction) - .routeId(route.getId()) - .customHeaders(customHeaders) - .build(); } } // none of selected backends available @@ -545,7 +551,7 @@ public void addBackend(BackendConfiguration backend) throws ConfigurationNotVali if (backends.put(backend.id(), backend) != null) { throw new ConfigurationNotValidException("backend " + backend.id() + " is already configured"); } - allbackendids.add(backend.id()); + allBackendIds.add(backend.id()); } public void addAction(ActionConfiguration action) throws ConfigurationNotValidException { diff --git a/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java b/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java index 2f88ad7a6..9d9282828 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java @@ -27,6 +27,7 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import com.github.tomakehurst.wiremock.junit.WireMockRule; import java.util.Properties; @@ -36,6 +37,7 @@ import org.carapaceproxy.configstore.PropertiesConfigurationStore; import org.carapaceproxy.core.HttpProxyServer; import org.carapaceproxy.core.ProxyRequestsManager; +import org.carapaceproxy.server.backends.BackendHealthStatus; import org.carapaceproxy.server.config.ActionConfiguration; import org.carapaceproxy.server.config.BackendConfiguration; import org.carapaceproxy.server.config.DirectorConfiguration; @@ -118,7 +120,10 @@ public void testBackendUnreachableOnStuckRequest(boolean backendsUnreachableOnSt assertThat((int) ProxyRequestsManager.PENDING_REQUESTS_GAUGE.get(), is(0)); assertThat((int) ProxyRequestsManager.STUCK_REQUESTS_COUNTER.get() > 0, is(true)); - assertEquals(backendsUnreachableOnStuckRequests, !server.getBackendHealthManager().isAvailable(key)); + final BackendHealthStatus.Status expected = backendsUnreachableOnStuckRequests + ? BackendHealthStatus.Status.DOWN + : BackendHealthStatus.Status.COLD; + assertSame(expected, server.getBackendHealthManager().getBackendStatus(key)); try (RawHttpClient client = new RawHttpClient("localhost", port)) { RawHttpClient.HttpResponse resp = client.executeRequest("GET /good-index.html HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n"); diff --git a/carapace-server/src/test/java/org/carapaceproxy/backends/UnreachableBackendTest.java b/carapace-server/src/test/java/org/carapaceproxy/backends/UnreachableBackendTest.java index 8430b06bb..caafb2750 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/backends/UnreachableBackendTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/backends/UnreachableBackendTest.java @@ -26,17 +26,17 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import com.github.tomakehurst.wiremock.http.Fault; import com.github.tomakehurst.wiremock.junit.WireMockRule; -import java.util.Arrays; -import java.util.Collection; +import java.util.List; import java.util.Properties; import org.carapaceproxy.configstore.PropertiesConfigurationStore; import org.carapaceproxy.core.EndpointKey; import org.carapaceproxy.core.HttpProxyServer; import org.carapaceproxy.core.ProxyRequestsManager; +import org.carapaceproxy.server.backends.BackendHealthStatus; import org.carapaceproxy.server.config.NetworkListenerConfiguration; import org.carapaceproxy.utils.RawHttpClient; import org.carapaceproxy.utils.TestEndpointMapper; @@ -50,15 +50,9 @@ @RunWith(Parameterized.class) public class UnreachableBackendTest { - @Parameters - public static Collection data() { - return Arrays.asList(new Object[][]{ - {true /* - * useCache = true - */}, {false /* - * useCache = false - */} - }); + @Parameters(name = "useCache = {0}") + public static Iterable data() { + return List.of(true, false); } @Rule @@ -67,7 +61,7 @@ public static Collection data() { @Rule public TemporaryFolder tmpDir = new TemporaryFolder(); - private boolean useCache = false; + private final boolean useCache; public UnreachableBackendTest(boolean useCache) { this.useCache = useCache; @@ -104,7 +98,7 @@ public void testWithUnreachableBackend() throws Exception { """, resp.getBodyString()); } - assertFalse(server.getBackendHealthManager().isAvailable(key)); + assertSame(server.getBackendHealthManager().getBackendStatus(key), BackendHealthStatus.Status.DOWN); assertThat((int) ProxyRequestsManager.PENDING_REQUESTS_GAUGE.get(), is(0)); } } @@ -142,7 +136,7 @@ public void testEmptyResponse() throws Exception { """, resp.getBodyString()); } - assertTrue(server.getBackendHealthManager().isAvailable(key)); // no troubles for new connections + assertSame(server.getBackendHealthManager().getBackendStatus(key), BackendHealthStatus.Status.COLD); // no troubles for new connections assertThat((int) ProxyRequestsManager.PENDING_REQUESTS_GAUGE.get(), is(0)); } } @@ -174,7 +168,7 @@ public void testConnectionResetByPeer() throws Exception { """, resp.getBodyString()); } - assertTrue(server.getBackendHealthManager().isAvailable(key)); // no troubles for new connections + assertSame(server.getBackendHealthManager().getBackendStatus(key), BackendHealthStatus.Status.COLD); // no troubles for new connections assertThat((int) ProxyRequestsManager.PENDING_REQUESTS_GAUGE.get(), is(0)); } } @@ -210,7 +204,7 @@ public void testNonHttpResponseThenClose() throws Exception { """, resp.getBodyString()); } - assertTrue(server.getBackendHealthManager().isAvailable(key)); + assertSame(server.getBackendHealthManager().getBackendStatus(key), BackendHealthStatus.Status.COLD); assertThat((int) ProxyRequestsManager.PENDING_REQUESTS_GAUGE.get(), is(0)); } } diff --git a/carapace-server/src/test/java/org/carapaceproxy/server/mapper/BasicStandardEndpointMapperTest.java b/carapace-server/src/test/java/org/carapaceproxy/server/mapper/BasicStandardEndpointMapperTest.java index eb8049076..4835e0574 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/server/mapper/BasicStandardEndpointMapperTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/server/mapper/BasicStandardEndpointMapperTest.java @@ -49,6 +49,7 @@ import org.carapaceproxy.core.HttpProxyServer; import org.carapaceproxy.core.StaticContentsManager; import org.carapaceproxy.server.backends.BackendHealthManager; +import org.carapaceproxy.server.backends.BackendHealthStatus; import org.carapaceproxy.server.certificates.DynamicCertificatesManager; import org.carapaceproxy.server.config.ActionConfiguration; import org.carapaceproxy.server.config.BackendConfiguration; @@ -66,7 +67,7 @@ public class BasicStandardEndpointMapperTest { @Rule - public WireMockRule backend1 = new WireMockRule(0); + public WireMockRule backend = new WireMockRule(0); @Rule public TemporaryFolder tmpDir = new TemporaryFolder(); @@ -91,7 +92,7 @@ public void test() throws Exception { .withHeader("Content-Type", "text/html") .withBody("it works !!"))); - int backendPort = backend1.port(); + int backendPort = backend.port(); StandardEndpointMapper mapper = new StandardEndpointMapper(); mapper.addBackend(new BackendConfiguration("backend-a", "localhost", backendPort, "/")); @@ -181,7 +182,7 @@ public void testRouteErrors() throws Exception { configuration.put("backend.1.id", "backend"); configuration.put("backend.1.host", "localhost"); - configuration.put("backend.1.port", String.valueOf(backend1.port())); + configuration.put("backend.1.port", String.valueOf(backend.port())); configuration.put("backend.1.enabled", "true"); configuration.put("director.1.id", "director"); @@ -191,7 +192,7 @@ public void testRouteErrors() throws Exception { // unreachable backend -> expected service unavailable configuration.put("backend.2.id", "backend-down"); configuration.put("backend.2.host", "localhost-down"); - configuration.put("backend.2.port", String.valueOf(backend1.port())); + configuration.put("backend.2.port", String.valueOf(backend.port())); configuration.put("backend.2.enabled", "true"); configuration.put("director.2.id", "director-down"); @@ -255,15 +256,21 @@ public void testRouteErrors() throws Exception { PropertiesConfigurationStore config = new PropertiesConfigurationStore(configuration); BackendHealthManager bhMan = mock(BackendHealthManager.class); - when(bhMan.isAvailable(eq(EndpointKey.make("localhost:" + backend1.port())))).thenReturn(true); - when(bhMan.isAvailable(eq(EndpointKey.make("localhost-down:" + backend1.port())))).thenReturn(false); // simulate unreachable backend -> expected 500 error + final EndpointKey alive = EndpointKey.make("localhost:" + backend.port()); + final BackendHealthStatus mockAliveStatus = mock(BackendHealthStatus.class); + when(mockAliveStatus.getStatus()).thenReturn(BackendHealthStatus.Status.STABLE); + when(bhMan.getBackendStatus(eq(alive))).thenReturn(mockAliveStatus.getStatus()); + final EndpointKey down = EndpointKey.make("localhost-down:" + backend.port()); + final BackendHealthStatus mockDownStatus = mock(BackendHealthStatus.class); + when(mockDownStatus.getStatus()).thenReturn(BackendHealthStatus.Status.DOWN); + when(bhMan.getBackendStatus(eq(down))).thenReturn(mockDownStatus.getStatus()); // simulate unreachable backend -> expected 500 error server.setBackendHealthManager(bhMan); server.configureAtBoot(config); server.start(); Thread.sleep(5_000); - // route-custom error (Internal Errror) + // route-custom error (Internal Error) { HttpURLConnection conn = (HttpURLConnection) URI.create("http://localhost:" + server.getLocalPort() + "/custom-error.html").toURL().openConnection(); System.out.println("response core " + conn.getResponseCode()); @@ -307,7 +314,7 @@ public void testDefaultRoute() throws Exception { .withHeader("Content-Type", "text/html") .withBody("it works !!"))); - int backendPort = backend1.port(); + int backendPort = backend.port(); StandardEndpointMapper mapper = new StandardEndpointMapper(); mapper.addBackend(new BackendConfiguration("backend", "localhost", backendPort, "/")); @@ -321,8 +328,14 @@ public void testDefaultRoute() throws Exception { mapper.addRoute(new RouteConfiguration("route-default", "cache", true, new RegexpRequestMatcher(PROPERTY_URI, ".*html"))); BackendHealthManager bhMan = mock(BackendHealthManager.class); - when(bhMan.isAvailable(eq(EndpointKey.make("localhost:" + backendPort)))).thenReturn(true); - when(bhMan.isAvailable(eq(EndpointKey.make("localhost-down:" + backendPort)))).thenReturn(false); // simulate unreachable backend -> expected 500 error + final EndpointKey alive = EndpointKey.make("localhost:" + backendPort); + final BackendHealthStatus mockAliveStatus = mock(BackendHealthStatus.class); + when(mockAliveStatus.getStatus()).thenReturn(BackendHealthStatus.Status.STABLE); + when(bhMan.getBackendStatus(eq(alive))).thenReturn(mockAliveStatus.getStatus()); + final EndpointKey down = EndpointKey.make("localhost-down:" + backendPort); + final BackendHealthStatus mockDownStatus = mock(BackendHealthStatus.class); + when(mockDownStatus.getStatus()).thenReturn(BackendHealthStatus.Status.DOWN); + when(bhMan.getBackendStatus(eq(down))).thenReturn(mockDownStatus.getStatus()); // simulate unreachable backend -> expected 500 error try (HttpProxyServer server = HttpProxyServer.buildForTests("localhost", 0, mapper, tmpDir.newFolder())) { server.setBackendHealthManager(bhMan); @@ -362,7 +375,7 @@ public void testAlwaysServeStaticContent() throws Exception { Properties configuration = new Properties(); configuration.put("backend.1.id", "foo"); configuration.put("backend.1.host", "localhost"); - configuration.put("backend.1.port", String.valueOf(backend1.port())); + configuration.put("backend.1.port", String.valueOf(backend.port())); configuration.put("backend.1.enabled", "true"); configuration.put("director.1.id", "*"); @@ -439,7 +452,7 @@ public void testServeACMEChallengeToken() throws Exception { Properties configuration = new Properties(); configuration.put("backend.1.id", "foo"); configuration.put("backend.1.host", "localhost"); - configuration.put("backend.1.port", String.valueOf(backend1.port())); + configuration.put("backend.1.port", String.valueOf(backend.port())); configuration.put("backend.1.enabled", "true"); configuration.put("director.1.id", "*"); @@ -513,11 +526,11 @@ public void testCustomAndDebuggingHeaders() throws Exception { Properties configuration = new Properties(); configuration.put("backend.1.id", "b1"); configuration.put("backend.1.host", "localhost"); - configuration.put("backend.1.port", String.valueOf(backend1.port())); + configuration.put("backend.1.port", String.valueOf(backend.port())); configuration.put("backend.1.enabled", "true"); configuration.put("backend.2.id", "b2"); configuration.put("backend.2.host", "localhost"); - configuration.put("backend.2.port", String.valueOf(backend1.port())); + configuration.put("backend.2.port", String.valueOf(backend.port())); configuration.put("backend.2.enabled", "true"); configuration.put("director.1.id", "d1"); diff --git a/carapace-server/src/test/java/org/carapaceproxy/server/mapper/HealthCheckTest.java b/carapace-server/src/test/java/org/carapaceproxy/server/mapper/HealthCheckTest.java index de0728841..41f392b67 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/server/mapper/HealthCheckTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/server/mapper/HealthCheckTest.java @@ -86,16 +86,16 @@ public void test() throws Exception { final BackendHealthStatus _status = status.get(b1conf.hostPort()); assertThat(_status, is(not(nullValue()))); assertThat(_status.getHostPort(), is(b1conf.hostPort())); - assertThat(_status.isAvailable(), is(true)); - assertThat(_status.isReportedAsUnreachable(), is(false)); - assertThat(_status.getReportedAsUnreachableTs(), is(0L)); + assertThat(_status.getStatus() != BackendHealthStatus.Status.DOWN, is(true)); + assertThat(_status.getStatus() == BackendHealthStatus.Status.DOWN, is(false)); + assertThat(_status.getLastUnreachableTs(), is(0L)); final BackendHealthCheck lastProbe = _status.getLastProbe(); assertThat(lastProbe, is(not(nullValue()))); assertThat(lastProbe.path(), is("/status.html")); assertThat(lastProbe.endTs() >= startTs, is(true)); assertThat(lastProbe.endTs() <= endTs, is(true)); - assertThat(lastProbe.isOk(), is(true)); + assertThat(lastProbe.ok(), is(true)); assertThat(lastProbe.httpResponse(), is("200 OK")); assertThat(lastProbe.httpBody(), is("Ok...")); } @@ -125,18 +125,18 @@ public void test() throws Exception { final BackendHealthStatus _status = status.get(b1conf.hostPort()); assertThat(_status, is(not(nullValue()))); assertThat(_status.getHostPort(), is(b1conf.hostPort())); - assertThat(_status.isAvailable(), is(false)); - assertThat(_status.isReportedAsUnreachable(), is(true)); - assertThat(_status.getReportedAsUnreachableTs() >= startTs, is(true)); - assertThat(_status.getReportedAsUnreachableTs() <= endTs, is(true)); - reportedAsUnreachableTs = _status.getReportedAsUnreachableTs(); + assertThat(_status.getStatus() != BackendHealthStatus.Status.DOWN, is(false)); + assertThat(_status.getStatus() == BackendHealthStatus.Status.DOWN, is(true)); + assertThat(_status.getLastUnreachableTs() >= startTs, is(true)); + assertThat(_status.getLastUnreachableTs() <= endTs, is(true)); + reportedAsUnreachableTs = _status.getLastUnreachableTs(); final BackendHealthCheck lastProbe = _status.getLastProbe(); assertThat(lastProbe, is(not(nullValue()))); assertThat(lastProbe.path(), is("/status.html")); assertThat(lastProbe.endTs() >= startTs, is(true)); assertThat(lastProbe.endTs() <= endTs, is(true)); - assertThat(lastProbe.isOk(), is(false)); + assertThat(lastProbe.ok(), is(false)); System.out.println("HTTP MESSAGE: " + lastProbe.httpResponse()); System.out.println("STATUS INFO: " + lastProbe.httpBody()); assertThat(lastProbe.httpResponse(), is("500 Server Error")); @@ -167,16 +167,16 @@ public void test() throws Exception { final BackendHealthStatus _status = status.get(b1conf.hostPort()); assertThat(_status, is(not(nullValue()))); assertThat(_status.getHostPort(), is(b1conf.hostPort())); - assertThat(_status.isAvailable(), is(false)); - assertThat(_status.isReportedAsUnreachable(), is(true)); - assertThat(_status.getReportedAsUnreachableTs(), is(reportedAsUnreachableTs)); + assertThat(_status.getStatus() != BackendHealthStatus.Status.DOWN, is(false)); + assertThat(_status.getStatus() == BackendHealthStatus.Status.DOWN, is(true)); + assertThat(_status.getLastUnreachableTs(), is(reportedAsUnreachableTs)); final BackendHealthCheck lastProbe = _status.getLastProbe(); assertThat(lastProbe, is(not(nullValue()))); assertThat(lastProbe.path(), is("/status.html")); assertThat(lastProbe.endTs() >= startTs, is(true)); assertThat(lastProbe.endTs() <= endTs, is(true)); - assertThat(lastProbe.isOk(), is(false)); + assertThat(lastProbe.ok(), is(false)); System.out.println("HTTP MESSAGE: " + lastProbe.httpResponse()); System.out.println("STATUS INFO: " + lastProbe.httpBody()); assertThat(lastProbe.httpResponse(), is("500 Server Error")); @@ -207,16 +207,16 @@ public void test() throws Exception { final BackendHealthStatus _status = status.get(b1conf.hostPort()); assertThat(_status, is(not(nullValue()))); assertThat(_status.getHostPort(), is(b1conf.hostPort())); - assertThat(_status.isAvailable(), is(true)); - assertThat(_status.isReportedAsUnreachable(), is(false)); - assertThat(_status.getReportedAsUnreachableTs(), is(0L)); + assertThat(_status.getStatus() != BackendHealthStatus.Status.DOWN, is(true)); + assertThat(_status.getStatus() == BackendHealthStatus.Status.DOWN, is(false)); + assertThat(_status.getLastUnreachableTs(), is(0L)); final BackendHealthCheck lastProbe = _status.getLastProbe(); assertThat(lastProbe, is(not(nullValue()))); assertThat(lastProbe.path(), is("/status.html")); assertThat(lastProbe.endTs() >= startTs, is(true)); assertThat(lastProbe.endTs() <= endTs, is(true)); - assertThat(lastProbe.isOk(), is(true)); + assertThat(lastProbe.ok(), is(true)); System.out.println("HTTP MESSAGE: " + lastProbe.httpResponse()); System.out.println("STATUS INFO: " + lastProbe.httpBody()); assertThat(lastProbe.httpResponse(), is("201 Created")); From 7c4768ceaa3f370d30a1ceee29a644e529267a70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Maltoni?= Date: Thu, 21 Nov 2024 13:03:33 +0100 Subject: [PATCH 07/15] feat: implement the feature by spreading the backend status --- .../carapaceproxy/api/BackendsResource.java | 2 +- .../org/carapaceproxy/core/ProxyRequest.java | 4 +- .../core/ProxyRequestsManager.java | 30 ++++--- .../server/backends/BackendHealthManager.java | 14 ++- .../server/backends/BackendHealthStatus.java | 88 +++++++++++++++---- .../server/config/ActionConfiguration.java | 17 +++- .../server/mapper/MapResult.java | 11 ++- .../server/mapper/StandardEndpointMapper.java | 70 +++++++++------ .../backends/StuckRequestsTest.java | 2 +- .../backends/UnreachableBackendTest.java | 8 +- .../BasicStandardEndpointMapperTest.java | 8 +- .../server/mapper/HealthCheckTest.java | 35 ++++---- .../utils/TestEndpointMapper.java | 37 +++++--- 13 files changed, 218 insertions(+), 108 deletions(-) diff --git a/carapace-server/src/main/java/org/carapaceproxy/api/BackendsResource.java b/carapace-server/src/main/java/org/carapaceproxy/api/BackendsResource.java index c9411479b..add9e0efe 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/api/BackendsResource.java +++ b/carapace-server/src/main/java/org/carapaceproxy/api/BackendsResource.java @@ -98,7 +98,7 @@ public Map getAll() { if (bhs != null) { bean.available = bhs.getStatus() != BackendHealthStatus.Status.DOWN; bean.reportedAsUnreachable = bhs.getStatus() == BackendHealthStatus.Status.DOWN; - bean.reportedAsUnreachableTs = bhs.getLastUnreachableTs(); + bean.reportedAsUnreachableTs = bhs.getUnreachableSince(); BackendHealthCheck lastProbe = bhs.getLastProbe(); if (lastProbe != null) { bean.lastProbeTs = lastProbe.endTs(); diff --git a/carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequest.java b/carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequest.java index 66d735cd9..94a38d5be 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequest.java +++ b/carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequest.java @@ -213,8 +213,8 @@ public boolean isValidHostAndPort(final String hostAndPort) { if (hostAndPort == null) { return false; } - HostAndPort parsed = HostAndPort.fromString(hostAndPort); - String host = parsed.getHost(); + final HostAndPort parsed = HostAndPort.fromString(hostAndPort); + final String host = parsed.getHost(); if (parsed.hasPort()) { return !host.isBlank() && (InternetDomainName.isValid(host) || InetAddresses.isInetAddress(host)) diff --git a/carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequestsManager.java b/carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequestsManager.java index b3c731c82..507735b5d 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequestsManager.java +++ b/carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequestsManager.java @@ -61,6 +61,7 @@ import org.apache.http.HttpStatus; import org.carapaceproxy.EndpointStats; import org.carapaceproxy.SimpleHTTPResponse; +import org.carapaceproxy.server.backends.BackendHealthStatus; import org.carapaceproxy.server.cache.ContentsCache; import org.carapaceproxy.server.config.BackendConfiguration; import org.carapaceproxy.server.config.ConnectionPoolConfiguration; @@ -106,7 +107,7 @@ public ProxyRequestsManager(HttpProxyServer parent) { this.parent = parent; } - public EndpointStats getEndpointStats(EndpointKey key) { + public EndpointStats getEndpointStats(final EndpointKey key) { return endpointsStats.get(key); } @@ -148,8 +149,8 @@ public Publisher processRequest(ProxyRequest request) { case BAD_REQUEST -> serveBadRequestMessage(request); case STATIC, ACME_CHALLENGE -> serveStaticMessage(request); case REDIRECT -> serveRedirect(request); - case PROXY -> forward(request, false); - case CACHE -> serveFromCache(request); // cached content + case PROXY -> forward(request, false, action.getHealthStatus()); + case CACHE -> serveFromCache(request, action.getHealthStatus()); // cached content default -> throw new IllegalStateException("Action " + action.getAction() + " not supported"); }; } finally { @@ -347,20 +348,22 @@ private static void addCustomResponseHeaders(final HttpHeaders responseHeaders, * * @param request the unpacked incoming request to forward to the corresponding backend endpoint * @param cache whether the request is cacheable or not + * @param healthStatus the health status of the chosen backend; it should be notified when connection starts and ends * @return a {@link Flux} forwarding the returned {@link Publisher} sequence */ - public Publisher forward(ProxyRequest request, boolean cache) { + public Publisher forward(final ProxyRequest request, final boolean cache, final BackendHealthStatus healthStatus) { Objects.requireNonNull(request.getAction()); final String endpointHost = request.getAction().getHost(); + Objects.requireNonNull(endpointHost); final int endpointPort = request.getAction().getPort(); - EndpointKey key = EndpointKey.make(endpointHost, endpointPort); - EndpointStats endpointStats = endpointsStats.computeIfAbsent(key, EndpointStats::new); + final EndpointKey key = EndpointKey.make(endpointHost, endpointPort); + final EndpointStats endpointStats = endpointsStats.computeIfAbsent(key, EndpointStats::new); - var connectionToEndpoint = connectionsManager.apply(request); - ConnectionPoolConfiguration connectionConfig = connectionToEndpoint.getKey(); - ConnectionProvider connectionProvider = connectionToEndpoint.getValue(); + final var connectionToEndpoint = connectionsManager.apply(request); + final ConnectionPoolConfiguration connectionConfig = connectionToEndpoint.getKey(); + final ConnectionProvider connectionProvider = connectionToEndpoint.getValue(); if (LOGGER.isDebugEnabled()) { - Map stats = parent.getConnectionPoolsStats().get(key); + final Map stats = parent.getConnectionPoolsStats().get(key); if (stats != null) { LOGGER.debug( "Connection {} stats: {}", @@ -416,6 +419,7 @@ public Publisher forward(ProxyRequest request, boolean cache) { } }).doAfterResponseSuccess((resp, conn) -> { PENDING_REQUESTS_GAUGE.dec(); + healthStatus.decrementConnections(); endpointStats.getLastActivity().set(System.currentTimeMillis()); })); @@ -429,6 +433,7 @@ public Publisher forward(ProxyRequest request, boolean cache) { } PENDING_REQUESTS_GAUGE.inc(); + healthStatus.incrementConnections(); return forwarder.request(request.getMethod()) .uri(request.getUri()) .send((req, out) -> { @@ -489,6 +494,7 @@ public Publisher forward(ProxyRequest request, boolean cache) { })); }).onErrorResume(err -> { // custom endpoint request/response error handling PENDING_REQUESTS_GAUGE.dec(); + healthStatus.decrementConnections(); EndpointKey endpoint = EndpointKey.make(request.getAction().getHost(), request.getAction().getPort()); if (err instanceof ReadTimeoutException) { @@ -557,11 +563,11 @@ private void addCachedResponseHeaders(ProxyRequest request) { } } - private Publisher serveFromCache(ProxyRequest request) { + private Publisher serveFromCache(ProxyRequest request, final BackendHealthStatus healthStatus) { ContentsCache.ContentSender cacheSender = parent.getCache().getCacheSender(request); if (cacheSender == null) { // content non cached, forwarding and caching... - return forward(request, true); + return forward(request, true, healthStatus); } request.setServedFromCache(true); diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java b/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java index a8f667bb1..6e6375251 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java @@ -136,7 +136,7 @@ public void run() { } for (final BackendConfiguration backend : mapper.getBackends().values()) { final EndpointKey endpoint = backend.hostPort(); - final BackendHealthStatus status = findStatus(endpoint); + final BackendHealthStatus status = getBackendStatus(endpoint); final BackendHealthCheck checkResult = BackendHealthCheck.check(backend, connectTimeout); if (checkResult.ok()) { @@ -173,23 +173,19 @@ private void cleanup() { } public void reportBackendReachable(final EndpointKey hostPort, final long timestamp) { - findStatus(hostPort).reportAsReachable(timestamp); + getBackendStatus(hostPort).reportAsReachable(timestamp); } public void reportBackendUnreachable(final EndpointKey hostPort, final long timestamp, final String cause) { - findStatus(hostPort).reportAsUnreachable(timestamp, cause); - } - - private BackendHealthStatus findStatus(final EndpointKey hostPort) { - return backends.computeIfAbsent(hostPort, BackendHealthStatus::new); + getBackendStatus(hostPort).reportAsUnreachable(timestamp, cause); } public Map getBackendsSnapshot() { return Map.copyOf(backends); } - public BackendHealthStatus.Status getBackendStatus(final EndpointKey hostPort) { - return findStatus(hostPort).getStatus(); + public BackendHealthStatus getBackendStatus(final EndpointKey hostPort) { + return backends.computeIfAbsent(hostPort, BackendHealthStatus::new); } @VisibleForTesting diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthStatus.java b/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthStatus.java index 6fddd1586..22024d393 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthStatus.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthStatus.java @@ -21,6 +21,7 @@ import java.sql.Timestamp; import java.time.Duration; +import java.util.concurrent.atomic.AtomicInteger; import org.carapaceproxy.core.EndpointKey; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -32,22 +33,46 @@ */ public class BackendHealthStatus { - // todo replace this with a property of some kind + // todo replace this with a configurable property of some kind public static final long WARMUP_MILLIS = Duration.ofMinutes(1).toMillis(); private static final Logger LOG = LoggerFactory.getLogger(BackendHealthStatus.class); private final EndpointKey hostPort; + private final AtomicInteger connections; + private volatile Status status; - private volatile long lastUnreachableTs; - private volatile long lastReachableTs; + private volatile long unreachableSince; + private volatile long lastUnreachable; + private volatile long lastReachable; private volatile BackendHealthCheck lastProbe; public BackendHealthStatus(final EndpointKey hostPort) { this.hostPort = hostPort; - // todo using DOWN would break BasicStandardEndpointMapperTest, UnreachableBackendTest, and StuckRequestsTest + // todo cannot start with a DOWN backend (+ current time) as it would break: + // - BasicStandardEndpointMapperTest, + // - UnreachableBackendTest, + // - StuckRequestsTest, + // - HealthCheckTest + // we assume that the backend is reachable when status is created this.status = Status.COLD; - this.lastUnreachableTs = System.currentTimeMillis(); + this.unreachableSince = 0L; + final long created = System.currentTimeMillis(); + this.lastUnreachable = created; + this.lastReachable = created; + this.connections = new AtomicInteger(); + } + + public long getUnreachableSince() { + return unreachableSince; + } + + public long getLastUnreachable() { + return lastUnreachable; + } + + public long getLastReachable() { + return lastReachable; } public EndpointKey getHostPort() { @@ -66,22 +91,33 @@ public Status getStatus() { return status; } - public long getLastUnreachableTs() { - return lastUnreachableTs; - } - public void reportAsUnreachable(final long timestamp, final String cause) { LOG.info("{}: reportAsUnreachable {}, cause {}", hostPort, new Timestamp(timestamp), cause); - this.lastUnreachableTs = timestamp; - this.status = Status.DOWN; + if (this.status != Status.DOWN) { + this.status = Status.DOWN; + this.unreachableSince = timestamp; + } + this.lastUnreachable = timestamp; + this.connections.set(0); } public void reportAsReachable(final long timestamp) { - this.lastReachableTs = timestamp; - if (this.lastReachableTs - this.lastUnreachableTs >= WARMUP_MILLIS) { - this.status = Status.STABLE; - } else { - this.status = Status.COLD; + LOG.info("{}: reportAsUnreachable {}", hostPort, new Timestamp(timestamp)); + switch (this.status) { + case DOWN: + this.status = Status.COLD; + this.unreachableSince = 0; + this.lastReachable = timestamp; + break; + case COLD: + this.lastReachable = timestamp; + if (this.lastReachable - this.lastUnreachable > WARMUP_MILLIS) { + this.status = Status.STABLE; + } + break; + case STABLE: + this.lastReachable = timestamp; + break; } } @@ -89,13 +125,29 @@ public void reportAsReachable(final long timestamp) { public String toString() { return "BackendHealthStatus{" + " hostPort=" + this.hostPort + + ", connections=" + this.connections + ", status=" + this.status - + ", lastUnreachableTs=" + this.lastUnreachableTs - + ", lastReachableTs=" + this.lastReachableTs + + ", unreachableSince=" + this.unreachableSince + + ", unreachableUntil=" + this.lastUnreachable + + ", lastReachable=" + this.lastReachable + ", lastProbe=" + this.lastProbe + '}'; } + public int getConnections() { + return this.connections.get(); + } + + public void incrementConnections() { + this.connections.incrementAndGet(); + } + + public void decrementConnections() { + if (this.connections.getAcquire() > 0) { + this.connections.decrementAndGet(); + } + } + /** * The enum models a simple status of the backend. */ diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/config/ActionConfiguration.java b/carapace-server/src/main/java/org/carapaceproxy/server/config/ActionConfiguration.java index 1b813d243..68bcdadd4 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/config/ActionConfiguration.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/config/ActionConfiguration.java @@ -33,10 +33,25 @@ @Data public class ActionConfiguration { + /** + * @see org.carapaceproxy.server.mapper.MapResult.Action#PROXY + */ public static final String TYPE_PROXY = "proxy"; + /** + * @see org.carapaceproxy.server.mapper.MapResult.Action#CACHE + */ public static final String TYPE_CACHE = "cache"; + /** + * @see org.carapaceproxy.server.mapper.MapResult.Action#STATIC + */ public static final String TYPE_STATIC = "static"; + /** + * @see org.carapaceproxy.server.mapper.MapResult.Action#ACME_CHALLENGE + */ public static final String TYPE_ACME_CHALLENGE = "acme-challenge"; + /** + * @see org.carapaceproxy.server.mapper.MapResult.Action#REDIRECT + */ public static final String TYPE_REDIRECT = "redirect"; private final String id; @@ -61,7 +76,7 @@ public ActionConfiguration(String id, String type, String director, String file, } public ActionConfiguration setCustomHeaders(List customHeaders) { - this.customHeaders = Collections.unmodifiableList(customHeaders == null ? new ArrayList() : customHeaders); + this.customHeaders = Collections.unmodifiableList(customHeaders == null ? new ArrayList<>() : customHeaders); return this; } diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/mapper/MapResult.java b/carapace-server/src/main/java/org/carapaceproxy/server/mapper/MapResult.java index addbfb2b6..30b42245e 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/mapper/MapResult.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/mapper/MapResult.java @@ -20,8 +20,13 @@ package org.carapaceproxy.server.mapper; import java.util.List; +import lombok.AccessLevel; import lombok.Builder; import lombok.Data; +import lombok.EqualsAndHashCode; +import lombok.Setter; +import lombok.ToString; +import org.carapaceproxy.server.backends.BackendHealthStatus; @Data @Builder @@ -78,7 +83,6 @@ public enum Action { public static final String REDIRECT_PROTO_HTTPS = "https"; public static final String REDIRECT_PROTO_HTTP = "http"; - // todo we don't actually want to have these nullable: probably we should have different classes for each case private String host; private int port; private Action action; @@ -90,6 +94,11 @@ public enum Action { private String redirectProto; private String redirectPath; + @Setter(AccessLevel.NONE) + @EqualsAndHashCode.Exclude + @ToString.Exclude + private BackendHealthStatus healthStatus; + public static MapResult notFound(String routeId) { return MapResult.builder() .action(Action.NOTFOUND) diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java b/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java index 6c8793b08..579be44f0 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java @@ -72,6 +72,9 @@ public class StandardEndpointMapper extends EndpointMapper { public static final String DEBUGGING_HEADER_DEFAULT_NAME = "X-Proxy-Path"; public static final String DEBUGGING_HEADER_ID = "mapper-debug"; + // todo replace this with a configurable property of some kind + private static final int THRESHOLD = 100; + // The map is wiped out whenever a new configuration is applied private final SequencedMap backends = new LinkedHashMap<>(); private final Map directors = new HashMap<>(); @@ -111,13 +114,13 @@ public MapResult map(final ProxyRequest request) { } return MapResult.badRequest(); } - if (!request.isValidHostAndPort(request.getRequestHostname())) { //Invalid header host + // Invalid header host + if (!request.isValidHostAndPort(request.getRequestHostname())) { if (LOG.isTraceEnabled()) { LOG.trace("Invalid header host {} for request {}", request.getRequestHostname(), request.getUri()); } return MapResult.badRequest(); } - for (final RouteConfiguration route : routes) { if (!route.isEnabled()) { continue; @@ -142,6 +145,7 @@ public MapResult map(final ProxyRequest request) { LOG.info("no action \"{}\" -> not-found for {}, valid {}", route.getAction(), request.getUri(), actions.keySet()); return MapResult.internalError(route.getId()); } + switch (action.getType()) { case ActionConfiguration.TYPE_REDIRECT -> { return MapResult.builder() @@ -199,36 +203,48 @@ public MapResult map(final ProxyRequest request) { LOG.trace("selected {} backends for {}, director is {}", selectedBackends, request.getUri(), director); } - for (final String backendId : selectedBackends) { - final Action selectedAction; - switch (action.getType()) { - case ActionConfiguration.TYPE_PROXY -> selectedAction = Action.PROXY; - case ActionConfiguration.TYPE_CACHE -> selectedAction = Action.CACHE; - default -> { - return MapResult.internalError(route.getId()); - } + final Action selectedAction; + switch (action.getType()) { + case ActionConfiguration.TYPE_PROXY -> selectedAction = Action.PROXY; + case ActionConfiguration.TYPE_CACHE -> selectedAction = Action.CACHE; + default -> { + return MapResult.internalError(route.getId()); } + } + for (final String backendId : selectedBackends) { final BackendConfiguration backend = this.backends.get(backendId); if (backend != null) { - BackendHealthManager backendHealthManager = getBackendHealthManager(); - if (backendHealthManager.getBackendStatus(backend.hostPort()) != BackendHealthStatus.Status.DOWN) { - List customHeaders = action.getCustomHeaders(); - if (this.debuggingHeaderEnabled) { - customHeaders = new ArrayList<>(customHeaders); - String routingPath = route.getId() + ";" - + action.getId() + ";" - + action.getDirector() + ";" - + backendId; - customHeaders.add(new CustomHeader(DEBUGGING_HEADER_ID, debuggingHeaderName, routingPath, HeaderMode.ADD)); + final BackendHealthManager backendHealthManager = getBackendHealthManager(); + final BackendHealthStatus backendStatus = backendHealthManager.getBackendStatus(backend.hostPort()); + switch (backendStatus.getStatus()) { + case DOWN: + continue; + case COLD: + if (backendStatus.getConnections() > THRESHOLD) { + // can't use this + continue; + } + // falls through + case STABLE: { + List customHeaders = action.getCustomHeaders(); + if (this.debuggingHeaderEnabled) { + customHeaders = new ArrayList<>(customHeaders); + final String routingPath = route.getId() + ";" + + action.getId() + ";" + + action.getDirector() + ";" + + backendId; + customHeaders.add(new CustomHeader(DEBUGGING_HEADER_ID, debuggingHeaderName, routingPath, HeaderMode.ADD)); + } + return MapResult.builder() + .host(backend.host()) + .port(backend.port()) + .action(selectedAction) + .routeId(route.getId()) + .customHeaders(customHeaders) + .healthStatus(backendStatus) + .build(); } - return MapResult.builder() - .host(backend.host()) - .port(backend.port()) - .action(selectedAction) - .routeId(route.getId()) - .customHeaders(customHeaders) - .build(); } } } diff --git a/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java b/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java index 9d9282828..451644496 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java @@ -123,7 +123,7 @@ public void testBackendUnreachableOnStuckRequest(boolean backendsUnreachableOnSt final BackendHealthStatus.Status expected = backendsUnreachableOnStuckRequests ? BackendHealthStatus.Status.DOWN : BackendHealthStatus.Status.COLD; - assertSame(expected, server.getBackendHealthManager().getBackendStatus(key)); + assertSame(expected, server.getBackendHealthManager().getBackendStatus(key).getStatus()); try (RawHttpClient client = new RawHttpClient("localhost", port)) { RawHttpClient.HttpResponse resp = client.executeRequest("GET /good-index.html HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n"); diff --git a/carapace-server/src/test/java/org/carapaceproxy/backends/UnreachableBackendTest.java b/carapace-server/src/test/java/org/carapaceproxy/backends/UnreachableBackendTest.java index caafb2750..9b20e13d8 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/backends/UnreachableBackendTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/backends/UnreachableBackendTest.java @@ -98,7 +98,7 @@ public void testWithUnreachableBackend() throws Exception { """, resp.getBodyString()); } - assertSame(server.getBackendHealthManager().getBackendStatus(key), BackendHealthStatus.Status.DOWN); + assertSame(server.getBackendHealthManager().getBackendStatus(key).getStatus(), BackendHealthStatus.Status.DOWN); assertThat((int) ProxyRequestsManager.PENDING_REQUESTS_GAUGE.get(), is(0)); } } @@ -136,7 +136,7 @@ public void testEmptyResponse() throws Exception { """, resp.getBodyString()); } - assertSame(server.getBackendHealthManager().getBackendStatus(key), BackendHealthStatus.Status.COLD); // no troubles for new connections + assertSame(server.getBackendHealthManager().getBackendStatus(key).getStatus(), BackendHealthStatus.Status.COLD); // no troubles for new connections assertThat((int) ProxyRequestsManager.PENDING_REQUESTS_GAUGE.get(), is(0)); } } @@ -168,7 +168,7 @@ public void testConnectionResetByPeer() throws Exception { """, resp.getBodyString()); } - assertSame(server.getBackendHealthManager().getBackendStatus(key), BackendHealthStatus.Status.COLD); // no troubles for new connections + assertSame(server.getBackendHealthManager().getBackendStatus(key).getStatus(), BackendHealthStatus.Status.COLD); // no troubles for new connections assertThat((int) ProxyRequestsManager.PENDING_REQUESTS_GAUGE.get(), is(0)); } } @@ -204,7 +204,7 @@ public void testNonHttpResponseThenClose() throws Exception { """, resp.getBodyString()); } - assertSame(server.getBackendHealthManager().getBackendStatus(key), BackendHealthStatus.Status.COLD); + assertSame(server.getBackendHealthManager().getBackendStatus(key).getStatus(), BackendHealthStatus.Status.COLD); assertThat((int) ProxyRequestsManager.PENDING_REQUESTS_GAUGE.get(), is(0)); } } diff --git a/carapace-server/src/test/java/org/carapaceproxy/server/mapper/BasicStandardEndpointMapperTest.java b/carapace-server/src/test/java/org/carapaceproxy/server/mapper/BasicStandardEndpointMapperTest.java index 4835e0574..7ebf5042c 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/server/mapper/BasicStandardEndpointMapperTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/server/mapper/BasicStandardEndpointMapperTest.java @@ -259,11 +259,11 @@ public void testRouteErrors() throws Exception { final EndpointKey alive = EndpointKey.make("localhost:" + backend.port()); final BackendHealthStatus mockAliveStatus = mock(BackendHealthStatus.class); when(mockAliveStatus.getStatus()).thenReturn(BackendHealthStatus.Status.STABLE); - when(bhMan.getBackendStatus(eq(alive))).thenReturn(mockAliveStatus.getStatus()); + when(bhMan.getBackendStatus(eq(alive))).thenReturn(mockAliveStatus); final EndpointKey down = EndpointKey.make("localhost-down:" + backend.port()); final BackendHealthStatus mockDownStatus = mock(BackendHealthStatus.class); when(mockDownStatus.getStatus()).thenReturn(BackendHealthStatus.Status.DOWN); - when(bhMan.getBackendStatus(eq(down))).thenReturn(mockDownStatus.getStatus()); // simulate unreachable backend -> expected 500 error + when(bhMan.getBackendStatus(eq(down))).thenReturn(mockDownStatus); // simulate unreachable backend -> expected 500 error server.setBackendHealthManager(bhMan); server.configureAtBoot(config); server.start(); @@ -331,11 +331,11 @@ public void testDefaultRoute() throws Exception { final EndpointKey alive = EndpointKey.make("localhost:" + backendPort); final BackendHealthStatus mockAliveStatus = mock(BackendHealthStatus.class); when(mockAliveStatus.getStatus()).thenReturn(BackendHealthStatus.Status.STABLE); - when(bhMan.getBackendStatus(eq(alive))).thenReturn(mockAliveStatus.getStatus()); + when(bhMan.getBackendStatus(eq(alive))).thenReturn(mockAliveStatus); final EndpointKey down = EndpointKey.make("localhost-down:" + backendPort); final BackendHealthStatus mockDownStatus = mock(BackendHealthStatus.class); when(mockDownStatus.getStatus()).thenReturn(BackendHealthStatus.Status.DOWN); - when(bhMan.getBackendStatus(eq(down))).thenReturn(mockDownStatus.getStatus()); // simulate unreachable backend -> expected 500 error + when(bhMan.getBackendStatus(eq(down))).thenReturn(mockDownStatus); // simulate unreachable backend -> expected 500 error try (HttpProxyServer server = HttpProxyServer.buildForTests("localhost", 0, mapper, tmpDir.newFolder())) { server.setBackendHealthManager(bhMan); diff --git a/carapace-server/src/test/java/org/carapaceproxy/server/mapper/HealthCheckTest.java b/carapace-server/src/test/java/org/carapaceproxy/server/mapper/HealthCheckTest.java index 41f392b67..bbbf12310 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/server/mapper/HealthCheckTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/server/mapper/HealthCheckTest.java @@ -23,10 +23,14 @@ import static com.github.tomakehurst.wiremock.client.WireMock.get; import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; +import static org.hamcrest.CoreMatchers.allOf; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.CoreMatchers.nullValue; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.lessThan; +import static org.hamcrest.Matchers.lessThanOrEqualTo; import com.github.tomakehurst.wiremock.junit.WireMockRule; import java.util.HashMap; import java.util.Map; @@ -86,9 +90,10 @@ public void test() throws Exception { final BackendHealthStatus _status = status.get(b1conf.hostPort()); assertThat(_status, is(not(nullValue()))); assertThat(_status.getHostPort(), is(b1conf.hostPort())); - assertThat(_status.getStatus() != BackendHealthStatus.Status.DOWN, is(true)); - assertThat(_status.getStatus() == BackendHealthStatus.Status.DOWN, is(false)); - assertThat(_status.getLastUnreachableTs(), is(0L)); + assertThat(_status.getStatus(), is(BackendHealthStatus.Status.COLD)); + assertThat(_status.getUnreachableSince(), is(0L)); + assertThat(_status.getLastUnreachable(), lessThan(_status.getLastReachable())); + assertThat(_status.getLastReachable(), allOf(greaterThanOrEqualTo(startTs), lessThanOrEqualTo(endTs))); final BackendHealthCheck lastProbe = _status.getLastProbe(); assertThat(lastProbe, is(not(nullValue()))); @@ -99,7 +104,6 @@ public void test() throws Exception { assertThat(lastProbe.httpResponse(), is("200 OK")); assertThat(lastProbe.httpBody(), is("Ok...")); } - final long reportedAsUnreachableTs; { // Backend returns 500, marking it unavailable. stubFor(get(urlEqualTo("/status.html")) @@ -125,11 +129,10 @@ public void test() throws Exception { final BackendHealthStatus _status = status.get(b1conf.hostPort()); assertThat(_status, is(not(nullValue()))); assertThat(_status.getHostPort(), is(b1conf.hostPort())); - assertThat(_status.getStatus() != BackendHealthStatus.Status.DOWN, is(false)); - assertThat(_status.getStatus() == BackendHealthStatus.Status.DOWN, is(true)); - assertThat(_status.getLastUnreachableTs() >= startTs, is(true)); - assertThat(_status.getLastUnreachableTs() <= endTs, is(true)); - reportedAsUnreachableTs = _status.getLastUnreachableTs(); + assertThat(_status.getStatus(), is(BackendHealthStatus.Status.DOWN)); + assertThat(_status.getLastReachable(), allOf(lessThan(startTs), lessThan(endTs))); + assertThat(_status.getUnreachableSince(), allOf(greaterThanOrEqualTo(startTs), lessThanOrEqualTo(endTs))); + assertThat(_status.getLastUnreachable(), is(_status.getUnreachableSince())); final BackendHealthCheck lastProbe = _status.getLastProbe(); assertThat(lastProbe, is(not(nullValue()))); @@ -167,9 +170,10 @@ public void test() throws Exception { final BackendHealthStatus _status = status.get(b1conf.hostPort()); assertThat(_status, is(not(nullValue()))); assertThat(_status.getHostPort(), is(b1conf.hostPort())); - assertThat(_status.getStatus() != BackendHealthStatus.Status.DOWN, is(false)); - assertThat(_status.getStatus() == BackendHealthStatus.Status.DOWN, is(true)); - assertThat(_status.getLastUnreachableTs(), is(reportedAsUnreachableTs)); + assertThat(_status.getStatus(), is(BackendHealthStatus.Status.DOWN)); + assertThat(_status.getLastReachable(), allOf(lessThan(startTs), lessThan(endTs))); + assertThat(_status.getUnreachableSince(), allOf(lessThan(startTs), lessThan(endTs))); + assertThat(_status.getLastUnreachable(), allOf(greaterThanOrEqualTo(startTs), lessThanOrEqualTo(endTs))); final BackendHealthCheck lastProbe = _status.getLastProbe(); assertThat(lastProbe, is(not(nullValue()))); @@ -207,9 +211,10 @@ public void test() throws Exception { final BackendHealthStatus _status = status.get(b1conf.hostPort()); assertThat(_status, is(not(nullValue()))); assertThat(_status.getHostPort(), is(b1conf.hostPort())); - assertThat(_status.getStatus() != BackendHealthStatus.Status.DOWN, is(true)); - assertThat(_status.getStatus() == BackendHealthStatus.Status.DOWN, is(false)); - assertThat(_status.getLastUnreachableTs(), is(0L)); + assertThat(_status.getStatus(), is(BackendHealthStatus.Status.COLD)); + assertThat(_status.getUnreachableSince(), is(0L)); + assertThat(_status.getLastUnreachable(), allOf(lessThan(startTs), lessThan(endTs))); + assertThat(_status.getLastReachable(), allOf(greaterThanOrEqualTo(startTs), lessThanOrEqualTo(endTs))); final BackendHealthCheck lastProbe = _status.getLastProbe(); assertThat(lastProbe, is(not(nullValue()))); diff --git a/carapace-server/src/test/java/org/carapaceproxy/utils/TestEndpointMapper.java b/carapace-server/src/test/java/org/carapaceproxy/utils/TestEndpointMapper.java index 5caea9b93..7f461312b 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/utils/TestEndpointMapper.java +++ b/carapace-server/src/test/java/org/carapaceproxy/utils/TestEndpointMapper.java @@ -26,6 +26,7 @@ import java.util.SequencedMap; import org.carapaceproxy.configstore.ConfigurationStore; import org.carapaceproxy.core.ProxyRequest; +import org.carapaceproxy.server.backends.BackendHealthStatus; import org.carapaceproxy.server.config.ActionConfiguration; import org.carapaceproxy.server.config.BackendConfiguration; import org.carapaceproxy.server.config.DirectorConfiguration; @@ -44,6 +45,7 @@ public class TestEndpointMapper extends EndpointMapper { private final List actions = new ArrayList<>(); private final List directors = new ArrayList<>(); private final List headers = new ArrayList<>(); + private BackendHealthStatus healthStatus; public TestEndpointMapper(String host, int port) { this(host, port, false); @@ -60,6 +62,10 @@ public TestEndpointMapper(String host, int port, boolean cacheAll, Map(backends); } + public BackendHealthStatus getHealthStatus() { + return healthStatus; + } + @Override public MapResult map(ProxyRequest request) { String uri = request.getUri(); @@ -70,20 +76,25 @@ public MapResult map(ProxyRequest request) { .action(MapResult.Action.SYSTEM) .routeId(MapResult.NO_ROUTE) .build(); - } else if (cacheAll) { - return MapResult.builder() - .host(host) - .port(port) - .action(MapResult.Action.CACHE) - .routeId(MapResult.NO_ROUTE) - .build(); } else { - return MapResult.builder() - .host(host) - .port(port) - .action(MapResult.Action.PROXY) - .routeId(MapResult.NO_ROUTE) - .build(); + healthStatus = new BackendHealthStatus(request.getListener()); + if (cacheAll) { + return MapResult.builder() + .host(host) + .port(port) + .action(MapResult.Action.CACHE) + .routeId(MapResult.NO_ROUTE) + .healthStatus(healthStatus) + .build(); + } else { + return MapResult.builder() + .host(host) + .port(port) + .action(MapResult.Action.PROXY) + .routeId(MapResult.NO_ROUTE) + .healthStatus(healthStatus) + .build(); + } } } From 2e083423d680ee5b861f80536b972f995869a0f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Maltoni?= Date: Thu, 21 Nov 2024 11:55:04 +0100 Subject: [PATCH 08/15] feat: add new configurations * add `healthmanager.warmupperiod` configuration (default 30s) * add `backend..coldcapacity` configuration (default 10) * add tolerance configuration --- .../core/RuntimeServerConfiguration.java | 10 ++++++ .../server/backends/BackendHealthManager.java | 36 +++++++++++++++++-- .../server/backends/BackendHealthStatus.java | 14 ++++---- .../server/config/BackendConfiguration.java | 6 ++-- .../server/mapper/StandardEndpointMapper.java | 18 +++++----- .../backends/StuckRequestsTest.java | 2 +- .../BasicStandardEndpointMapperTest.java | 8 ++--- .../server/mapper/ForceBackendTest.java | 4 +-- .../server/mapper/HealthCheckTest.java | 2 +- .../utils/TestEndpointMapper.java | 5 +-- 10 files changed, 76 insertions(+), 29 deletions(-) diff --git a/carapace-server/src/main/java/org/carapaceproxy/core/RuntimeServerConfiguration.java b/carapace-server/src/main/java/org/carapaceproxy/core/RuntimeServerConfiguration.java index 0d99a81c9..ed8d55e31 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/core/RuntimeServerConfiguration.java +++ b/carapace-server/src/main/java/org/carapaceproxy/core/RuntimeServerConfiguration.java @@ -40,6 +40,7 @@ import java.security.NoSuchAlgorithmException; import java.sql.Timestamp; import java.text.SimpleDateFormat; +import java.time.Duration; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -75,6 +76,7 @@ public class RuntimeServerConfiguration { private static final Logger LOG = LoggerFactory.getLogger(RuntimeServerConfiguration.class); private static final int DEFAULT_PROBE_PERIOD = 0; + public static final long DEFAULT_WARMUP_PERIOD = Duration.ofSeconds(30).toMillis(); private final List listeners = new ArrayList<>(); private final Map certificates = new HashMap<>(); @@ -116,6 +118,8 @@ public class RuntimeServerConfiguration { private String userRealmClassname; private int healthProbePeriod = DEFAULT_PROBE_PERIOD; private int healthConnectTimeout = 5_000; + private long warmupPeriod = DEFAULT_WARMUP_PERIOD; + private boolean tolerant = false; private int dynamicCertificatesManagerPeriod = 0; private int keyPairsSize = DEFAULT_KEYPAIRS_SIZE; private Set domainsCheckerIPAddresses; @@ -231,6 +235,12 @@ public void configure(ConfigurationStore properties) throws ConfigurationNotVali LOG.warn("BACKEND-HEALTH-MANAGER DISABLED"); } + warmupPeriod = properties.getLong("healthmanager.warmupperiod", DEFAULT_WARMUP_PERIOD); + LOG.info("healthmanager.warmupperiod={}", warmupPeriod); + + tolerant = properties.getBoolean("healthmanager.tolerant", false); + LOG.info("healthmanager.tolerant={}", tolerant); + healthConnectTimeout = properties.getInt("healthmanager.connecttimeout", healthConnectTimeout); LOG.info("healthmanager.connecttimeout={}", healthConnectTimeout); if (healthConnectTimeout < 0) { diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java b/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java index 6e6375251..a6871e187 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java @@ -22,6 +22,7 @@ import com.google.common.annotations.VisibleForTesting; import io.prometheus.client.Gauge; import java.util.Map; +import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; @@ -60,13 +61,17 @@ public class BackendHealthManager implements Runnable { private volatile int connectTimeout; // keep track of start() calling private volatile boolean started; + private volatile long warmupPeriod; + private volatile boolean tolerant; public BackendHealthManager(final RuntimeServerConfiguration conf, final EndpointMapper mapper) { this.mapper = mapper; + this.connectTimeout = conf.getHealthConnectTimeout(); + this.warmupPeriod = conf.getWarmupPeriod(); + this.tolerant = conf.isTolerant(); // will be overridden before start this.period = DEFAULT_PERIOD; - this.connectTimeout = conf.getHealthConnectTimeout(); } public int getPeriod() { @@ -122,6 +127,17 @@ public synchronized void reloadConfiguration(RuntimeServerConfiguration newConfi LOG.info("Applying new connect timeout {} ms", this.connectTimeout); } + if (this.warmupPeriod != newConfiguration.getWarmupPeriod()) { + this.warmupPeriod = newConfiguration.getWarmupPeriod(); + this.backends.values().forEach(it -> it.setWarmupPeriod(warmupPeriod)); + LOG.info("Applying new warmup period of {} ms", this.warmupPeriod); + } + + if (this.tolerant != newConfiguration.isTolerant()) { + this.tolerant = newConfiguration.isTolerant(); + LOG.info("Applying new health tolerance configuration {}; cold backends now {} exceed safe capacity", this.tolerant, this.tolerant ? "may" : "may not"); + } + this.mapper = mapper; if (restart || started) { @@ -185,7 +201,23 @@ public Map getBackendsSnapshot() { } public BackendHealthStatus getBackendStatus(final EndpointKey hostPort) { - return backends.computeIfAbsent(hostPort, BackendHealthStatus::new); + return backends.computeIfAbsent(hostPort, key -> new BackendHealthStatus(key, warmupPeriod)); + } + + public BackendHealthStatus getBackendStatus(final String backendId) { + final BackendConfiguration backendConfiguration = this.mapper.getBackends().get(backendId); + Objects.requireNonNull(backendConfiguration); + return getBackendStatus(backendConfiguration.hostPort()); + } + + public boolean exceedsCapacity(final String backendId) { + final BackendConfiguration backendConfiguration = this.mapper.getBackends().get(backendId); + Objects.requireNonNull(backendConfiguration); + if (backendConfiguration.coldCapacity() <= 0) { + return false; + } + final BackendHealthStatus backendStatus = getBackendStatus(backendConfiguration.hostPort()); + return backendConfiguration.coldCapacity() > backendStatus.getConnections() && !this.tolerant; } @VisibleForTesting diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthStatus.java b/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthStatus.java index 22024d393..ad2609b98 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthStatus.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthStatus.java @@ -20,7 +20,6 @@ package org.carapaceproxy.server.backends; import java.sql.Timestamp; -import java.time.Duration; import java.util.concurrent.atomic.AtomicInteger; import org.carapaceproxy.core.EndpointKey; import org.slf4j.Logger; @@ -33,9 +32,6 @@ */ public class BackendHealthStatus { - // todo replace this with a configurable property of some kind - public static final long WARMUP_MILLIS = Duration.ofMinutes(1).toMillis(); - private static final Logger LOG = LoggerFactory.getLogger(BackendHealthStatus.class); private final EndpointKey hostPort; @@ -46,8 +42,9 @@ public class BackendHealthStatus { private volatile long lastUnreachable; private volatile long lastReachable; private volatile BackendHealthCheck lastProbe; + private volatile long warmupPeriod; - public BackendHealthStatus(final EndpointKey hostPort) { + public BackendHealthStatus(final EndpointKey hostPort, final long warmupPeriod) { this.hostPort = hostPort; // todo cannot start with a DOWN backend (+ current time) as it would break: // - BasicStandardEndpointMapperTest, @@ -61,6 +58,7 @@ public BackendHealthStatus(final EndpointKey hostPort) { this.lastUnreachable = created; this.lastReachable = created; this.connections = new AtomicInteger(); + this.warmupPeriod = warmupPeriod; } public long getUnreachableSince() { @@ -111,7 +109,7 @@ public void reportAsReachable(final long timestamp) { break; case COLD: this.lastReachable = timestamp; - if (this.lastReachable - this.lastUnreachable > WARMUP_MILLIS) { + if (this.lastReachable - this.lastUnreachable > this.warmupPeriod) { this.status = Status.STABLE; } break; @@ -148,6 +146,10 @@ public void decrementConnections() { } } + public void setWarmupPeriod(final long warmupPeriod) { + this.warmupPeriod = warmupPeriod; + } + /** * The enum models a simple status of the backend. */ diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/config/BackendConfiguration.java b/carapace-server/src/main/java/org/carapaceproxy/server/config/BackendConfiguration.java index 7fc42e453..f845de084 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/config/BackendConfiguration.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/config/BackendConfiguration.java @@ -24,10 +24,10 @@ /** * Configuration of a single backend server */ -public record BackendConfiguration(String id, EndpointKey hostPort, String probePath) { +public record BackendConfiguration(String id, EndpointKey hostPort, String probePath, int coldCapacity) { - public BackendConfiguration(final String id, final String host, final int port, final String probePath) { - this(id, new EndpointKey(host, port), probePath); + public BackendConfiguration(final String id, final String host, final int port, final String probePath, final int coldCapacity) { + this(id, new EndpointKey(host, port), probePath, coldCapacity); } public String host() { diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java b/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java index 579be44f0..086218dcb 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java @@ -71,6 +71,7 @@ public class StandardEndpointMapper extends EndpointMapper { public static final String ACME_CHALLENGE_ROUTE_ACTION_ID = "acme-challenge"; public static final String DEBUGGING_HEADER_DEFAULT_NAME = "X-Proxy-Path"; public static final String DEBUGGING_HEADER_ID = "mapper-debug"; + private static final int DEFAULT_CAPACITY = 10; // number of connections // todo replace this with a configurable property of some kind private static final int THRESHOLD = 100; @@ -221,7 +222,8 @@ public MapResult map(final ProxyRequest request) { case DOWN: continue; case COLD: - if (backendStatus.getConnections() > THRESHOLD) { + final int capacity = backend.coldCapacity(); + if (capacity > 0 && backendStatus.getConnections() > capacity) { // can't use this continue; } @@ -464,14 +466,14 @@ public void configure(ConfigurationStore properties) throws ConfigurationNotVali String prefix = "backend." + i + "."; String id = properties.getString(prefix + "id", ""); if (!id.isEmpty()) { - boolean enabled = properties.getBoolean(prefix + "enabled", false); - String host = properties.getString(prefix + "host", "localhost"); - int port = properties.getInt(prefix + "port", 8086); - String probePath = properties.getString(prefix + "probePath", ""); - LOG.info("configured backend {} {}:{} enabled:{}", id, host, port, enabled); + final boolean enabled = properties.getBoolean(prefix + "enabled", false); + final String host = properties.getString(prefix + "host", "localhost"); + final int port = properties.getInt(prefix + "port", 8086); + final String probePath = properties.getString(prefix + "probePath", ""); + final int coldCapacity = properties.getInt(prefix + "coldCapacity", DEFAULT_CAPACITY); + LOG.info("configured backend {} {}:{} enabled={} capacity={}", id, host, port, enabled, coldCapacity); if (enabled) { - BackendConfiguration config = new BackendConfiguration(id, host, port, probePath); - addBackend(config); + addBackend(new BackendConfiguration(id, host, port, probePath, coldCapacity)); } } } diff --git a/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java b/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java index 451644496..fe4d6bc2e 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java @@ -84,7 +84,7 @@ public void testBackendUnreachableOnStuckRequest(boolean backendsUnreachableOnSt EndpointKey key = new EndpointKey("localhost", theport); StandardEndpointMapper mapper = new StandardEndpointMapper(); - mapper.addBackend(new BackendConfiguration("backend-a", "localhost", theport, "/")); + mapper.addBackend(new BackendConfiguration("backend-a", "localhost", theport, "/", -1)); mapper.addDirector(new DirectorConfiguration("director-1").addBackend("backend-a")); mapper.addAction(new ActionConfiguration("proxy-1", ActionConfiguration.TYPE_PROXY, "director-1", null, -1)); mapper.addRoute(new RouteConfiguration("route-1", "proxy-1", true, new RegexpRequestMatcher(PROPERTY_URI, ".*index.html.*"))); diff --git a/carapace-server/src/test/java/org/carapaceproxy/server/mapper/BasicStandardEndpointMapperTest.java b/carapace-server/src/test/java/org/carapaceproxy/server/mapper/BasicStandardEndpointMapperTest.java index 7ebf5042c..bf1ef0934 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/server/mapper/BasicStandardEndpointMapperTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/server/mapper/BasicStandardEndpointMapperTest.java @@ -95,8 +95,8 @@ public void test() throws Exception { int backendPort = backend.port(); StandardEndpointMapper mapper = new StandardEndpointMapper(); - mapper.addBackend(new BackendConfiguration("backend-a", "localhost", backendPort, "/")); - mapper.addBackend(new BackendConfiguration("backend-b", "localhost", backendPort, "/")); + mapper.addBackend(new BackendConfiguration("backend-a", "localhost", backendPort, "/", -1)); + mapper.addBackend(new BackendConfiguration("backend-b", "localhost", backendPort, "/", -1)); mapper.addDirector(new DirectorConfiguration("director-1").addBackend("backend-a")); mapper.addDirector(new DirectorConfiguration("director-2").addBackend("backend-b")); mapper.addDirector(new DirectorConfiguration("director-all").addBackend("*")); // all the known backends @@ -317,8 +317,8 @@ public void testDefaultRoute() throws Exception { int backendPort = backend.port(); StandardEndpointMapper mapper = new StandardEndpointMapper(); - mapper.addBackend(new BackendConfiguration("backend", "localhost", backendPort, "/")); - mapper.addBackend(new BackendConfiguration("backend-down", "localhost-down", backendPort, "/")); + mapper.addBackend(new BackendConfiguration("backend", "localhost", backendPort, "/", -1)); + mapper.addBackend(new BackendConfiguration("backend-down", "localhost-down", backendPort, "/", -1)); mapper.addDirector(new DirectorConfiguration("director").addBackend("backend")); mapper.addDirector(new DirectorConfiguration("director-down").addBackend("backend-down")); mapper.addAction(new ActionConfiguration("cache", ActionConfiguration.TYPE_CACHE, "director", null, -1)); diff --git a/carapace-server/src/test/java/org/carapaceproxy/server/mapper/ForceBackendTest.java b/carapace-server/src/test/java/org/carapaceproxy/server/mapper/ForceBackendTest.java index 852b17ded..bb3793a6d 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/server/mapper/ForceBackendTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/server/mapper/ForceBackendTest.java @@ -77,8 +77,8 @@ public void test() throws Exception { assertEquals("thedirector", mapper.getForceDirectorParameter()); assertEquals("thebackend", mapper.getForceBackendParameter()); - mapper.addBackend(new BackendConfiguration("backend-a", "localhost", backendPort, "/")); - mapper.addBackend(new BackendConfiguration("backend-b", "localhost", backendPort, "/")); + mapper.addBackend(new BackendConfiguration("backend-a", "localhost", backendPort, "/", -1)); + mapper.addBackend(new BackendConfiguration("backend-b", "localhost", backendPort, "/", -1)); mapper.addDirector(new DirectorConfiguration("director-1").addBackend("backend-a")); mapper.addDirector(new DirectorConfiguration("director-2").addBackend("backend-b")); diff --git a/carapace-server/src/test/java/org/carapaceproxy/server/mapper/HealthCheckTest.java b/carapace-server/src/test/java/org/carapaceproxy/server/mapper/HealthCheckTest.java index bbbf12310..ef9339305 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/server/mapper/HealthCheckTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/server/mapper/HealthCheckTest.java @@ -60,7 +60,7 @@ public class HealthCheckTest { @Test public void test() throws Exception { final Map backends = new HashMap<>(); - final BackendConfiguration b1conf = new BackendConfiguration("myid", "localhost", wireMockRule.port(), "/status.html"); + final BackendConfiguration b1conf = new BackendConfiguration("myid", "localhost", wireMockRule.port(), "/status.html", -1); backends.put(b1conf.hostPort().toString(), b1conf); final EndpointMapper mapper = new TestEndpointMapper(null, 0, false, backends); final RuntimeServerConfiguration conf = new RuntimeServerConfiguration(); diff --git a/carapace-server/src/test/java/org/carapaceproxy/utils/TestEndpointMapper.java b/carapace-server/src/test/java/org/carapaceproxy/utils/TestEndpointMapper.java index 7f461312b..73e41e48e 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/utils/TestEndpointMapper.java +++ b/carapace-server/src/test/java/org/carapaceproxy/utils/TestEndpointMapper.java @@ -19,6 +19,7 @@ */ package org.carapaceproxy.utils; +import static org.carapaceproxy.core.RuntimeServerConfiguration.DEFAULT_WARMUP_PERIOD; import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; @@ -52,7 +53,7 @@ public TestEndpointMapper(String host, int port) { } public TestEndpointMapper(String host, int port, boolean cacheAll) { - this(host, port, cacheAll, Map.of(host, new BackendConfiguration(host, host, port, "/index.html"))); + this(host, port, cacheAll, Map.of(host, new BackendConfiguration(host, host, port, "/index.html", -1))); } public TestEndpointMapper(String host, int port, boolean cacheAll, Map backends) { @@ -77,7 +78,7 @@ public MapResult map(ProxyRequest request) { .routeId(MapResult.NO_ROUTE) .build(); } else { - healthStatus = new BackendHealthStatus(request.getListener()); + healthStatus = new BackendHealthStatus(request.getListener(), DEFAULT_WARMUP_PERIOD); if (cacheAll) { return MapResult.builder() .host(host) From 48b5bf3aff94bb8bcd43d675c85d664ae109a144 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Maltoni?= Date: Mon, 11 Nov 2024 15:45:01 +0100 Subject: [PATCH 09/15] chore: cleanup todo comments --- .../carapaceproxy/server/backends/BackendHealthStatus.java | 7 +------ .../server/mapper/StandardEndpointMapper.java | 3 --- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthStatus.java b/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthStatus.java index ad2609b98..13826d43d 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthStatus.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthStatus.java @@ -46,12 +46,7 @@ public class BackendHealthStatus { public BackendHealthStatus(final EndpointKey hostPort, final long warmupPeriod) { this.hostPort = hostPort; - // todo cannot start with a DOWN backend (+ current time) as it would break: - // - BasicStandardEndpointMapperTest, - // - UnreachableBackendTest, - // - StuckRequestsTest, - // - HealthCheckTest - // we assume that the backend is reachable when status is created + // we assume that the backend just become reachable when the status is created this.status = Status.COLD; this.unreachableSince = 0L; final long created = System.currentTimeMillis(); diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java b/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java index 086218dcb..b69d921b1 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java @@ -73,9 +73,6 @@ public class StandardEndpointMapper extends EndpointMapper { public static final String DEBUGGING_HEADER_ID = "mapper-debug"; private static final int DEFAULT_CAPACITY = 10; // number of connections - // todo replace this with a configurable property of some kind - private static final int THRESHOLD = 100; - // The map is wiped out whenever a new configuration is applied private final SequencedMap backends = new LinkedHashMap<>(); private final Map directors = new HashMap<>(); From 7e348bbed50c9d3ab14e3b14a63ddf5a798ef8f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Maltoni?= Date: Thu, 21 Nov 2024 15:08:02 +0100 Subject: [PATCH 10/15] feat: introduce SafeBackendSelector --- .../server/backends/BackendHealthManager.java | 4 +- .../server/config/BackendConfiguration.java | 25 ++++++-- .../server/config/BackendSelector.java | 14 +++++ .../server/config/SafeBackendSelector.java | 62 +++++++++++++++++++ .../server/mapper/EndpointMapper.java | 2 +- .../server/mapper/RandomBackendSelector.java | 18 ++++-- .../server/mapper/StandardEndpointMapper.java | 27 ++++---- .../backends/StuckRequestsTest.java | 5 +- .../BasicStandardEndpointMapperTest.java | 5 +- .../server/mapper/ForceBackendTest.java | 3 +- 10 files changed, 135 insertions(+), 30 deletions(-) create mode 100644 carapace-server/src/main/java/org/carapaceproxy/server/config/SafeBackendSelector.java diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java b/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java index a6871e187..1cfbb5539 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java @@ -213,11 +213,11 @@ public BackendHealthStatus getBackendStatus(final String backendId) { public boolean exceedsCapacity(final String backendId) { final BackendConfiguration backendConfiguration = this.mapper.getBackends().get(backendId); Objects.requireNonNull(backendConfiguration); - if (backendConfiguration.coldCapacity() <= 0) { + if (backendConfiguration.safeCapacity() <= 0) { return false; } final BackendHealthStatus backendStatus = getBackendStatus(backendConfiguration.hostPort()); - return backendConfiguration.coldCapacity() > backendStatus.getConnections() && !this.tolerant; + return backendConfiguration.safeCapacity() > backendStatus.getConnections() && !this.tolerant; } @VisibleForTesting diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/config/BackendConfiguration.java b/carapace-server/src/main/java/org/carapaceproxy/server/config/BackendConfiguration.java index f845de084..a7897cc99 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/config/BackendConfiguration.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/config/BackendConfiguration.java @@ -20,14 +20,29 @@ package org.carapaceproxy.server.config; import org.carapaceproxy.core.EndpointKey; +import org.carapaceproxy.server.backends.BackendHealthStatus.Status; /** - * Configuration of a single backend server + * Configuration of a single backend server. + * + * @param id an arbitrary ID of the backend + * @param hostPort the host:port tuple for the backend + * @param probePath a path to use to probe the backend for reachability + * @param safeCapacity a capacity that is considered safe even when {@link Status#COLD cold} */ -public record BackendConfiguration(String id, EndpointKey hostPort, String probePath, int coldCapacity) { - - public BackendConfiguration(final String id, final String host, final int port, final String probePath, final int coldCapacity) { - this(id, new EndpointKey(host, port), probePath, coldCapacity); +public record BackendConfiguration(String id, EndpointKey hostPort, String probePath, int safeCapacity) { + + /** + * Configuration of a single backend server. + * + * @param id an arbitrary ID of the backend + * @param host the host name + * @param port the port to use + * @param probePath a path to use to probe the backend for reachability + * @param safeCapacity a capacity that is considered safe even when {@link Status#COLD cold}, or 0 for an infinite capacity + */ + public BackendConfiguration(final String id, final String host, final int port, final String probePath, final int safeCapacity) { + this(id, new EndpointKey(host, port), probePath, safeCapacity); } public String host() { diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/config/BackendSelector.java b/carapace-server/src/main/java/org/carapaceproxy/server/config/BackendSelector.java index f157178b5..2fae705fe 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/config/BackendSelector.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/config/BackendSelector.java @@ -20,6 +20,8 @@ package org.carapaceproxy.server.config; import java.util.List; +import java.util.function.Function; +import org.carapaceproxy.server.mapper.EndpointMapper; /** * Chooses the backend which can serve the request @@ -27,4 +29,16 @@ public interface BackendSelector { List selectBackends(String userId, String sessionId, String director); + + /** + * Functional interface that models a factory for selectors given the mapper they should be applied to. + * + * @see SafeBackendSelector#forMapper(EndpointMapper) + */ + @FunctionalInterface + interface SelectorFactory extends Function { + + @Override + BackendSelector apply(EndpointMapper endpointMapper); + } } diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/config/SafeBackendSelector.java b/carapace-server/src/main/java/org/carapaceproxy/server/config/SafeBackendSelector.java new file mode 100644 index 000000000..7fbd4bae0 --- /dev/null +++ b/carapace-server/src/main/java/org/carapaceproxy/server/config/SafeBackendSelector.java @@ -0,0 +1,62 @@ +package org.carapaceproxy.server.config; + +import static org.carapaceproxy.server.config.DirectorConfiguration.ALL_BACKENDS; +import java.util.Comparator; +import java.util.List; +import java.util.Map; +import java.util.SequencedCollection; +import java.util.function.Function; +import java.util.stream.Collectors; +import org.carapaceproxy.server.backends.BackendHealthManager; +import org.carapaceproxy.server.backends.BackendHealthStatus; +import org.carapaceproxy.server.mapper.EndpointMapper; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class SafeBackendSelector implements BackendSelector { + private static final Logger LOGGER = LoggerFactory.getLogger(SafeBackendSelector.class); + private final SequencedCollection backends; + private final Map directors; + private final BackendHealthManager healthManager; + + public static BackendSelector forMapper(final EndpointMapper mapper) { + final var directors = mapper.getDirectors() + .stream() + .collect(Collectors.toUnmodifiableMap(DirectorConfiguration::getId, Function.identity())); + return new SafeBackendSelector(mapper.getBackends().sequencedKeySet(), directors, mapper.getBackendHealthManager()); + } + + public SafeBackendSelector(final SequencedCollection allBackendIds, final Map directors, final BackendHealthManager healthManager) { + this.backends = allBackendIds; + this.directors = directors; + this.healthManager = healthManager; + } + + @Override + public List selectBackends(final String userId, final String sessionId, final String director) { + if (!directors.containsKey(director)) { + LOGGER.error("Director \"{}\" not configured, while handling request userId={} sessionId={}", director, userId, sessionId); + return List.of(); + } + final DirectorConfiguration directorConfig = directors.get(director); + if (directorConfig.getBackends().contains(ALL_BACKENDS)) { + return sortByConnections(backends); + } + return sortByConnections(directorConfig.getBackends()); + } + + public List sortByConnections(final SequencedCollection backendIds) { + return backendIds.stream().sorted(Comparator.comparingLong(this::connections)).toList(); + } + + private long connections(final String backendId) { + final BackendHealthStatus backendStatus = healthManager.getBackendStatus(backendId); + return switch (backendStatus.getStatus()) { + case DOWN -> Long.MAX_VALUE; // backends that are down are put last, but not dropped + case COLD -> healthManager.exceedsCapacity(backendId) + ? Long.MAX_VALUE - 1 // cold backends that exceed safe capacity are put last, just before down ones + : backendStatus.getConnections(); + case STABLE -> backendStatus.getConnections(); + }; + } +} diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/mapper/EndpointMapper.java b/carapace-server/src/main/java/org/carapaceproxy/server/mapper/EndpointMapper.java index a12b1f703..c610ea3cb 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/mapper/EndpointMapper.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/mapper/EndpointMapper.java @@ -125,7 +125,7 @@ protected final DynamicCertificatesManager getDynamicCertificatesManager() { return parent.getDynamicCertificatesManager(); } - protected final BackendHealthManager getBackendHealthManager() { + public final BackendHealthManager getBackendHealthManager() { Objects.requireNonNull(parent); return parent.getBackendHealthManager(); } diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/mapper/RandomBackendSelector.java b/carapace-server/src/main/java/org/carapaceproxy/server/mapper/RandomBackendSelector.java index 6dfbfb997..52d6bd6c6 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/mapper/RandomBackendSelector.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/mapper/RandomBackendSelector.java @@ -6,7 +6,10 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.SequencedCollection; +import java.util.function.Function; import java.util.random.RandomGenerator; +import java.util.stream.Collectors; import org.carapaceproxy.server.config.BackendSelector; import org.carapaceproxy.server.config.DirectorConfiguration; import org.slf4j.Logger; @@ -19,14 +22,21 @@ * @see Collections#shuffle(List, RandomGenerator) * @see SecureRandom */ -class RandomBackendSelector implements BackendSelector { +public class RandomBackendSelector implements BackendSelector { private static final Logger LOG = LoggerFactory.getLogger(RandomBackendSelector.class); private static final RandomGenerator RANDOM = new SecureRandom(); - private final List allBackendIds; + private final SequencedCollection allBackendIds; private final Map directors; - public RandomBackendSelector(final List allBackendIds, final Map directors) { + public static BackendSelector forMapper(final EndpointMapper mapper) { + final var directors = mapper.getDirectors() + .stream() + .collect(Collectors.toUnmodifiableMap(DirectorConfiguration::getId, Function.identity())); + return new RandomBackendSelector(mapper.getBackends().sequencedKeySet(), directors); + } + + private RandomBackendSelector(final SequencedCollection allBackendIds, final Map directors) { this.allBackendIds = allBackendIds; this.directors = directors; } @@ -44,7 +54,7 @@ public List selectBackends(final String userId, final String sessionId, return shuffleCopy(directorConfig.getBackends()); } - public List shuffleCopy(final List ids) { + public List shuffleCopy(final SequencedCollection ids) { if (ids.isEmpty()) { return List.of(); } diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java b/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java index b69d921b1..c182d039e 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java @@ -94,12 +94,8 @@ public class StandardEndpointMapper extends EndpointMapper { private String debuggingHeaderName = DEBUGGING_HEADER_DEFAULT_NAME; private boolean debuggingHeaderEnabled = false; - public StandardEndpointMapper(final BackendSelector backendSelector) { - this.backendSelector = backendSelector; - } - - public StandardEndpointMapper() { - this.backendSelector = new RandomBackendSelector(allBackendIds, directors); + public StandardEndpointMapper(final BackendSelector.SelectorFactory backendSelector) { + this.backendSelector = backendSelector.apply(this); } @Override @@ -219,10 +215,15 @@ public MapResult map(final ProxyRequest request) { case DOWN: continue; case COLD: - final int capacity = backend.coldCapacity(); - if (capacity > 0 && backendStatus.getConnections() > capacity) { - // can't use this - continue; + final int capacity = backend.safeCapacity(); + if (backendHealthManager.exceedsCapacity(backendId)) { + /* + * backends are returned by the mapper sorted + * from the most desirable to the less desirable; + * if the execution reaches this point, + * we should use a cold backend even if over the recommended capacity anyway... + */ + LOG.warn("Backend {} exceeds cold capacity of {}, but will use it anyway", backendId, capacity); } // falls through case STABLE: { @@ -467,10 +468,10 @@ public void configure(ConfigurationStore properties) throws ConfigurationNotVali final String host = properties.getString(prefix + "host", "localhost"); final int port = properties.getInt(prefix + "port", 8086); final String probePath = properties.getString(prefix + "probePath", ""); - final int coldCapacity = properties.getInt(prefix + "coldCapacity", DEFAULT_CAPACITY); - LOG.info("configured backend {} {}:{} enabled={} capacity={}", id, host, port, enabled, coldCapacity); + final int safeCapacity = properties.getInt(prefix + "safeCapacity", DEFAULT_CAPACITY); + LOG.info("configured backend {} {}:{} enabled={} capacity={}", id, host, port, enabled, safeCapacity); if (enabled) { - addBackend(new BackendConfiguration(id, host, port, probePath, coldCapacity)); + addBackend(new BackendConfiguration(id, host, port, probePath, safeCapacity)); } } } diff --git a/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java b/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java index fe4d6bc2e..7a21d981a 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java @@ -33,8 +33,8 @@ import java.util.Properties; import junitparams.JUnitParamsRunner; import junitparams.Parameters; -import org.carapaceproxy.core.EndpointKey; import org.carapaceproxy.configstore.PropertiesConfigurationStore; +import org.carapaceproxy.core.EndpointKey; import org.carapaceproxy.core.HttpProxyServer; import org.carapaceproxy.core.ProxyRequestsManager; import org.carapaceproxy.server.backends.BackendHealthStatus; @@ -43,6 +43,7 @@ import org.carapaceproxy.server.config.DirectorConfiguration; import org.carapaceproxy.server.config.NetworkListenerConfiguration; import org.carapaceproxy.server.config.RouteConfiguration; +import org.carapaceproxy.server.config.SafeBackendSelector; import org.carapaceproxy.server.mapper.StandardEndpointMapper; import org.carapaceproxy.server.mapper.requestmatcher.RegexpRequestMatcher; import org.carapaceproxy.utils.RawHttpClient; @@ -83,7 +84,7 @@ public void testBackendUnreachableOnStuckRequest(boolean backendsUnreachableOnSt final int theport = wireMockRule.port(); EndpointKey key = new EndpointKey("localhost", theport); - StandardEndpointMapper mapper = new StandardEndpointMapper(); + StandardEndpointMapper mapper = new StandardEndpointMapper(SafeBackendSelector::forMapper); mapper.addBackend(new BackendConfiguration("backend-a", "localhost", theport, "/", -1)); mapper.addDirector(new DirectorConfiguration("director-1").addBackend("backend-a")); mapper.addAction(new ActionConfiguration("proxy-1", ActionConfiguration.TYPE_PROXY, "director-1", null, -1)); diff --git a/carapace-server/src/test/java/org/carapaceproxy/server/mapper/BasicStandardEndpointMapperTest.java b/carapace-server/src/test/java/org/carapaceproxy/server/mapper/BasicStandardEndpointMapperTest.java index bf1ef0934..f9c3cdbdb 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/server/mapper/BasicStandardEndpointMapperTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/server/mapper/BasicStandardEndpointMapperTest.java @@ -55,6 +55,7 @@ import org.carapaceproxy.server.config.BackendConfiguration; import org.carapaceproxy.server.config.DirectorConfiguration; import org.carapaceproxy.server.config.RouteConfiguration; +import org.carapaceproxy.server.config.SafeBackendSelector; import org.carapaceproxy.server.mapper.requestmatcher.RegexpRequestMatcher; import org.junit.Rule; import org.junit.Test; @@ -93,7 +94,7 @@ public void test() throws Exception { .withBody("it works !!"))); int backendPort = backend.port(); - StandardEndpointMapper mapper = new StandardEndpointMapper(); + StandardEndpointMapper mapper = new StandardEndpointMapper(SafeBackendSelector::forMapper); mapper.addBackend(new BackendConfiguration("backend-a", "localhost", backendPort, "/", -1)); mapper.addBackend(new BackendConfiguration("backend-b", "localhost", backendPort, "/", -1)); @@ -316,7 +317,7 @@ public void testDefaultRoute() throws Exception { int backendPort = backend.port(); - StandardEndpointMapper mapper = new StandardEndpointMapper(); + StandardEndpointMapper mapper = new StandardEndpointMapper(SafeBackendSelector::forMapper); mapper.addBackend(new BackendConfiguration("backend", "localhost", backendPort, "/", -1)); mapper.addBackend(new BackendConfiguration("backend-down", "localhost-down", backendPort, "/", -1)); mapper.addDirector(new DirectorConfiguration("director").addBackend("backend")); diff --git a/carapace-server/src/test/java/org/carapaceproxy/server/mapper/ForceBackendTest.java b/carapace-server/src/test/java/org/carapaceproxy/server/mapper/ForceBackendTest.java index bb3793a6d..bcd9ae301 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/server/mapper/ForceBackendTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/server/mapper/ForceBackendTest.java @@ -37,6 +37,7 @@ import org.carapaceproxy.server.config.BackendConfiguration; import org.carapaceproxy.server.config.DirectorConfiguration; import org.carapaceproxy.server.config.RouteConfiguration; +import org.carapaceproxy.server.config.SafeBackendSelector; import org.carapaceproxy.server.mapper.requestmatcher.RegexpRequestMatcher; import org.junit.Rule; import org.junit.Test; @@ -69,7 +70,7 @@ public void test() throws Exception { .withBody("it works !!"))); int backendPort = backend1.port(); - StandardEndpointMapper mapper = new StandardEndpointMapper(); + StandardEndpointMapper mapper = new StandardEndpointMapper(SafeBackendSelector::forMapper); Properties properties = new Properties(); properties.put("mapper.forcedirector.parameter", "thedirector"); properties.put("mapper.forcebackend.parameter", "thebackend"); From d27050837238526b1f89135092376ebe8b23d25b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Maltoni?= Date: Thu, 21 Nov 2024 15:44:31 +0100 Subject: [PATCH 11/15] feat: invert HttpProxyServer dependency for mapper  Conflicts:  carapace-server/src/test/java/org/carapaceproxy/listeners/ListenerConfigurationTest.java --- .../carapaceproxy/api/DirectorsResource.java | 7 +- .../carapaceproxy/core/HttpProxyServer.java | 98 +++++++++---------- .../carapaceproxy/launcher/ServerMain.java | 4 +- .../server/backends/BackendHealthManager.java | 6 +- .../server/config/BackendSelector.java | 8 +- .../server/config/SafeBackendSelector.java | 27 ++--- .../server/mapper/EndpointMapper.java | 23 +++-- .../server/mapper/RandomBackendSelector.java | 9 -- .../server/mapper/StandardEndpointMapper.java | 31 ++++-- .../carapaceproxy/ApplyConfigurationTest.java | 12 +-- .../DatabaseConfigurationTest.java | 9 +- .../org/carapaceproxy/api/UseAdminServer.java | 2 +- .../backends/StuckRequestsTest.java | 37 +++++-- .../backends/UnreachableBackendTest.java | 8 +- .../listeners/ListenerConfigurationTest.java | 3 +- .../BasicStandardEndpointMapperTest.java | 86 ++++++++-------- .../server/mapper/ForceBackendTest.java | 32 +++--- .../users/FileUserRealmTest.java | 6 +- .../utils/TestEndpointMapper.java | 58 ++++++----- 19 files changed, 249 insertions(+), 217 deletions(-) diff --git a/carapace-server/src/main/java/org/carapaceproxy/api/DirectorsResource.java b/carapace-server/src/main/java/org/carapaceproxy/api/DirectorsResource.java index 999293b94..25eb06387 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/api/DirectorsResource.java +++ b/carapace-server/src/main/java/org/carapaceproxy/api/DirectorsResource.java @@ -59,11 +59,10 @@ public List getBackends() { @GET public List getAll() { - final List directors = new ArrayList(); + final List directors = new ArrayList<>(); HttpProxyServer server = (HttpProxyServer) context.getAttribute("server"); - server.getMapper().getDirectors().forEach(director -> { - directors.add(new DirectorBean(director.getId(), director.getBackends())); - }); + server.getMapper().getDirectors() + .forEach((directorId, director) -> directors.add(new DirectorBean(directorId, director.getBackends()))); return directors; } diff --git a/carapace-server/src/main/java/org/carapaceproxy/core/HttpProxyServer.java b/carapace-server/src/main/java/org/carapaceproxy/core/HttpProxyServer.java index 52e54557e..d8237031d 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/core/HttpProxyServer.java +++ b/carapace-server/src/main/java/org/carapaceproxy/core/HttpProxyServer.java @@ -203,7 +203,7 @@ public class HttpProxyServer implements AutoCloseable { */ private final ReentrantLock configurationLock = new ReentrantLock(); - private Server adminserver; + private Server adminServer; private String adminAccessLogPath = "admin.access.log"; private String adminAccessLogTimezone = "GMT"; private int adminLogRetentionDays = 90; @@ -226,31 +226,35 @@ public class HttpProxyServer implements AutoCloseable { @Getter private int listenersOffsetPort = 0; - public static HttpProxyServer buildForTests(String host, int port, EndpointMapper mapper, File baseDir) throws ConfigurationNotValidException { - HttpProxyServer res = new HttpProxyServer(mapper, baseDir.getAbsoluteFile()); - res.currentConfiguration.addListener(new NetworkListenerConfiguration(host, port)); - res.proxyRequestsManager.reloadConfiguration(res.currentConfiguration, mapper.getBackends().values()); - - return res; - } - - public HttpProxyServer(EndpointMapper mapper, File basePath) { + @VisibleForTesting + public static HttpProxyServer buildForTests( + final String host, + final int port, + final EndpointMapper.Factory mapperFactory, + final File baseDir + ) throws ConfigurationNotValidException { + final HttpProxyServer server = new HttpProxyServer(mapperFactory, baseDir.getAbsoluteFile()); + final EndpointMapper mapper = server.getMapper(); + server.currentConfiguration.addListener(new NetworkListenerConfiguration(host, port)); + server.proxyRequestsManager.reloadConfiguration(server.currentConfiguration, mapper.getBackends().values()); + return server; + } + + public HttpProxyServer(final EndpointMapper.Factory mapperFactory, File basePath) throws ConfigurationNotValidException { // metrics - statsProvider = new PrometheusMetricsProvider(); - mainLogger = statsProvider.getStatsLogger(""); - prometheusRegistry = new PrometheusMeterRegistry(PrometheusConfig.DEFAULT); - prometheusRegistry.config().meterFilter(new PrometheusRenameFilter()); - Metrics.globalRegistry.add(prometheusRegistry); + this.statsProvider = new PrometheusMetricsProvider(); + this.mainLogger = this.statsProvider.getStatsLogger(""); + this.prometheusRegistry = new PrometheusMeterRegistry(PrometheusConfig.DEFAULT); + this.prometheusRegistry.config().meterFilter(new PrometheusRenameFilter()); + Metrics.globalRegistry.add(this.prometheusRegistry); Metrics.globalRegistry.config() .meterFilter(MeterFilter.denyNameStartsWith(("reactor.netty.http.server.data"))) // spam .meterFilter(MeterFilter.denyNameStartsWith(("reactor.netty.http.server.response"))) // spam .meterFilter(MeterFilter.denyNameStartsWith(("reactor.netty.http.server.errors"))); // spam - this.mapper = mapper; this.basePath = basePath; this.filters = new ArrayList<>(); this.currentConfiguration = new RuntimeServerConfiguration(); - this.backendHealthManager = new BackendHealthManager(currentConfiguration, mapper); this.listeners = new Listeners(this); this.cache = new ContentsCache(currentConfiguration); this.requestsLogger = new RequestsLogger(currentConfiguration); @@ -258,16 +262,16 @@ public HttpProxyServer(EndpointMapper mapper, File basePath) { this.trustStoreManager = new TrustStoreManager(currentConfiguration, this); this.ocspStaplingManager = new OcspStaplingManager(trustStoreManager); this.proxyRequestsManager = new ProxyRequestsManager(this); - if (mapper != null) { - mapper.setParent(this); - this.proxyRequestsManager.reloadConfiguration(currentConfiguration, mapper.getBackends().values()); - } + this.mapper = mapperFactory.build(this); + this.backendHealthManager = new BackendHealthManager(currentConfiguration, this.mapper); + this.proxyRequestsManager.reloadConfiguration(currentConfiguration, this.mapper.getBackends().values()); this.usePooledByteBufAllocator = Boolean.getBoolean("cache.allocator.usepooledbytebufallocator"); this.cachePoolAllocator = usePooledByteBufAllocator - ? new PooledByteBufAllocator(true) : new UnpooledByteBufAllocator(true); + ? new PooledByteBufAllocator(true) + : new UnpooledByteBufAllocator(true); this.cacheByteBufMemoryUsageMetric = new CacheByteBufMemoryUsageMetric(this); - //Best practice is to reuse EventLoopGroup + // Best practice is to reuse EventLoopGroup // http://normanmaurer.me/presentations/2014-facebook-eng-netty/slides.html#25.0 this.eventLoopGroup = Epoll.isAvailable() ? new EpollEventLoopGroup() : new NioEventLoopGroup(); } @@ -283,13 +287,6 @@ public int getLocalPort() { return listeners.getLocalPort(); } - @VisibleForTesting - public void setMapper(EndpointMapper mapper) { - Objects.requireNonNull(mapper); - mapper.setParent(this); - this.mapper = mapper; - } - public void startAdminInterface() throws Exception { if (!adminServerEnabled) { return; @@ -299,17 +296,17 @@ public void startAdminInterface() throws Exception { throw new RuntimeException("To enable admin interface at least one between http and https port must be set"); } - adminserver = new Server(); + adminServer = new Server(); ServerConnector httpConnector = null; if (adminServerHttpPort >= 0) { LOG.info("Starting Admin UI over HTTP"); - httpConnector = new ServerConnector(adminserver); + httpConnector = new ServerConnector(adminServer); httpConnector.setPort(adminServerHttpPort); httpConnector.setHost(adminServerHost); - adminserver.addConnector(httpConnector); + adminServer.addConnector(httpConnector); } ServerConnector httpsConnector = null; @@ -333,17 +330,17 @@ public void startAdminInterface() throws Exception { https.setSecurePort(adminServerHttpsPort); https.addCustomizer(new SecureRequestCustomizer()); - httpsConnector = new ServerConnector(adminserver, + httpsConnector = new ServerConnector(adminServer, new SslConnectionFactory(sslContextFactory, "http/1.1"), new HttpConnectionFactory(https)); httpsConnector.setPort(adminServerHttpsPort); httpsConnector.setHost(adminServerHost); - adminserver.addConnector(httpsConnector); + adminServer.addConnector(httpsConnector); } ContextHandlerCollection contexts = new ContextHandlerCollection(); - adminserver.setHandler(constrainTraceMethod(contexts)); + adminServer.setHandler(constrainTraceMethod(contexts)); File webUi = new File(basePath, "web/ui"); if (webUi.isDirectory()) { @@ -379,7 +376,7 @@ public void startAdminInterface() throws Exception { contexts.addHandler(requestLogHandler); - adminserver.start(); + adminServer.start(); LOG.info("Admin UI started"); @@ -472,13 +469,13 @@ public void close() { ocspStaplingManager.stop(); cacheByteBufMemoryUsageMetric.stop(); - if (adminserver != null) { + if (adminServer != null) { try { - adminserver.stop(); + adminServer.stop(); } catch (Exception err) { LOG.error("Error while stopping admin server", err); } finally { - adminserver = null; + adminServer = null; } } statsProvider.stop(); @@ -496,21 +493,20 @@ public void close() { staticContentsManager.close(); if (dynamicConfigurationStore != null) { - // this will also shutdown embedded database + // this will also shut down embedded database dynamicConfigurationStore.close(); } } - private static EndpointMapper buildMapper(String className, ConfigurationStore properties) throws ConfigurationNotValidException { + private static EndpointMapper buildMapper(final String className, final HttpProxyServer parent, final ConfigurationStore properties) throws ConfigurationNotValidException { try { - EndpointMapper res = (EndpointMapper) Class.forName(className).getConstructor().newInstance(); + final Class mapperClass = Class.forName(className).asSubclass(EndpointMapper.class); + final EndpointMapper res = mapperClass.getConstructor(HttpProxyServer.class).newInstance(parent); res.configure(properties); return res; - } catch (ClassNotFoundException err) { + } catch (final ClassNotFoundException | ClassCastException | NoSuchMethodException err) { throw new ConfigurationNotValidException(err); - } catch (IllegalAccessException | IllegalArgumentException - | InstantiationException | NoSuchMethodException - | SecurityException | InvocationTargetException err) { + } catch (final IllegalAccessException | IllegalArgumentException | InstantiationException | SecurityException | InvocationTargetException err) { throw new RuntimeException(err); } } @@ -654,7 +650,7 @@ public RuntimeServerConfiguration buildValidConfiguration(ConfigurationStore sim // Try to perform a service configuration from the passed store. newConfiguration.configure(simpleStore); - buildMapper(newConfiguration.getMapperClassname(), simpleStore); + buildMapper(newConfiguration.getMapperClassname(), this, simpleStore); buildRealm(userRealmClassname, simpleStore); return newConfiguration; @@ -747,8 +743,7 @@ private void applyDynamicConfiguration(ConfigurationStore newConfigurationStore, } try { RuntimeServerConfiguration newConfiguration = buildValidConfiguration(storeWithConfig); - EndpointMapper newMapper = buildMapper(newConfiguration.getMapperClassname(), storeWithConfig); - newMapper.setParent(this); + EndpointMapper newMapper = buildMapper(newConfiguration.getMapperClassname(), this, storeWithConfig); UserRealm newRealm = buildRealm(userRealmClassname, storeWithConfig); this.filters = buildFilters(newConfiguration); @@ -876,7 +871,8 @@ public Map> getConnectionPoolsStat if (!metric.getName().startsWith(CONNECTION_PROVIDER_PREFIX)) { return; } - EndpointKey key = EndpointKey.make(metric.getTag(REMOTE_ADDRESS)); + final String remoteAddress = Objects.requireNonNull(metric.getTag(REMOTE_ADDRESS)); + EndpointKey key = EndpointKey.make(remoteAddress); Map pools = res.computeIfAbsent(key, k -> new HashMap<>()); String poolName = metric.getTag(NAME); ConnectionPoolStats stats = pools.computeIfAbsent(poolName, k -> new ConnectionPoolStats()); diff --git a/carapace-server/src/main/java/org/carapaceproxy/launcher/ServerMain.java b/carapace-server/src/main/java/org/carapaceproxy/launcher/ServerMain.java index 1d8db4f35..1be84f2af 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/launcher/ServerMain.java +++ b/carapace-server/src/main/java/org/carapaceproxy/launcher/ServerMain.java @@ -35,6 +35,7 @@ import org.carapaceproxy.configstore.PropertiesConfigurationStore; import org.carapaceproxy.core.HttpProxyServer; import org.carapaceproxy.server.config.ConfigurationNotValidException; +import org.carapaceproxy.server.mapper.StandardEndpointMapper; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -174,8 +175,7 @@ public void join() { */ public void start() throws Exception { pidFileLocker.lock(); - - server = new HttpProxyServer(null, basePath); + server = new HttpProxyServer(StandardEndpointMapper::new, basePath); server.configureAtBoot(configuration); server.start(); server.startMetrics(); diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java b/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java index 1cfbb5539..296232a28 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/backends/BackendHealthManager.java @@ -74,6 +74,10 @@ public BackendHealthManager(final RuntimeServerConfiguration conf, final Endpoin this.period = DEFAULT_PERIOD; } + public boolean isTolerant() { + return tolerant; + } + public int getPeriod() { return period; } @@ -217,7 +221,7 @@ public boolean exceedsCapacity(final String backendId) { return false; } final BackendHealthStatus backendStatus = getBackendStatus(backendConfiguration.hostPort()); - return backendConfiguration.safeCapacity() > backendStatus.getConnections() && !this.tolerant; + return backendConfiguration.safeCapacity() > backendStatus.getConnections(); } @VisibleForTesting diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/config/BackendSelector.java b/carapace-server/src/main/java/org/carapaceproxy/server/config/BackendSelector.java index 2fae705fe..91248a1c1 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/config/BackendSelector.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/config/BackendSelector.java @@ -20,7 +20,6 @@ package org.carapaceproxy.server.config; import java.util.List; -import java.util.function.Function; import org.carapaceproxy.server.mapper.EndpointMapper; /** @@ -32,13 +31,10 @@ public interface BackendSelector { /** * Functional interface that models a factory for selectors given the mapper they should be applied to. - * - * @see SafeBackendSelector#forMapper(EndpointMapper) */ @FunctionalInterface - interface SelectorFactory extends Function { + interface SelectorFactory { - @Override - BackendSelector apply(EndpointMapper endpointMapper); + BackendSelector build(EndpointMapper endpointMapper); } } diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/config/SafeBackendSelector.java b/carapace-server/src/main/java/org/carapaceproxy/server/config/SafeBackendSelector.java index 7fbd4bae0..362a6a4a0 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/config/SafeBackendSelector.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/config/SafeBackendSelector.java @@ -5,9 +5,6 @@ import java.util.List; import java.util.Map; import java.util.SequencedCollection; -import java.util.function.Function; -import java.util.stream.Collectors; -import org.carapaceproxy.server.backends.BackendHealthManager; import org.carapaceproxy.server.backends.BackendHealthStatus; import org.carapaceproxy.server.mapper.EndpointMapper; import org.slf4j.Logger; @@ -15,32 +12,22 @@ public class SafeBackendSelector implements BackendSelector { private static final Logger LOGGER = LoggerFactory.getLogger(SafeBackendSelector.class); - private final SequencedCollection backends; - private final Map directors; - private final BackendHealthManager healthManager; + private final EndpointMapper mapper; - public static BackendSelector forMapper(final EndpointMapper mapper) { - final var directors = mapper.getDirectors() - .stream() - .collect(Collectors.toUnmodifiableMap(DirectorConfiguration::getId, Function.identity())); - return new SafeBackendSelector(mapper.getBackends().sequencedKeySet(), directors, mapper.getBackendHealthManager()); - } - - public SafeBackendSelector(final SequencedCollection allBackendIds, final Map directors, final BackendHealthManager healthManager) { - this.backends = allBackendIds; - this.directors = directors; - this.healthManager = healthManager; + public SafeBackendSelector(final EndpointMapper mapper) { + this.mapper = mapper; } @Override public List selectBackends(final String userId, final String sessionId, final String director) { + final Map directors = mapper.getDirectors(); if (!directors.containsKey(director)) { LOGGER.error("Director \"{}\" not configured, while handling request userId={} sessionId={}", director, userId, sessionId); return List.of(); } final DirectorConfiguration directorConfig = directors.get(director); if (directorConfig.getBackends().contains(ALL_BACKENDS)) { - return sortByConnections(backends); + return sortByConnections(mapper.getBackends().sequencedKeySet()); } return sortByConnections(directorConfig.getBackends()); } @@ -50,10 +37,10 @@ public List sortByConnections(final SequencedCollection backendI } private long connections(final String backendId) { - final BackendHealthStatus backendStatus = healthManager.getBackendStatus(backendId); + final BackendHealthStatus backendStatus = mapper.getBackendHealthManager().getBackendStatus(backendId); return switch (backendStatus.getStatus()) { case DOWN -> Long.MAX_VALUE; // backends that are down are put last, but not dropped - case COLD -> healthManager.exceedsCapacity(backendId) + case COLD -> mapper.getBackendHealthManager().exceedsCapacity(backendId) ? Long.MAX_VALUE - 1 // cold backends that exceed safe capacity are put last, just before down ones : backendStatus.getConnections(); case STABLE -> backendStatus.getConnections(); diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/mapper/EndpointMapper.java b/carapace-server/src/main/java/org/carapaceproxy/server/mapper/EndpointMapper.java index c610ea3cb..c5e597fa5 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/mapper/EndpointMapper.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/mapper/EndpointMapper.java @@ -44,7 +44,11 @@ */ public abstract class EndpointMapper { - private HttpProxyServer parent; + private final HttpProxyServer parent; + + protected EndpointMapper(final HttpProxyServer parent) { + this.parent = parent; + } /** * Get the pool of {@link BackendConfiguration backends} to choose from. @@ -71,9 +75,9 @@ public abstract class EndpointMapper { /** * Get all the configured {@link DirectorConfiguration directors} for the {@link BackendConfiguration backends}. * - * @return a list of directors, sorted according to configuration order + * @return a map of directors, sorted according to configuration order */ - public abstract List getDirectors(); + public abstract SequencedMap getDirectors(); /** * Get all the {@link CustomHeader custom headers} that can be applied to the requests. @@ -86,7 +90,8 @@ public abstract class EndpointMapper { * Process a request for a {@link ProxyRequestsManager}. *

* According to the incoming request and the underlying configuration, - * it computes an {@link MapResult#getAction() action} to execute on a specific {@link MapResult#routeId route}. + * it computes an {@link MapResult#getAction() action} + * to execute on a specific {@link MapResult#getRouteId() route}. * * @param request the request of a resource to be proxied * @return the result of the mapping process @@ -116,10 +121,6 @@ public SimpleHTTPResponse mapBadRequest() { public abstract void configure(ConfigurationStore properties) throws ConfigurationNotValidException; - public final void setParent(HttpProxyServer parent) { - this.parent = parent; - } - protected final DynamicCertificatesManager getDynamicCertificatesManager() { Objects.requireNonNull(parent); return parent.getDynamicCertificatesManager(); @@ -134,4 +135,10 @@ protected final RuntimeServerConfiguration getCurrentConfiguration() { Objects.requireNonNull(parent); return parent.getCurrentConfiguration(); } + + @FunctionalInterface + public interface Factory { + + EndpointMapper build(HttpProxyServer httpProxyServer) throws ConfigurationNotValidException; + } } diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/mapper/RandomBackendSelector.java b/carapace-server/src/main/java/org/carapaceproxy/server/mapper/RandomBackendSelector.java index 52d6bd6c6..7c7a86e30 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/mapper/RandomBackendSelector.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/mapper/RandomBackendSelector.java @@ -7,9 +7,7 @@ import java.util.List; import java.util.Map; import java.util.SequencedCollection; -import java.util.function.Function; import java.util.random.RandomGenerator; -import java.util.stream.Collectors; import org.carapaceproxy.server.config.BackendSelector; import org.carapaceproxy.server.config.DirectorConfiguration; import org.slf4j.Logger; @@ -29,13 +27,6 @@ public class RandomBackendSelector implements BackendSelector { private final SequencedCollection allBackendIds; private final Map directors; - public static BackendSelector forMapper(final EndpointMapper mapper) { - final var directors = mapper.getDirectors() - .stream() - .collect(Collectors.toUnmodifiableMap(DirectorConfiguration::getId, Function.identity())); - return new RandomBackendSelector(mapper.getBackends().sequencedKeySet(), directors); - } - private RandomBackendSelector(final SequencedCollection allBackendIds, final Map directors) { this.allBackendIds = allBackendIds; this.directors = directors; diff --git a/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java b/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java index c182d039e..3da586a3e 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java +++ b/carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java @@ -36,6 +36,7 @@ import java.util.Set; import org.carapaceproxy.SimpleHTTPResponse; import org.carapaceproxy.configstore.ConfigurationStore; +import org.carapaceproxy.core.HttpProxyServer; import org.carapaceproxy.core.ProxyRequest; import org.carapaceproxy.server.backends.BackendHealthManager; import org.carapaceproxy.server.backends.BackendHealthStatus; @@ -45,6 +46,7 @@ import org.carapaceproxy.server.config.ConfigurationNotValidException; import org.carapaceproxy.server.config.DirectorConfiguration; import org.carapaceproxy.server.config.RouteConfiguration; +import org.carapaceproxy.server.config.SafeBackendSelector; import org.carapaceproxy.server.filters.UrlEncodedQueryString; import org.carapaceproxy.server.mapper.CustomHeader.HeaderMode; import org.carapaceproxy.server.mapper.MapResult.Action; @@ -75,8 +77,7 @@ public class StandardEndpointMapper extends EndpointMapper { // The map is wiped out whenever a new configuration is applied private final SequencedMap backends = new LinkedHashMap<>(); - private final Map directors = new HashMap<>(); - private final List allBackendIds = new ArrayList<>(); + private final SequencedMap directors = new LinkedHashMap<>(); private final List routes = new ArrayList<>(); private final Map actions = new HashMap<>(); public final Map headers = new HashMap<>(); @@ -94,8 +95,13 @@ public class StandardEndpointMapper extends EndpointMapper { private String debuggingHeaderName = DEBUGGING_HEADER_DEFAULT_NAME; private boolean debuggingHeaderEnabled = false; - public StandardEndpointMapper(final BackendSelector.SelectorFactory backendSelector) { - this.backendSelector = backendSelector.apply(this); + public StandardEndpointMapper(final HttpProxyServer parent) { + this(parent, SafeBackendSelector::new); + } + + public StandardEndpointMapper(final HttpProxyServer parent, final BackendSelector.SelectorFactory backendSelector) { + super(parent); + this.backendSelector = backendSelector.build(this); } @Override @@ -213,17 +219,23 @@ public MapResult map(final ProxyRequest request) { final BackendHealthStatus backendStatus = backendHealthManager.getBackendStatus(backend.hostPort()); switch (backendStatus.getStatus()) { case DOWN: + LOG.info("Backend {} is down, skipping...", backendId); continue; case COLD: - final int capacity = backend.safeCapacity(); if (backendHealthManager.exceedsCapacity(backendId)) { + final int capacity = backend.safeCapacity(); + if (!backendHealthManager.isTolerant()) { + // default behavior, exceeding safe capacity is not tolerated... + LOG.info("Backend {} is cold and exceeds safe capacity of {} connections, skipping...", backendId, capacity); + continue; + } /* * backends are returned by the mapper sorted * from the most desirable to the less desirable; * if the execution reaches this point, - * we should use a cold backend even if over the recommended capacity anyway... + * we may use the cold backend even if over the recommended capacity anyway... */ - LOG.warn("Backend {} exceeds cold capacity of {}, but will use it anyway", backendId, capacity); + LOG.warn("Cold backend {} exceeds safe capacity of {} connections, but will use it anyway", backendId, capacity); } // falls through case STABLE: { @@ -567,7 +579,6 @@ public void addBackend(BackendConfiguration backend) throws ConfigurationNotVali if (backends.put(backend.id(), backend) != null) { throw new ConfigurationNotValidException("backend " + backend.id() + " is already configured"); } - allBackendIds.add(backend.id()); } public void addAction(ActionConfiguration action) throws ConfigurationNotValidException { @@ -599,8 +610,8 @@ public List getActions() { } @Override - public List getDirectors() { - return new ArrayList<>(directors.values()); + public SequencedMap getDirectors() { + return Collections.unmodifiableSequencedMap(directors); } @Override diff --git a/carapace-server/src/test/java/org/carapaceproxy/ApplyConfigurationTest.java b/carapace-server/src/test/java/org/carapaceproxy/ApplyConfigurationTest.java index a9cd7a899..40f9af3ae 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/ApplyConfigurationTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/ApplyConfigurationTest.java @@ -93,7 +93,7 @@ public StaticEndpointMapper() { @Test public void testChangeListenersConfig() throws Exception { - try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder())) { + try (HttpProxyServer server = new HttpProxyServer(StandardEndpointMapper::new, tmpDir.newFolder());) { { Properties configuration = new Properties(); @@ -220,7 +220,7 @@ public void testChangeListenersConfig() throws Exception { @Test public void testReloadMapper() throws Exception { - try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder())) { + try (HttpProxyServer server = new HttpProxyServer(StandardEndpointMapper::new, tmpDir.newFolder());) { { Properties configuration = new Properties(); @@ -292,7 +292,7 @@ public void testReloadMapper() throws Exception { public void testUserRealm() throws Exception { // Default UserRealm - try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder())) { + try (HttpProxyServer server = new HttpProxyServer(StandardEndpointMapper::new, tmpDir.newFolder())) { Properties configuration = new Properties(); server.configureAtBoot(new PropertiesConfigurationStore(configuration)); server.start(); @@ -310,7 +310,7 @@ public void testUserRealm() throws Exception { } // TestUserRealm - try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder())) { + try (HttpProxyServer server = new HttpProxyServer(StandardEndpointMapper::new, tmpDir.newFolder())) { Properties configuration = new Properties(); configuration.put("userrealm.class", "org.carapaceproxy.utils.TestUserRealm"); configuration.put("user.test1", "pass1"); @@ -339,7 +339,7 @@ public void testUserRealm() throws Exception { @Test public void testChangeFiltersConfiguration() throws Exception { - try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder())) { + try (HttpProxyServer server = new HttpProxyServer(StandardEndpointMapper::new, tmpDir.newFolder());) { { Properties configuration = new Properties(); @@ -378,7 +378,7 @@ public void testChangeFiltersConfiguration() throws Exception { @Test public void testChangeBackendHealthManagerConfiguration() throws Exception { - try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder())) { + try (HttpProxyServer server = new HttpProxyServer(StandardEndpointMapper::new, tmpDir.newFolder());) { { Properties configuration = new Properties(); diff --git a/carapace-server/src/test/java/org/carapaceproxy/DatabaseConfigurationTest.java b/carapace-server/src/test/java/org/carapaceproxy/DatabaseConfigurationTest.java index 492904bf7..e15733826 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/DatabaseConfigurationTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/DatabaseConfigurationTest.java @@ -31,6 +31,7 @@ import org.carapaceproxy.core.HttpProxyServer; import org.carapaceproxy.server.filters.RegexpMapUserIdFilter; import org.carapaceproxy.server.filters.XForwardedForRequestFilter; +import org.carapaceproxy.server.mapper.StandardEndpointMapper; import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; @@ -48,7 +49,7 @@ public class DatabaseConfigurationTest { @Test public void testBootWithDatabaseStore() throws Exception { - try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder())) { + try (HttpProxyServer server = new HttpProxyServer(StandardEndpointMapper::new, tmpDir.newFolder())) { Properties configuration = new Properties(); @@ -70,7 +71,7 @@ public void testBootWithDatabaseStore() throws Exception { public void testChangeFiltersConfiguration() throws Exception { File databaseFolder = tmpDir.newFolder(); - try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder())) { + try (HttpProxyServer server = new HttpProxyServer(StandardEndpointMapper::new, tmpDir.newFolder())) { Properties configurationAtBoot = new Properties(); configurationAtBoot.put("db.jdbc.url", "jdbc:herddb:localhost"); @@ -100,7 +101,7 @@ public void testChangeFiltersConfiguration() throws Exception { } // reboot, new configuration MUST be kept - try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder())) { + try (HttpProxyServer server = new HttpProxyServer(StandardEndpointMapper::new, tmpDir.newFolder())) { Properties configurationAtBoot = new Properties(); configurationAtBoot.put("db.jdbc.url", "jdbc:herddb:localhost"); configurationAtBoot.put("db.server.base.dir", tmpDir.newFolder().getAbsolutePath()); @@ -119,7 +120,7 @@ public void testChangeFiltersConfiguration() throws Exception { } // reboot, new configuration MUST be kept - try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder())) { + try (HttpProxyServer server = new HttpProxyServer(StandardEndpointMapper::new, tmpDir.newFolder())) { Properties configurationAtBoot = new Properties(); configurationAtBoot.put("db.jdbc.url", "jdbc:herddb:localhost"); configurationAtBoot.put("db.server.base.dir", tmpDir.newFolder().getAbsolutePath()); diff --git a/carapace-server/src/test/java/org/carapaceproxy/api/UseAdminServer.java b/carapace-server/src/test/java/org/carapaceproxy/api/UseAdminServer.java index 91cc6dd66..36963ddcc 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/api/UseAdminServer.java +++ b/carapace-server/src/test/java/org/carapaceproxy/api/UseAdminServer.java @@ -62,7 +62,7 @@ public void buildNewServer() throws Exception { assertNull(server); credentials = new RawHttpClient.BasicAuthCredentials(DEFAULT_USERNAME, DEFAULT_PASSWORD); File serverRoot = tmpDir.getRoot(); // at every reboot we must keep the same directory - server = buildForTests("localhost", 0, new TestEndpointMapper("localhost", 0), serverRoot); + server = buildForTests("localhost", 0, parent -> new TestEndpointMapper("localhost", 0), serverRoot); } @After diff --git a/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java b/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java index 7a21d981a..9c16d1b23 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java @@ -44,6 +44,7 @@ import org.carapaceproxy.server.config.NetworkListenerConfiguration; import org.carapaceproxy.server.config.RouteConfiguration; import org.carapaceproxy.server.config.SafeBackendSelector; +import org.carapaceproxy.server.mapper.EndpointMapper; import org.carapaceproxy.server.mapper.StandardEndpointMapper; import org.carapaceproxy.server.mapper.requestmatcher.RegexpRequestMatcher; import org.carapaceproxy.utils.RawHttpClient; @@ -63,6 +64,7 @@ public class StuckRequestsTest { @Test @Parameters({"true", "false"}) + @junitparams.naming.TestCaseName("test(backend unreachable on stuck request: {0})") public void testBackendUnreachableOnStuckRequest(boolean backendsUnreachableOnStuckRequests) throws Exception { stubFor(get(urlEqualTo("/index.html")) @@ -84,20 +86,39 @@ public void testBackendUnreachableOnStuckRequest(boolean backendsUnreachableOnSt final int theport = wireMockRule.port(); EndpointKey key = new EndpointKey("localhost", theport); - StandardEndpointMapper mapper = new StandardEndpointMapper(SafeBackendSelector::forMapper); - mapper.addBackend(new BackendConfiguration("backend-a", "localhost", theport, "/", -1)); - mapper.addDirector(new DirectorConfiguration("director-1").addBackend("backend-a")); - mapper.addAction(new ActionConfiguration("proxy-1", ActionConfiguration.TYPE_PROXY, "director-1", null, -1)); - mapper.addRoute(new RouteConfiguration("route-1", "proxy-1", true, new RegexpRequestMatcher(PROPERTY_URI, ".*index.html.*"))); - - try (HttpProxyServer server = new HttpProxyServer(mapper, tmpDir.newFolder());) { + final EndpointMapper.Factory mapperFactory = parent -> { + StandardEndpointMapper mapper = new StandardEndpointMapper(parent, SafeBackendSelector::new); + mapper.addBackend(new BackendConfiguration("backend-a", "localhost", theport, "/", -1)); + mapper.addDirector(new DirectorConfiguration("director-1").addBackend("backend-a")); + mapper.addAction(new ActionConfiguration("proxy-1", ActionConfiguration.TYPE_PROXY, "director-1", null, -1)); + mapper.addRoute(new RouteConfiguration("route-1", "proxy-1", true, new RegexpRequestMatcher(PROPERTY_URI, ".*index.html.*"))); + return mapper; + }; + try (HttpProxyServer server = new HttpProxyServer(mapperFactory, tmpDir.newFolder())) { Properties properties = new Properties(); + properties.put("healthmanager.tolerant", "true"); + properties.put("backend.1.id", "backend-a"); + properties.put("backend.1.enabled", "true"); + properties.put("backend.1.host", "localhost"); + properties.put("backend.1.port", theport); + properties.put("backend.1.probePath", "/"); + properties.put("director.1.id", "director-1"); + properties.put("director.1.backends", properties.getProperty("backend.1.id")); + properties.put("director.1.enabled", "true"); + properties.put("action.1.id", "proxy-1"); + properties.put("action.1.enabled", "true"); + properties.put("action.1.type", ActionConfiguration.TYPE_PROXY); + properties.put("action.1.director", properties.getProperty("director.1.id")); + properties.put("route.100.id", "route-1"); + properties.put("route.100.enabled", "true"); + properties.put("route.100.match", "request.uri ~ \".*index.html.*\""); + properties.put("route.100.action", properties.getProperty("action.1.id")); + properties.put("connectionsmanager.stuckrequesttimeout", "100"); // ms properties.put("connectionsmanager.backendsunreachableonstuckrequests", backendsUnreachableOnStuckRequests + ""); // configure resets all listeners configurations server.configureAtBoot(new PropertiesConfigurationStore(properties)); server.addListener(new NetworkListenerConfiguration("localhost", 0)); - server.setMapper(mapper); assertEquals(100, server.getCurrentConfiguration().getStuckRequestTimeout()); assertEquals(backendsUnreachableOnStuckRequests, server.getCurrentConfiguration().isBackendsUnreachableOnStuckRequests()); server.start(); diff --git a/carapace-server/src/test/java/org/carapaceproxy/backends/UnreachableBackendTest.java b/carapace-server/src/test/java/org/carapaceproxy/backends/UnreachableBackendTest.java index 9b20e13d8..ebbaceb66 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/backends/UnreachableBackendTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/backends/UnreachableBackendTest.java @@ -114,12 +114,12 @@ public void testEmptyResponse() throws Exception { TestEndpointMapper mapper = new TestEndpointMapper("localhost", dummyport, useCache); EndpointKey key = new EndpointKey("localhost", dummyport); - try (HttpProxyServer server = new HttpProxyServer(mapper, tmpDir.newFolder());) { + try (HttpProxyServer server = new HttpProxyServer(mapper, tmpDir.newFolder())) { Properties properties = new Properties(); + properties.put("healthmanager.tolerant", "true"); // configure resets all listeners configurations server.configureAtBoot(new PropertiesConfigurationStore(properties)); server.addListener(new NetworkListenerConfiguration("localhost", 0)); - server.setMapper(mapper); server.start(); int port = server.getLocalPort(); assertTrue(port > 0); @@ -183,11 +183,11 @@ public void testNonHttpResponseThenClose() throws Exception { TestEndpointMapper mapper = new TestEndpointMapper("localhost", dummyport, useCache); EndpointKey key = new EndpointKey("localhost", dummyport); - try (HttpProxyServer server = new HttpProxyServer(mapper, tmpDir.newFolder());) { + try (HttpProxyServer server = new HttpProxyServer(mapper, tmpDir.newFolder())) { Properties properties = new Properties(); + properties.put("healthmanager.tolerant", "true"); server.configureAtBoot(new PropertiesConfigurationStore(properties)); server.addListener(new NetworkListenerConfiguration("localhost", 0)); - server.setMapper(mapper); server.start(); int port = server.getLocalPort(); diff --git a/carapace-server/src/test/java/org/carapaceproxy/listeners/ListenerConfigurationTest.java b/carapace-server/src/test/java/org/carapaceproxy/listeners/ListenerConfigurationTest.java index d34c096f5..5a955eb6a 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/listeners/ListenerConfigurationTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/listeners/ListenerConfigurationTest.java @@ -10,6 +10,7 @@ import org.carapaceproxy.core.HttpProxyServer; import org.carapaceproxy.core.Listeners; import org.carapaceproxy.server.config.ConfigurationChangeInProgressException; +import org.carapaceproxy.server.mapper.StandardEndpointMapper; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -21,7 +22,7 @@ public class ListenerConfigurationTest { @Test public void testListenerKeepAliveConfiguration() throws Exception { - try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder());) { + try (HttpProxyServer server = new HttpProxyServer(StandardEndpointMapper::new, tmpDir.newFolder())) { { Properties configuration = new Properties(); diff --git a/carapace-server/src/test/java/org/carapaceproxy/server/mapper/BasicStandardEndpointMapperTest.java b/carapace-server/src/test/java/org/carapaceproxy/server/mapper/BasicStandardEndpointMapperTest.java index f9c3cdbdb..d12988a87 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/server/mapper/BasicStandardEndpointMapperTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/server/mapper/BasicStandardEndpointMapperTest.java @@ -94,28 +94,32 @@ public void test() throws Exception { .withBody("it works !!"))); int backendPort = backend.port(); - StandardEndpointMapper mapper = new StandardEndpointMapper(SafeBackendSelector::forMapper); - - mapper.addBackend(new BackendConfiguration("backend-a", "localhost", backendPort, "/", -1)); - mapper.addBackend(new BackendConfiguration("backend-b", "localhost", backendPort, "/", -1)); - mapper.addDirector(new DirectorConfiguration("director-1").addBackend("backend-a")); - mapper.addDirector(new DirectorConfiguration("director-2").addBackend("backend-b")); - mapper.addDirector(new DirectorConfiguration("director-all").addBackend("*")); // all the known backends - mapper.addAction(new ActionConfiguration("proxy-1", ActionConfiguration.TYPE_PROXY, "director-1", null, -1)); - mapper.addAction(new ActionConfiguration("cache-1", ActionConfiguration.TYPE_CACHE, "director-2", null, -1)); - mapper.addAction(new ActionConfiguration("all-1", ActionConfiguration.TYPE_CACHE, "director-all", null, -1)); - - mapper.addAction(new ActionConfiguration("not-found-custom", ActionConfiguration.TYPE_STATIC, null, StaticContentsManager.DEFAULT_NOT_FOUND, 404)); - mapper.addAction(new ActionConfiguration("error-custom", ActionConfiguration.TYPE_STATIC, null, StaticContentsManager.DEFAULT_INTERNAL_SERVER_ERROR, 500)); - mapper.addAction(new ActionConfiguration("static-custom", ActionConfiguration.TYPE_STATIC, null, CLASSPATH_RESOURCE + "/test-static-page.html", 200)); - - mapper.addRoute(new RouteConfiguration("route-1", "proxy-1", true, new RegexpRequestMatcher(PROPERTY_URI, ".*index.html.*"))); - mapper.addRoute(new RouteConfiguration("route-1b", "cache-1", true, new RegexpRequestMatcher(PROPERTY_URI, ".*index2.html.*"))); - mapper.addRoute(new RouteConfiguration("route-1c", "all-1", true, new RegexpRequestMatcher(PROPERTY_URI, ".*index3.html.*"))); - mapper.addRoute(new RouteConfiguration("route-2-not-found", "not-found-custom", true, new RegexpRequestMatcher(PROPERTY_URI, ".*notfound.html.*"))); - mapper.addRoute(new RouteConfiguration("route-3-error", "error-custom", true, new RegexpRequestMatcher(PROPERTY_URI, ".*error.html.*"))); - mapper.addRoute(new RouteConfiguration("route-4-static", "static-custom", true, new RegexpRequestMatcher(PROPERTY_URI, ".*static.html.*"))); - try (HttpProxyServer server = HttpProxyServer.buildForTests("localhost", 0, mapper, tmpDir.newFolder())) { + final EndpointMapper.Factory mapperFactory = parent -> { + StandardEndpointMapper mapper = new StandardEndpointMapper(parent, SafeBackendSelector::new); + + mapper.addBackend(new BackendConfiguration("backend-a", "localhost", backendPort, "/", -1)); + mapper.addBackend(new BackendConfiguration("backend-b", "localhost", backendPort, "/", -1)); + mapper.addDirector(new DirectorConfiguration("director-1").addBackend("backend-a")); + mapper.addDirector(new DirectorConfiguration("director-2").addBackend("backend-b")); + mapper.addDirector(new DirectorConfiguration("director-all").addBackend("*")); // all the known backends + mapper.addAction(new ActionConfiguration("proxy-1", ActionConfiguration.TYPE_PROXY, "director-1", null, -1)); + mapper.addAction(new ActionConfiguration("cache-1", ActionConfiguration.TYPE_CACHE, "director-2", null, -1)); + mapper.addAction(new ActionConfiguration("all-1", ActionConfiguration.TYPE_CACHE, "director-all", null, -1)); + + mapper.addAction(new ActionConfiguration("not-found-custom", ActionConfiguration.TYPE_STATIC, null, StaticContentsManager.DEFAULT_NOT_FOUND, 404)); + mapper.addAction(new ActionConfiguration("error-custom", ActionConfiguration.TYPE_STATIC, null, StaticContentsManager.DEFAULT_INTERNAL_SERVER_ERROR, 500)); + mapper.addAction(new ActionConfiguration("static-custom", ActionConfiguration.TYPE_STATIC, null, CLASSPATH_RESOURCE + "/test-static-page.html", 200)); + + mapper.addRoute(new RouteConfiguration("route-1", "proxy-1", true, new RegexpRequestMatcher(PROPERTY_URI, ".*index.html.*"))); + mapper.addRoute(new RouteConfiguration("route-1b", "cache-1", true, new RegexpRequestMatcher(PROPERTY_URI, ".*index2.html.*"))); + mapper.addRoute(new RouteConfiguration("route-1c", "all-1", true, new RegexpRequestMatcher(PROPERTY_URI, ".*index3.html.*"))); + mapper.addRoute(new RouteConfiguration("route-2-not-found", "not-found-custom", true, new RegexpRequestMatcher(PROPERTY_URI, ".*notfound.html.*"))); + mapper.addRoute(new RouteConfiguration("route-3-error", "error-custom", true, new RegexpRequestMatcher(PROPERTY_URI, ".*error.html.*"))); + mapper.addRoute(new RouteConfiguration("route-4-static", "static-custom", true, new RegexpRequestMatcher(PROPERTY_URI, ".*static.html.*"))); + + return mapper; + }; + try (HttpProxyServer server = HttpProxyServer.buildForTests("localhost", 0, mapperFactory, tmpDir.newFolder())) { server.start(); int port = server.getLocalPort(); { @@ -173,7 +177,7 @@ public void testRouteErrors() throws Exception { .withHeader("Content-Type", "text/html") .withBody("it works !!"))); - try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder())) { + try (HttpProxyServer server = new HttpProxyServer(StandardEndpointMapper::new, tmpDir.newFolder())) { Properties configuration = new Properties(); configuration.put("listener.1.host", "0.0.0.0"); @@ -317,17 +321,19 @@ public void testDefaultRoute() throws Exception { int backendPort = backend.port(); - StandardEndpointMapper mapper = new StandardEndpointMapper(SafeBackendSelector::forMapper); - mapper.addBackend(new BackendConfiguration("backend", "localhost", backendPort, "/", -1)); - mapper.addBackend(new BackendConfiguration("backend-down", "localhost-down", backendPort, "/", -1)); - mapper.addDirector(new DirectorConfiguration("director").addBackend("backend")); - mapper.addDirector(new DirectorConfiguration("director-down").addBackend("backend-down")); - mapper.addAction(new ActionConfiguration("cache", ActionConfiguration.TYPE_CACHE, "director", null, -1)); - mapper.addAction(new ActionConfiguration("cache-down", ActionConfiguration.TYPE_CACHE, "director-down", null, -1)); - mapper.addRoute(new RouteConfiguration("route", "cache", true, new RegexpRequestMatcher(PROPERTY_URI, ".*index.html.*"))); - mapper.addRoute(new RouteConfiguration("route-down", "cache-down", true, new RegexpRequestMatcher(PROPERTY_URI, ".*down.html.*"))); - mapper.addRoute(new RouteConfiguration("route-default", "cache", true, new RegexpRequestMatcher(PROPERTY_URI, ".*html"))); - + final EndpointMapper.Factory mapperFactory = parent -> { + StandardEndpointMapper mapper = new StandardEndpointMapper(parent, SafeBackendSelector::new); + mapper.addBackend(new BackendConfiguration("backend", "localhost", backendPort, "/", -1)); + mapper.addBackend(new BackendConfiguration("backend-down", "localhost-down", backendPort, "/", -1)); + mapper.addDirector(new DirectorConfiguration("director").addBackend("backend")); + mapper.addDirector(new DirectorConfiguration("director-down").addBackend("backend-down")); + mapper.addAction(new ActionConfiguration("cache", ActionConfiguration.TYPE_CACHE, "director", null, -1)); + mapper.addAction(new ActionConfiguration("cache-down", ActionConfiguration.TYPE_CACHE, "director-down", null, -1)); + mapper.addRoute(new RouteConfiguration("route", "cache", true, new RegexpRequestMatcher(PROPERTY_URI, ".*index.html.*"))); + mapper.addRoute(new RouteConfiguration("route-down", "cache-down", true, new RegexpRequestMatcher(PROPERTY_URI, ".*down.html.*"))); + mapper.addRoute(new RouteConfiguration("route-default", "cache", true, new RegexpRequestMatcher(PROPERTY_URI, ".*html"))); + return mapper; + }; BackendHealthManager bhMan = mock(BackendHealthManager.class); final EndpointKey alive = EndpointKey.make("localhost:" + backendPort); final BackendHealthStatus mockAliveStatus = mock(BackendHealthStatus.class); @@ -338,7 +344,7 @@ public void testDefaultRoute() throws Exception { when(mockDownStatus.getStatus()).thenReturn(BackendHealthStatus.Status.DOWN); when(bhMan.getBackendStatus(eq(down))).thenReturn(mockDownStatus); // simulate unreachable backend -> expected 500 error - try (HttpProxyServer server = HttpProxyServer.buildForTests("localhost", 0, mapper, tmpDir.newFolder())) { + try (HttpProxyServer server = HttpProxyServer.buildForTests("localhost", 0, mapperFactory, tmpDir.newFolder())) { server.setBackendHealthManager(bhMan); server.start(); int port = server.getLocalPort(); @@ -370,10 +376,11 @@ public void testAlwaysServeStaticContent() throws Exception { .withHeader("Content-Type", "text/html") .withBody("it works !!"))); - try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder())) { + try (HttpProxyServer server = new HttpProxyServer(StandardEndpointMapper::new, tmpDir.newFolder())) { { Properties configuration = new Properties(); + configuration.put("healthmanager.tolerant", "true"); configuration.put("backend.1.id", "foo"); configuration.put("backend.1.host", "localhost"); configuration.put("backend.1.port", String.valueOf(backend.port())); @@ -443,7 +450,7 @@ public void testAlwaysServeStaticContent() throws Exception { @Test public void testServeACMEChallengeToken() throws Exception { - try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder())) { + try (HttpProxyServer server = new HttpProxyServer(StandardEndpointMapper::new, tmpDir.newFolder())) { final String tokenName = "test-token"; final String tokenData = "test-token-data-content"; DynamicCertificatesManager dynamicCertificateManager = mock(DynamicCertificatesManager.class); @@ -523,8 +530,9 @@ public void testCustomAndDebuggingHeaders() throws Exception { .withHeader("Content-Type", "text/html") .withBody("it works !!"))); - try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder())) { + try (HttpProxyServer server = new HttpProxyServer(StandardEndpointMapper::new, tmpDir.newFolder())) { Properties configuration = new Properties(); + configuration.put("healthmanager.tolerant", "true"); configuration.put("backend.1.id", "b1"); configuration.put("backend.1.host", "localhost"); configuration.put("backend.1.port", String.valueOf(backend.port())); @@ -648,7 +656,7 @@ public void testCustomAndDebuggingHeaders() throws Exception { @Test public void testActionRedirect() throws Exception { - try (HttpProxyServer server = new HttpProxyServer(null, tmpDir.newFolder())) { + try (HttpProxyServer server = new HttpProxyServer(StandardEndpointMapper::new, tmpDir.newFolder())) { Properties configuration = new Properties(); configuration.put("listener.1.host", "0.0.0.0"); configuration.put("listener.1.port", "1425"); diff --git a/carapace-server/src/test/java/org/carapaceproxy/server/mapper/ForceBackendTest.java b/carapace-server/src/test/java/org/carapaceproxy/server/mapper/ForceBackendTest.java index bcd9ae301..78120c1af 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/server/mapper/ForceBackendTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/server/mapper/ForceBackendTest.java @@ -70,24 +70,28 @@ public void test() throws Exception { .withBody("it works !!"))); int backendPort = backend1.port(); - StandardEndpointMapper mapper = new StandardEndpointMapper(SafeBackendSelector::forMapper); - Properties properties = new Properties(); - properties.put("mapper.forcedirector.parameter", "thedirector"); - properties.put("mapper.forcebackend.parameter", "thebackend"); - mapper.configure(new PropertiesConfigurationStore(properties)); - assertEquals("thedirector", mapper.getForceDirectorParameter()); - assertEquals("thebackend", mapper.getForceBackendParameter()); - mapper.addBackend(new BackendConfiguration("backend-a", "localhost", backendPort, "/", -1)); - mapper.addBackend(new BackendConfiguration("backend-b", "localhost", backendPort, "/", -1)); - mapper.addDirector(new DirectorConfiguration("director-1").addBackend("backend-a")); - mapper.addDirector(new DirectorConfiguration("director-2").addBackend("backend-b")); + final EndpointMapper.Factory mapperFactory = parent -> { + StandardEndpointMapper mapper = new StandardEndpointMapper(parent, SafeBackendSelector::new); + Properties properties = new Properties(); + properties.put("mapper.forcedirector.parameter", "thedirector"); + properties.put("mapper.forcebackend.parameter", "thebackend"); + mapper.configure(new PropertiesConfigurationStore(properties)); + assertEquals("thedirector", mapper.getForceDirectorParameter()); + assertEquals("thebackend", mapper.getForceBackendParameter()); - mapper.addAction(new ActionConfiguration("proxy-1", ActionConfiguration.TYPE_PROXY, "director-1", null, -1)); + mapper.addBackend(new BackendConfiguration("backend-a", "localhost", backendPort, "/", -1)); + mapper.addBackend(new BackendConfiguration("backend-b", "localhost", backendPort, "/", -1)); + mapper.addDirector(new DirectorConfiguration("director-1").addBackend("backend-a")); + mapper.addDirector(new DirectorConfiguration("director-2").addBackend("backend-b")); - mapper.addRoute(new RouteConfiguration("route-1", "proxy-1", true, new RegexpRequestMatcher(PROPERTY_URI, ".*index.html.*"))); + mapper.addAction(new ActionConfiguration("proxy-1", ActionConfiguration.TYPE_PROXY, "director-1", null, -1)); - try (HttpProxyServer server = HttpProxyServer.buildForTests("localhost", 0, mapper, tmpDir.newFolder());) { + mapper.addRoute(new RouteConfiguration("route-1", "proxy-1", true, new RegexpRequestMatcher(PROPERTY_URI, ".*index.html.*"))); + return mapper; + }; + + try (HttpProxyServer server = HttpProxyServer.buildForTests("localhost", 0, mapperFactory, tmpDir.newFolder())) { server.start(); int port = server.getLocalPort(); { diff --git a/carapace-server/src/test/java/org/carapaceproxy/users/FileUserRealmTest.java b/carapace-server/src/test/java/org/carapaceproxy/users/FileUserRealmTest.java index de197517a..c1482cc16 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/users/FileUserRealmTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/users/FileUserRealmTest.java @@ -64,7 +64,7 @@ private File createUserFile(Map users) throws Exception { @Test public void testFileUserRealm() throws Exception { - try (HttpProxyServer server = buildForTests("localhost", 0, new TestEndpointMapper("localhost", 0), tmpDir.newFolder())) { + try (HttpProxyServer server = buildForTests("localhost", 0, parent -> new TestEndpointMapper("localhost", 0), tmpDir.newFolder())) { Properties prop = new Properties(); prop.setProperty("http.admin.enabled", "true"); prop.setProperty("http.admin.port", "8761"); @@ -104,7 +104,7 @@ public void testFileUserRealm() throws Exception { @Test public void testFileUserRealmRefresh() throws Exception { - try (HttpProxyServer server = buildForTests("localhost", 0, new TestEndpointMapper("localhost", 0), tmpDir.newFolder())) { + try (HttpProxyServer server = buildForTests("localhost", 0, parent -> new TestEndpointMapper("localhost", 0), tmpDir.newFolder())) { Map users = new HashMap<>(); users.put("test1", "pass1"); @@ -149,7 +149,7 @@ public void testFileUserRealmRefresh() throws Exception { @Test public void testFileRelativePath() throws Exception { - try (HttpProxyServer server = buildForTests("localhost", 0, new TestEndpointMapper("localhost", 0), tmpDir.newFolder())) { + try (HttpProxyServer server = buildForTests("localhost", 0, parent -> new TestEndpointMapper("localhost", 0), tmpDir.newFolder())) { Properties prop = new Properties(); prop.setProperty("http.admin.enabled", "true"); prop.setProperty("http.admin.port", "8761"); diff --git a/carapace-server/src/test/java/org/carapaceproxy/utils/TestEndpointMapper.java b/carapace-server/src/test/java/org/carapaceproxy/utils/TestEndpointMapper.java index 73e41e48e..c2863a5b3 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/utils/TestEndpointMapper.java +++ b/carapace-server/src/test/java/org/carapaceproxy/utils/TestEndpointMapper.java @@ -26,6 +26,7 @@ import java.util.Map; import java.util.SequencedMap; import org.carapaceproxy.configstore.ConfigurationStore; +import org.carapaceproxy.core.HttpProxyServer; import org.carapaceproxy.core.ProxyRequest; import org.carapaceproxy.server.backends.BackendHealthStatus; import org.carapaceproxy.server.config.ActionConfiguration; @@ -36,7 +37,7 @@ import org.carapaceproxy.server.mapper.EndpointMapper; import org.carapaceproxy.server.mapper.MapResult; -public class TestEndpointMapper extends EndpointMapper { +public class TestEndpointMapper extends EndpointMapper implements EndpointMapper.Factory { private final String host; private final int port; @@ -44,9 +45,8 @@ public class TestEndpointMapper extends EndpointMapper { private final SequencedMap backends; private final List routes = new ArrayList<>(); private final List actions = new ArrayList<>(); - private final List directors = new ArrayList<>(); + private final SequencedMap directors; private final List headers = new ArrayList<>(); - private BackendHealthStatus healthStatus; public TestEndpointMapper(String host, int port) { this(host, port, false); @@ -57,14 +57,16 @@ public TestEndpointMapper(String host, int port, boolean cacheAll) { } public TestEndpointMapper(String host, int port, boolean cacheAll, Map backends) { + super(null); this.host = host; this.port = port; this.cacheAll = cacheAll; this.backends = new LinkedHashMap<>(backends); + this.directors = new LinkedHashMap<>(); } - public BackendHealthStatus getHealthStatus() { - return healthStatus; + public TestEndpointMapper(final HttpProxyServer ignoredServer) { + this("localhost", 0); // required for reflective construction } @Override @@ -72,31 +74,30 @@ public MapResult map(ProxyRequest request) { String uri = request.getUri(); if (uri.contains("not-found")) { return MapResult.notFound(MapResult.NO_ROUTE); - } else if (uri.contains("debug")) { + } + if (uri.contains("debug")) { return MapResult.builder() .action(MapResult.Action.SYSTEM) .routeId(MapResult.NO_ROUTE) .build(); - } else { - healthStatus = new BackendHealthStatus(request.getListener(), DEFAULT_WARMUP_PERIOD); - if (cacheAll) { - return MapResult.builder() - .host(host) - .port(port) - .action(MapResult.Action.CACHE) - .routeId(MapResult.NO_ROUTE) - .healthStatus(healthStatus) - .build(); - } else { - return MapResult.builder() - .host(host) - .port(port) - .action(MapResult.Action.PROXY) - .routeId(MapResult.NO_ROUTE) - .healthStatus(healthStatus) - .build(); - } } + final BackendHealthStatus healthStatus = new BackendHealthStatus(request.getListener(), DEFAULT_WARMUP_PERIOD); + if (cacheAll) { + return MapResult.builder() + .host(host) + .port(port) + .action(MapResult.Action.CACHE) + .routeId(MapResult.NO_ROUTE) + .healthStatus(healthStatus) + .build(); + } + return MapResult.builder() + .host(host) + .port(port) + .action(MapResult.Action.PROXY) + .routeId(MapResult.NO_ROUTE) + .healthStatus(healthStatus) + .build(); } @Override @@ -115,7 +116,7 @@ public List getActions() { } @Override - public List getDirectors() { + public SequencedMap getDirectors() { return directors; } @@ -127,4 +128,9 @@ public List getHeaders() { @Override public void configure(ConfigurationStore properties) { } + + @Override + public EndpointMapper build(final HttpProxyServer httpProxyServer) { + return this; + } } From e0633bacd0d394f925c4d4a668f022c942455f66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Maltoni?= Date: Fri, 29 Nov 2024 18:24:37 +0100 Subject: [PATCH 12/15] refactor: cleanup tests --- .../src/test/java/org/carapaceproxy/api/UseAdminServer.java | 2 +- .../java/org/carapaceproxy/backends/StuckRequestsTest.java | 3 ++- .../java/org/carapaceproxy/users/FileUserRealmTest.java | 6 +++--- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/carapace-server/src/test/java/org/carapaceproxy/api/UseAdminServer.java b/carapace-server/src/test/java/org/carapaceproxy/api/UseAdminServer.java index 36963ddcc..91cc6dd66 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/api/UseAdminServer.java +++ b/carapace-server/src/test/java/org/carapaceproxy/api/UseAdminServer.java @@ -62,7 +62,7 @@ public void buildNewServer() throws Exception { assertNull(server); credentials = new RawHttpClient.BasicAuthCredentials(DEFAULT_USERNAME, DEFAULT_PASSWORD); File serverRoot = tmpDir.getRoot(); // at every reboot we must keep the same directory - server = buildForTests("localhost", 0, parent -> new TestEndpointMapper("localhost", 0), serverRoot); + server = buildForTests("localhost", 0, new TestEndpointMapper("localhost", 0), serverRoot); } @After diff --git a/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java b/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java index 9c16d1b23..96792464c 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java @@ -33,6 +33,7 @@ import java.util.Properties; import junitparams.JUnitParamsRunner; import junitparams.Parameters; +import junitparams.naming.TestCaseName; import org.carapaceproxy.configstore.PropertiesConfigurationStore; import org.carapaceproxy.core.EndpointKey; import org.carapaceproxy.core.HttpProxyServer; @@ -64,7 +65,7 @@ public class StuckRequestsTest { @Test @Parameters({"true", "false"}) - @junitparams.naming.TestCaseName("test(backend unreachable on stuck request: {0})") + @TestCaseName("test(backend unreachable on stuck request: {0})") public void testBackendUnreachableOnStuckRequest(boolean backendsUnreachableOnStuckRequests) throws Exception { stubFor(get(urlEqualTo("/index.html")) diff --git a/carapace-server/src/test/java/org/carapaceproxy/users/FileUserRealmTest.java b/carapace-server/src/test/java/org/carapaceproxy/users/FileUserRealmTest.java index c1482cc16..de197517a 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/users/FileUserRealmTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/users/FileUserRealmTest.java @@ -64,7 +64,7 @@ private File createUserFile(Map users) throws Exception { @Test public void testFileUserRealm() throws Exception { - try (HttpProxyServer server = buildForTests("localhost", 0, parent -> new TestEndpointMapper("localhost", 0), tmpDir.newFolder())) { + try (HttpProxyServer server = buildForTests("localhost", 0, new TestEndpointMapper("localhost", 0), tmpDir.newFolder())) { Properties prop = new Properties(); prop.setProperty("http.admin.enabled", "true"); prop.setProperty("http.admin.port", "8761"); @@ -104,7 +104,7 @@ public void testFileUserRealm() throws Exception { @Test public void testFileUserRealmRefresh() throws Exception { - try (HttpProxyServer server = buildForTests("localhost", 0, parent -> new TestEndpointMapper("localhost", 0), tmpDir.newFolder())) { + try (HttpProxyServer server = buildForTests("localhost", 0, new TestEndpointMapper("localhost", 0), tmpDir.newFolder())) { Map users = new HashMap<>(); users.put("test1", "pass1"); @@ -149,7 +149,7 @@ public void testFileUserRealmRefresh() throws Exception { @Test public void testFileRelativePath() throws Exception { - try (HttpProxyServer server = buildForTests("localhost", 0, parent -> new TestEndpointMapper("localhost", 0), tmpDir.newFolder())) { + try (HttpProxyServer server = buildForTests("localhost", 0, new TestEndpointMapper("localhost", 0), tmpDir.newFolder())) { Properties prop = new Properties(); prop.setProperty("http.admin.enabled", "true"); prop.setProperty("http.admin.port", "8761"); From 75cfd276e4f27ea9852cc7e2ef7b9beb53d0bdc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Maltoni?= Date: Sun, 1 Dec 2024 17:44:23 +0100 Subject: [PATCH 13/15] test: fix ManagersExecutionTest and StuckRequestsTest --- .../backends/StuckRequestsTest.java | 19 ++----------------- pom.xml | 1 + 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java b/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java index 96792464c..ad29c3245 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/backends/StuckRequestsTest.java @@ -23,7 +23,6 @@ import static com.github.tomakehurst.wiremock.client.WireMock.get; import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; -import static org.carapaceproxy.core.ProxyRequest.PROPERTY_URI; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; @@ -40,14 +39,8 @@ import org.carapaceproxy.core.ProxyRequestsManager; import org.carapaceproxy.server.backends.BackendHealthStatus; import org.carapaceproxy.server.config.ActionConfiguration; -import org.carapaceproxy.server.config.BackendConfiguration; -import org.carapaceproxy.server.config.DirectorConfiguration; import org.carapaceproxy.server.config.NetworkListenerConfiguration; -import org.carapaceproxy.server.config.RouteConfiguration; -import org.carapaceproxy.server.config.SafeBackendSelector; -import org.carapaceproxy.server.mapper.EndpointMapper; import org.carapaceproxy.server.mapper.StandardEndpointMapper; -import org.carapaceproxy.server.mapper.requestmatcher.RegexpRequestMatcher; import org.carapaceproxy.utils.RawHttpClient; import org.junit.Rule; import org.junit.Test; @@ -87,21 +80,13 @@ public void testBackendUnreachableOnStuckRequest(boolean backendsUnreachableOnSt final int theport = wireMockRule.port(); EndpointKey key = new EndpointKey("localhost", theport); - final EndpointMapper.Factory mapperFactory = parent -> { - StandardEndpointMapper mapper = new StandardEndpointMapper(parent, SafeBackendSelector::new); - mapper.addBackend(new BackendConfiguration("backend-a", "localhost", theport, "/", -1)); - mapper.addDirector(new DirectorConfiguration("director-1").addBackend("backend-a")); - mapper.addAction(new ActionConfiguration("proxy-1", ActionConfiguration.TYPE_PROXY, "director-1", null, -1)); - mapper.addRoute(new RouteConfiguration("route-1", "proxy-1", true, new RegexpRequestMatcher(PROPERTY_URI, ".*index.html.*"))); - return mapper; - }; - try (HttpProxyServer server = new HttpProxyServer(mapperFactory, tmpDir.newFolder())) { + try (HttpProxyServer server = new HttpProxyServer(StandardEndpointMapper::new, tmpDir.newFolder())) { Properties properties = new Properties(); properties.put("healthmanager.tolerant", "true"); properties.put("backend.1.id", "backend-a"); properties.put("backend.1.enabled", "true"); properties.put("backend.1.host", "localhost"); - properties.put("backend.1.port", theport); + properties.put("backend.1.port", String.valueOf(theport)); properties.put("backend.1.probePath", "/"); properties.put("director.1.id", "director-1"); properties.put("director.1.backends", properties.getProperty("backend.1.id")); diff --git a/pom.xml b/pom.xml index 85586439e..4ffb2a0c7 100644 --- a/pom.xml +++ b/pom.xml @@ -354,6 +354,7 @@ --add-opens java.base/java.util=ALL-UNNAMED --add-opens java.base/java.util.concurrent=ALL-UNNAMED --add-opens java.rmi/sun.rmi.transport=ALL-UNNAMED + --add-opens java.xml/jdk.xml.internal=ALL-UNNAMED From 5f995b2eca9610edc39cb94a9a2a2a3c0657362c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Maltoni?= Date: Sun, 1 Dec 2024 23:47:27 +0100 Subject: [PATCH 14/15] test: fix MaintenanceModeTest --- .../java/org/carapaceproxy/MaintenanceModeTest.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/carapace-server/src/test/java/org/carapaceproxy/MaintenanceModeTest.java b/carapace-server/src/test/java/org/carapaceproxy/MaintenanceModeTest.java index daa47a2e2..19e1ea560 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/MaintenanceModeTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/MaintenanceModeTest.java @@ -55,12 +55,12 @@ public void test() throws Exception { config.put("backend.1.id", "localhost"); config.put("backend.1.enabled", "true"); config.put("backend.1.host", "localhost"); - config.put("backend.1.port", wireMockRule.port() + ""); + config.put("backend.1.port", String.valueOf(wireMockRule.port())); config.put("backend.2.id", "localhost2"); config.put("backend.2.enabled", "true"); config.put("backend.2.host", "localhost2"); - config.put("backend.2.port", wireMockRule.port() + ""); + config.put("backend.2.port", String.valueOf(wireMockRule.port())); // Default director config.put("director.1.id", "*"); @@ -113,6 +113,7 @@ public void maintenanceModeApiTest() throws Exception { .withBody("it works !!"))); config = new Properties(HTTP_ADMIN_SERVER_CONFIG); + config.put("healthmanager.tolerant", "true"); startServer(config); // Default certificate @@ -131,12 +132,12 @@ public void maintenanceModeApiTest() throws Exception { config.put("backend.1.id", "localhost"); config.put("backend.1.enabled", "true"); config.put("backend.1.host", "localhost"); - config.put("backend.1.port", wireMockRule.port() + ""); + config.put("backend.1.port", String.valueOf(wireMockRule.port())); config.put("backend.2.id", "localhost2"); config.put("backend.2.enabled", "true"); config.put("backend.2.host", "localhost2"); - config.put("backend.2.port", wireMockRule.port() + ""); + config.put("backend.2.port", String.valueOf(wireMockRule.port())); // Default director config.put("director.1.id", "*"); @@ -162,7 +163,7 @@ public void maintenanceModeApiTest() throws Exception { HttpResponse response = httpClient.send(request, HttpResponse.BodyHandlers.ofString()); assertEquals("it works !!", response.body()); - //ENABLE MAINTENANCE MODE VIA API + // ENABLE MAINTENANCE MODE VIA API HttpRequest enableMaintenanceRequest = HttpRequest.newBuilder() .POST(HttpRequest.BodyPublishers.noBody()) .uri(URI.create("http://localhost:" + DEFAULT_ADMIN_PORT + "/api/config/maintenance?enable=true")) From a10a07a5a5cc54034006c18502f3308b7a0b7ee9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Maltoni?= Date: Sun, 1 Dec 2024 23:49:22 +0100 Subject: [PATCH 15/15] test: fixing UnreachableBackendTest --- .../backends/UnreachableBackendTest.java | 44 +++++++++++++++++-- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/carapace-server/src/test/java/org/carapaceproxy/backends/UnreachableBackendTest.java b/carapace-server/src/test/java/org/carapaceproxy/backends/UnreachableBackendTest.java index ebbaceb66..87b915e57 100644 --- a/carapace-server/src/test/java/org/carapaceproxy/backends/UnreachableBackendTest.java +++ b/carapace-server/src/test/java/org/carapaceproxy/backends/UnreachableBackendTest.java @@ -23,6 +23,7 @@ import static com.github.tomakehurst.wiremock.client.WireMock.get; import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; +import static org.carapaceproxy.server.backends.BackendHealthStatus.Status.COLD; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; @@ -37,6 +38,7 @@ import org.carapaceproxy.core.HttpProxyServer; import org.carapaceproxy.core.ProxyRequestsManager; import org.carapaceproxy.server.backends.BackendHealthStatus; +import org.carapaceproxy.server.config.ActionConfiguration; import org.carapaceproxy.server.config.NetworkListenerConfiguration; import org.carapaceproxy.utils.RawHttpClient; import org.carapaceproxy.utils.TestEndpointMapper; @@ -98,7 +100,7 @@ public void testWithUnreachableBackend() throws Exception { """, resp.getBodyString()); } - assertSame(server.getBackendHealthManager().getBackendStatus(key).getStatus(), BackendHealthStatus.Status.DOWN); + assertSame(BackendHealthStatus.Status.DOWN, server.getBackendHealthManager().getBackendStatus(key).getStatus()); assertThat((int) ProxyRequestsManager.PENDING_REQUESTS_GAUGE.get(), is(0)); } } @@ -117,6 +119,23 @@ public void testEmptyResponse() throws Exception { try (HttpProxyServer server = new HttpProxyServer(mapper, tmpDir.newFolder())) { Properties properties = new Properties(); properties.put("healthmanager.tolerant", "true"); + properties.put("backend.1.id", "backend-a"); + properties.put("backend.1.enabled", "true"); + properties.put("backend.1.host", "localhost"); + properties.put("backend.1.port", String.valueOf(wireMockRule.port())); + properties.put("backend.1.probePath", "/"); + properties.put("director.1.id", "director-1"); + properties.put("director.1.backends", properties.getProperty("backend.1.id")); + properties.put("director.1.enabled", "true"); + properties.put("action.1.id", "proxy-1"); + properties.put("action.1.enabled", "true"); + properties.put("action.1.type", ActionConfiguration.TYPE_PROXY); + properties.put("action.1.director", properties.getProperty("director.1.id")); + properties.put("route.100.id", "route-1"); + properties.put("route.100.enabled", "true"); + properties.put("route.100.match", "request.uri ~ \".*index.html.*\""); + properties.put("route.100.action", properties.getProperty("action.1.id")); + properties.put("healthmanager.tolerant", "true"); // configure resets all listeners configurations server.configureAtBoot(new PropertiesConfigurationStore(properties)); server.addListener(new NetworkListenerConfiguration("localhost", 0)); @@ -136,7 +155,7 @@ public void testEmptyResponse() throws Exception { """, resp.getBodyString()); } - assertSame(server.getBackendHealthManager().getBackendStatus(key).getStatus(), BackendHealthStatus.Status.COLD); // no troubles for new connections + assertSame(COLD, server.getBackendHealthManager().getBackendStatus(key).getStatus()); // no troubles for new connections assertThat((int) ProxyRequestsManager.PENDING_REQUESTS_GAUGE.get(), is(0)); } } @@ -168,7 +187,7 @@ public void testConnectionResetByPeer() throws Exception { """, resp.getBodyString()); } - assertSame(server.getBackendHealthManager().getBackendStatus(key).getStatus(), BackendHealthStatus.Status.COLD); // no troubles for new connections + assertSame(COLD, server.getBackendHealthManager().getBackendStatus(key).getStatus()); // no troubles for new connections assertThat((int) ProxyRequestsManager.PENDING_REQUESTS_GAUGE.get(), is(0)); } } @@ -186,6 +205,23 @@ public void testNonHttpResponseThenClose() throws Exception { try (HttpProxyServer server = new HttpProxyServer(mapper, tmpDir.newFolder())) { Properties properties = new Properties(); properties.put("healthmanager.tolerant", "true"); + properties.put("backend.1.id", "backend-a"); + properties.put("backend.1.enabled", "true"); + properties.put("backend.1.host", "localhost"); + properties.put("backend.1.port", String.valueOf(wireMockRule.port())); + properties.put("backend.1.probePath", "/"); + properties.put("director.1.id", "director-1"); + properties.put("director.1.backends", properties.getProperty("backend.1.id")); + properties.put("director.1.enabled", "true"); + properties.put("action.1.id", "proxy-1"); + properties.put("action.1.enabled", "true"); + properties.put("action.1.type", ActionConfiguration.TYPE_PROXY); + properties.put("action.1.director", properties.getProperty("director.1.id")); + properties.put("route.100.id", "route-1"); + properties.put("route.100.enabled", "true"); + properties.put("route.100.match", "request.uri ~ \".*index.html.*\""); + properties.put("route.100.action", properties.getProperty("action.1.id")); + properties.put("healthmanager.tolerant", "true"); server.configureAtBoot(new PropertiesConfigurationStore(properties)); server.addListener(new NetworkListenerConfiguration("localhost", 0)); server.start(); @@ -204,7 +240,7 @@ public void testNonHttpResponseThenClose() throws Exception { """, resp.getBodyString()); } - assertSame(server.getBackendHealthManager().getBackendStatus(key).getStatus(), BackendHealthStatus.Status.COLD); + assertSame(COLD, server.getBackendHealthManager().getBackendStatus(key).getStatus()); assertThat((int) ProxyRequestsManager.PENDING_REQUESTS_GAUGE.get(), is(0)); } }