You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by dj...@apache.org on 2021/09/29 22:57:50 UTC

[cassandra-sidecar] branch trunk updated: Avoid having sidecar's health response code depend on Cassandra's health information

This is an automated email from the ASF dual-hosted git repository.

djoshi pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra-sidecar.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 1012c0d  Avoid having sidecar's health response code depend on Cassandra's health information
1012c0d is described below

commit 1012c0dac56b64599d06815b500d4e7d41d34f03
Author: Saranya Krishnakumar <sa...@apple.com>
AuthorDate: Wed Sep 22 15:50:39 2021 -0700

    Avoid having sidecar's health response code depend on Cassandra's health information
    
    patch by Saranya Krishnakumar; reviewed by Yifan Cai, Dinesh Joshi for CASSANDRASC-29
---
 .../org/apache/cassandra/sidecar/MainModule.java   |  5 +++-
 ...lthService.java => CassandraHealthService.java} |  8 +++----
 .../cassandra/sidecar/routes/HealthService.java    | 28 +++++-----------------
 .../sidecar/AbstractHealthServiceTest.java         | 28 ++++++++++++++++++----
 .../cassandra/sidecar/FilePathBuilderTest.java     |  1 -
 .../org/apache/cassandra/sidecar/TestModule.java   |  9 -------
 6 files changed, 37 insertions(+), 42 deletions(-)

diff --git a/src/main/java/org/apache/cassandra/sidecar/MainModule.java b/src/main/java/org/apache/cassandra/sidecar/MainModule.java
index 223ab97..701900c 100644
--- a/src/main/java/org/apache/cassandra/sidecar/MainModule.java
+++ b/src/main/java/org/apache/cassandra/sidecar/MainModule.java
@@ -45,6 +45,7 @@ import org.apache.cassandra.sidecar.cassandra40.Cassandra40Factory;
 import org.apache.cassandra.sidecar.common.CQLSession;
 import org.apache.cassandra.sidecar.common.CassandraAdapterDelegate;
 import org.apache.cassandra.sidecar.common.CassandraVersionProvider;
+import org.apache.cassandra.sidecar.routes.CassandraHealthService;
 import org.apache.cassandra.sidecar.routes.HealthService;
 import org.apache.cassandra.sidecar.routes.StreamSSTableComponent;
 import org.apache.cassandra.sidecar.routes.SwaggerOpenApiResource;
@@ -102,7 +103,8 @@ public class MainModule extends AbstractModule
     @Singleton
     private VertxRequestHandler configureServices(Vertx vertx,
                                                   HealthService healthService,
-                                                  StreamSSTableComponent ssTableComponent)
+                                                  StreamSSTableComponent ssTableComponent,
+                                                  CassandraHealthService cassandraHealthService)
     {
         VertxResteasyDeployment deployment = new VertxResteasyDeployment();
         deployment.start();
@@ -111,6 +113,7 @@ public class MainModule extends AbstractModule
         r.addPerInstanceResource(SwaggerOpenApiResource.class);
         r.addSingletonResource(healthService);
         r.addSingletonResource(ssTableComponent);
+        r.addSingletonResource(cassandraHealthService);
 
         return new VertxRequestHandler(vertx, deployment);
     }
diff --git a/src/main/java/org/apache/cassandra/sidecar/routes/HealthService.java b/src/main/java/org/apache/cassandra/sidecar/routes/CassandraHealthService.java
similarity index 93%
copy from src/main/java/org/apache/cassandra/sidecar/routes/HealthService.java
copy to src/main/java/org/apache/cassandra/sidecar/routes/CassandraHealthService.java
index bd15fde..cc17ef8 100644
--- a/src/main/java/org/apache/cassandra/sidecar/routes/HealthService.java
+++ b/src/main/java/org/apache/cassandra/sidecar/routes/CassandraHealthService.java
@@ -40,14 +40,14 @@ import org.apache.cassandra.sidecar.common.CassandraAdapterDelegate;
  * Provides a simple REST endpoint to determine if a node is available
  */
 @Singleton
