You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@yunikorn.apache.org by GitBox <gi...@apache.org> on 2020/10/29 17:52:05 UTC

[GitHub] [incubator-yunikorn-k8shim] kingamarton commented on a change in pull request #203: [YUNIKORN-453] add taskGroup info to apps/tasks

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



##########
File path: pkg/cache/application.go
##########
@@ -359,6 +402,50 @@ func (app *Application) handleRecoverApplicationEvent(event *fsm.Event) {
 	}
 }
 
+func (app *Application) postAppAccepted(event *fsm.Event) {
+	// if app has taskGroups defined, it goes to the Reserving state before getting to Running
+	var ev events.SchedulingEvent
+	if app.taskGroups != nil {
+		ev = NewSimpleApplicationEvent(app.applicationID, events.TryReserve)
+		dispatcher.Dispatch(ev)
+	} else {
+		ev = NewRunApplicationEvent(app.applicationID)
+	}
+	dispatcher.Dispatch(ev)
+}
+
+func (app *Application) onReserving(event *fsm.Event) {
+	go func() {
+		// while doing reserving
+		if err := GetPlaceholderManager().createAppPlaceholders(app); err != nil {
+			// creating placeholder failed
+			// put the app into recycling queue and turn the app to running state
+			GetPlaceholderManager().Recycle(app.applicationID)
+			ev := NewRunApplicationEvent(app.applicationID)

Review comment:
       If the placeholder creation failed, why do we transit the application into running state? 

##########
File path: pkg/cache/placeholder_manager.go
##########
@@ -0,0 +1,84 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+     http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+package cache
+
+import (
+	"sync"
+
+	"go.uber.org/zap"
+
+	"github.com/apache/incubator-yunikorn-k8shim/pkg/client"
+	"github.com/apache/incubator-yunikorn-k8shim/pkg/common/utils"
+	"github.com/apache/incubator-yunikorn-k8shim/pkg/log"
+)
+
+// placeholder manager is a service to manage the lifecycle of app placeholders
+type PlaceholderManager struct {
+	clients *client.Clients
+	sync.RWMutex
+}
+
+var placeholderMgr *PlaceholderManager
+var once sync.Once
+
+func GetPlaceholderManager() *PlaceholderManager {
+	once.Do(func() {
+		placeholderMgr = &PlaceholderManager{}

Review comment:
       I can't see where the clients got its value

##########
File path: pkg/cache/application_test.go
##########
@@ -181,19 +190,192 @@ func TestGetNonTerminatedTaskAlias(t *testing.T) {
 	assert.Assert(t, is.Contains(res, "/test-00001"))
 	assert.Assert(t, is.Contains(res, "/test-00002"))
 
-	//set two tasks to terminated states
+	// set two tasks to terminated states
 	task1.sm.SetState(events.States().Task.Rejected)
 	task2.sm.SetState(events.States().Task.Rejected)
 	// check the tasks both in terminated states
 	// res should retuen empty
 	res = app.getNonTerminatedTaskAlias()
 	assert.Equal(t, len(res), 0)
 
-	//set two tasks to one is terminated, another is non-terminated
+	// set two tasks to one is terminated, another is non-terminated
 	task1.sm.SetState(events.States().Task.Rejected)
 	task2.sm.SetState(events.States().Task.Allocated)
 	// check the task, should only return task2's alias
 	res = app.getNonTerminatedTaskAlias()
 	assert.Equal(t, len(res), 1)
 	assert.Equal(t, res[0], "/test-00002")
 }
+
+func TestSetTaskGroupsAndSchedulingPolicy(t *testing.T) {
+	app := NewApplication("app01", "root.a", "test-user", map[string]string{}, newMockSchedulerAPI())
+	assert.Assert(t, app.getSchedulingPolicy() == nil)
+	assert.Assert(t, app.getTaskGroups() == nil)
+
+	app.setSchedulingPolicy(&v1alpha1.SchedulingPolicy{
+		Type: v1alpha1.TryReserve,
+		Parameters: map[string]string{
+			"option-1": "value-1",
+			"option-2": "value-2",
+		},
+	})
+
+	assert.Equal(t, app.getSchedulingPolicy().Type, v1alpha1.TryReserve)
+	assert.Equal(t, len(app.getSchedulingPolicy().Parameters), 2)
+	assert.Equal(t, app.getSchedulingPolicy().Parameters["option-1"], "value-1")
+	assert.Equal(t, app.getSchedulingPolicy().Parameters["option-2"], "value-2")
+
+	duration := int64(3000)
+	app.setTaskGroups([]*v1alpha1.TaskGroup{
+		{
+			Name:      "test-group-1",
+			MinMember: 10,
+			MinResource: map[string]resource.Quantity{
+				"cpu":    resource.MustParse("500m"),
+				"memory": resource.MustParse("500Mi"),

Review comment:
       use constants.CPU and constants.Memory

##########
File path: pkg/cache/placeholder_manager_test.go
##########
@@ -0,0 +1,88 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+     http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+package cache
+
+import (
+	"fmt"
+	"sync"
+	"testing"
+
+	"gotest.tools/assert"
+	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
+
+	"github.com/apache/incubator-yunikorn-k8shim/pkg/apis/yunikorn.apache.org/v1alpha1"
+	"github.com/apache/incubator-yunikorn-k8shim/pkg/client"
+	"github.com/apache/incubator-yunikorn-k8shim/pkg/common/constants"
+)
+
+func TestCreateAppPlaceholders(t *testing.T) {
+	const (
+		appID     = "app01"
+		queue     = "root.default"
+		namespace = "test"
+	)
+	mockedSchedulerAPI := newMockSchedulerAPI()
+	app := NewApplication(appID, queue,
+		"bob", map[string]string{constants.AppTagNamespace: namespace}, mockedSchedulerAPI)
+	app.setTaskGroups([]*v1alpha1.TaskGroup{
+		{
+			Name:      "test-group-1",
+			MinMember: 10,
+			MinResource: map[string]resource.Quantity{
+				"cpu":    resource.MustParse("500m"),
+				"memory": resource.MustParse("1024M"),

Review comment:
       Use constants

##########
File path: pkg/cache/placeholder_test.go
##########
@@ -0,0 +1,67 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+     http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+package cache
+
+import (
+	"testing"
+
+	"gotest.tools/assert"
+	"k8s.io/apimachinery/pkg/api/resource"
+
+	"github.com/apache/incubator-yunikorn-k8shim/pkg/apis/yunikorn.apache.org/v1alpha1"
+	"github.com/apache/incubator-yunikorn-k8shim/pkg/common"
+	"github.com/apache/incubator-yunikorn-k8shim/pkg/common/constants"
+)
+
+func TestNewPlaceholder(t *testing.T) {
+	const (
+		appID     = "app01"
+		queue     = "root.default"
+		namespace = "test"
+	)
+	mockedSchedulerAPI := newMockSchedulerAPI()
+	app := NewApplication(appID, queue,
+		"bob", map[string]string{constants.AppTagNamespace: namespace}, mockedSchedulerAPI)
+	app.setTaskGroups([]*v1alpha1.TaskGroup{
+		{
+			Name:      "test-group-1",
+			MinMember: 10,
+			MinResource: map[string]resource.Quantity{
+				"cpu":    resource.MustParse("500m"),
+				"memory": resource.MustParse("1024M"),

Review comment:
       use constants

##########
File path: pkg/cache/application.go
##########
@@ -359,6 +402,50 @@ func (app *Application) handleRecoverApplicationEvent(event *fsm.Event) {
 	}
 }
 
+func (app *Application) postAppAccepted(event *fsm.Event) {
+	// if app has taskGroups defined, it goes to the Reserving state before getting to Running
+	var ev events.SchedulingEvent
+	if app.taskGroups != nil {

Review comment:
       isn't it better to use len(app.taskGroups) == 0? This way the empty task group will be covered as well.

##########
File path: pkg/cache/placeholder_manager.go
##########
@@ -0,0 +1,84 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+     http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+package cache
+
+import (
+	"sync"
+
+	"go.uber.org/zap"
+
+	"github.com/apache/incubator-yunikorn-k8shim/pkg/client"
+	"github.com/apache/incubator-yunikorn-k8shim/pkg/common/utils"
+	"github.com/apache/incubator-yunikorn-k8shim/pkg/log"
+)
+
+// placeholder manager is a service to manage the lifecycle of app placeholders
+type PlaceholderManager struct {
+	clients *client.Clients
+	sync.RWMutex
+}
+
+var placeholderMgr *PlaceholderManager
+var once sync.Once
+
+func GetPlaceholderManager() *PlaceholderManager {
+	once.Do(func() {
+		placeholderMgr = &PlaceholderManager{}
+	})
+	return placeholderMgr
+}
+
+func (mgr *PlaceholderManager) createAppPlaceholders(app *Application) error {
+	mgr.Lock()
+	defer mgr.Unlock()
+
+	// iterate all task groups, create placeholders for all the min members
+	for _, tg := range app.getTaskGroups() {
+		for i := int32(0); i < tg.MinMember; i++ {
+			placeholderName := utils.GeneratePlaceholderName(tg.Name, app.GetApplicationID(), i)
+			placeholder := newPlaceholder(placeholderName, app, tg)
+			// create the placeholder on K8s
+			_, err := mgr.clients.KubeClient.Create(placeholder.pod)
+			if err != nil {
+				// if failed to create the place holder pod
+				// caller should handle this error
+				log.Logger().Error("failed to create placeholder pod",
+					zap.Error(err))
+				return err
+			}
+			log.Logger().Info("placeholder created",
+				zap.String("placeholder", placeholder.String()))
+		}
+	}
+
+	return nil
+}
+
+// recycle all the placeholders for an application
+func (mgr *PlaceholderManager) Recycle(appID string) {
+	log.Logger().Info("start to recycle app placeholders",
+		zap.String("appID", appID))

Review comment:
       Here will come the implementation in some of the next subtasks, right?

##########
File path: pkg/cache/application.go
##########
@@ -28,25 +28,29 @@ import (
 	v1 "k8s.io/api/core/v1"
 
 	"github.com/apache/incubator-yunikorn-core/pkg/api"
+	"github.com/apache/incubator-yunikorn-k8shim/pkg/apis/yunikorn.apache.org/v1alpha1"
 	"github.com/apache/incubator-yunikorn-k8shim/pkg/appmgmt/interfaces"
 	"github.com/apache/incubator-yunikorn-k8shim/pkg/common/constants"
 	"github.com/apache/incubator-yunikorn-k8shim/pkg/common/events"
+	"github.com/apache/incubator-yunikorn-k8shim/pkg/common/utils"
 	"github.com/apache/incubator-yunikorn-k8shim/pkg/conf"
 	"github.com/apache/incubator-yunikorn-k8shim/pkg/dispatcher"
 	"github.com/apache/incubator-yunikorn-k8shim/pkg/log"
 	"github.com/apache/incubator-yunikorn-scheduler-interface/lib/go/si"
 )
 
 type Application struct {
-	applicationID string
-	queue         string
-	partition     string
-	user          string
-	taskMap       map[string]*Task
-	tags          map[string]string
-	sm            *fsm.FSM
-	lock          *sync.RWMutex
-	schedulerAPI  api.SchedulerAPI
+	applicationID    string
+	queue            string
+	partition        string
+	user             string
+	taskMap          map[string]*Task
+	tags             map[string]string
+	schedulingPolicy *v1alpha1.SchedulingPolicy
+	taskGroups       []*v1alpha1.TaskGroup

Review comment:
       This 2 fields are from the Application CRD. I cannot decide whether it is good to reuse them in a general code. It might work,  but I would rather create a new structure for it here, even if it will be a duplicated code, but at least we will not depend on the CRD structure. What do you think @yangwwei?

##########
File path: pkg/cache/placeholder.go
##########
@@ -0,0 +1,85 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+     http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+package cache
+
+import (
+	"fmt"
+
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/apache/incubator-yunikorn-k8shim/pkg/apis/yunikorn.apache.org/v1alpha1"
+	"github.com/apache/incubator-yunikorn-k8shim/pkg/common/constants"
+	"github.com/apache/incubator-yunikorn-k8shim/pkg/common/utils"
+)
+
+type Placeholder struct {
+	appID         string
+	taskGroupName string
+	pod           *v1.Pod
+	stage         PlaceholderState
+}
+
+type PlaceholderState string
+
+const (
+	Acquiring PlaceholderState = "Acquiring"
+	Acquired  PlaceholderState = "Acquired"
+)
+
+func newPlaceholder(placeholderName string, app *Application, taskGroup *v1alpha1.TaskGroup) *Placeholder {
+	placeholderPod := &v1.Pod{

Review comment:
       On K8s side, how we can differentiate a placeholder from a real pod? Shouldn't we add a label or something to differentiate it from the real pod?

##########
File path: pkg/cache/application_test.go
##########
@@ -181,19 +190,192 @@ func TestGetNonTerminatedTaskAlias(t *testing.T) {
 	assert.Assert(t, is.Contains(res, "/test-00001"))
 	assert.Assert(t, is.Contains(res, "/test-00002"))
 
-	//set two tasks to terminated states
+	// set two tasks to terminated states
 	task1.sm.SetState(events.States().Task.Rejected)
 	task2.sm.SetState(events.States().Task.Rejected)
 	// check the tasks both in terminated states
 	// res should retuen empty
 	res = app.getNonTerminatedTaskAlias()
 	assert.Equal(t, len(res), 0)
 
-	//set two tasks to one is terminated, another is non-terminated
+	// set two tasks to one is terminated, another is non-terminated
 	task1.sm.SetState(events.States().Task.Rejected)
 	task2.sm.SetState(events.States().Task.Allocated)
 	// check the task, should only return task2's alias
 	res = app.getNonTerminatedTaskAlias()
 	assert.Equal(t, len(res), 1)
 	assert.Equal(t, res[0], "/test-00002")
 }
+
+func TestSetTaskGroupsAndSchedulingPolicy(t *testing.T) {
+	app := NewApplication("app01", "root.a", "test-user", map[string]string{}, newMockSchedulerAPI())
+	assert.Assert(t, app.getSchedulingPolicy() == nil)
+	assert.Assert(t, app.getTaskGroups() == nil)
+
+	app.setSchedulingPolicy(&v1alpha1.SchedulingPolicy{
+		Type: v1alpha1.TryReserve,
+		Parameters: map[string]string{
+			"option-1": "value-1",
+			"option-2": "value-2",
+		},
+	})
+
+	assert.Equal(t, app.getSchedulingPolicy().Type, v1alpha1.TryReserve)
+	assert.Equal(t, len(app.getSchedulingPolicy().Parameters), 2)
+	assert.Equal(t, app.getSchedulingPolicy().Parameters["option-1"], "value-1")
+	assert.Equal(t, app.getSchedulingPolicy().Parameters["option-2"], "value-2")
+
+	duration := int64(3000)
+	app.setTaskGroups([]*v1alpha1.TaskGroup{
+		{
+			Name:      "test-group-1",
+			MinMember: 10,
+			MinResource: map[string]resource.Quantity{
+				"cpu":    resource.MustParse("500m"),
+				"memory": resource.MustParse("500Mi"),
+			},
+		},
+		{
+			Name:      "test-group-2",
+			MinMember: 20,
+			MinResource: map[string]resource.Quantity{
+				"cpu":    resource.MustParse("1000m"),
+				"memory": resource.MustParse("1000Mi"),

Review comment:
       use constants.CPU and constants.Memory

##########
File path: pkg/cache/application_test.go
##########
@@ -181,19 +190,192 @@ func TestGetNonTerminatedTaskAlias(t *testing.T) {
 	assert.Assert(t, is.Contains(res, "/test-00001"))
 	assert.Assert(t, is.Contains(res, "/test-00002"))
 
-	//set two tasks to terminated states
+	// set two tasks to terminated states
 	task1.sm.SetState(events.States().Task.Rejected)
 	task2.sm.SetState(events.States().Task.Rejected)
 	// check the tasks both in terminated states
 	// res should retuen empty
 	res = app.getNonTerminatedTaskAlias()
 	assert.Equal(t, len(res), 0)
 
-	//set two tasks to one is terminated, another is non-terminated
+	// set two tasks to one is terminated, another is non-terminated
 	task1.sm.SetState(events.States().Task.Rejected)
 	task2.sm.SetState(events.States().Task.Allocated)
 	// check the task, should only return task2's alias
 	res = app.getNonTerminatedTaskAlias()
 	assert.Equal(t, len(res), 1)
 	assert.Equal(t, res[0], "/test-00002")
 }
+
+func TestSetTaskGroupsAndSchedulingPolicy(t *testing.T) {
+	app := NewApplication("app01", "root.a", "test-user", map[string]string{}, newMockSchedulerAPI())
+	assert.Assert(t, app.getSchedulingPolicy() == nil)
+	assert.Assert(t, app.getTaskGroups() == nil)
+
+	app.setSchedulingPolicy(&v1alpha1.SchedulingPolicy{
+		Type: v1alpha1.TryReserve,
+		Parameters: map[string]string{
+			"option-1": "value-1",
+			"option-2": "value-2",
+		},
+	})
+
+	assert.Equal(t, app.getSchedulingPolicy().Type, v1alpha1.TryReserve)
+	assert.Equal(t, len(app.getSchedulingPolicy().Parameters), 2)
+	assert.Equal(t, app.getSchedulingPolicy().Parameters["option-1"], "value-1")
+	assert.Equal(t, app.getSchedulingPolicy().Parameters["option-2"], "value-2")
+
+	duration := int64(3000)
+	app.setTaskGroups([]*v1alpha1.TaskGroup{
+		{
+			Name:      "test-group-1",
+			MinMember: 10,
+			MinResource: map[string]resource.Quantity{
+				"cpu":    resource.MustParse("500m"),
+				"memory": resource.MustParse("500Mi"),
+			},
+		},
+		{
+			Name:      "test-group-2",
+			MinMember: 20,
+			MinResource: map[string]resource.Quantity{
+				"cpu":    resource.MustParse("1000m"),
+				"memory": resource.MustParse("1000Mi"),
+			},
+			NodeSelector: metav1.LabelSelector{
+				MatchExpressions: []metav1.LabelSelectorRequirement{
+					{
+						Key:      "security",
+						Operator: metav1.LabelSelectorOpIn,
+						Values:   []string{"S1", "value2"},
+					},
+				},
+			},
+			Tolerations: []v1.Toleration{
+				{
+					Key:               "nodeType",
+					Operator:          v1.TolerationOpEqual,
+					Value:             "infra",
+					Effect:            v1.TaintEffectNoSchedule,
+					TolerationSeconds: &duration,
+				},
+			},
+		},
+	})
+
+	assert.Assert(t, app.getTaskGroups() != nil)
+	assert.Equal(t, len(app.getTaskGroups()), 2)
+
+	// sort the slice to give us a stable order
+	sort.Slice(app.getTaskGroups(), func(i, j int) bool {
+		return strings.Compare(app.getTaskGroups()[i].Name, app.getTaskGroups()[j].Name) < 0
+	})
+
+	tg1 := app.getTaskGroups()[0]
+	assert.Equal(t, tg1.Name, "test-group-1")
+	assert.Equal(t, tg1.MinMember, int32(10))
+	assert.Equal(t, tg1.MinResource["cpu"], resource.MustParse("500m"))
+	assert.Equal(t, tg1.MinResource["memory"], resource.MustParse("500Mi"))

Review comment:
       use constants.CPU and constants.Memory

##########
File path: pkg/cache/placeholder_manager_test.go
##########
@@ -0,0 +1,88 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+     http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+package cache
+
+import (
+	"fmt"
+	"sync"
+	"testing"
+
+	"gotest.tools/assert"
+	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
+
+	"github.com/apache/incubator-yunikorn-k8shim/pkg/apis/yunikorn.apache.org/v1alpha1"
+	"github.com/apache/incubator-yunikorn-k8shim/pkg/client"
+	"github.com/apache/incubator-yunikorn-k8shim/pkg/common/constants"
+)
+
+func TestCreateAppPlaceholders(t *testing.T) {
+	const (
+		appID     = "app01"
+		queue     = "root.default"
+		namespace = "test"
+	)
+	mockedSchedulerAPI := newMockSchedulerAPI()
+	app := NewApplication(appID, queue,
+		"bob", map[string]string{constants.AppTagNamespace: namespace}, mockedSchedulerAPI)
+	app.setTaskGroups([]*v1alpha1.TaskGroup{
+		{
+			Name:      "test-group-1",
+			MinMember: 10,
+			MinResource: map[string]resource.Quantity{
+				"cpu":    resource.MustParse("500m"),
+				"memory": resource.MustParse("1024M"),
+			},
+		},
+		{
+			Name:      "test-group-2",
+			MinMember: 20,
+			MinResource: map[string]resource.Quantity{
+				"cpu":    resource.MustParse("1000m"),
+				"memory": resource.MustParse("2048M"),

Review comment:
       Use constants
   

##########
File path: pkg/cache/application_test.go
##########
@@ -181,19 +190,192 @@ func TestGetNonTerminatedTaskAlias(t *testing.T) {
 	assert.Assert(t, is.Contains(res, "/test-00001"))
 	assert.Assert(t, is.Contains(res, "/test-00002"))
 
-	//set two tasks to terminated states
+	// set two tasks to terminated states
 	task1.sm.SetState(events.States().Task.Rejected)
 	task2.sm.SetState(events.States().Task.Rejected)
 	// check the tasks both in terminated states
 	// res should retuen empty
 	res = app.getNonTerminatedTaskAlias()
 	assert.Equal(t, len(res), 0)
 
-	//set two tasks to one is terminated, another is non-terminated
+	// set two tasks to one is terminated, another is non-terminated
 	task1.sm.SetState(events.States().Task.Rejected)
 	task2.sm.SetState(events.States().Task.Allocated)
 	// check the task, should only return task2's alias
 	res = app.getNonTerminatedTaskAlias()
 	assert.Equal(t, len(res), 1)
 	assert.Equal(t, res[0], "/test-00002")
 }
+
+func TestSetTaskGroupsAndSchedulingPolicy(t *testing.T) {
+	app := NewApplication("app01", "root.a", "test-user", map[string]string{}, newMockSchedulerAPI())
+	assert.Assert(t, app.getSchedulingPolicy() == nil)
+	assert.Assert(t, app.getTaskGroups() == nil)
+
+	app.setSchedulingPolicy(&v1alpha1.SchedulingPolicy{
+		Type: v1alpha1.TryReserve,
+		Parameters: map[string]string{
+			"option-1": "value-1",
+			"option-2": "value-2",
+		},
+	})
+
+	assert.Equal(t, app.getSchedulingPolicy().Type, v1alpha1.TryReserve)
+	assert.Equal(t, len(app.getSchedulingPolicy().Parameters), 2)
+	assert.Equal(t, app.getSchedulingPolicy().Parameters["option-1"], "value-1")
+	assert.Equal(t, app.getSchedulingPolicy().Parameters["option-2"], "value-2")

Review comment:
       nit: add assertion message 




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