Skip to content

Commit

Permalink
go: eliminate helpers in favor of min/max
Browse files Browse the repository at this point in the history
  • Loading branch information
shoenig committed Aug 11, 2023
1 parent 284a028 commit 8d53637
Show file tree
Hide file tree
Showing 18 changed files with 37 additions and 111 deletions.
3 changes: 1 addition & 2 deletions client/allocrunner/taskrunner/driver_handle.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"time"

cstructs "github.com/hashicorp/nomad/client/structs"
"github.com/hashicorp/nomad/helper"
"github.com/hashicorp/nomad/nomad/structs"
"github.com/hashicorp/nomad/plugins/drivers"
)
Expand All @@ -26,7 +25,7 @@ func NewDriverHandle(
net: net,
taskID: taskID,
killSignal: task.KillSignal,
killTimeout: helper.Min(task.KillTimeout, maxKillTimeout),
killTimeout: min(task.KillTimeout, maxKillTimeout),
}
}

Expand Down
15 changes: 7 additions & 8 deletions client/allocrunner/taskrunner/getter/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (

"github.com/hashicorp/go-cleanhttp"
"github.com/hashicorp/go-getter"
"github.com/hashicorp/nomad/helper"
"golang.org/x/exp/maps"
"golang.org/x/exp/slices"
)
Expand Down Expand Up @@ -65,13 +64,13 @@ func (p *parameters) read(r io.Reader) error {
// terminated via signal.
func (p *parameters) deadline() time.Duration {
const minimum = 30 * time.Minute
max := minimum
max = helper.Max(max, p.HTTPReadTimeout)
max = helper.Max(max, p.GCSTimeout)
max = helper.Max(max, p.GitTimeout)
max = helper.Max(max, p.HgTimeout)
max = helper.Max(max, p.S3Timeout)
return max + 1*time.Minute
maximum := minimum
maximum = max(maximum, p.HTTPReadTimeout)
maximum = max(maximum, p.GCSTimeout)
maximum = max(maximum, p.GitTimeout)
maximum = max(maximum, p.HgTimeout)
maximum = max(maximum, p.S3Timeout)
return maximum + 1*time.Minute
}

// Equal returns whether p and o are the same.
Expand Down
2 changes: 1 addition & 1 deletion client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -3005,7 +3005,7 @@ func (c *Client) consulDiscoveryImpl() error {
// datacenterQueryLimit, the next heartbeat will pick
// a new set of servers so it's okay.
shuffleStrings(dcs[1:])
dcs = dcs[0:helper.Min(len(dcs), datacenterQueryLimit)]
dcs = dcs[0:min(len(dcs), datacenterQueryLimit)]
}

serviceName := c.GetConfig().ConsulConfig.ServerServiceName
Expand Down
3 changes: 1 addition & 2 deletions command/job_restart.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"github.com/hashicorp/go-set"
"github.com/hashicorp/nomad/api"
"github.com/hashicorp/nomad/api/contexts"
"github.com/hashicorp/nomad/helper"
"github.com/posener/complete"
)

Expand Down Expand Up @@ -370,7 +369,7 @@ func (c *JobRestartCommand) Run(args []string) int {
"[bold]==> %s: Restarting %s batch of %d allocations[reset]",
formatTime(time.Now()),
humanize.Ordinal(batchNumber),
helper.Min(c.batchSize, remaining),
min(c.batchSize, remaining),
)))
}

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/hashicorp/nomad

go 1.20
go 1.21

