Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add timestamps to evals #5881

Merged
merged 10 commits into from
Aug 7, 2019
Merged

add timestamps to evals #5881

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
472c7f0
add timestamps to evals
jazzyfresh Jun 24, 2019
0a72f11
add timestamps to eval instances
jazzyfresh Jul 3, 2019
bd0dcad
add more eval timestamps
jazzyfresh Jul 11, 2019
a8d464c
refactor for code review: adjust correct places to update/create time…
jazzyfresh Jul 22, 2019
9a384b8
tests
jazzyfresh Jul 24, 2019
56e60d3
linting fixes
jazzyfresh Jul 24, 2019
0ce1aca
add pretty printed timestamps to eval status command
jazzyfresh Aug 6, 2019
04b943c
eval CreateTime and ModifyTime are equal if eval is unmodified
jazzyfresh Aug 6, 2019
a405a3a
guarantee eval ModifyTime will always be after CreateTime
jazzyfresh Aug 6, 2019
d855aaa
comment formatting
jazzyfresh Aug 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions api/evaluations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ type Evaluation struct {
SnapshotIndex uint64
CreateIndex uint64
ModifyIndex uint64
CreateTime int64
ModifyTime int64
}

// EvalIndexSort is a wrapper to sort evaluations by CreateIndex.
Expand Down
13 changes: 13 additions & 0 deletions command/eval_status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"sort"
"strings"
"time"

"github.com/hashicorp/nomad/api"
"github.com/hashicorp/nomad/api/contexts"
Expand Down Expand Up @@ -203,9 +204,21 @@ func (c *EvalStatusCommand) Run(args []string) int {
statusDesc = eval.Status
}

// Format eval timestamps
var formattedCreateTime, formattedModifyTime string
if verbose {
formattedCreateTime = formatUnixNanoTime(eval.CreateTime)
formattedModifyTime = formatUnixNanoTime(eval.ModifyTime)
} else {
formattedCreateTime = prettyTimeDiff(time.Unix(0, eval.CreateTime), time.Now())
formattedModifyTime = prettyTimeDiff(time.Unix(0, eval.ModifyTime), time.Now())
}

