pbacsko commented on code in PR #628:
URL: https://github.com/apache/yunikorn-k8shim/pull/628#discussion_r1269582166
##########
pkg/appmgmt/general/podevent_handler_test.go:
##########
@@ -99,7 +106,117 @@ func TestRecoveryDone(t *testing.T) {
assert.Equal(t, false, podEventHandler.recoveryRunning)
}
+func TestSingleUserPerApplication(t *testing.T) {
+ conf.GetSchedulerConf().SetTestMode(true)
+ api := client.NewMockedAPIProvider(false)
+ cache.NewContext(api)
+ recorder, ok := events.GetRecorder().(*k8sEvents.FakeRecorder)
+ if !ok {
+ t.Fatal("the EventRecorder is expected to be of type
FakeRecorder")
+ }
+
+ amprotocol := cache.NewMockedAMProtocol()
+ podEvent := NewPodEventHandler(amprotocol, false)
+
+ 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,
+ },
+ }
+
+ // submit the app
+ 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,
+ },
+ }
+
+ // submit the same app with different user
+ am.AddPod(pod)
+
+ message := "Rejecting the application because already application
exists with different user"
+ reason := "SingleUserPerApplication has been configured to true. So
rejecting the application because it has been submitted by different user. " +
+ "Either you can disable the check by not setting
SingleUserPerApplication or try submitting a application by same use. " +
+ "By default, SingleUserPerApplication is false"
+
+ // ensure there is no event
+ err := utils.WaitForCondition(func() bool {
+ for {
+ select {
+ case event := <-recorder.Events:
+ if strings.Contains(event, reason) &&
strings.Contains(event, message) {
+ return true
+ }
+ default:
+ return false
+ }
+ }
+ }, 50*time.Millisecond, 20*time.Millisecond)
Review Comment:
Timeout is smaller then the interval. Use a timeout of 1 second (just
`time.Second` instead of `20*time.Millisecond`) to be safe and avoid timeouts
on slower machines.
##########
pkg/appmgmt/general/podevent_handler_test.go:
##########
@@ -99,7 +106,117 @@ func TestRecoveryDone(t *testing.T) {
assert.Equal(t, false, podEventHandler.recoveryRunning)
}
+func TestSingleUserPerApplication(t *testing.T) {
+ conf.GetSchedulerConf().SetTestMode(true)
+ api := client.NewMockedAPIProvider(false)
+ cache.NewContext(api)
+ recorder, ok := events.GetRecorder().(*k8sEvents.FakeRecorder)
+ if !ok {
+ t.Fatal("the EventRecorder is expected to be of type
FakeRecorder")
+ }
+
+ amprotocol := cache.NewMockedAMProtocol()
+ podEvent := NewPodEventHandler(amprotocol, false)
+
+ 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,
+ },
+ }
+
+ // submit the app
+ 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,
+ },
+ }
+
+ // submit the same app with different user
+ am.AddPod(pod)
+
+ message := "Rejecting the application because already application
exists with different user"
+ reason := "SingleUserPerApplication has been configured to true. So
rejecting the application because it has been submitted by different user. " +
+ "Either you can disable the check by not setting
SingleUserPerApplication or try submitting a application by same use. " +
+ "By default, SingleUserPerApplication is false"
+
+ // ensure there is no event
+ err := utils.WaitForCondition(func() bool {
+ for {
+ select {
+ case event := <-recorder.Events:
+ if strings.Contains(event, reason) &&
strings.Contains(event, message) {
+ return true
+ }
+ default:
+ return false
+ }
+ }
+ }, 50*time.Millisecond, 20*time.Millisecond)
+ assert.Error(t, err, "timeout waiting for condition")
+
+ // set SingleUserPerApplication to true
+ err = conf.UpdateConfigMaps([]*v1.ConfigMap{
+ {Data: map[string]string{conf.CMSvcSingleUserPerApplication:
"true"}},
+ }, true)
+ assert.NilError(t, err, "UpdateConfigMap failed")
+
+ // submit the same app with different user and ensure specific
rejection event has been published
+ am.AddPod(pod)
+
+ err = utils.WaitForCondition(func() bool {
+ for {
+ select {
+ case event := <-recorder.Events:
+ if strings.Contains(event, reason) &&
strings.Contains(event, message) {
+ return true
+ }
+ default:
+ return false
+ }
+ }
+ }, 50*time.Millisecond, 20*time.Millisecond)
Review Comment:
Timeout is smaller then the interval. Use a timeout of 1 second (just
`time.Second` instead of `20*time.Millisecond`) to be safe and avoid timeouts
on slower machines.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]