Merge pull request #3022 from hashicorp/b-check-validation

Fix timeout validation for script checks

CHANGELOG.md

@@ -21,6 +21,7 @@ BUG FIXES:
  * cli: Fix panic when using 0.6.0 cli with an older cluster [GH-2929]
  * deployment: Fix alloc health with services/checks using interpolation
    [GH-2984]
+ * discovery: Fix timeout validation for script checks [GH-3022]
  * driver/docker: Fix leaking plugin file used by syslog server [GH-2937]
 
 ## 0.6.0 (July 26, 2017)

nomad/structs/structs.go

@@ -2696,28 +2696,15 @@ func (sc *ServiceCheck) Canonicalize(serviceName string) {
 func (sc *ServiceCheck) validate() error {
 	switch strings.ToLower(sc.Type) {
 	case ServiceCheckTCP:
-		if sc.Timeout == 0 {
-			return fmt.Errorf("missing required value timeout. Timeout cannot be less than %v", minCheckInterval)
-		} else if sc.Timeout < minCheckTimeout {
-			return fmt.Errorf("timeout (%v) is lower than required minimum timeout %v", sc.Timeout, minCheckInterval)
-		}
 	case ServiceCheckHTTP:
 		if sc.Path == "" {
 			return fmt.Errorf("http type must have a valid http path")
 		}
 
-		if sc.Timeout == 0 {
-			return fmt.Errorf("missing required value timeout. Timeout cannot be less than %v", minCheckInterval)
-		} else if sc.Timeout < minCheckTimeout {
-			return fmt.Errorf("timeout (%v) is lower than required minimum timeout %v", sc.Timeout, minCheckInterval)
-		}
 	case ServiceCheckScript:
 		if sc.Command == "" {
 			return fmt.Errorf("script type must have a valid script path")
 		}
-
-		// TODO: enforce timeout on the Client side and reenable
-		// validation.
 	default:
 		return fmt.Errorf(`invalid type (%+q), must be one of "http", "tcp", or "script" type`, sc.Type)
 	}
@@ -2728,6 +2715,12 @@ func (sc *ServiceCheck) validate() error {
 		return fmt.Errorf("interval (%v) cannot be lower than %v", sc.Interval, minCheckInterval)
 	}
 
+	if sc.Timeout == 0 {
+		return fmt.Errorf("missing required value timeout. Timeout cannot be less than %v", minCheckInterval)
+	} else if sc.Timeout < minCheckTimeout {
+		return fmt.Errorf("timeout (%v) is lower than required minimum timeout %v", sc.Timeout, minCheckInterval)
+	}
+
 	switch sc.InitialStatus {
 	case "":
 		// case api.HealthUnknown: TODO: Add when Consul releases 0.7.1

nomad/structs/structs_test.go

@@ -1084,15 +1084,26 @@ func TestTask_Validate_Services(t *testing.T) {
 
 func TestTask_Validate_Service_Check(t *testing.T) {
 
+	invalidCheck := ServiceCheck{
+		Name:     "check-name",
+		Command:  "/bin/true",
+		Type:     ServiceCheckScript,
+		Interval: 10 * time.Second,
+	}
+
+	err := invalidCheck.validate()
+	if err == nil || !strings.Contains(err.Error(), "Timeout cannot be less") {
+		t.Fatalf("expected a timeout validation error but received: %q", err)
+	}
+
 	check1 := ServiceCheck{
 		Name:     "check-name",
 		Type:     ServiceCheckTCP,
 		Interval: 10 * time.Second,
 		Timeout:  2 * time.Second,
 	}
 
-	err := check1.validate()
-	if err != nil {
+	if err := check1.validate(); err != nil {
 		t.Fatalf("err: %v", err)
 	}