[YUNIKORN-2188] Improve state transition event to include the eventin…

…fo (apache#730) Closes: apache#730 Signed-off-by: Peter Bacsko <[email protected]>
doupache · Nov 24, 2023 · 4ae99d6 · 4ae99d6
1 parent 88bb82c
commit 4ae99d6
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 34 deletions.
diff --git a/pkg/scheduler/objects/application_events.go b/pkg/scheduler/objects/application_events.go
@@ -117,19 +117,11 @@ func (evt *applicationEvents) sendRemoveApplicationEvent() {
 	evt.eventSystem.AddEvent(event)
 }
 
-func (evt *applicationEvents) sendRejectApplicationEvent(eventInfo string) {
-	if !evt.eventSystem.IsEventTrackingEnabled() {
-		return
-	}
-	event := events.CreateAppEventRecord(evt.app.ApplicationID, eventInfo, "", si.EventRecord_REMOVE, si.EventRecord_APP_REJECT, evt.app.allocatedResource)
-	evt.eventSystem.AddEvent(event)
-}
-
-func (evt *applicationEvents) sendStateChangeEvent(changeDetail si.EventRecord_ChangeDetail) {
+func (evt *applicationEvents) sendStateChangeEvent(changeDetail si.EventRecord_ChangeDetail, eventInfo string) {
 	if !evt.eventSystem.IsEventTrackingEnabled() || !evt.app.sendStateChangeEvents {
 		return
 	}
-	event := events.CreateAppEventRecord(evt.app.ApplicationID, "", "", si.EventRecord_SET, changeDetail, evt.app.allocatedResource)
+	event := events.CreateAppEventRecord(evt.app.ApplicationID, eventInfo, "", si.EventRecord_SET, changeDetail, evt.app.allocatedResource)
 	evt.eventSystem.AddEvent(event)
 }
 

diff --git a/pkg/scheduler/objects/application_events_test.go b/pkg/scheduler/objects/application_events_test.go
@@ -346,47 +346,41 @@ func TestSendRemoveApplicationEvent(t *testing.T) {
 	assert.Equal(t, "", event.Message)
 }
 
-func TestSendRejectApplicationEvent(t *testing.T) {
+func TestSendStateChangeEvent(t *testing.T) {
 	app := &Application{
-		ApplicationID: appID0,
-		queuePath:     "root.test",
+		ApplicationID:         appID0,
+		queuePath:             "root.test",
+		sendStateChangeEvents: true,
 	}
 	mock := newEventSystemMockDisabled()
 	appEvents := newApplicationEvents(app, mock)
-	appEvents.sendRejectApplicationEvent("ResourceReservationTimeout")
+	appEvents.sendStateChangeEvent(si.EventRecord_APP_RUNNING, "")
 	assert.Equal(t, 0, len(mock.events), "unexpected event")
 
 	mock = newEventSystemMock()
 	appEvents = newApplicationEvents(app, mock)
-	appEvents.sendRejectApplicationEvent("ResourceReservationTimeout")
+	appEvents.sendStateChangeEvent(si.EventRecord_APP_RUNNING, "The application is running")
 	event := mock.events[0]
 	assert.Equal(t, si.EventRecord_APP, event.Type)
-	assert.Equal(t, si.EventRecord_REMOVE, event.EventChangeType)
-	assert.Equal(t, si.EventRecord_APP_REJECT, event.EventChangeDetail)
+	assert.Equal(t, si.EventRecord_SET, event.EventChangeType)
+	assert.Equal(t, si.EventRecord_APP_RUNNING, event.EventChangeDetail)
 	assert.Equal(t, "app-0", event.ObjectID)
 	assert.Equal(t, "", event.ReferenceID)
-	assert.Equal(t, "ResourceReservationTimeout", event.Message)
-}
+	assert.Equal(t, "The application is running", event.Message)
 
-func TestSendStateChangeEvent(t *testing.T) {
-	app := &Application{
-		ApplicationID:         appID0,
-		queuePath:             "root.test",
-		sendStateChangeEvents: true,
-	}
-	mock := newEventSystemMockDisabled()
-	appEvents := newApplicationEvents(app, mock)
-	appEvents.sendStateChangeEvent(si.EventRecord_APP_RUNNING)
+	mock = newEventSystemMockDisabled()
+	appEvents = newApplicationEvents(app, mock)
+	appEvents.sendStateChangeEvent(si.EventRecord_APP_RUNNING, "ResourceReservationTimeout")
 	assert.Equal(t, 0, len(mock.events), "unexpected event")
 
 	mock = newEventSystemMock()
 	appEvents = newApplicationEvents(app, mock)
-	appEvents.sendStateChangeEvent(si.EventRecord_APP_RUNNING)
-	event := mock.events[0]
+	appEvents.sendStateChangeEvent(si.EventRecord_APP_REJECT, "Failed to add application to partition (placement rejected)")
+	event = mock.events[0]
 	assert.Equal(t, si.EventRecord_APP, event.Type)
 	assert.Equal(t, si.EventRecord_SET, event.EventChangeType)
-	assert.Equal(t, si.EventRecord_APP_RUNNING, event.EventChangeDetail)
+	assert.Equal(t, si.EventRecord_APP_REJECT, event.EventChangeDetail)
 	assert.Equal(t, "app-0", event.ObjectID)
 	assert.Equal(t, "", event.ReferenceID)
-	assert.Equal(t, "", event.Message)
+	assert.Equal(t, "Failed to add application to partition (placement rejected)", event.Message)
 }
diff --git a/pkg/scheduler/objects/application_state.go b/pkg/scheduler/objects/application_state.go
@@ -149,8 +149,10 @@ func NewAppState() *fsm.FSM {
 					zap.String("source", event.Src),
 					zap.String("destination", event.Dst),
 					zap.String("event", event.Event))
+
+				eventInfo := ""
 				if len(event.Args) == 2 {
-					eventInfo := event.Args[1].(string) //nolint:errcheck
+					eventInfo = event.Args[1].(string) //nolint:errcheck
 					app.OnStateChange(event, eventInfo)
 				} else {
 					app.OnStateChange(event, "")
@@ -161,7 +163,7 @@ func NewAppState() *fsm.FSM {
 						zap.String("state", event.Dst))
 					return
 				}
-				app.appEvents.sendStateChangeEvent(eventDetails)
+				app.appEvents.sendStateChangeEvent(eventDetails, eventInfo)
 			},
 			"leave_state": func(_ context.Context, event *fsm.Event) {
 				event.Args[0].(*Application).clearStateTimer() //nolint:errcheck