[4/6] mesos git commit: Improved health checks validations.

[al...@apache.org]

Improved health checks validations.

Add validation for the "general" fields, such as `timeout_seconds`.

Review: https://reviews.apache.org/r/59902/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/ec40f359
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/ec40f359
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/ec40f359

Branch: refs/heads/master
Commit: ec40f359ba70e354580865ee3934461047c0fdac
Parents: cb302f9
Author: Gastón Kleiman <ga...@mesosphere.io>
Authored: Fri Jun 9 15:00:40 2017 +0200
Committer: Alexander Rukletsov <al...@apache.org>
Committed: Fri Jun 9 15:47:50 2017 +0200

----------------------------------------------------------------------
 src/checks/health_checker.cpp    | 42 +++++++++++++++++++++++++----------
 src/tests/health_check_tests.cpp | 26 ++++++++++++++++++++++
 2 files changed, 56 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/ec40f359/src/checks/health_checker.cpp
----------------------------------------------------------------------
diff --git a/src/checks/health_checker.cpp b/src/checks/health_checker.cpp
index c26e606..99ff965 100644
--- a/src/checks/health_checker.cpp
+++ b/src/checks/health_checker.cpp
@@ -370,19 +370,19 @@ void HealthChecker::success()
 
 namespace validation {
 
-Option<Error> healthCheck(const HealthCheck& check)
+Option<Error> healthCheck(const HealthCheck& healthCheck)
 {
-  if (!check.has_type()) {
+  if (!healthCheck.has_type()) {
     return Error("HealthCheck must specify 'type'");
   }
 
-  switch (check.type()) {
+  switch (healthCheck.type()) {
     case HealthCheck::COMMAND: {
-      if (!check.has_command()) {
+      if (!healthCheck.has_command()) {
         return Error("Expecting 'command' to be set for COMMAND health check");
       }
 
-      const CommandInfo& command = check.command();
+      const CommandInfo& command = healthCheck.command();
 
       if (!command.has_value()) {
         string commandType =
@@ -398,15 +398,16 @@ Option<Error> healthCheck(const HealthCheck& check)
             "Health check's `CommandInfo` is invalid: " + error->message);
       }
 
+      // TODO(alexr): Make sure irrelevant fields, e.g., `uris` are not set.
+
       break;
     }
-
     case HealthCheck::HTTP: {
-      if (!check.has_http()) {
+      if (!healthCheck.has_http()) {
         return Error("Expecting 'http' to be set for HTTP health check");
       }
 
-      const HealthCheck::HTTPCheckInfo& http = check.http();
+      const HealthCheck::HTTPCheckInfo& http = healthCheck.http();
 
       if (http.has_scheme() &&
           http.scheme() != "http" &&
@@ -423,22 +424,39 @@ Option<Error> healthCheck(const HealthCheck& check)
 
       break;
     }
-
     case HealthCheck::TCP: {
-      if (!check.has_tcp()) {
+      if (!healthCheck.has_tcp()) {
         return Error("Expecting 'tcp' to be set for TCP health check");
       }
 
       break;
     }
-
     case HealthCheck::UNKNOWN: {
       return Error(
-          "'" + HealthCheck::Type_Name(check.type()) + "'"
+          "'" + HealthCheck::Type_Name(healthCheck.type()) + "'"
           " is not a valid health check type");
     }
   }
 
+  if (healthCheck.has_delay_seconds() && healthCheck.delay_seconds() < 0.0) {
+    return Error("Expecting 'delay_seconds' to be non-negative");
+  }
+
+  if (healthCheck.has_grace_period_seconds() &&
+      healthCheck.grace_period_seconds() < 0.0) {
+    return Error("Expecting 'grace_period_seconds' to be non-negative");
+  }
+
+  if (healthCheck.has_interval_seconds() &&
+      healthCheck.interval_seconds() < 0.0) {
+    return Error("Expecting 'interval_seconds' to be non-negative");
+  }
+
+  if (healthCheck.has_timeout_seconds() &&
+      healthCheck.timeout_seconds() < 0.0) {
+    return Error("Expecting 'timeout_seconds' to be non-negative");
+  }
+
   return None();
 }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/ec40f359/src/tests/health_check_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/health_check_tests.cpp b/src/tests/health_check_tests.cpp
index 7917a22..caaac8d 100644
--- a/src/tests/health_check_tests.cpp
+++ b/src/tests/health_check_tests.cpp
@@ -221,6 +221,32 @@ TEST_F(HealthCheckTest, HealthCheckProtobufValidation)
     EXPECT_SOME(validate);
   }
 
+  // Duration parameters must be non-negative.
+  {
+    HealthCheck healthCheckProto;
+
+    healthCheckProto.set_type(HealthCheck::HTTP);
+    healthCheckProto.mutable_http()->set_port(8080);
+
+    healthCheckProto.set_delay_seconds(-1.0);
+    Option<Error> validate = validation::healthCheck(healthCheckProto);
+    EXPECT_SOME(validate);
+
+    healthCheckProto.set_delay_seconds(0.0);
+    healthCheckProto.set_interval_seconds(-1.0);
+    validate = validation::healthCheck(healthCheckProto);
+    EXPECT_SOME(validate);
+
+    healthCheckProto.set_interval_seconds(0.0);
+    healthCheckProto.set_timeout_seconds(-1.0);
+    validate = validation::healthCheck(healthCheckProto);
+    EXPECT_SOME(validate);
+
+    healthCheckProto.set_timeout_seconds(0.0);
+    validate = validation::healthCheck(healthCheckProto);
+    EXPECT_NONE(validate);
+  }
+
   // Command health check must specify an actual command in `command.value`.
   {
     HealthCheck healthCheckProto;