Skip to content

Commit

Permalink
Addressed review comments * Added more test cases * Cleanedup test code
Browse files Browse the repository at this point in the history
  • Loading branch information
manirajv06 committed Aug 23, 2023
1 parent 64a9714 commit 92fd1e7
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 44 deletions.
4 changes: 3 additions & 1 deletion pkg/appmgmt/general/podevent_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
v1 "k8s.io/api/core/v1"

"github.com/apache/yunikorn-k8shim/pkg/appmgmt/interfaces"
"github.com/apache/yunikorn-k8shim/pkg/common/constants"
"github.com/apache/yunikorn-k8shim/pkg/common/events"
"github.com/apache/yunikorn-k8shim/pkg/conf"
"github.com/apache/yunikorn-k8shim/pkg/log"
Expand Down Expand Up @@ -130,7 +131,8 @@ func (p *PodEventHandler) addPod(pod *v1.Pod, eventSource EventSource) interface
Metadata: appMeta,
})
} else {
if conf.GetSchedulerConf().GetSingleUserPerApplication() && app.GetUser() != appMeta.User {
if app.GetApplicationID() != constants.AutoGenAppPrefix+"-"+pod.Namespace+"-"+constants.AutoGenAppSuffix &&
conf.GetSchedulerConf().GetSingleUserPerApplication() && app.GetUser() != appMeta.User {
log.Log(log.ShimAppMgmtGeneral).Warn("rejecting application as it has been submitted by different user",
zap.String("app id ", appMeta.ApplicationID),
zap.String("app user", app.GetUser()),
Expand Down
91 changes: 48 additions & 43 deletions pkg/appmgmt/general/podevent_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,50 +120,12 @@ func TestSingleUserPerApplication(t *testing.T) {

am := NewManager(client.NewMockedAPIProvider(false), podEvent)

pod1 := v1.Pod{
TypeMeta: apis.TypeMeta{
Kind: "Pod",
APIVersion: "v1",
},
ObjectMeta: apis.ObjectMeta{
Name: "pod00001",
Namespace: "default",
UID: "UID-POD-00001",
Labels: map[string]string{
"queue": "root.a",
"yunikorn.apache.org/username": "test",
"applicationId": appID,
},
},
Spec: v1.PodSpec{SchedulerName: constants.SchedulerName},
Status: v1.PodStatus{
Phase: v1.PodPending,
},
}
pod1 := newPodByUser("pod00001", "test", appID)

// submit the app
am.AddPod(&pod1)
am.AddPod(pod1)

pod := &v1.Pod{
TypeMeta: apis.TypeMeta{
Kind: "Pod",
APIVersion: "v1",
},
ObjectMeta: apis.ObjectMeta{
Name: "pod00002",
Namespace: "default",
UID: "UID-POD-00002",
Labels: map[string]string{
"queue": "root.a",
"yunikorn.apache.org/username": "test1",
"applicationId": appID,
},
},
Spec: v1.PodSpec{SchedulerName: constants.SchedulerName},
Status: v1.PodStatus{
Phase: v1.PodPending,
},
}
pod := newPodByUser("pod00002", "test1", appID)

// submit the same app with different user
am.AddPod(pod)
Expand Down Expand Up @@ -207,13 +169,56 @@ func TestSingleUserPerApplication(t *testing.T) {
}
}, 50*time.Millisecond, time.Second)
assert.NilError(t, err, "event should have been emitted")

autogenAppID := constants.AutoGenAppPrefix + "-default-" + constants.AutoGenAppSuffix
pod2 := newPodByUser("pod00003", "test", autogenAppID)

// submit the autogen app
am.AddPod(pod2)

message = "Rejecting pod because application ID " + autogenAppID + " belongs to a different user"

// ensure there is no event
err = utils.WaitForCondition(func() bool {
for {
select {
case event := <-recorder.Events:
if strings.Contains(event, message) {
return true
}
default:
return false
}
}
}, 50*time.Millisecond, time.Second)
assert.Error(t, err, "timeout waiting for condition")

pod3 := newPodByUser("pod00004", "test", autogenAppID)

// submit the same autogen app with different user and ensure no event has been published because same autogen app submission by different users are allowed
am.AddPod(pod3)

// ensure there is no event even though auto gen app already exists
err = utils.WaitForCondition(func() bool {
for {
select {
case event := <-recorder.Events:
if strings.Contains(event, message) {
return true
}
default:
return false
}
}
}, 50*time.Millisecond, time.Second)
assert.Error(t, err, "timeout waiting for condition")
}

func newPod(name string) *v1.Pod {
return newPodByUser(name, "nobody")
return newPodByUser(name, "nobody", appID)
}

func newPodByUser(name string, user string) *v1.Pod {
func newPodByUser(name string, user string, appID string) *v1.Pod {
return &v1.Pod{
TypeMeta: apis.TypeMeta{
Kind: "Pod",
Expand Down

0 comments on commit 92fd1e7

Please sign in to comment.