// Pinned dependencies are noted in github.com/hashicorp/nomad/issues/11826
replace (
Expand Down
2 changes: 1 addition & 1 deletion helper/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func TestCluster_RandomStagger(t *testing.T) {
}

abs := func(d time.Duration) time.Duration {
return Max(d, -d)
return max(d, -d)
}

for _, tc := range cases {
Expand Down
17 changes: 0 additions & 17 deletions helper/funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
multierror "github.com/hashicorp/go-multierror"
"github.com/hashicorp/go-set"
"github.com/hashicorp/hcl/hcl/ast"
"golang.org/x/exp/constraints"
"golang.org/x/exp/maps"
"golang.org/x/exp/slices"
)
Expand Down Expand Up @@ -82,22 +81,6 @@ func HashUUID(input string) (output string, hashed bool) {
return output, true
}

// Min returns the minimum of a and b.
func Min[T constraints.Ordered](a, b T) T {
if a < b {
return a
}
return b
}

// Max returns the maximum of a and b.
func Max[T constraints.Ordered](a, b T) T {
if a > b {
return a
}
return b
}

// UniqueMapSliceValues returns the union of values from each slice in a map[K][]V.
func UniqueMapSliceValues[K, V comparable](m map[K][]V) []V {
s := set.New[V](0)
Expand Down
46 changes: 0 additions & 46 deletions helper/funcs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,52 +15,6 @@ import (
"golang.org/x/exp/maps"
)

func Test_Min(t *testing.T) {
t.Run("int", func(t *testing.T) {
a := 1
b := 2
must.Eq(t, 1, Min(a, b))
must.Eq(t, 1, Min(b, a))
})

t.Run("float64", func(t *testing.T) {
a := 1.1
b := 2.2
must.Eq(t, 1.1, Min(a, b))
must.Eq(t, 1.1, Min(b, a))
})

t.Run("string", func(t *testing.T) {
a := "cat"
b := "dog"
must.Eq(t, "cat", Min(a, b))
must.Eq(t, "cat", Min(b, a))
})
}

func Test_Max(t *testing.T) {
t.Run("int", func(t *testing.T) {
a := 1
b := 2
must.Eq(t, 2, Max(a, b))
must.Eq(t, 2, Max(b, a))
})

t.Run("float64", func(t *testing.T) {
a := 1.1
b := 2.2
must.Eq(t, 2.2, Max(a, b))
must.Eq(t, 2.2, Max(b, a))
})

t.Run("string", func(t *testing.T) {
a := "cat"
b := "dog"
must.Eq(t, "dog", Max(a, b))
must.Eq(t, "dog", Max(b, a))
})
}

func TestIsSubset(t *testing.T) {
l := []string{"a", "b", "c"}
s := []string{"d"}
Expand Down
6 changes: 3 additions & 3 deletions nomad/blocked_evals.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ func latestEvalIndex(eval *structs.Evaluation) uint64 {
return 0
}

return helper.Max(eval.CreateIndex, eval.SnapshotIndex)
return max(eval.CreateIndex, eval.SnapshotIndex)
}

// missedUnblock returns whether an evaluation missed an unblock while it was in
Expand Down Expand Up @@ -545,9 +545,9 @@ func (b *BlockedEvals) unblock(computedClass, quota string, index uint64) {

// Every eval that has escaped computed node class has to be unblocked
// because any node could potentially be feasible.
numEscaped := len(b.escaped)
numQuotaLimit := 0
unblocked := make(map[*structs.Evaluation]string, helper.Max(numEscaped, 4))
numEscaped := len(b.escaped)
unblocked := make(map[*structs.Evaluation]string, max(uint64(numEscaped), 4))

if numEscaped != 0 && computedClass != "" {
for id, wrapped := range b.escaped {
Expand Down
2 changes: 1 addition & 1 deletion nomad/drainer/watch_jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ func handleTaskGroup(snap *state.StateSnapshot, batch bool, tg *structs.TaskGrou
// Determine how many we can drain
thresholdCount := tg.Count - tg.Migrate.MaxParallel
numToDrain := healthy - thresholdCount
numToDrain = helper.Min(len(drainable), numToDrain)
numToDrain = min(len(drainable), numToDrain)
if numToDrain <= 0 {
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion nomad/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -1502,7 +1502,7 @@ func (j *Job) List(args *structs.JobListRequest, reply *structs.JobListResponse)
if err != nil {
return err
}
reply.Index = helper.Max(jindex, sindex)
reply.Index = max(jindex, sindex)

// Set the query response
j.srv.setQueryMeta(&reply.QueryMeta)
Expand Down
10 changes: 4 additions & 6 deletions nomad/node_pool_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ import (
metrics "github.com/armon/go-metrics"
"github.com/hashicorp/go-memdb"
multierror "github.com/hashicorp/go-multierror"

"github.com/hashicorp/nomad/acl"
"github.com/hashicorp/nomad/helper"
"github.com/hashicorp/nomad/nomad/state"
"github.com/hashicorp/nomad/nomad/state/paginator"
"github.com/hashicorp/nomad/nomad/structs"
Expand Down Expand Up @@ -106,7 +104,7 @@ func (n *NodePool) List(args *structs.NodePoolListRequest, reply *structs.NodePo
if err != nil {
return err
}
reply.Index = helper.Max(1, index)
reply.Index = max(1, index)

// Set the query response.
n.srv.setQueryMeta(&reply.QueryMeta)
Expand Down Expand Up @@ -161,7 +159,7 @@ func (n *NodePool) GetNodePool(args *structs.NodePoolSpecificRequest, reply *str
if err != nil {
return err
}
reply.Index = helper.Max(1, index)
reply.Index = max(1, index)
}
return nil
}}
Expand Down Expand Up @@ -503,7 +501,7 @@ func (n *NodePool) ListJobs(args *structs.NodePoolJobsRequest, reply *structs.No
if err != nil {
return err
}
reply.Index = helper.Max(jindex, sindex)
reply.Index = max(jindex, sindex)

// Set the query response
n.srv.setQueryMeta(&reply.QueryMeta)
Expand Down Expand Up @@ -593,7 +591,7 @@ func (n *NodePool) ListNodes(args *structs.NodePoolNodesRequest, reply *structs.
if err != nil {
return err
}
reply.Index = helper.Max(1, index)
reply.Index = max(1, index)

// Set the query response.
n.srv.setQueryMeta(&reply.QueryMeta)
Expand Down
6 changes: 2 additions & 4 deletions nomad/scaling_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@ import (
"github.com/armon/go-metrics"
"github.com/hashicorp/go-hclog"
"github.com/hashicorp/go-memdb"

"github.com/hashicorp/nomad/acl"
"github.com/hashicorp/nomad/helper"
"github.com/hashicorp/nomad/nomad/state"
"github.com/hashicorp/nomad/nomad/structs"
)
Expand Down Expand Up @@ -148,7 +146,7 @@ func (p *Scaling) GetPolicy(args *structs.ScalingPolicySpecificRequest,
if err != nil {
return err
}
reply.Index = helper.Max(1, index)
reply.Index = max(1, index)
}
return nil
}}
Expand Down Expand Up @@ -212,7 +210,7 @@ func (p *Scaling) listAllNamespaces(args *structs.ScalingPolicyListRequest, repl
if err != nil {
return err
}
reply.Index = helper.Max(1, index)
reply.Index = max(1, index)

// Set the query response
p.srv.setQueryMeta(&reply.QueryMeta)
Expand Down
4 changes: 2 additions & 2 deletions nomad/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -977,7 +977,7 @@ func (s *Server) setupBootstrapHandler() error {
// walk all datacenter until it finds enough hosts to
// form a quorum.
shuffleStrings(dcs[1:])
dcs = dcs[0:helper.Min(len(dcs), datacenterQueryLimit)]
dcs = dcs[0:min(len(dcs), datacenterQueryLimit)]
}

nomadServerServiceName := s.config.ConsulConfig.ServerServiceName
Expand Down Expand Up @@ -2010,7 +2010,7 @@ func (s *Server) setReplyQueryMeta(stateStore *state.StateStore, table string, r
if err != nil {
return err
}
reply.Index = helper.Max(1, index)
reply.Index = max(1, index)

// Set the query response.
s.setQueryMeta(reply)
Expand Down
6 changes: 2 additions & 4 deletions nomad/state/state_store_variables.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import (
"math"

"github.com/hashicorp/go-memdb"

"github.com/hashicorp/nomad/helper"
"github.com/hashicorp/nomad/nomad/structs"
)

Expand Down Expand Up @@ -251,7 +249,7 @@ func (s *StateStore) varSetTxn(tx WriteTxn, idx uint64, req *structs.VarApplySta
if quotaChange > 0 {
quotaUsed.Size += quotaChange
} else if quotaChange < 0 {
quotaUsed.Size -= helper.Min(quotaUsed.Size, -quotaChange)
quotaUsed.Size -= min(quotaUsed.Size, -quotaChange)
}

err = s.enforceVariablesQuota(idx, tx, sv.Namespace, quotaChange)
Expand Down Expand Up @@ -392,7 +390,7 @@ func (s *StateStore) svDeleteTxn(tx WriteTxn, idx uint64, req *structs.VarApplyS
if existingQuota != nil {
quotaUsed := existingQuota.(*structs.VariablesQuota)
quotaUsed = quotaUsed.Copy()
quotaUsed.Size -= helper.Min(quotaUsed.Size, int64(len(sv.Data)))
quotaUsed.Size -= min(quotaUsed.Size, int64(len(sv.Data)))
quotaUsed.ModifyIndex = idx
if err := tx.Insert(TableVariablesQuotas, quotaUsed); err != nil {
return req.ErrorResponse(idx, fmt.Errorf("variable quota insert failed: %v", err))
Expand Down
2 changes: 1 addition & 1 deletion nomad/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -9105,7 +9105,7 @@ func (e *TaskEvent) SetValidationError(err error) *TaskEvent {
}

func (e *TaskEvent) SetKillTimeout(timeout, maxTimeout time.Duration) *TaskEvent {
actual := helper.Min(timeout, maxTimeout)
actual := min(timeout, maxTimeout)
e.KillTimeout = actual
e.Details["kill_timeout"] = actual.String()
return e
Expand Down
4 changes: 1 addition & 3 deletions scheduler/propertyset.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import (
log "github.com/hashicorp/go-hclog"
memdb "github.com/hashicorp/go-memdb"
"github.com/hashicorp/go-set"

"github.com/hashicorp/nomad/helper"
"github.com/hashicorp/nomad/nomad/structs"
)

Expand Down Expand Up @@ -263,7 +261,7 @@ func (p *propertySet) UsedCount(option *structs.Node, _ string) (string, string,
// existing and proposed allocations. It also takes into account any stopped
// allocations
func (p *propertySet) GetCombinedUseMap() map[string]uint64 {
combinedUse := make(map[string]uint64, helper.Max(len(p.existingValues), len(p.proposedValues)))
combinedUse := make(map[string]uint64, max(len(p.existingValues), len(p.proposedValues)))
for _, usedValues := range []map[string]uint64{p.existingValues, p.proposedValues} {
for propertyValue, usedCount := range usedValues {
targetPropertyValue := p.targetedPropertyValue(propertyValue)
Expand Down
Loading

0 comments on commit 8d53637

Please sign in to comment.