fix(controller): report Failed when a group fails

checkOverallStatus overwrote the Failed status with Running. A failed workflow
stayed Running and kept requeueing. It now stays Failed and the failure event
fires once. Also remove err shadowing in the visualization client.
This commit is contained in:
2026-10-02 03:59:44 +01:00
parent 5ebe8d8c72
commit bdfc781044
3 changed files with 15 additions and 14 deletions
+12 -8
View File
@@ -311,24 +311,28 @@ func (cp *connPackage) executeJob(j *jobsmanagerv1beta1.ManagedJobDefinition, g
func (cp *connPackage) checkOverallStatus() { func (cp *connPackage) checkOverallStatus() {
groupsCompleted := 0 groupsCompleted := 0
failedGroup := ""
negativeStatuses := []string{ExecutionStatusFailed, ExecutionStatusAborted} negativeStatuses := []string{ExecutionStatusFailed, ExecutionStatusAborted}
for _, group := range cp.mj.Spec.Groups { for _, group := range cp.mj.Spec.Groups {
if group.Status == ExecutionStatusSucceeded { if group.Status == ExecutionStatusSucceeded {
groupsCompleted++ groupsCompleted++
} else if pandati.ExistsInSlice(negativeStatuses, group.Status) { } else if failedGroup == "" && pandati.ExistsInSlice(negativeStatuses, group.Status) {
cp.mj.Status = ExecutionStatusFailed failedGroup = group.Name
cp.r.Recorder.Eventf(cp.mj, corev1.EventTypeWarning, "Failure", "Run failed in group %s", group.Name)
} else {
continue
} }
} }
if groupsCompleted == len(cp.mj.Spec.Groups) { switch {
case failedGroup != "":
if cp.mj.Status != ExecutionStatusFailed {
cp.r.Recorder.Eventf(cp.mj, corev1.EventTypeWarning, "Failure", "Run failed in group %s", failedGroup)
}
cp.mj.Status = ExecutionStatusFailed
case groupsCompleted == len(cp.mj.Spec.Groups):
if cp.mj.Status != ExecutionStatusSucceeded { if cp.mj.Status != ExecutionStatusSucceeded {
cp.r.Recorder.Eventf(cp.mj, corev1.EventTypeNormal, "Success", "Run completed successfuly") cp.r.Recorder.Eventf(cp.mj, corev1.EventTypeNormal, "Success", "Run completed successfully")
} }
cp.mj.Status = ExecutionStatusSucceeded cp.mj.Status = ExecutionStatusSucceeded
} else { default:
cp.mj.Status = ExecutionStatusRunning cp.mj.Status = ExecutionStatusRunning
} }
if err := cp.r.Status().Update(cp.ctx, cp.mj); err != nil { if err := cp.r.Status().Update(cp.ctx, cp.mj); err != nil {
+1 -4
View File
@@ -315,10 +315,7 @@ func (s *ScrapperTestSuite) TestCheckOverallStatus_GroupFailed() {
cp := s.newConnPackage(mj) cp := s.newConnPackage(mj)
cp.checkOverallStatus() cp.checkOverallStatus()
// Note: Current implementation sets running status after checking failed s.Equal(ExecutionStatusFailed, mj.Status)
// This is the actual behavior - the status goes through the failed branch
// but then gets overwritten by the final else block
s.Equal(ExecutionStatusRunning, mj.Status)
} }
func (s *ScrapperTestSuite) TestCheckOverallStatus_GroupRunning() { func (s *ScrapperTestSuite) TestCheckOverallStatus_GroupRunning() {
+2 -2
View File
@@ -25,10 +25,10 @@ func NewClient() (*Client, error) {
} }
scheme := runtime.NewScheme() scheme := runtime.NewScheme()
if err := clientgoscheme.AddToScheme(scheme); err != nil { if err = clientgoscheme.AddToScheme(scheme); err != nil {
return nil, fmt.Errorf("failed to add client-go scheme: %w", err) return nil, fmt.Errorf("failed to add client-go scheme: %w", err)
} }
if err := jobsmanagerv1beta1.AddToScheme(scheme); err != nil { if err = jobsmanagerv1beta1.AddToScheme(scheme); err != nil {
return nil, fmt.Errorf("failed to add jobsmanager scheme: %w", err) return nil, fmt.Errorf("failed to add jobsmanager scheme: %w", err)
} }