[GitHub] [yunikorn-k8shim] bgrams commented on a diff in pull request #598: [YUNIKORN-1555] Deduplicate async pod events during recovery

["bgrams (via GitHub)" <gi...@apache.org>]

bgrams commented on code in PR #598:
URL: https://github.com/apache/yunikorn-k8shim/pull/598#discussion_r1203435160


##########
pkg/appmgmt/general/podevent_handler_test.go:
##########
@@ -73,21 +73,48 @@ func TestHandleAsyncEventWhenNotRecovering(t *testing.T) {
 func TestRecoveryDone(t *testing.T) {
 	amProtocol := cache.NewMockedAMProtocol()
 	podEventHandler := NewPodEventHandler(amProtocol, true)
+
 	pod1 := newPod("pod1")
+	pod2 := newPod("pod2")
+	pod3 := newPod("pod3")
+	pod3v1 := newPodWithResourceVersion(pod3.GetName(), "1")
+	pod3v2 := newPodWithResourceVersion(pod3.GetName(), "2")
+
 	podEventHandler.HandleEvent(AddPod, Informers, pod1)
+	podEventHandler.HandleEvent(AddPod, Informers, pod2)
 	podEventHandler.HandleEvent(DeletePod, Informers, pod1)
+	podEventHandler.HandleEvent(AddPod, Informers, pod3v1)
+	podEventHandler.HandleEvent(UpdatePod, Informers, pod3v2)
 
-	podEventHandler.RecoveryDone()
+	seenEvents := map[types.UID]string{
+		pod2.UID: pod2.GetResourceVersion(), // should not be added
+		pod3.UID: pod3.GetResourceVersion(), // should be updated with new version and ultimately completed
+	}
+	podEventHandler.RecoveryDone(seenEvents)
 
 	assert.Equal(t, len(podEventHandler.asyncEvents), 0)
 	app := amProtocol.GetApplication(appID)
+
 	task, err := app.GetTask("pod1")
 	assert.NilError(t, err)
 	assert.Equal(t, cache.TaskStates().Completed, task.GetTaskState())
+
+	_, err = app.GetTask("pod2")
+	assert.ErrorContains(t, err, "task pod2 doesn't exist in application")
+
+	task, err = app.GetTask("pod3")
+	assert.NilError(t, err)
+	assert.Equal(t, "1", task.GetTaskPod().GetResourceVersion())

Review Comment:
   I'd like to hear feedback on this assertion specifically, as the `UpdatePod` event simply completes the task without actually updating the associated pod. So the tests are passing but this is not extremely intuitive.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org