Merge pull request #463 from NDLANO/liveness-readiness

Split health checks into multiple endpoints
NDLANO · Apr 29, 2024 · 49aafeb · 49aafeb
2 parents edc6cfa + a46d28e
commit 49aafeb
Show file tree

Hide file tree

Showing 7 changed files with 58 additions and 26 deletions.
diff --git a/audio-api/src/main/scala/no/ndla/audioapi/controller/HealthController.scala b/audio-api/src/main/scala/no/ndla/audioapi/controller/HealthController.scala
@@ -19,7 +19,7 @@ trait HealthController {
 
   class HealthController extends TapirHealthController[Eff] {
 
-    override def checkHealth(): Either[String, String] = {
+    override def checkReadiness(): Either[String, String] = {
       audioRepository
         .getRandomAudio()
         .flatMap(audio => {

diff --git a/audio-api/src/test/scala/no/ndla/audioapi/controller/HealthControllerTest.scala b/audio-api/src/test/scala/no/ndla/audioapi/controller/HealthControllerTest.scala
@@ -53,40 +53,49 @@ class HealthControllerTest extends UnitSuite with TestEnvironment with TapirCont
   healthControllerResponse = 404
   when(audioRepository.getRandomAudio()).thenReturn(None)
 
-  test("that /health returns 200 on success") {
+  test("that /health/readiness returns 200 on success") {
     healthControllerResponse = 200
     when(audioRepository.getRandomAudio()).thenReturn(Some(audioMeta))
     when(audioStorage.objectExists("file.mp3")).thenReturn(true)
 
     val request =
       quickRequest
-        .get(uri"http://localhost:$serverPort/health")
+        .get(uri"http://localhost:$serverPort/health/readiness")
 
     val response = simpleHttpClient.send(request)
     response.code.code should be(200)
   }
 
-  test("that /health returns 500 on failure") {
+  test("that /health/readiness returns 500 on failure") {
     healthControllerResponse = 500
     when(audioRepository.getRandomAudio()).thenReturn(Some(audioMeta))
     when(audioStorage.objectExists("file.mp3")).thenReturn(false)
 
     val request =
       quickRequest
-        .get(uri"http://localhost:$serverPort/health")
+        .get(uri"http://localhost:$serverPort/health/readiness")
 
     val response = simpleHttpClient.send(request)
     response.code.code should be(500)
   }
 
+  test("that /health/liveness returns 200") {
+    val request =
+      quickRequest
+        .get(uri"http://localhost:$serverPort/health/liveness")
+
+    val response = simpleHttpClient.send(request)
+    response.code.code should be(200)
+  }
+
   test("that /health returns 200 on no audios") {
     healthControllerResponse = 404
     when(audioRepository.getRandomAudio()).thenReturn(None)
     when(audioStorage.objectExists("file.mp3")).thenReturn(false)
 
     val request =
       quickRequest
-        .get(uri"http://localhost:$serverPort/health")
+        .get(uri"http://localhost:$serverPort/health/readiness")
 
     val response = simpleHttpClient.send(request)
     response.code.code should be(200)

diff --git a/image-api/src/main/scala/no/ndla/imageapi/controller/HealthController.scala b/image-api/src/main/scala/no/ndla/imageapi/controller/HealthController.scala
@@ -19,7 +19,7 @@ trait HealthController {
 
   class HealthController extends TapirHealthController[Eff] {
 
-    override def checkHealth(): Either[String, String] = {
+    override def checkReadiness(): Either[String, String] = {
       imageRepository
         .getRandomImage()
         .flatMap(image => {

diff --git a/image-api/src/test/scala/no/ndla/imageapi/controller/HealthControllerTest.scala b/image-api/src/test/scala/no/ndla/imageapi/controller/HealthControllerTest.scala
@@ -61,27 +61,37 @@ class HealthControllerTest extends UnitSuite with TestEnvironment with TapirCont
     Seq.empty
   )
 
-  test("that /health returns 200 on success") {
+  test("that /health/readiness returns 200 on success") {
     healthControllerResponse = 200
     when(imageRepository.getRandomImage()).thenReturn(Some(imageMeta))
     when(imageStorage.objectExists("file.jpg")).thenReturn(true)
 
     val request =
       quickRequest
-        .get(uri"http://localhost:$serverPort/health")
+        .get(uri"http://localhost:$serverPort/health/readiness")
 
     val response = simpleHttpClient.send(request)
     response.code.code should be(200)
   }
 
-  test("that /health returns 500 on failure") {
+  test("that /health/liveness returns 200") {
+    healthControllerResponse = 200
+    val request =
+      quickRequest
+        .get(uri"http://localhost:$serverPort/health/liveness")
+
+    val response = simpleHttpClient.send(request)
+    response.code.code should be(200)
+  }
+
+  test("that /health/readiness returns 500 on failure") {
     healthControllerResponse = 500
     when(imageRepository.getRandomImage()).thenReturn(Some(imageMeta))
     when(imageStorage.objectExists("file.jpg")).thenReturn(false)
 
     val request =
       quickRequest
-        .get(uri"http://localhost:$serverPort/health")
+        .get(uri"http://localhost:$serverPort/health/readiness")
 
     val response = simpleHttpClient.send(request)
     response.code.code should be(500)

diff --git a/network/src/main/scala/no/ndla/network/tapir/NdlaTapirMain.scala b/network/src/main/scala/no/ndla/network/tapir/NdlaTapirMain.scala
@@ -44,7 +44,9 @@ trait NdlaTapirMain[F[_]] {
     setPropsFromEnv()
 
     logCopyrightHeader()
-    beforeStart()
-    startServer(props.ApplicationName, props.ApplicationPort) { performWarmup() }
+    startServer(props.ApplicationName, props.ApplicationPort) {
+      beforeStart()
+      performWarmup()
+    }
   }
 }
diff --git a/network/src/main/scala/no/ndla/network/tapir/Routes.scala b/network/src/main/scala/no/ndla/network/tapir/Routes.scala
@@ -89,9 +89,10 @@ trait Routes[F[_]] {
 
     object JDKMiddleware {
       private def shouldLogRequest(req: ServerRequest): Boolean = {
-        if (req.uri.path.size != 1) return true
-        if (req.uri.path.head == "metrics") return false
-        if (req.uri.path.head == "health") return false
+        if (req.uri.path.size == 1) {
+          if (req.uri.path.head == "metrics") return false
+          if (req.uri.path.head == "health") return false
+        } else if (req.uri.path.size > 1 && req.uri.path.head == "health") return false
         true
       }
 

diff --git a/network/src/main/scala/no/ndla/network/tapir/TapirHealthController.scala b/network/src/main/scala/no/ndla/network/tapir/TapirHealthController.scala
@@ -11,26 +11,36 @@ import no.ndla.common.Warmup
 import sttp.model.StatusCode
 import sttp.tapir.EndpointInput
 import sttp.tapir.server.ServerEndpoint
-import sttp.tapir._
+import sttp.tapir.*
 
 trait TapirHealthController {
   class TapirHealthController[F[_]] extends Warmup with Service[F] {
     override val enableSwagger: Boolean = false
     val prefix: EndpointInput[Unit]     = "health"
 
-    protected def checkHealth(): Either[String, String] = Right("Health check succeeded")
+    private def checkLiveness(): Either[String, String] = Right("Healthy")
+    protected def checkReadiness(): Either[String, String] = {
+      if (isWarmedUp) Right("Ready")
+      else Left("Service is not ready")
+    }
 
     override val endpoints: List[ServerEndpoint[Any, F]] = List(
       endpoint.get
+        .description("Readiness probe. Returns 200 if the service is ready to serve traffic.")
+        .in("readiness")
         .out(stringBody)
-        .errorOut(
-          statusCode(StatusCode.InternalServerError)
-            .and(stringBody)
-        )
-        .serverLogicPure { _ =>
-          if (!isWarmedUp) Left("Warmup hasn't finished")
-          else checkHealth()
-        }
+        .errorOut(statusCode(StatusCode.InternalServerError).and(stringBody))
+        .serverLogicPure(_ => checkReadiness()),
+      endpoint.get
+        .description("Liveness probe. Returns 200 if the service is alive, but not necessarily ready.")
+        .in("liveness")
+        .out(stringBody)
+        .errorOut(statusCode(StatusCode.InternalServerError).and(stringBody))
+        .serverLogicPure(_ => checkLiveness()),
+      endpoint.get
+        .out(stringBody)
+        .errorOut(statusCode(StatusCode.InternalServerError).and(stringBody))
+        .serverLogicPure(_ => checkLiveness())
     )
   }
 }