Merge pull request #974 from hashicorp/b-remove-gc-field

Remove the GC field on the job and use the job type

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)

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 {

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

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
 }
 

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

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 {

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.

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)
 	}