Skip to content

Commit

Permalink
[KS-590][bugfix] Auto-approval of workflow deletion - use correct ID (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
bolekk authored Dec 4, 2024
1 parent 52676f1 commit c7cb608
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 9 deletions.
16 changes: 11 additions & 5 deletions core/services/feeds/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -529,12 +529,18 @@ func (s *service) DeleteJob(ctx context.Context, args *DeleteJobArgs) (int64, er
return proposal.ID, nil
}
if job.WorkflowSpecID != nil { // this is a Workflow job
specID := int64(*job.WorkflowSpecID)
if err := s.CancelSpec(ctx, proposal.ID); err != nil {
logger.Errorw("Failed to auto-cancel workflow spec", "id", specID, "err", err, "name", job.Name)
return 0, fmt.Errorf("failed to auto-cancel workflow spec %d: %w", specID, err)
jobSpecID := int64(*job.WorkflowSpecID)
jpSpec, err2 := s.orm.GetApprovedSpec(ctx, proposal.ID)
if err2 != nil {
logger.Errorw("GetApprovedSpec failed - no approved specs to cancel?", "id", proposal.ID, "err", err2, "name", job.Name)
// return success if there are no approved specs to cancel
return proposal.ID, nil
}
logger.Infow("Successfully auto-cancelled a workflow spec", "id", specID)
if err := s.CancelSpec(ctx, jpSpec.ID); err != nil {
logger.Errorw("Failed to auto-cancel workflow spec", "jobProposalID", proposal.ID, "jobProposalSpecID", jpSpec.ID, "jobSpecID", jobSpecID, "err", err, "name", job.Name)
return 0, fmt.Errorf("failed to auto-cancel workflow spec (job proposal spec ID: %d): %w", jpSpec.ID, err)
}
logger.Infow("Successfully auto-cancelled a workflow spec", "jobProposalID", proposal.ID, "jobProposalSpecID", jpSpec.ID, "jobSpecID", jobSpecID, "name", job.Name)
}

return proposal.ID, nil
Expand Down
9 changes: 5 additions & 4 deletions core/services/feeds/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1341,7 +1341,7 @@ func Test_Service_DeleteJob(t *testing.T) {
ID: 1,
WorkflowSpecID: &wfSpecID,
}
spec = &feeds.JobProposalSpec{
jobProposalSpec = &feeds.JobProposalSpec{
ID: 20,
Status: feeds.SpecStatusApproved,
JobProposalID: approved.ID,
Expand Down Expand Up @@ -1415,21 +1415,22 @@ func Test_Service_DeleteJob(t *testing.T) {
svc.orm.On("DeleteProposal", mock.Anything, approved.ID).Return(nil)
svc.orm.On("CountJobProposalsByStatus", mock.Anything).Return(&feeds.JobProposalCounts{}, nil)
svc.jobORM.On("FindJobByExternalJobID", mock.Anything, approved.ExternalJobID.UUID).Return(workflowJob, nil)
svc.orm.On("GetApprovedSpec", mock.Anything, approved.ID).Return(jobProposalSpec, nil)

// mocks for CancelSpec()
svc.orm.On("GetSpec", mock.Anything, approved.ID).Return(spec, nil)
svc.orm.On("GetSpec", mock.Anything, jobProposalSpec.ID).Return(jobProposalSpec, nil)
svc.orm.On("GetJobProposal", mock.Anything, approved.ID).Return(&approved, nil)
svc.connMgr.On("GetClient", mock.Anything).Return(svc.fmsClient, nil)

svc.orm.On("CancelSpec", mock.Anything, approved.ID).Return(nil)
svc.orm.On("CancelSpec", mock.Anything, jobProposalSpec.ID).Return(nil)
svc.jobORM.On("FindJobByExternalJobID", mock.Anything, approved.ExternalJobID.UUID).Return(workflowJob, nil)
svc.spawner.On("DeleteJob", mock.Anything, mock.Anything, workflowJob.ID).Return(nil)

svc.fmsClient.On("CancelledJob",
mock.MatchedBy(func(ctx context.Context) bool { return true }),
&proto.CancelledJobRequest{
Uuid: approved.RemoteUUID.String(),
Version: int64(spec.Version),
Version: int64(jobProposalSpec.Version),
},
).Return(&proto.CancelledJobResponse{}, nil)
svc.orm.On("CountJobProposalsByStatus", mock.Anything).Return(&feeds.JobProposalCounts{}, nil)
Expand Down

0 comments on commit c7cb608

Please sign in to comment.