Skip to content

Commit

Permalink
fix: OnAdd() panics when called for running job (#12)
Browse files Browse the repository at this point in the history
  • Loading branch information
lucaspin authored Jan 31, 2024
1 parent 46e2e12 commit c528bbe
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 5 deletions.
5 changes: 1 addition & 4 deletions pkg/controller/job_scheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,15 +310,12 @@ func (s *JobScheduler) isJobRunning(logger logr.Logger, jobID string, job *batch
return s.kubernetesMajorVersion, s.kubernetesMinorVersion
})

if running {
s.current[jobID].Running = true
}

return running
}

func (s *JobScheduler) handleInProgress(logger logr.Logger, jobID string, job *batchv1.Job) {
if s.isJobRunning(logger, jobID, job) {
s.current[jobID].Running = true
logger.Info("Job is running", "for", time.Since(job.Status.StartTime.Time))
return
}
Expand Down
27 changes: 26 additions & 1 deletion pkg/controller/job_scheduler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func Test__JobScheduler(t *testing.T) {

require.NoError(t, err)

t.Run("job is loaded on startup", func(t *testing.T) {
t.Run("non-running job is loaded on startup", func(t *testing.T) {
clear(scheduler.current)
defer clear(scheduler.current)

Expand All @@ -62,6 +62,31 @@ func Test__JobScheduler(t *testing.T) {
require.True(t, scheduler.IsCurrentJob(jobID))
})

t.Run("running job is loaded on startup", func(t *testing.T) {
clear(scheduler.current)
defer clear(scheduler.current)

jobID := randJobID()
require.False(t, scheduler.IsCurrentJob(jobID))

ready := int32(1)
j := &batchv1.Job{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
config.JobIDLabel: jobID,
config.AgentTypeLabel: agentType.AgentTypeName,
},
},
Status: batchv1.JobStatus{
Ready: &ready,
},
}

scheduler.OnAdd(j, false)
require.True(t, scheduler.IsCurrentJob(jobID))
require.True(t, scheduler.current[jobID].Running)
})

t.Run("job is created", func(t *testing.T) {
clear(scheduler.current)
defer clear(scheduler.current)
Expand Down

0 comments on commit c528bbe

Please sign in to comment.