Merge pull request #1912 from hashicorp/b-validation

Change the 0 timeout, interval validation to be more user friendly

nomad/structs/structs.go

@@ -1785,7 +1785,9 @@ func (sc *ServiceCheck) Canonicalize(serviceName string) {
 func (sc *ServiceCheck) validate() error {
 	switch strings.ToLower(sc.Type) {
 	case ServiceCheckTCP:
-		if sc.Timeout < minCheckTimeout {
+		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:
@@ -1793,7 +1795,9 @@ func (sc *ServiceCheck) validate() error {
 			return fmt.Errorf("http type must have a valid http path")
 		}
 
-		if sc.Timeout < minCheckTimeout {
+		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:
@@ -1807,8 +1811,10 @@ func (sc *ServiceCheck) validate() error {
 		return fmt.Errorf(`invalid type (%+q), must be one of "http", "tcp", or "script" type`, sc.Type)
 	}
 
-	if sc.Interval < minCheckInterval {
-		return fmt.Errorf("interval (%v) can not be lower than %v", sc.Interval, minCheckInterval)
+	if sc.Interval == 0 {
+		return fmt.Errorf("missing required value interval. Interval cannot be less than %v", minCheckInterval)
+	} else if sc.Interval < minCheckInterval {
+		return fmt.Errorf("interval (%v) cannot be lower than %v", sc.Interval, minCheckInterval)
 	}
 
 	switch sc.InitialStatus {
@@ -2150,7 +2156,7 @@ func (t *Task) Validate(ephemeralDisk *EphemeralDisk) error {
 	if strings.ContainsAny(t.Name, `/\`) {
 		// We enforce this so that when creating the directory on disk it will
 		// not have any slashes.
-		mErr.Errors = append(mErr.Errors, errors.New("Task name can not include slashes"))
+		mErr.Errors = append(mErr.Errors, errors.New("Task name cannot include slashes"))
 	}
 	if t.Driver == "" {
 		mErr.Errors = append(mErr.Errors, errors.New("Missing task driver"))
@@ -2759,7 +2765,7 @@ func (ta *TaskArtifact) Validate() error {
 	if check, ok := ta.GetterOptions["checksum"]; ok {
 		check = strings.TrimSpace(check)
 		if check == "" {
-			mErr.Errors = append(mErr.Errors, fmt.Errorf("checksum value can not be empty"))
+			mErr.Errors = append(mErr.Errors, fmt.Errorf("checksum value cannot be empty"))
 			return mErr.ErrorOrNil()
 		}
 
@@ -2955,7 +2961,7 @@ func (v *Vault) Validate() error {
 	}
 
 	if len(v.Policies) == 0 {
-		return fmt.Errorf("Policy list can not be empty")
+		return fmt.Errorf("Policy list cannot be empty")
 	}
 
 	switch v.ChangeMode {

nomad/structs/structs_test.go

@@ -500,6 +500,11 @@ func TestTask_Validate_Services(t *testing.T) {
 				Type:    ServiceCheckTCP,
 				Timeout: 2 * time.Second,
 			},
+			{
+				Name:     "check-name",
+				Type:     ServiceCheckTCP,
+				Interval: 1 * time.Second,
+			},
 		},
 	}
 
@@ -536,7 +541,11 @@ func TestTask_Validate_Services(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
-	if !strings.Contains(err.Error(), "interval (0s) can not be lower") {
+	if !strings.Contains(err.Error(), "missing required value interval") {
+		t.Fatalf("err: %v", err)
+	}
+
+	if !strings.Contains(err.Error(), "cannot be less than") {
 		t.Fatalf("err: %v", err)
 	}
 }