From c24e8ba7d88abd3ab9606b7b62f4be73c783fda2 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 3 Aug 2016 16:58:12 -0700 Subject: [PATCH] Not updating summary if job is de-registered --- nomad/state/state_store.go | 10 +++++++ nomad/state/state_store_test.go | 47 ++++++++++++++++++++++++++++++++- 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/nomad/state/state_store.go b/nomad/state/state_store.go index 062c0e2de..3451386f5 100644 --- a/nomad/state/state_store.go +++ b/nomad/state/state_store.go @@ -1453,6 +1453,16 @@ func (s *StateStore) updateSummaryWithAlloc(index uint64, alloc *structs.Allocat return fmt.Errorf("unable to lookup job summary for job id %q: %v", err) } if summaryRaw == nil { + // Check if the job is de-registered + rawJob, err := txn.First("jobs", "id", alloc.JobID) + if err != nil { + return fmt.Errorf("unable to query job: %v", err) + } + + // If the job is de-registered then we skip updating it's summary + if rawJob == nil { + return nil + } return fmt.Errorf("job summary for job %q is not present", alloc.JobID) } summary := summaryRaw.(structs.JobSummary) diff --git a/nomad/state/state_store_test.go b/nomad/state/state_store_test.go index c7bad31bb..f883a3c49 100644 --- a/nomad/state/state_store_test.go +++ b/nomad/state/state_store_test.go @@ -2073,7 +2073,7 @@ func TestStateStore_ReconcileJobSummary(t *testing.T) { // DeleteJobSummary is a helper method and doesn't modify the indexes table state.DeleteJobSummary(130, alloc.Job.ID) - state.ReconcileJobSummaries() + state.ReconcileJobSummaries(120) summary, _ := state.JobSummaryByID(alloc.Job.ID) expectedSummary := structs.JobSummary{ @@ -2092,6 +2092,51 @@ func TestStateStore_ReconcileJobSummary(t *testing.T) { } } +func TestStateStore_UpdateAlloc_JobNotPresent(t *testing.T) { + state := testStateStore(t) + + alloc := mock.Alloc() + state.UpsertJob(100, alloc.Job) + state.UpsertAllocs(200, []*structs.Allocation{alloc}) + + // Delete the job + state.DeleteJob(300, alloc.Job.ID) + + // Update the alloc + alloc1 := alloc.Copy() + alloc1.ClientStatus = structs.AllocClientStatusRunning + + // Updating allocation should not throw any error + if err := state.UpdateAllocsFromClient(400, []*structs.Allocation{alloc1}); err != nil { + t.Fatalf("expect err: %v", err) + } + + // Re-Register the job + state.UpsertJob(500, alloc.Job) + + // Update the alloc again + alloc2 := alloc.Copy() + alloc2.ClientStatus = structs.AllocClientStatusComplete + if err := state.UpdateAllocsFromClient(400, []*structs.Allocation{alloc1}); err != nil { + t.Fatalf("expect err: %v", err) + } + + // Job Summary of the newly registered job shouldn't account for the + // allocation update for the older job + expectedSummary := structs.JobSummary{ + JobID: alloc1.JobID, + Summary: map[string]structs.TaskGroupSummary{ + "web": structs.TaskGroupSummary{}, + }, + CreateIndex: 500, + ModifyIndex: 500, + } + summary, _ := state.JobSummaryByID(alloc.Job.ID) + if !reflect.DeepEqual(&expectedSummary, summary) { + t.Fatalf("expected: %v, actual: %v", expectedSummary, summary) + } +} + func TestStateStore_EvictAlloc_Alloc(t *testing.T) { state := testStateStore(t) alloc := mock.Alloc()