From 80a1fa0e11681d039c30bfd3f8bbb1440adf7239 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 23 Mar 2016 18:02:01 -0700 Subject: [PATCH] remove the GC field on the job and use the job type --- nomad/core_sched_test.go | 4 +-- nomad/job_endpoint.go | 15 -------- nomad/job_endpoint_test.go | 62 --------------------------------- nomad/periodic.go | 1 - nomad/state/schema.go | 5 ++- nomad/state/state_store_test.go | 11 ++++-- nomad/structs/structs.go | 9 ----- nomad/system_endpoint_test.go | 2 +- 8 files changed, 15 insertions(+), 94 deletions(-) diff --git a/nomad/core_sched_test.go b/nomad/core_sched_test.go index 249b071f995..bc6275f60c2 100644 --- a/nomad/core_sched_test.go +++ b/nomad/core_sched_test.go @@ -238,7 +238,7 @@ func TestCoreScheduler_JobGC(t *testing.T) { // Insert job. state := s1.fsm.State() job := mock.Job() - job.GC = true + job.Type = structs.JobTypeBatch err := state.UpsertJob(1000, job) if err != nil { t.Fatalf("test(%s) err: %v", test.test, err) @@ -342,7 +342,7 @@ func TestCoreScheduler_JobGC_Force(t *testing.T) { // Insert job. state := s1.fsm.State() job := mock.Job() - job.GC = true + job.Type = structs.JobTypeBatch err := state.UpsertJob(1000, job) if err != nil { t.Fatalf("test(%s) err: %v", test.test, err) diff --git a/nomad/job_endpoint.go b/nomad/job_endpoint.go index eb6205e06e0..44dee6e2ff6 100644 --- a/nomad/job_endpoint.go +++ b/nomad/job_endpoint.go @@ -1,7 +1,6 @@ package nomad import ( - "errors" "fmt" "time" @@ -28,10 +27,6 @@ func (j *Job) Register(args *structs.JobRegisterRequest, reply *structs.JobRegis return fmt.Errorf("missing job for registration") } - if err := j.checkBlacklist(args.Job); err != nil { - return err - } - // Initialize the job fields (sets defaults and any necessary init work). args.Job.InitFields() @@ -89,16 +84,6 @@ func (j *Job) Register(args *structs.JobRegisterRequest, reply *structs.JobRegis return nil } -// checkBlacklist returns an error if the user has set any blacklisted field in -// the job. -func (j *Job) checkBlacklist(job *structs.Job) error { - if job.GC { - return errors.New("GC field of a job is used only internally and should not be set by user") - } - - return nil -} - // Evaluate is used to force a job for re-evaluation func (j *Job) Evaluate(args *structs.JobEvaluateRequest, reply *structs.JobRegisterResponse) error { if done, err := j.srv.forward("Job.Evaluate", args, args, reply); done { diff --git a/nomad/job_endpoint_test.go b/nomad/job_endpoint_test.go index 7c313e8d01a..87696c5e19d 100644 --- a/nomad/job_endpoint_test.go +++ b/nomad/job_endpoint_test.go @@ -171,68 +171,6 @@ func TestJobEndpoint_Register_Existing(t *testing.T) { } } -func TestJobEndpoint_Register_Batch(t *testing.T) { - s1 := testServer(t, func(c *Config) { - c.NumSchedulers = 0 // Prevent automatic dequeue - }) - defer s1.Shutdown() - codec := rpcClient(t, s1) - testutil.WaitForLeader(t, s1.RPC) - - // Create the register request - job := mock.Job() - job.Type = structs.JobTypeBatch - req := &structs.JobRegisterRequest{ - Job: job, - WriteRequest: structs.WriteRequest{Region: "global"}, - } - - // Fetch the response - var resp structs.JobRegisterResponse - if err := msgpackrpc.CallWithCodec(codec, "Job.Register", req, &resp); err != nil { - t.Fatalf("err: %v", err) - } - if resp.Index == 0 { - t.Fatalf("bad index: %d", resp.Index) - } - - // Check for the node in the FSM - state := s1.fsm.State() - out, err := state.JobByID(job.ID) - if err != nil { - t.Fatalf("err: %v", err) - } - if out == nil { - t.Fatalf("expected job") - } - if !out.GC { - t.Fatal("expect batch job to be made garbage collectible") - } -} - -func TestJobEndpoint_Register_GC_Set(t *testing.T) { - s1 := testServer(t, func(c *Config) { - c.NumSchedulers = 0 // Prevent automatic dequeue - }) - defer s1.Shutdown() - codec := rpcClient(t, s1) - testutil.WaitForLeader(t, s1.RPC) - - // Create the register request - job := mock.Job() - job.GC = true - req := &structs.JobRegisterRequest{ - Job: job, - WriteRequest: structs.WriteRequest{Region: "global"}, - } - - // Fetch the response - var resp structs.JobRegisterResponse - if err := msgpackrpc.CallWithCodec(codec, "Job.Register", req, &resp); err == nil { - t.Fatalf("expect err") - } -} - func TestJobEndpoint_Register_Periodic(t *testing.T) { s1 := testServer(t, func(c *Config) { c.NumSchedulers = 0 // Prevent automatic dequeue diff --git a/nomad/periodic.go b/nomad/periodic.go index 4cba0e845dd..ccbdd809d7d 100644 --- a/nomad/periodic.go +++ b/nomad/periodic.go @@ -413,7 +413,6 @@ func (p *PeriodicDispatch) deriveJob(periodicJob *structs.Job, time time.Time) ( derived.ID = p.derivedJobID(periodicJob, time) derived.Name = derived.ID derived.Periodic = nil - derived.GC = true return } diff --git a/nomad/state/schema.go b/nomad/state/schema.go index 10553856d86..35d201b9fc5 100644 --- a/nomad/state/schema.go +++ b/nomad/state/schema.go @@ -129,7 +129,10 @@ func jobIsGCable(obj interface{}) (bool, error) { return false, fmt.Errorf("Unexpected type: %v", obj) } - return j.GC, nil + // The job is GCable if it is batch and it is not periodic + periodic := j.Periodic != nil && j.Periodic.Enabled + gcable := j.Type == structs.JobTypeBatch && !periodic + return gcable, nil } // jobIsPeriodic satisfies the ConditionalIndexFunc interface and creates an index diff --git a/nomad/state/state_store_test.go b/nomad/state/state_store_test.go index f7d8351d287..22166ef1cec 100644 --- a/nomad/state/state_store_test.go +++ b/nomad/state/state_store_test.go @@ -681,8 +681,13 @@ func TestStateStore_JobsByGC(t *testing.T) { state := testStateStore(t) var gc, nonGc []*structs.Job - for i := 0; i < 10; i++ { - job := mock.Job() + for i := 0; i < 20; i++ { + var job *structs.Job + if i%2 == 0 { + job = mock.Job() + } else { + job = mock.PeriodicJob() + } nonGc = append(nonGc, job) if err := state.UpsertJob(1000+uint64(i), job); err != nil { @@ -692,7 +697,7 @@ func TestStateStore_JobsByGC(t *testing.T) { for i := 0; i < 10; i++ { job := mock.Job() - job.GC = true + job.Type = structs.JobTypeBatch gc = append(gc, job) if err := state.UpsertJob(2000+uint64(i), job); err != nil { diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index c92841a9505..8c13a06839d 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -900,10 +900,6 @@ type Job struct { // Periodic is used to define the interval the job is run at. Periodic *PeriodicConfig - // GC is used to mark the job as available for garbage collection after it - // has no outstanding evaluations or allocations. - GC bool - // Meta is used to associate arbitrary metadata with this // job. This is opaque to Nomad. Meta map[string]string @@ -926,11 +922,6 @@ func (j *Job) InitFields() { for _, tg := range j.TaskGroups { tg.InitFields(j) } - - // If the job is batch then make it GC. - if j.Type == JobTypeBatch { - j.GC = true - } } // Copy returns a deep copy of the Job. It is expected that callers use recover. diff --git a/nomad/system_endpoint_test.go b/nomad/system_endpoint_test.go index 746ed27e50f..4302d9435dc 100644 --- a/nomad/system_endpoint_test.go +++ b/nomad/system_endpoint_test.go @@ -22,7 +22,7 @@ func TestSystemEndpoint_GarbageCollect(t *testing.T) { // Insert a job that can be GC'd state := s1.fsm.State() job := mock.Job() - job.GC = true + job.Type = structs.JobTypeBatch if err := state.UpsertJob(0, job); err != nil { t.Fatalf("UpsertAllocs() failed: %v", err) }