-@Path("/api/v1/__health")
-public class HealthService
+@Path("/api/v1/cassandra/__health")
+public class CassandraHealthService
 {
     private static final Logger logger = LoggerFactory.getLogger(HealthService.class);
     private final CassandraAdapterDelegate cassandra;
 
     @Inject
-    public HealthService(CassandraAdapterDelegate cassandra)
+    public CassandraHealthService(CassandraAdapterDelegate cassandra)
     {
         this.cassandra = cassandra;
     }
@@ -60,7 +60,7 @@ public class HealthService
     })
     @Produces(MediaType.APPLICATION_JSON)
     @GET
-    public Response doGet()
+    public Response getCassandraHealth()
     {
         Boolean up = cassandra.isUp();
         int status = up ? HttpResponseStatus.OK.code() : HttpResponseStatus.SERVICE_UNAVAILABLE.code();
diff --git a/src/main/java/org/apache/cassandra/sidecar/routes/HealthService.java b/src/main/java/org/apache/cassandra/sidecar/routes/HealthService.java
index bd15fde..dd0a46a 100644
--- a/src/main/java/org/apache/cassandra/sidecar/routes/HealthService.java
+++ b/src/main/java/org/apache/cassandra/sidecar/routes/HealthService.java
@@ -26,45 +26,29 @@ import javax.ws.rs.core.Response;
 
 import com.google.common.collect.ImmutableMap;
 
-import com.google.inject.Inject;
 import com.google.inject.Singleton;
 import io.netty.handler.codec.http.HttpResponseStatus;
 import io.swagger.v3.oas.annotations.Operation;
-import io.swagger.v3.oas.annotations.responses.ApiResponse;
 import io.vertx.core.json.Json;
 import io.vertx.core.logging.Logger;
 import io.vertx.core.logging.LoggerFactory;
-import org.apache.cassandra.sidecar.common.CassandraAdapterDelegate;
 
 /**
- * Provides a simple REST endpoint to determine if a node is available
+ * Provides a simple REST endpoint to determine if Sidecar is available
  */
 @Singleton
 @Path("/api/v1/__health")
 public class HealthService
 {
     private static final Logger logger = LoggerFactory.getLogger(HealthService.class);
-    private final CassandraAdapterDelegate cassandra;
 
-    @Inject
-    public HealthService(CassandraAdapterDelegate cassandra)
-    {
-        this.cassandra = cassandra;
-    }
-
-    @Operation(summary = "Health Check for Cassandra's status",
-    description = "Returns HTTP 200 if Cassandra is available, 503 otherwise",
-    responses = {
-    @ApiResponse(responseCode = "200", description = "Cassandra is available"),
-    @ApiResponse(responseCode = "503", description = "Cassandra is not available")
-    })
+    @Operation(summary = "Health Check for Sidecar's status",
+    description = "Returns HTTP 200 if Sidecar is available")
     @Produces(MediaType.APPLICATION_JSON)
     @GET
-    public Response doGet()
+    public Response getSidecarHealth()
     {
-        Boolean up = cassandra.isUp();
-        int status = up ? HttpResponseStatus.OK.code() : HttpResponseStatus.SERVICE_UNAVAILABLE.code();
-        return Response.status(status).entity(Json.encode(ImmutableMap.of("status", up ?
-                                                                                    "OK" : "NOT_OK"))).build();
+        return Response.status(HttpResponseStatus.OK.code()).entity(Json.encode(ImmutableMap.of("status", "OK")))
+                       .build();
     }
 }
diff --git a/src/test/java/org/apache/cassandra/sidecar/AbstractHealthServiceTest.java b/src/test/java/org/apache/cassandra/sidecar/AbstractHealthServiceTest.java
index 946be3c..9b19b8f 100644
--- a/src/test/java/org/apache/cassandra/sidecar/AbstractHealthServiceTest.java
+++ b/src/test/java/org/apache/cassandra/sidecar/AbstractHealthServiceTest.java
@@ -96,11 +96,10 @@ public abstract class AbstractHealthServiceTest
             logger.error("Close event timed out.");
     }
 
-    @DisplayName("Should return HTTP 200 OK when check=True")
+    @DisplayName("Should return HTTP 200 OK if sidecar server is running")
     @Test
