From 92fd1e7b5387bc4bc0e485a98f885ee4c59ea1ad Mon Sep 17 00:00:00 2001 From: Manikandan R Date: Wed, 23 Aug 2023 13:30:09 +0530 Subject: [PATCH] Addressed review comments * Added more test cases * Cleanedup test code --- pkg/appmgmt/general/podevent_handler.go | 4 +- pkg/appmgmt/general/podevent_handler_test.go | 91 +++++++++++--------- 2 files changed, 51 insertions(+), 44 deletions(-) diff --git a/pkg/appmgmt/general/podevent_handler.go b/pkg/appmgmt/general/podevent_handler.go index 7e7bccf9a..b641768f1 100644 --- a/pkg/appmgmt/general/podevent_handler.go +++ b/pkg/appmgmt/general/podevent_handler.go @@ -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" @@ -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()), diff --git a/pkg/appmgmt/general/podevent_handler_test.go b/pkg/appmgmt/general/podevent_handler_test.go index c938e601b..0941fd3f0 100644 --- a/pkg/appmgmt/general/podevent_handler_test.go +++ b/pkg/appmgmt/general/podevent_handler_test.go @@ -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) @@ -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",