Merge pull request #2279 from Sharathmk99/bugfix/queue-closed-state

Validate queue closed state in Pod validation webhook
volcano-sh · Jul 5, 2022 · 0b52e8f · 0b52e8f
2 parents 61574b3 + 4cb9b68
commit 0b52e8f
Show file tree

Hide file tree

Showing 2 changed files with 198 additions and 1 deletion.
diff --git a/pkg/webhooks/admission/pods/validate/admit_pod.go b/pkg/webhooks/admission/pods/validate/admit_pod.go
@@ -115,10 +115,20 @@ func validatePod(pod *v1.Pod, reviewResponse *admissionv1.AdmissionResponse) str
 		if err := checkPG(pod, pgName, true); err != nil {
 			msg = err.Error()
 			reviewResponse.Allowed = false
+		} else if err := checkPGQueueState(pod, pgName); err != nil {
+			msg = err.Error()
+			reviewResponse.Allowed = false
 		}
 		return msg
 	}
-
+	if pod.Annotations != nil && pod.Annotations[vcv1beta1.QueueNameAnnotationKey] != "" {
+		queueName := pod.Annotations[vcv1beta1.QueueNameAnnotationKey]
+		if err := checkQueueState(queueName); err != nil {
+			msg = err.Error()
+			reviewResponse.Allowed = false
+			return msg
+		}
+	}
 	// normal pod, SN == volcano
 	pgName = helpers.GeneratePodgroupName(pod)
 	if err := checkPG(pod, pgName, false); err != nil {
@@ -146,6 +156,30 @@ func checkPG(pod *v1.Pod, pgName string, isVCJob bool) error {
 	return nil
 }
 
+func checkPGQueueState(pod *v1.Pod, pgName string) error {
+	pgObj, err := config.VolcanoClient.SchedulingV1beta1().PodGroups(pod.Namespace).Get(context.TODO(), pgName, metav1.GetOptions{})
+	if err == nil {
+		if errQueue := checkQueueState(pgObj.Spec.Queue); errQueue != nil {
+			return fmt.Errorf("failed : %v;", errQueue)
+		}
+	}
+	return nil
+}
+
+func checkQueueState(queueName string) error {
+	if queueName == "" {
+		return nil
+	}
+	queue, err := config.VolcanoClient.SchedulingV1beta1().Queues().Get(context.TODO(), queueName, metav1.GetOptions{})
+	if err != nil {
+		return fmt.Errorf(" unable to find job queue: %v;", err)
+	} else if queue.Status.State != vcv1beta1.QueueStateOpen {
+		return fmt.Errorf(" can only submit job to queue with state `Open`, "+
+			"queue `%s` status is `%s`;", queue.Name, queue.Status.State)
+	}
+	return nil
+}
+
 func validateAnnotation(pod *v1.Pod) error {
 	num := 0
 	if len(pod.Annotations) > 0 {

diff --git a/pkg/webhooks/admission/pods/validate/admit_pod_test.go b/pkg/webhooks/admission/pods/validate/admit_pod_test.go
@@ -41,6 +41,8 @@ func TestValidatePod(t *testing.T) {
 		reviewResponse admissionv1.AdmissionResponse
 		ret            string
 		disabledPG     bool
+		queueName      string
+		queueState     vcschedulingv1.QueueState
 	}{
 		// validate normal pod with default-scheduler
 		{
@@ -86,6 +88,147 @@ func TestValidatePod(t *testing.T) {
 			ExpectErr:      true,
 			disabledPG:     true,
 		},
+		// validate volcano pod with volcano scheduler when queue is closed when no pg
+		{
+			Name: "validate pod when volcano queue is closed when no pg",
+			Pod: v1.Pod{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: "v1",
+					Kind:       "Pod",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace:   namespace,
+					Name:        "volcano-pod-3",
+					Annotations: map[string]string{vcschedulingv1.QueueNameAnnotationKey: "queue-closed"},
+				},
+				Spec: v1.PodSpec{
+					SchedulerName: "volcano",
+				},
+			},
+
+			reviewResponse: admissionv1.AdmissionResponse{Allowed: false},
+			ret:            "can only submit job to queue with state `Open`, queue `queue-closed` status is `Closed`",
+			ExpectErr:      true,
+			queueState:     vcschedulingv1.QueueStateClosed,
+			queueName:      "queue-closed",
+		},
+		// validate volcano pod with volcano scheduler when queue is Open when no pg
+		{
+			Name: "validate pod when volcano queue is open when no pg",
+			Pod: v1.Pod{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: "v1",
+					Kind:       "Pod",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace:   namespace,
+					Name:        "volcano-pod-4",
+					Annotations: map[string]string{vcschedulingv1.QueueNameAnnotationKey: "queue-open"},
+				},
+				Spec: v1.PodSpec{
+					SchedulerName: "volcano",
+				},
+			},
+
+			reviewResponse: admissionv1.AdmissionResponse{Allowed: true},
+			ret:            "",
+			ExpectErr:      false,
+			queueState:     vcschedulingv1.QueueStateOpen,
+			queueName:      "queue-open",
+		},
+		// validate volcano pod with volcano scheduler when queue is Open when pg
+		{
+			Name: "validate pod when volcano queue is open when pg",
+			Pod: v1.Pod{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: "v1",
+					Kind:       "Pod",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace:   namespace,
+					Name:        "volcano-pod-5",
+					Annotations: map[string]string{vcschedulingv1.KubeGroupNameAnnotationKey: pgName},
+				},
+				Spec: v1.PodSpec{
+					SchedulerName: "volcano",
+				},
+			},
+
+			reviewResponse: admissionv1.AdmissionResponse{Allowed: true},
+			ret:            "",
+			ExpectErr:      false,
+			queueName:      "queue-open",
+			queueState:     vcschedulingv1.QueueStateOpen,
+		},
+		// validate volcano pod with volcano scheduler when queue is Closed when pg
+		{
+			Name: "validate pod when volcano queue is closed when pg",
+			Pod: v1.Pod{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: "v1",
+					Kind:       "Pod",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace:   namespace,
+					Name:        "volcano-pod-6",
+					Annotations: map[string]string{vcschedulingv1.KubeGroupNameAnnotationKey: pgName},
+				},
+				Spec: v1.PodSpec{
+					SchedulerName: "volcano",
+				},
+			},
+
+			reviewResponse: admissionv1.AdmissionResponse{Allowed: true},
+			ret:            "can only submit job to queue with state `Open`, queue `queue-closed` status is `Closed`",
+			ExpectErr:      true,
+			queueName:      "queue-closed",
+			queueState:     vcschedulingv1.QueueStateClosed,
+		},
+		// validate volcano pod with volcano scheduler when no queue and no pg
+		{
+			Name: "validate volcano pod with volcano scheduler when no queue and no pg",
+			Pod: v1.Pod{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: "v1",
+					Kind:       "Pod",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: namespace,
+					Name:      "volcano-pod-7",
+				},
+				Spec: v1.PodSpec{
+					SchedulerName: "volcano",
+				},
+			},
+
+			reviewResponse: admissionv1.AdmissionResponse{Allowed: true},
+			ret:            "",
+			ExpectErr:      false,
+			disabledPG:     true,
+		},
+		// validate volcano pod with volcano scheduler when queue name is empty and when pg
+		{
+			Name: "validate volcano pod with volcano scheduler when no queue and no pg",
+			Pod: v1.Pod{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: "v1",
+					Kind:       "Pod",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace:   namespace,
+					Name:        "volcano-pod-8",
+					Annotations: map[string]string{vcschedulingv1.KubeGroupNameAnnotationKey: pgName},
+				},
+				Spec: v1.PodSpec{
+					SchedulerName: "volcano",
+				},
+			},
+
+			reviewResponse: admissionv1.AdmissionResponse{Allowed: true},
+			ret:            "",
+			ExpectErr:      false,
+			queueName:      "",
+		},
 	}
 
 	for _, testCase := range testCases {
@@ -97,11 +240,23 @@ func TestValidatePod(t *testing.T) {
 			},
 			Spec: vcschedulingv1.PodGroupSpec{
 				MinMember: 1,
+				Queue:     testCase.queueName,
 			},
 			Status: vcschedulingv1.PodGroupStatus{
 				Phase: vcschedulingv1.PodGroupPending,
 			},
 		}
+		queue := vcschedulingv1.Queue{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: testCase.queueName,
+			},
+			Spec: vcschedulingv1.QueueSpec{
+				Weight: 1,
+			},
+			Status: vcschedulingv1.QueueStatus{
+				State: testCase.queueState,
+			},
+		}
 
 		// create fake volcano clientset
 		config.VolcanoClient = vcclient.NewSimpleClientset()
@@ -114,6 +269,14 @@ func TestValidatePod(t *testing.T) {
 			}
 		}
 
+		if testCase.queueName != "" && testCase.queueState != "" {
+			//create default queue
+			_, err := config.VolcanoClient.SchedulingV1beta1().Queues().Create(context.TODO(), &queue, metav1.CreateOptions{})
+			if err != nil {
+				t.Error("Queue Creation Failed")
+			}
+		}
+
 		ret := validatePod(&testCase.Pod, &testCase.reviewResponse)
 
 		if testCase.ExpectErr == true && ret == "" {