Skip to content

Commit

Permalink
Fix non existing dependent job (#1297)
Browse files Browse the repository at this point in the history
* Update CHANGELOG

* Fix panic on non-existent dependent job
  • Loading branch information
Victor Castell authored Mar 17, 2023
1 parent c12588c commit 18dc23d
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 32 deletions.
2 changes: 1 addition & 1 deletion dkron/grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,10 +246,10 @@ func (grpcs *GRPCServer) ExecutionDone(ctx context.Context, execDoneReq *proto.E
if len(job.DependentJobs) > 0 && job.Status == StatusSuccess {
for _, djn := range job.DependentJobs {
dj, err := grpcs.agent.Store.GetJob(djn, nil)
dj.Agent = grpcs.agent
if err != nil {
return nil, err
}
dj.Agent = grpcs.agent
grpcs.logger.WithField("job", djn).Debug("grpc: Running dependent job")
dj.Run()
}
Expand Down
83 changes: 52 additions & 31 deletions dkron/grpc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func TestGRPCExecutionDone(t *testing.T) {
require.NoError(t, err)

testExecution := &Execution{
JobName: "test",
JobName: testJob.Name,
Group: time.Now().UnixNano(),
StartedAt: time.Now(),
NodeName: "testNode",
Expand All @@ -76,45 +76,66 @@ func TestGRPCExecutionDone(t *testing.T) {

log := getTestLogger()
rc := NewGRPCClient(nil, a, log)
rc.ExecutionDone(a.advertiseRPCAddr(), testExecution)
execs, err := a.Store.GetExecutions("test", &ExecutionOptions{})
require.NoError(t, err)

assert.Len(t, execs, 1)
assert.Equal(t, string(testExecution.Output), string(execs[0].Output))
t.Run("Should run job", func(t *testing.T) {
err = rc.ExecutionDone(a.advertiseRPCAddr(), testExecution)
require.NoError(t, err)

// Test run a dependent job
execs, err = a.Store.GetExecutions("child-test", &ExecutionOptions{})
require.NoError(t, err)
execs, err := a.Store.GetExecutions("test", &ExecutionOptions{})
require.NoError(t, err)

assert.Len(t, execs, 1)
assert.Len(t, execs, 1)
assert.Equal(t, string(testExecution.Output), string(execs[0].Output))
})

// Test job with dependents no delete
_, err = a.Store.DeleteJob(testJob.Name)
require.Error(t, err)
t.Run("Should run a dependent job", func(t *testing.T) {
execs, err := a.Store.GetExecutions("child-test", &ExecutionOptions{})
require.NoError(t, err)

// Remove dependents and parent
_, err = a.Store.DeleteJob(testChildJob.Name)
require.NoError(t, err)
_, err = a.Store.DeleteJob(testJob.Name)
require.NoError(t, err)
assert.Len(t, execs, 1)
})

// Test store execution on a deleted job
testExecution.FinishedAt = time.Now()
err = rc.ExecutionDone(a.advertiseRPCAddr(), testExecution)
t.Run("Should store execution on a deleted job", func(t *testing.T) {
// Test job with dependents no delete
_, err = a.Store.DeleteJob(testJob.Name)
require.Error(t, err)

assert.Error(t, err, ErrExecutionDoneForDeletedJob)
// Remove dependents and parent
_, err = a.Store.DeleteJob(testChildJob.Name)
require.NoError(t, err)
_, err = a.Store.DeleteJob(testJob.Name)
require.NoError(t, err)

// Test ephemeral jobs
testJob.Ephemeral = true
// Test store execution on a deleted job
testExecution.FinishedAt = time.Now()
err = rc.ExecutionDone(a.advertiseRPCAddr(), testExecution)

err = a.Store.SetJob(testJob, true)
require.NoError(t, err)
assert.Error(t, err, ErrExecutionDoneForDeletedJob)
})

t.Run("Test ephemeral jobs", func(t *testing.T) {
testJob.Ephemeral = true

err = a.Store.SetJob(testJob, true)
require.NoError(t, err)

err = rc.ExecutionDone(a.advertiseRPCAddr(), testExecution)
assert.NoError(t, err)

j, err := a.Store.GetJob("test", nil)
assert.Error(t, err)
assert.Nil(t, j)
})

t.Run("Test job with non-existent dependent", func(t *testing.T) {
testJob.Name = "test2"
testJob.DependentJobs = []string{"non-existent"}
testExecution.JobName = testJob.Name

err = rc.ExecutionDone(a.advertiseRPCAddr(), testExecution)
assert.NoError(t, err)
err = a.Store.SetJob(testJob, true)
require.NoError(t, err)

j, err := a.Store.GetJob("test", nil)
assert.Error(t, err)
assert.Nil(t, j)
err = rc.ExecutionDone(a.advertiseRPCAddr(), testExecution)
assert.Error(t, err)
})
}

0 comments on commit 18dc23d

Please sign in to comment.