Add ModifyTime to Allocation and update it both on plan applies and c… #3446
Conversation
…lient initiated updates
nomad/node_endpoint.go
Outdated
| @@ -776,6 +776,10 @@ func (n *Node) UpdateAlloc(args *structs.AllocUpdateRequest, reply *structs.Gene | |||
|
|
|||
| // Add this to the batch | |||
| n.updatesLock.Lock() | |||
| now := time.Now().UTC().UnixNano() | |||
There was a problem hiding this comment.
Can you add a comment on what this code is doing
| @@ -1445,6 +1447,10 @@ func TestClientEndpoint_GetAllocs_Blocking(t *testing.T) { | |||
| t.Fatalf("bad: %#v", resp2.Allocs) | |||
| } | |||
|
|
|||
| if resp2.Allocs[0].ModifyTime != now { | |||
There was a problem hiding this comment.
I'm not sure this is testing anything?
There was a problem hiding this comment.
I had added this test to debug serialization (before I realized there was generated code that needed to be updated). Left it here because it helps catch the problem if you update the struct but not the serializer.
There was a problem hiding this comment.
Although, now that I think about this uses msgpack codec and not the generated codec so I don't know where I was going with that. Seems like this test is still useful as it verifies that the modtime persisted correctly in the state store.
nomad/plan_apply.go
Outdated
| @@ -149,6 +149,8 @@ func (s *Server) applyPlan(plan *structs.Plan, result *structs.PlanResult, snap | |||
| for _, alloc := range req.Alloc { | |||
| if alloc.CreateTime == 0 { | |||
| alloc.CreateTime = now | |||
| } else { | |||
There was a problem hiding this comment.
I think this should always be updated (remove the else and always run). The reason I say this is that when the scheduler first creates the allocation we want CreateTime == ModifyTime
| @@ -226,6 +226,10 @@ func TestPlanApply_applyPlan(t *testing.T) { | |||
| t.Fatalf("missing job") | |||
| } | |||
|
|
|||
| if out.ModifyTime <= 0 { | |||
There was a problem hiding this comment.
Can you test that when it is created it gets a modify time too
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
…lient initiated updates