Enforce bounds on MaxQueryTime (hashicorp#9064)

The MaxQueryTime value used in QueryOptions.HasTimedOut() can be set to
an invalid value that would throw off how RPC requests are retried.

This fix uses the same logic that enforces the MaxQueryTime bounds in the
blockingRPC() call.

nomad/rpc.go

@@ -31,13 +31,6 @@ import (
 )
 
 const (
-	// maxQueryTime is used to bound the limit of a blocking query
-	maxQueryTime = 300 * time.Second
-
-	// defaultQueryTime is the amount of time we block waiting for a change
-	// if no time is specified. Previously we would wait the maxQueryTime.
-	defaultQueryTime = 300 * time.Second
-
 	// Warn if the Raft command is larger than this.
 	// If it's over 1MB something is probably being abusive.
 	raftWarnSize = 1024 * 1024
@@ -788,10 +781,10 @@ func (r *rpcHandler) blockingRPC(opts *blockingOptions) error {
 	}
 
 	// Restrict the max query time, and ensure there is always one
-	if opts.queryOpts.MaxQueryTime > maxQueryTime {
-		opts.queryOpts.MaxQueryTime = maxQueryTime
+	if opts.queryOpts.MaxQueryTime > structs.MaxBlockingRPCQueryTime {
+		opts.queryOpts.MaxQueryTime = structs.MaxBlockingRPCQueryTime
 	} else if opts.queryOpts.MaxQueryTime <= 0 {
-		opts.queryOpts.MaxQueryTime = defaultQueryTime
+		opts.queryOpts.MaxQueryTime = structs.DefaultBlockingRPCQueryTime
 	}
 
 	// Apply a small amount of jitter to the request

nomad/structs/structs.go

@@ -24,6 +24,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/hashicorp/consul/lib"
 	"github.com/hashicorp/cronexpr"
 	"github.com/hashicorp/go-msgpack/codec"
 	"github.com/hashicorp/go-multierror"
@@ -163,6 +164,13 @@ const (
 
 	// Normalized scorer name
 	NormScorerName = "normalized-score"
+
+	// MaxBlockingRPCQueryTime is used to bound the limit of a blocking query
+	MaxBlockingRPCQueryTime = 300 * time.Second
+
+	// DefaultBlockingRPCQueryTime is the amount of time we block waiting for a change
+	// if no time is specified. Previously we would wait the MaxBlockingRPCQueryTime.
+	DefaultBlockingRPCQueryTime = 300 * time.Second
 )
 
 // Context defines the scope in which a search for Nomad object operates, and
@@ -289,6 +297,14 @@ func (q QueryOptions) AllowStaleRead() bool {
 
 func (q QueryOptions) HasTimedOut(start time.Time, rpcHoldTimeout time.Duration) bool {
 	if q.MinQueryIndex > 0 {
+		// Restrict the max query time, and ensure there is always one
+		if q.MaxQueryTime > MaxBlockingRPCQueryTime {
+			q.MaxQueryTime = MaxBlockingRPCQueryTime
+		} else if q.MaxQueryTime <= 0 {
+			q.MaxQueryTime = DefaultBlockingRPCQueryTime
+		}
+		q.MaxQueryTime += lib.RandomStagger(q.MaxQueryTime / JitterFraction)
+
 		return time.Since(start) > (q.MaxQueryTime + rpcHoldTimeout)
 	}
 	return time.Since(start) > rpcHoldTimeout