Skip to content

Commit

Permalink
Work in progress
Browse files Browse the repository at this point in the history
  • Loading branch information
ElaadF committed Oct 25, 2020
1 parent 5d063f6 commit 0869157
Showing 1 changed file with 40 additions and 1 deletion.
Expand Up @@ -3,14 +3,20 @@ package com.normation.rudder.services.healthcheck
import java.lang.Runtime.getRuntime

import com.normation.errors.IOResult
import com.normation.errors.Inconsistency
import com.normation.rudder.domain.logger.{HealthcheckLoggerPure => logger}
import com.normation.rudder.hooks.Cmd
import com.normation.rudder.hooks.RunNuCommand
import com.normation.rudder.services.healthcheck.HealthcheckResult.Critical
import com.normation.rudder.services.healthcheck.HealthcheckResult.Warning
import com.normation.rudder.services.healthcheck.HealthcheckResult.Ok
import com.normation.rudder.services.nodes.NodeInfoService
import zio.UIO
import zio.ZIO
import zio.syntax.ToZio

import scala.util.Try

final case class CheckName(value: String)

trait Check {
Expand Down Expand Up @@ -62,7 +68,7 @@ class HealthcheckService(checks: List[Check]) {
}
}

final object CheckCoreNumber extends Check {
final class CheckCoreNumber extends Check {
def name: CheckName = CheckName("CPU Cores available")
def run: IOResult[HealthcheckResult] = for {
availableCores <- IOResult.effect(getRuntime.availableProcessors)
Expand All @@ -73,4 +79,37 @@ final object CheckCoreNumber extends Check {
case n => Ok(name, s"${n} cores available")
}
}
}

// How to get a service in this check, or find another way to get number of node
// Should I transform this into a class ?

This comment has been minimized.

Copy link
@fanf

fanf Oct 25, 2020

I conventionnaly prefer a class for service, so that you know when the instanctiation is (or more "where" than "when"). But from a semantic point of view, it's the same (modulo tech details).
So: more yes to be consistent with other part of rudder, but there is no scala reason for that.

final object CheckFileDescriptorLimit extends Check {
def name: CheckName = CheckName("File descriptor limit")
def run: IOResult[HealthcheckResult] = {
// Check the soft limit.
// That can be raise or lower by any user but cannot exceed the hard limit provided by parameter "-Hn"
// "-Sn" correspond to [S]oft and limit [n]umber of opened file descriptor
val cmd = Cmd("ulimit", "-Sn" :: Nil, Map.empty)

for {
fdLimitCmd <- RunNuCommand.run(cmd)
res <- fdLimitCmd.await
_ <- ZIO.when(res.code != 0) {
Inconsistency(
s"An error occurred while getting file descriptor soft limit with command '${cmd.display}':\n code: ${res.code}\n stderr: ${res.stderr}\n stdout: ${res.stdout}"
).fail
}
limit <- IOResult.effect(res.stdout.toInt) // not sure about to handle a toInt with a possible exception

This comment has been minimized.

Copy link
@fanf

fanf Oct 25, 2020

since you need to be consistant with IOResult in that for comprehension, it's a correct way to do it. You can use IORestul.effectNonBlocking since we are sure that toInt does not block, it will avoid a move to the blocking thread pool.

} yield {
val numNode = 31 // NodeInfoService.getNumberOfManagedNodes or NodeInfoServiceCached.getNumberOfManagedNodes

This comment has been minimized.

Copy link
@fanf

fanf Oct 25, 2020

that answers the question about abject: since you will have a dependency toward NodeInfoService, it needs to be a class with that dependency as parameter.

limit match {
case limit if limit <= 10_000 =>
Critical(name, s"The limit of ${limit} opened file descriptor is not enough")
case limit if limit <= (100 * numNode) => // what if nb node < 100 ?

This comment has been minimized.

Copy link
@fanf

fanf Oct 25, 2020

it's ok, we will get the first match and the corresponding critical. I really think we need to have fd > 10_000

Warning(name, s"The limit of ${limit} opened file descriptor may cause trouble")
case _ =>
Ok(name, s"The limit of ${limit} is acceptable")
}
}
}
}

0 comments on commit 0869157

Please sign in to comment.