[GitHub] [incubator-yunikorn-k8shim] kingamarton commented on a change in pull request #260: YUNIKORN-585: Create a Failing app state in shim side

[GitBox <gi...@apache.org>]

kingamarton commented on a change in pull request #260:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/260#discussion_r631785311



##########
File path: pkg/cache/application_test.go
##########
@@ -880,3 +880,144 @@ func TestTriggerAppRecovery(t *testing.T) {
 	err = app.TriggerAppRecovery()
 	assert.ErrorContains(t, err, "event RecoverApplication inappropriate in current state Submitted")
 }
+
+func TestReleaseAppAllocationInFailingState(t *testing.T) {
+	context := initContextForTest()
+	ms := &mockSchedulerAPI{}
+	resources := make(map[v1.ResourceName]resource.Quantity)
+	containers := make([]v1.Container, 0)
+	containers = append(containers, v1.Container{
+		Name: "container-01",
+		Resources: v1.ResourceRequirements{
+			Requests: resources,
+		},
+	})
+	pod := &v1.Pod{
+		TypeMeta: apis.TypeMeta{
+			Kind:       "Pod",
+			APIVersion: "v1",
+		},
+		ObjectMeta: apis.ObjectMeta{
+			Name: "pod-test-00001",
+			UID:  "UID-00001",
+		},
+		Spec: v1.PodSpec{
+			Containers: containers,
+		},
+	}
+	appID := "app-test-001"
+	UUID := "testUUID001"
+	app := NewApplication(appID, "root.abc", "testuser", map[string]string{}, ms)
+	task := NewTask("task01", app, context, pod)
+	app.addTask(task)
+	task.allocationUUID = UUID
+	// app must be running states
+	err := app.handle(NewReleaseAppAllocationEvent(appID, si.TerminationType_TIMEOUT, UUID))
+	if err == nil {
+		// this should give an error
+		t.Error("expecting error got 'nil'")
+	}
+	// set app states to running, let event can be trigger
+	app.SetState(events.States().Application.Running)
+	assertAppState(t, app, events.States().Application.Running, 3*time.Second)
+	err = app.handle(NewReleaseAppAllocationEvent(appID, si.TerminationType_TIMEOUT, UUID))
+	assert.NilError(t, err)
+	// after handle release event the states of app must be running
+	assertAppState(t, app, events.States().Application.Running, 3*time.Second)
+	app.SetState(events.States().Application.Failing)
+	err = app.handle(NewReleaseAppAllocationEvent(appID, si.TerminationType_TIMEOUT, UUID))
+	assert.NilError(t, err)
+	// after handle release event the states of app must be failed
+	assertAppState(t, app, events.States().Application.Failed, 3*time.Second)
+}
+
+func TestFailApplicationInFailingState(t *testing.T) {
+	context := initContextForTest()
+	dispatcher.RegisterEventHandler(dispatcher.EventTypeApp, context.ApplicationEventHandler())
+	dispatcher.Start()
+	defer dispatcher.Stop()
+
+	// inject the mocked clients to the placeholder manager
+	createdPods := newThreadSafePodsMap()
+	mockedAPIProvider := client.NewMockedAPIProvider()
+	mockedAPIProvider.MockCreateFn(func(pod *v1.Pod) (*v1.Pod, error) {
+		createdPods.add(pod)
+		return pod, nil
+	})
+	mgr := NewPlaceholderManager(mockedAPIProvider.GetAPIs())
+	mgr.Start()
+	defer mgr.Stop()
+
+	rt := &recorderTime{
+		time: int64(0),
+		lock: &sync.RWMutex{},
+	}
+	ms := &mockSchedulerAPI{}
+	// set test mode
+	conf.GetSchedulerConf().SetTestMode(true)
+	// set Recorder to mocked type
+	mr := events.NewMockedRecorder()
+	mr.OnEventf = func() {
+		rt.lock.Lock()
+		defer rt.lock.Unlock()
+		rt.time++
+	}
+	events.SetRecorderForTest(mr)
+	resources := make(map[v1.ResourceName]resource.Quantity)
+	containers := make([]v1.Container, 0)
+	containers = append(containers, v1.Container{
+		Name: "container-01",
+		Resources: v1.ResourceRequirements{
+			Requests: resources,
+		},
+	})
+	pod := &v1.Pod{
+		TypeMeta: apis.TypeMeta{
+			Kind:       "Pod",
+			APIVersion: "v1",
+		},
+		ObjectMeta: apis.ObjectMeta{
+			Name: "pod-test-00001",
+			UID:  "UID-00001",
+		},
+		Spec: v1.PodSpec{
+			Containers: containers,
+		},
+	}
+	appID := "app-test-001"
+	UUID := "testUUID001"
+	app := NewApplication(appID, "root.abc", "testuser", map[string]string{}, ms)
+	task1 := NewTask("task01", app, context, pod)
+	task2 := NewTask("task02", app, context, pod)
+	task3 := NewTask("task03", app, context, pod)
+	task4 := NewTask("task04", app, context, pod)
+	// set task states to new/pending/scheduling/running
+	task1.sm.SetState(events.States().Task.New)
+	task2.sm.SetState(events.States().Task.Pending)
+	task3.sm.SetState(events.States().Task.Scheduling)
+	task4.sm.SetState(events.States().Task.Allocated)
+	app.addTask(task1)
+	app.addTask(task2)
+	app.addTask(task3)
+	app.addTask(task4)
+	task1.allocationUUID = UUID
+	app.SetState(events.States().Application.Accepted)

Review comment:
       Why are you setting the app state to Accepted first? Couldn't we just set it directly to Running?




-- 
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.

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