From ca8fc7e616437d084111d434e0e938823e5a5015 Mon Sep 17 00:00:00 2001 From: James Sharkey Date: Tue, 17 Feb 2026 17:38:06 +0000 Subject: [PATCH 1/2] Fix ElasticSearch ping InfoFacade endpoint This endpoint was not working because the cluster needs an authenticated request. It has been broken for a long time, but upgrading ES is a good reason to fix it. Fix the tests to actually verify the expected behaviour, too; we always return a 200 OK, so they did not notice that the endpoint was broken. --- .../ac/cam/cl/dtg/segue/api/InfoFacade.java | 23 +++++++++++++++++-- .../api/AbstractIsaacIntegrationTest.java | 8 +++---- .../ac/cam/cl/dtg/isaac/api/InfoFacadeIT.java | 10 +++++++- 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/api/InfoFacade.java b/src/main/java/uk/ac/cam/cl/dtg/segue/api/InfoFacade.java index 64663f383d..2630b0da73 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/api/InfoFacade.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/api/InfoFacade.java @@ -16,6 +16,9 @@ package uk.ac.cam.cl.dtg.segue.api; +import co.elastic.clients.elasticsearch.ElasticsearchClient; +import co.elastic.clients.elasticsearch._types.HealthStatus; +import co.elastic.clients.elasticsearch.cluster.HealthResponse; import com.google.common.collect.ImmutableMap; import com.google.inject.Inject; import io.swagger.v3.oas.annotations.Operation; @@ -42,6 +45,7 @@ import java.net.http.HttpRequest; import java.net.http.HttpResponse; import java.util.Objects; +import java.util.Set; import static uk.ac.cam.cl.dtg.segue.api.Constants.*; @@ -56,6 +60,7 @@ public class InfoFacade extends AbstractSegueFacade { private static final Logger log = LoggerFactory.getLogger(InfoFacade.class); private final SegueJobService segueJobService; + private final ElasticsearchClient searchClient; private final HttpClient httpClient; /** @@ -66,9 +71,11 @@ public class InfoFacade extends AbstractSegueFacade { */ @Inject public InfoFacade(final AbstractConfigLoader properties, final SegueJobService segueJobService, + final ElasticsearchClient searchClient, final ILogManager logManager) { super(properties, logManager); this.segueJobService = segueJobService; + this.searchClient = searchClient; this.httpClient = HttpClient.newHttpClient(); } @@ -171,8 +178,20 @@ public Response pingETLServer() { @Operation(summary = "Check whether elasticsearch is running.") public Response pingElasticSearch() { - return pingUrlForStatus("http://" + getProperties().getProperty("SEARCH_CLUSTER_ADDRESS") + ":" - + getProperties().getProperty("SEARCH_CLUSTER_INFO_PORT") + "/_cat/health"); + boolean healthy = false; + try { + HealthResponse response = searchClient.cluster().health(); + // Green: all okay with replicas. Yellow: either indexing, no replica, or local dev. + healthy = Set.of(HealthStatus.Green, HealthStatus.Yellow).contains(response.status()); + } catch (final IOException e) { + log.warn("Error whilst checking ElasticSearch status! {}", e.toString()); + } + + if (healthy) { + return Response.ok(ImmutableMap.of("success", true)).build(); + } else { + return Response.ok(ImmutableMap.of("success", false)).build(); + } } /** diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AbstractIsaacIntegrationTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AbstractIsaacIntegrationTest.java index f863965d2e..3d4167e0ee 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AbstractIsaacIntegrationTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AbstractIsaacIntegrationTest.java @@ -1,5 +1,6 @@ package uk.ac.cam.cl.dtg.isaac.api; +import co.elastic.clients.elasticsearch.ElasticsearchClient; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.json.JsonMapper; import com.google.api.client.util.Maps; @@ -104,8 +105,6 @@ import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.replay; import static uk.ac.cam.cl.dtg.segue.api.Constants.*; -import static uk.ac.cam.cl.dtg.segue.api.Constants.EMAIL_SIGNATURE; -import static uk.ac.cam.cl.dtg.segue.api.Constants.HOST_NAME; /** * IMPORTANT: Rather than directly subclass this, use either IsaacIntegrationTestWithREST or IsaacIntegrationTest @@ -124,6 +123,7 @@ public class AbstractIsaacIntegrationTest { protected static AbstractConfigLoader properties; protected static Map globalTokens; protected static PostgresSqlDb postgresSqlDb; + protected static ElasticsearchClient elasticSearchClient; protected static ElasticSearchIndexer elasticSearchProvider; protected static SchoolListReader schoolListReader; protected static MainMapper mainMapper; @@ -188,13 +188,13 @@ public class AbstractIsaacIntegrationTest { elasticsearch.start(); try { - elasticSearchProvider = new ElasticSearchIndexer(ElasticSearchProvider.getClient( + elasticSearchClient = ElasticSearchProvider.getClient( "localhost", elasticsearch.getMappedPort(9200), "elastic", "elastic" - ) ); + elasticSearchProvider = new ElasticSearchIndexer(elasticSearchClient); } catch (UnknownHostException e) { throw new RuntimeException(e); } diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/InfoFacadeIT.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/InfoFacadeIT.java index 8a0101b988..2086e275c2 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/InfoFacadeIT.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/InfoFacadeIT.java @@ -23,7 +23,7 @@ public class InfoFacadeIT extends IsaacIntegrationTest { @BeforeEach public void setUp() throws RuntimeException, IOException { SegueJobService segueJobService = createNiceMock(SegueJobService.class); // new SegueJobService(new ArrayList<>(), postgresSqlDb); - infoFacade = new InfoFacade(properties, segueJobService, logManager); + infoFacade = new InfoFacade(properties, segueJobService, elasticSearchClient, logManager); } @Test @@ -59,6 +59,10 @@ public void etlPing_respondsOK() { // /info/etl/ping Response response = infoFacade.pingETLServer(); assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); + @SuppressWarnings("unchecked") + ImmutableMap status = (ImmutableMap) response.getEntity(); + // ETL does not run during IT Tests, expect failure: + assertEquals(false, status.get("success")); } @Test @@ -66,6 +70,10 @@ public void elasticsearchPing_respondsOK() { // /info/elasticsearch/ping Response response = infoFacade.pingElasticSearch(); assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); + @SuppressWarnings("unchecked") + ImmutableMap status = (ImmutableMap) response.getEntity(); + // ElasticSearch does run during IT Tests, expect success: + assertEquals(true, status.get("success")); } // NOTE: The other methods are probably less useful to test unless we also bring up the checkers From ca48004b23be3368d41c213b5faf8d16e0220e05 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Thu, 19 Feb 2026 09:36:55 +0000 Subject: [PATCH 2/2] Simplify success response Co-authored-by: James Sharkey --- src/main/java/uk/ac/cam/cl/dtg/segue/api/InfoFacade.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/api/InfoFacade.java b/src/main/java/uk/ac/cam/cl/dtg/segue/api/InfoFacade.java index 2630b0da73..d826569860 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/api/InfoFacade.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/api/InfoFacade.java @@ -187,11 +187,7 @@ public Response pingElasticSearch() { log.warn("Error whilst checking ElasticSearch status! {}", e.toString()); } - if (healthy) { - return Response.ok(ImmutableMap.of("success", true)).build(); - } else { - return Response.ok(ImmutableMap.of("success", false)).build(); - } + return Response.ok(ImmutableMap.of("success", healthy)).build(); } /**