Skip to content

Commit

Permalink
[YUNIKORN-2323] Gang scheduling user experience issues (#835)
Browse files Browse the repository at this point in the history
Closes: #835

Signed-off-by: Manikandan R <[email protected]>
  • Loading branch information
manirajv06 committed May 15, 2024
1 parent d5d70e3 commit 0c90ea5
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 0 deletions.
10 changes: 10 additions & 0 deletions pkg/cache/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,10 @@ func (app *Application) onReserving() {
// onReservationStateChange is called when there is an add or a release of a placeholder
// If we have all the required placeholders progress the application status, otherwise nothing happens
func (app *Application) onReservationStateChange() {
if app.originatingTask != nil {
events.GetRecorder().Eventf(app.originatingTask.GetTaskPod().DeepCopy(), nil, v1.EventTypeNormal, "GangScheduling",
"Placeholder Allocated", "Application %s placeholder has been allocated.", app.applicationID)
}
desireCounts := make(map[string]int32, len(app.taskGroups))
for _, tg := range app.taskGroups {
desireCounts[tg.Name] = tg.MinMember
Expand All @@ -542,6 +546,12 @@ func (app *Application) onReservationStateChange() {
return
}
}

if app.originatingTask != nil {
// Now that all placeholders has been allocated, send a final conclusion message
events.GetRecorder().Eventf(app.originatingTask.GetTaskPod().DeepCopy(), nil, v1.EventTypeNormal, "GangScheduling",
"Gang reservations completed. All placeholders are allocated.", "Application %s all placeholders are allocated. Transitioning to running state.", app.applicationID)
}
dispatcher.Dispatch(NewRunApplicationEvent(app.applicationID))
}

Expand Down
28 changes: 28 additions & 0 deletions pkg/cache/application_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1220,6 +1220,11 @@ func TestApplication_onReservationStateChange(t *testing.T) {
dispatcher.Start()
defer dispatcher.Stop()

recorder, ok := events.GetRecorder().(*k8sEvents.FakeRecorder)
if !ok {
t.Fatal("the EventRecorder is expected to be of type FakeRecorder")
}

app := NewApplication(appID, "root.a", "testuser", testGroups, map[string]string{}, newMockSchedulerAPI())
context.addApplicationToContext(app)

Expand Down Expand Up @@ -1265,6 +1270,7 @@ func TestApplication_onReservationStateChange(t *testing.T) {
app.addTask(task1)
app.addTask(task2)
app.addTask(task3)
app.setOriginatingTask(task1)

// app stays in accepted with taskgroups defined none bound
app.onReservationStateChange()
Expand All @@ -1287,6 +1293,28 @@ func TestApplication_onReservationStateChange(t *testing.T) {
task3.setTaskGroupName("test-group-2")
app.onReservationStateChange()
assertAppState(t, app, ApplicationStates().Running, 1*time.Second)

message := "placeholder has been allocated"
reason := "GangScheduling"
counter := 0
// check that the event has been published
err := utils.WaitForCondition(func() bool {
for {
select {
case event := <-recorder.Events:
print(event)
if strings.Contains(event, reason) && strings.Contains(event, message) {
counter++
if counter == 4 {
return true
}
}
default:
return false
}
}
}, 5*time.Millisecond, time.Second)
assert.NilError(t, err, "event should have been emitted")
}

func TestTaskRemoval(t *testing.T) {
Expand Down

0 comments on commit 0c90ea5

Please sign in to comment.