// Format the evaluation data
basic := []string{
fmt.Sprintf("ID|%s", limit(eval.ID, length)),
fmt.Sprintf("Create Time|%s", formattedCreateTime),
fmt.Sprintf("Modify Time|%s", formattedModifyTime),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pointing out that for old evals this will print an empty string, which may be ok but odd UX.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed it's not the greatest UX. This behavior is consistent with nomad alloc status, although it's unlikely at this point that there are old allocs w/o a CreateTime. Going to think about it, will most likely leave as is but add a note in the docs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving it as is sounds good to me if there's prior art already that does this.

fmt.Sprintf("Status|%s", eval.Status),
fmt.Sprintf("Status Description|%s", statusDesc),
fmt.Sprintf("Type|%s", eval.Type),
Expand Down
3 changes: 3 additions & 0 deletions nomad/alloc_endpoint.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ func (a *Alloc) Stop(args *structs.AllocStopRequest, reply *structs.AllocStopRes
return fmt.Errorf(structs.ErrUnknownAllocationPrefix)
}

now := time.Now().UTC().UnixNano()
eval := &structs.Evaluation{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Someday a helper func for making evals might be nice to ensure we always set the time properly:

func NewEval(namespace, type string) *Evaluation {
    // get time.now, generate uuid, and set basic fields here
}

Not a blocker for this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. This was discussed but I opted not to since a big refactor on all eval creations seemed risky and outside of the scope of this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too bad Go doesn't have constructors! 🐣

ID: uuid.Generate(),
Namespace: alloc.Namespace,
Expand All @@ -244,6 +245,8 @@ func (a *Alloc) Stop(args *structs.AllocStopRequest, reply *structs.AllocStopRes
JobID: alloc.Job.ID,
JobModifyIndex: alloc.Job.ModifyIndex,
Status: structs.EvalStatusPending,
CreateTime: now,
ModifyTime: now,
}

transitionReq := &structs.AllocUpdateDesiredTransitionRequest{
Expand Down
3 changes: 3 additions & 0 deletions nomad/deploymentwatcher/deployment_watcher.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,7 @@ func (w *deploymentWatcher) createBatchedUpdate(allowReplacements []string, forI

// getEval returns an evaluation suitable for the deployment
func (w *deploymentWatcher) getEval() *structs.Evaluation {
now := time.Now().UTC().UnixNano()
return &structs.Evaluation{
ID: uuid.Generate(),
Namespace: w.j.Namespace,
Expand All @@ -800,6 +801,8 @@ func (w *deploymentWatcher) getEval() *structs.Evaluation {
JobID: w.j.ID,
DeploymentID: w.deploymentID,
Status: structs.EvalStatusPending,
CreateTime: now,
ModifyTime: now,
}
}

Expand Down
3 changes: 3 additions & 0 deletions nomad/drainer/drainer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,7 @@ func (n *NodeDrainer) drainAllocs(future *structs.BatchFuture, allocs []*structs
}

evals := make([]*structs.Evaluation, 0, len(jobs))
now := time.Now().UTC().UnixNano()
schmichael marked this conversation as resolved.
Show resolved Hide resolved
for job, alloc := range jobs {
evals = append(evals, &structs.Evaluation{
ID: uuid.Generate(),
Expand All @@ -411,6 +412,8 @@ func (n *NodeDrainer) drainAllocs(future *structs.BatchFuture, allocs []*structs
TriggeredBy: structs.EvalTriggerNodeDrain,
JobID: job,
Status: structs.EvalStatusPending,
CreateTime: now,
ModifyTime: now,
})
}

Expand Down
19 changes: 19 additions & 0 deletions nomad/job_endpoint.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ func (j *Job) Register(args *structs.JobRegisterRequest, reply *structs.JobRegis
}

// Create a new evaluation
now := time.Now().UTC().UnixNano()
eval := &structs.Evaluation{
ID: uuid.Generate(),
Namespace: args.RequestNamespace(),
Expand All @@ -217,6 +218,8 @@ func (j *Job) Register(args *structs.JobRegisterRequest, reply *structs.JobRegis
JobID: args.Job.ID,
JobModifyIndex: reply.JobModifyIndex,
Status: structs.EvalStatusPending,
CreateTime: now,
ModifyTime: now,
}
update := &structs.EvalUpdateRequest{
Evals: []*structs.Evaluation{eval},
Expand Down Expand Up @@ -571,6 +574,7 @@ func (j *Job) Evaluate(args *structs.JobEvaluateRequest, reply *structs.JobRegis
}

// Create a new evaluation
now := time.Now().UTC().UnixNano()
eval := &structs.Evaluation{
ID: uuid.Generate(),
Namespace: args.RequestNamespace(),
Expand All @@ -580,6 +584,8 @@ func (j *Job) Evaluate(args *structs.JobEvaluateRequest, reply *structs.JobRegis
JobID: job.ID,
JobModifyIndex: job.ModifyIndex,
Status: structs.EvalStatusPending,
CreateTime: now,
ModifyTime: now,
}

// Create a AllocUpdateDesiredTransitionRequest request with the eval and any forced rescheduled allocs
Expand Down Expand Up @@ -650,6 +656,7 @@ func (j *Job) Deregister(args *structs.JobDeregisterRequest, reply *structs.JobD
// Create a new evaluation
// XXX: The job priority / type is strange for this, since it's not a high
// priority even if the job was.
now := time.Now().UTC().UnixNano()
eval := &structs.Evaluation{
ID: uuid.Generate(),
Namespace: args.RequestNamespace(),
Expand All @@ -659,6 +666,8 @@ func (j *Job) Deregister(args *structs.JobDeregisterRequest, reply *structs.JobD
JobID: args.JobID,
JobModifyIndex: index,
Status: structs.EvalStatusPending,
CreateTime: now,
ModifyTime: now,
}
update := &structs.EvalUpdateRequest{
Evals: []*structs.Evaluation{eval},
Expand Down Expand Up @@ -738,6 +747,7 @@ func (j *Job) BatchDeregister(args *structs.JobBatchDeregisterRequest, reply *st
}

// Create a new evaluation
now := time.Now().UTC().UnixNano()
eval := &structs.Evaluation{
ID: uuid.Generate(),
Namespace: jobNS.Namespace,
Expand All @@ -746,6 +756,8 @@ func (j *Job) BatchDeregister(args *structs.JobBatchDeregisterRequest, reply *st
TriggeredBy: structs.EvalTriggerJobDeregister,
JobID: jobNS.ID,
Status: structs.EvalStatusPending,
CreateTime: now,
ModifyTime: now,
}
args.Evals = append(args.Evals, eval)
}
Expand Down Expand Up @@ -1195,6 +1207,7 @@ func (j *Job) Plan(args *structs.JobPlanRequest, reply *structs.JobPlanResponse)
}

// Create an eval and mark it as requiring annotations and insert that as well
now := time.Now().UTC().UnixNano()
eval := &structs.Evaluation{
ID: uuid.Generate(),
Namespace: args.RequestNamespace(),
Expand All @@ -1205,6 +1218,9 @@ func (j *Job) Plan(args *structs.JobPlanRequest, reply *structs.JobPlanResponse)
JobModifyIndex: updatedIndex,
Status: structs.EvalStatusPending,
AnnotatePlan: true,
// Timestamps are added for consistency but this eval is never persisted
CreateTime: now,
ModifyTime: now,
}

snap.UpsertEvals(100, []*structs.Evaluation{eval})
Expand Down Expand Up @@ -1415,6 +1431,7 @@ func (j *Job) Dispatch(args *structs.JobDispatchRequest, reply *structs.JobDispa
// If the job is periodic, we don't create an eval.
if !dispatchJob.IsPeriodic() {
// Create a new evaluation
now := time.Now().UTC().UnixNano()
eval := &structs.Evaluation{
ID: uuid.Generate(),
Namespace: args.RequestNamespace(),
Expand All @@ -1424,6 +1441,8 @@ func (j *Job) Dispatch(args *structs.JobDispatchRequest, reply *structs.JobDispa
JobID: dispatchJob.ID,
JobModifyIndex: jobCreateIndex,
Status: structs.EvalStatusPending,
CreateTime: now,
ModifyTime: now,
}
update := &structs.EvalUpdateRequest{
Evals: []*structs.Evaluation{eval},
Expand Down
36 changes: 36 additions & 0 deletions nomad/job_endpoint_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,12 @@ func TestJobEndpoint_Register(t *testing.T) {
if eval.Status != structs.EvalStatusPending {
t.Fatalf("bad: %#v", eval)
}
if eval.CreateTime == 0 {
t.Fatalf("eval CreateTime is unset: %#v", eval)
}
if eval.ModifyTime == 0 {
t.Fatalf("eval ModifyTime is unset: %#v", eval)
}
}

func TestJobEndpoint_Register_ACL(t *testing.T) {
Expand Down Expand Up @@ -302,6 +308,12 @@ func TestJobEndpoint_Register_Existing(t *testing.T) {
if eval.Status != structs.EvalStatusPending {
t.Fatalf("bad: %#v", eval)
}
if eval.CreateTime == 0 {
t.Fatalf("eval CreateTime is unset: %#v", eval)
}
if eval.ModifyTime == 0 {
t.Fatalf("eval ModifyTime is unset: %#v", eval)
}

if err := msgpackrpc.CallWithCodec(codec, "Job.Register", req, &resp); err != nil {
t.Fatalf("err: %v", err)
Expand Down Expand Up @@ -1500,6 +1512,12 @@ func TestJobEndpoint_Evaluate(t *testing.T) {
if eval.Status != structs.EvalStatusPending {
t.Fatalf("bad: %#v", eval)
}
if eval.CreateTime == 0 {
t.Fatalf("eval CreateTime is unset: %#v", eval)
}
if eval.ModifyTime == 0 {
t.Fatalf("eval ModifyTime is unset: %#v", eval)
}
}

func TestJobEndpoint_ForceRescheduleEvaluate(t *testing.T) {
Expand Down Expand Up @@ -1569,6 +1587,8 @@ func TestJobEndpoint_ForceRescheduleEvaluate(t *testing.T) {
require.Equal(eval.JobID, job.ID)
require.Equal(eval.JobModifyIndex, resp.JobModifyIndex)
require.Equal(eval.Status, structs.EvalStatusPending)
require.NotZero(eval.CreateTime)
require.NotZero(eval.ModifyTime)

// Lookup the alloc, verify DesiredTransition ForceReschedule
alloc, err = state.AllocByID(ws, alloc.ID)
Expand Down Expand Up @@ -1647,6 +1667,8 @@ func TestJobEndpoint_Evaluate_ACL(t *testing.T) {
require.Equal(eval.JobID, job.ID)
require.Equal(eval.JobModifyIndex, validResp2.JobModifyIndex)
require.Equal(eval.Status, structs.EvalStatusPending)
require.NotZero(eval.CreateTime)
require.NotZero(eval.ModifyTime)
}

func TestJobEndpoint_Evaluate_Periodic(t *testing.T) {
Expand Down Expand Up @@ -1790,6 +1812,8 @@ func TestJobEndpoint_Deregister(t *testing.T) {
require.Equal(structs.EvalTriggerJobDeregister, eval.TriggeredBy)
require.Equal(job.ID, eval.JobID)
require.Equal(structs.EvalStatusPending, eval.Status)
require.NotZero(eval.CreateTime)
require.NotZero(eval.ModifyTime)

// Deregister and purge
dereg2 := &structs.JobDeregisterRequest{
Expand Down Expand Up @@ -1820,6 +1844,8 @@ func TestJobEndpoint_Deregister(t *testing.T) {
require.Equal(structs.EvalTriggerJobDeregister, eval.TriggeredBy)
require.Equal(job.ID, eval.JobID)
require.Equal(structs.EvalStatusPending, eval.Status)
require.NotZero(eval.CreateTime)
require.NotZero(eval.ModifyTime)
}

func TestJobEndpoint_Deregister_ACL(t *testing.T) {
Expand Down Expand Up @@ -1899,6 +1925,8 @@ func TestJobEndpoint_Deregister_ACL(t *testing.T) {
require.Equal(eval.JobID, job.ID)
require.Equal(eval.JobModifyIndex, validResp2.JobModifyIndex)
require.Equal(eval.Status, structs.EvalStatusPending)
require.NotZero(eval.CreateTime)
require.NotZero(eval.ModifyTime)
}

func TestJobEndpoint_Deregister_Nonexistent(t *testing.T) {
Expand Down Expand Up @@ -1959,6 +1987,12 @@ func TestJobEndpoint_Deregister_Nonexistent(t *testing.T) {
if eval.Status != structs.EvalStatusPending {
t.Fatalf("bad: %#v", eval)
}
if eval.CreateTime == 0 {
t.Fatalf("eval CreateTime is unset: %#v", eval)
}
if eval.ModifyTime == 0 {
t.Fatalf("eval ModifyTime is unset: %#v", eval)
}
}

func TestJobEndpoint_Deregister_Periodic(t *testing.T) {
Expand Down Expand Up @@ -2165,6 +2199,8 @@ func TestJobEndpoint_BatchDeregister(t *testing.T) {
require.Equal(structs.EvalTriggerJobDeregister, eval.TriggeredBy)
require.Equal(expectedJob.ID, eval.JobID)
require.Equal(structs.EvalStatusPending, eval.Status)
require.NotZero(eval.CreateTime)
require.NotZero(eval.ModifyTime)
}
}

Expand Down
3 changes: 3 additions & 0 deletions nomad/leader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,8 +532,10 @@ func (s *Server) reapFailedEvaluations(stopCh chan struct{}) {
// due to the fairly large backoff.
followupEvalWait := s.config.EvalFailedFollowupBaselineDelay +
time.Duration(rand.Int63n(int64(s.config.EvalFailedFollowupDelayRange)))

followupEval := eval.CreateFailedFollowUpEval(followupEvalWait)
updateEval.NextEval = followupEval.ID
updateEval.UpdateModifyTime()

// Update via Raft
req := structs.EvalUpdateRequest{
Expand Down Expand Up @@ -570,6 +572,7 @@ func (s *Server) reapDupBlockedEvaluations(stopCh chan struct{}) {
newEval := dup.Copy()
newEval.Status = structs.EvalStatusCancelled
newEval.StatusDescription = fmt.Sprintf("existing blocked evaluation exists for job %q", newEval.JobID)
newEval.UpdateModifyTime()
cancel[i] = newEval
}

Expand Down
15 changes: 9 additions & 6 deletions nomad/mock/mock.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,13 +402,16 @@ func PeriodicJob() *structs.Job {
}

func Eval() *structs.Evaluation {
now := time.Now().UTC().UnixNano()
eval := &structs.Evaluation{
ID: uuid.Generate(),
Namespace: structs.DefaultNamespace,
Priority: 50,
Type: structs.JobTypeService,
JobID: uuid.Generate(),
Status: structs.EvalStatusPending,
ID: uuid.Generate(),
Namespace: structs.DefaultNamespace,
Priority: 50,
Type: structs.JobTypeService,
JobID: uuid.Generate(),
Status: structs.EvalStatusPending,
CreateTime: now,
ModifyTime: now,
}
return eval
}
Expand Down
10 changes: 10 additions & 0 deletions nomad/node_endpoint.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1038,6 +1038,8 @@ func (n *Node) UpdateAlloc(args *structs.AllocUpdateRequest, reply *structs.Gene
Type: job.Type,
Priority: job.Priority,
Status: structs.EvalStatusPending,
CreateTime: now.UTC().UnixNano(),
ModifyTime: now.UTC().UnixNano(),
}
evals = append(evals, eval)
}
Expand Down Expand Up @@ -1095,6 +1097,9 @@ func (n *Node) batchUpdate(future *structs.BatchFuture, updates []*structs.Alloc
}
_, exists := evalsByJobId[namespacedID]
if !exists {
now := time.Now().UTC().UnixNano()
eval.CreateTime = now
eval.ModifyTime = now
trimmedEvals = append(trimmedEvals, eval)
evalsByJobId[namespacedID] = struct{}{}
}
Expand Down Expand Up @@ -1242,6 +1247,7 @@ func (n *Node) createNodeEvals(nodeID string, nodeIndex uint64) ([]string, uint6
var evals []*structs.Evaluation
var evalIDs []string
jobIDs := make(map[string]struct{})
now := time.Now().UTC().UnixNano()

for _, alloc := range allocs {
// Deduplicate on JobID
Expand All @@ -1261,6 +1267,8 @@ func (n *Node) createNodeEvals(nodeID string, nodeIndex uint64) ([]string, uint6
NodeID: nodeID,
NodeModifyIndex: nodeIndex,
Status: structs.EvalStatusPending,
CreateTime: now,
ModifyTime: now,
}
evals = append(evals, eval)
evalIDs = append(evalIDs, eval.ID)
Expand All @@ -1285,6 +1293,8 @@ func (n *Node) createNodeEvals(nodeID string, nodeIndex uint64) ([]string, uint6
NodeID: nodeID,
NodeModifyIndex: nodeIndex,
Status: structs.EvalStatusPending,
CreateTime: now,
ModifyTime: now,
}
evals = append(evals, eval)
evalIDs = append(evalIDs, eval.ID)
Expand Down