-    public void testHealthCheckReturns200OK(VertxTestContext testContext)
+    public void testSidecarHealthCheckReturnsOK(VertxTestContext testContext)
     {
-        when(cassandra.isUp()).thenReturn(true);
         WebClient client = getClient();
 
         client.get(config.getPort(), "localhost", "/api/v1/__health")
@@ -109,6 +108,7 @@ public abstract class AbstractHealthServiceTest
               .send(testContext.succeeding(response -> testContext.verify(() ->
               {
                   Assert.assertEquals(200, response.statusCode());
+                  Assert.assertEquals("{\"status\":\"OK\"}", response.body());
                   testContext.completeNow();
               })));
     }
@@ -129,20 +129,38 @@ public abstract class AbstractHealthServiceTest
         return options;
     }
 
+    @DisplayName("Should return HTTP 200 OK when check=True")
+    @Test
+    public void testHealthCheckReturns200OK(VertxTestContext testContext)
+    {
+        when(cassandra.isUp()).thenReturn(true);
+        WebClient client = getClient();
+
+        client.get(config.getPort(), "localhost", "/api/v1/cassandra/__health")
+              .as(BodyCodec.string())
+              .ssl(isSslEnabled())
+              .send(testContext.succeeding(response -> testContext.verify(() ->
+              {
+                  Assert.assertEquals(200, response.statusCode());
+                  Assert.assertEquals("{\"status\":\"OK\"}", response.body());
+                  testContext.completeNow();
+              })));
+    }
+
     @DisplayName("Should return HTTP 503 Failure when check=False")
     @Test
     public void testHealthCheckReturns503Failure(VertxTestContext testContext)
     {
-
         when(cassandra.isUp()).thenReturn(false);
         WebClient client = getClient();
 
-        client.get(config.getPort(), "localhost", "/api/v1/__health")
+        client.get(config.getPort(), "localhost", "/api/v1/cassandra/__health")
               .as(BodyCodec.string())
               .ssl(isSslEnabled())
               .send(testContext.succeeding(response -> testContext.verify(() ->
               {
                   Assert.assertEquals(503, response.statusCode());
+                  Assert.assertEquals("{\"status\":\"NOT_OK\"}", response.body());
                   testContext.completeNow();
               })));
     }
diff --git a/src/test/java/org/apache/cassandra/sidecar/FilePathBuilderTest.java b/src/test/java/org/apache/cassandra/sidecar/FilePathBuilderTest.java
index 734ca22..2f37749 100644
--- a/src/test/java/org/apache/cassandra/sidecar/FilePathBuilderTest.java
+++ b/src/test/java/org/apache/cassandra/sidecar/FilePathBuilderTest.java
@@ -18,7 +18,6 @@ import org.assertj.core.api.Assertions;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.junit.jupiter.api.Assertions.assertTrue;
 
 /**
  * FilePathBuilderTest
diff --git a/src/test/java/org/apache/cassandra/sidecar/TestModule.java b/src/test/java/org/apache/cassandra/sidecar/TestModule.java
index 755ad4a..4cf7d9f 100644
--- a/src/test/java/org/apache/cassandra/sidecar/TestModule.java
+++ b/src/test/java/org/apache/cassandra/sidecar/TestModule.java
@@ -29,7 +29,6 @@ import com.google.inject.Singleton;
 import org.apache.cassandra.sidecar.common.CassandraAdapterDelegate;
 import org.apache.cassandra.sidecar.common.CassandraVersionProvider;
 import org.apache.cassandra.sidecar.common.MockCassandraFactory;
-import org.apache.cassandra.sidecar.routes.HealthService;
 
 import static org.mockito.Mockito.mock;
 
@@ -47,14 +46,6 @@ public class TestModule extends AbstractModule
         return mock(CassandraAdapterDelegate.class);
     }
 
-    @Singleton
-    @Provides
-    public HealthService healthService(CassandraAdapterDelegate delegate)
-    {
-        return new HealthService(delegate);
-    }
-
-
     @Provides
     @Singleton
     public Configuration configuration()

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org