Skip to content

Commit 53e8d51

Browse files
Doris-xmDoris-xm
committed
fix: refactor TestValidate
Signed-off-by: Xinmin Du <2812493086@qq.com>
1 parent 49bd5b9 commit 53e8d51

1 file changed

Lines changed: 101 additions & 197 deletions

File tree

‎pkg/runtime/framework/plugins/volcano/volcano_test.go‎

Lines changed: 101 additions & 197 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"cmp"
2121
"context"
2222
"errors"
23+
"fmt"
2324
"strconv"
2425
"testing"
2526

@@ -32,12 +33,14 @@ import (
3233
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3334
apiruntime "k8s.io/apimachinery/pkg/runtime"
3435
"k8s.io/apimachinery/pkg/types"
36+
"k8s.io/apimachinery/pkg/util/validation/field"
3537
batchv1ac "k8s.io/client-go/applyconfigurations/batch/v1"
3638
corev1ac "k8s.io/client-go/applyconfigurations/core/v1"
3739
"k8s.io/klog/v2/ktesting"
3840
"k8s.io/utils/ptr"
3941
"sigs.k8s.io/controller-runtime/pkg/client"
4042
"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
43+
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
4144
jobsetv1alpha2 "sigs.k8s.io/jobset/api/jobset/v1alpha2"
4245
jobsetv1alpha2ac "sigs.k8s.io/jobset/client-go/applyconfiguration/jobset/v1alpha2"
4346
volcanov1beta1 "volcano.sh/apis/pkg/apis/scheduling/v1beta1"
@@ -397,229 +400,130 @@ func TestVolcano(t *testing.T) {
397400
}
398401

399402
func TestValidate(t *testing.T) {
400-
_, ctx := ktesting.NewTestContext(t)
401-
var cancel func()
402-
ctx, cancel = context.WithCancel(ctx)
403-
t.Cleanup(cancel)
404-
405-
createPriorityClass := func(name string) *schedulingv1.PriorityClass {
406-
return &schedulingv1.PriorityClass{
407-
ObjectMeta: metav1.ObjectMeta{
408-
Name: name,
409-
},
410-
Value: 100,
411-
}
412-
}
413-
414-
createJobSetSpecWithPriority := func(priorityClassName string) *jobsetv1alpha2ac.JobSetSpecApplyConfiguration {
415-
return jobsetv1alpha2ac.JobSetSpec().
416-
WithReplicatedJobs(
417-
jobsetv1alpha2ac.ReplicatedJob().
418-
WithTemplate(
419-
batchv1ac.JobTemplateSpec().
420-
WithSpec(
421-
batchv1ac.JobSpec().
422-
WithTemplate(
423-
corev1ac.PodTemplateSpec().
424-
WithSpec(
425-
corev1ac.PodSpec().
426-
WithPriorityClassName(priorityClassName),
427-
),
428-
),
429-
),
430-
),
431-
)
432-
}
433-
434403
cases := map[string]struct {
435404
info *runtime.Info
436-
trainJob *trainer.TrainJob
405+
oldObj *trainer.TrainJob
406+
newObj *trainer.TrainJob
437407
objs []client.Object
438-
wantErrors bool
439-
wantWarnings bool
408+
wantError field.ErrorList
409+
wantWarnings admission.Warnings
440410
}{
441-
"queue annotation missing": {
442-
info: &runtime.Info{
443-
Annotations: map[string]string{},
444-
RuntimePolicy: runtime.RuntimePolicy{
445-
PodGroupPolicy: &trainer.PodGroupPolicy{
446-
PodGroupPolicySource: trainer.PodGroupPolicySource{
447-
Volcano: &trainer.VolcanoPodGroupPolicySource{},
448-
},
449-
},
450-
},
451-
},
452-
trainJob: &trainer.TrainJob{},
453-
objs: []client.Object{},
454-
wantErrors: false,
455-
wantWarnings: false,
456-
},
457-
"queue annotation empty": {
458-
info: &runtime.Info{
459-
Annotations: map[string]string{
460-
volcanov1beta1.QueueNameAnnotationKey: "",
461-
},
462-
RuntimePolicy: runtime.RuntimePolicy{
463-
PodGroupPolicy: &trainer.PodGroupPolicy{
464-
PodGroupPolicySource: trainer.PodGroupPolicySource{
465-
Volcano: &trainer.VolcanoPodGroupPolicySource{},
466-
},
467-
},
468-
},
469-
},
470-
trainJob: &trainer.TrainJob{},
471-
objs: []client.Object{},
472-
wantErrors: true,
473-
wantWarnings: false,
474-
},
475-
"queue annotation valid": {
476-
info: &runtime.Info{
477-
Annotations: map[string]string{
478-
volcanov1beta1.QueueNameAnnotationKey: "default",
479-
},
480-
RuntimePolicy: runtime.RuntimePolicy{
481-
PodGroupPolicy: &trainer.PodGroupPolicy{
482-
PodGroupPolicySource: trainer.PodGroupPolicySource{
483-
Volcano: &trainer.VolcanoPodGroupPolicySource{},
484-
},
485-
},
486-
},
487-
},
488-
trainJob: &trainer.TrainJob{},
489-
objs: []client.Object{},
490-
wantErrors: false,
491-
wantWarnings: false,
411+
"no action when info is nil": {},
412+
"no action when Volcano policy not enabled": {
413+
info: runtime.NewInfo(),
414+
oldObj: utiltesting.MakeTrainJobWrapper(metav1.NamespaceDefault, "test").
415+
Obj(),
416+
newObj: utiltesting.MakeTrainJobWrapper(metav1.NamespaceDefault, "test").
417+
Obj(),
492418
},
493-
"priorityClassName is system-cluster-critical": {
494-
info: &runtime.Info{
495-
Annotations: map[string]string{},
496-
RuntimePolicy: runtime.RuntimePolicy{
497-
PodGroupPolicy: &trainer.PodGroupPolicy{
498-
PodGroupPolicySource: trainer.PodGroupPolicySource{
499-
Volcano: &trainer.VolcanoPodGroupPolicySource{},
500-
},
419+
"queue annotation is empty": {
420+
info: runtime.NewInfo(
421+
runtime.WithPodGroupPolicy(&trainer.PodGroupPolicy{
422+
PodGroupPolicySource: trainer.PodGroupPolicySource{
423+
Volcano: &trainer.VolcanoPodGroupPolicySource{},
501424
},
502-
},
503-
TemplateSpec: runtime.TemplateSpec{
504-
ObjApply: createJobSetSpecWithPriority("system-cluster-critical"),
505-
},
506-
},
507-
trainJob: &trainer.TrainJob{},
508-
objs: []client.Object{},
509-
wantErrors: false,
510-
wantWarnings: false,
511-
},
512-
"priorityClassName is system-node-critical": {
513-
info: &runtime.Info{
514-
Annotations: map[string]string{},
515-
RuntimePolicy: runtime.RuntimePolicy{
516-
PodGroupPolicy: &trainer.PodGroupPolicy{
517-
PodGroupPolicySource: trainer.PodGroupPolicySource{
518-
Volcano: &trainer.VolcanoPodGroupPolicySource{},
519-
},
520-
},
521-
},
522-
TemplateSpec: runtime.TemplateSpec{
523-
ObjApply: createJobSetSpecWithPriority("system-node-critical"),
524-
},
525-
},
526-
trainJob: &trainer.TrainJob{},
527-
objs: []client.Object{},
528-
wantErrors: false,
529-
wantWarnings: false,
530-
},
531-
"priorityClassName exists": {
532-
info: &runtime.Info{
533-
Annotations: map[string]string{},
534-
RuntimePolicy: runtime.RuntimePolicy{
535-
PodGroupPolicy: &trainer.PodGroupPolicy{
536-
PodGroupPolicySource: trainer.PodGroupPolicySource{
537-
Volcano: &trainer.VolcanoPodGroupPolicySource{},
538-
},
539-
},
540-
},
541-
TemplateSpec: runtime.TemplateSpec{
542-
ObjApply: createJobSetSpecWithPriority("existing-priority"),
543-
},
425+
}),
426+
runtime.WithAnnotations(map[string]string{
427+
volcanov1beta1.QueueNameAnnotationKey: "",
428+
}),
429+
),
430+
newObj: utiltesting.MakeTrainJobWrapper(metav1.NamespaceDefault, "test").
431+
Obj(),
432+
wantError: field.ErrorList{
433+
field.Invalid(
434+
field.NewPath("spec").Child("annotations").Key(volcanov1beta1.QueueNameAnnotationKey),
435+
"",
436+
"Volcano queue name must not be empty",
437+
),
544438
},
545-
trainJob: &trainer.TrainJob{},
546-
objs: []client.Object{createPriorityClass("existing-priority")},
547-
wantErrors: false,
548-
wantWarnings: false,
549439
},
550440
"priorityClassName does not exist": {
551-
info: &runtime.Info{
552-
Annotations: map[string]string{},
553-
RuntimePolicy: runtime.RuntimePolicy{
554-
PodGroupPolicy: &trainer.PodGroupPolicy{
555-
PodGroupPolicySource: trainer.PodGroupPolicySource{
556-
Volcano: &trainer.VolcanoPodGroupPolicySource{},
557-
},
441+
info: runtime.NewInfo(
442+
runtime.WithPodGroupPolicy(&trainer.PodGroupPolicy{
443+
PodGroupPolicySource: trainer.PodGroupPolicySource{
444+
Volcano: &trainer.VolcanoPodGroupPolicySource{},
558445
},
559-
},
560-
TemplateSpec: runtime.TemplateSpec{
561-
ObjApply: createJobSetSpecWithPriority("non-existent"),
562-
},
563-
},
564-
trainJob: &trainer.TrainJob{},
565-
objs: []client.Object{},
566-
wantErrors: true,
567-
wantWarnings: false,
568-
},
569-
"no volcano policy": {
570-
info: &runtime.Info{
571-
Annotations: map[string]string{},
572-
RuntimePolicy: runtime.RuntimePolicy{
573-
PodGroupPolicy: &trainer.PodGroupPolicy{
574-
PodGroupPolicySource: trainer.PodGroupPolicySource{
575-
Volcano: nil,
576-
},
577-
},
578-
},
446+
}),
447+
runtime.WithTemplateSpecObjApply(
448+
jobsetv1alpha2ac.JobSetSpec().
449+
WithReplicatedJobs(jobsetv1alpha2ac.ReplicatedJob().
450+
WithTemplate(batchv1ac.JobTemplateSpec().
451+
WithSpec(batchv1ac.JobSpec().
452+
WithTemplate(corev1ac.PodTemplateSpec().
453+
WithSpec(corev1ac.PodSpec().
454+
WithPriorityClassName("non-existent"),
455+
),
456+
),
457+
),
458+
),
459+
),
460+
),
461+
),
462+
newObj: utiltesting.MakeTrainJobWrapper(metav1.NamespaceDefault, "test").Obj(),
463+
wantError: field.ErrorList{
464+
field.Invalid(
465+
field.NewPath("spec").Child("templateSpec").Child("priorityClassName"),
466+
"non-existent",
467+
fmt.Sprintf(`PriorityClass "non-existent" doesn't exist: priorityclasses.scheduling.k8s.io "non-existent" not found`),
468+
),
579469
},
580-
trainJob: &trainer.TrainJob{},
581-
objs: []client.Object{},
582-
wantErrors: false,
583-
wantWarnings: false,
584470
},
585-
"nil info": {
586-
info: nil,
587-
trainJob: &trainer.TrainJob{},
588-
objs: []client.Object{},
589-
wantErrors: false,
590-
wantWarnings: false,
591-
},
592-
"nil trainJob": {
593-
info: &runtime.Info{
594-
RuntimePolicy: runtime.RuntimePolicy{
595-
PodGroupPolicy: &trainer.PodGroupPolicy{
596-
PodGroupPolicySource: trainer.PodGroupPolicySource{
597-
Volcano: &trainer.VolcanoPodGroupPolicySource{},
598-
},
471+
"priorityClassName exists": {
472+
info: runtime.NewInfo(
473+
runtime.WithPodGroupPolicy(&trainer.PodGroupPolicy{
474+
PodGroupPolicySource: trainer.PodGroupPolicySource{
475+
Volcano: &trainer.VolcanoPodGroupPolicySource{},
599476
},
477+
}),
478+
runtime.WithTemplateSpecObjApply(runtime.TemplateSpec{
479+
ObjApply: jobsetv1alpha2ac.JobSetSpec().
480+
WithReplicatedJobs(jobsetv1alpha2ac.ReplicatedJob().
481+
WithTemplate(batchv1ac.JobTemplateSpec().
482+
WithSpec(batchv1ac.JobSpec().
483+
WithTemplate(corev1ac.PodTemplateSpec().
484+
WithSpec(corev1ac.PodSpec().
485+
WithPriorityClassName("existing-priority"),
486+
),
487+
),
488+
),
489+
),
490+
),
491+
}),
492+
),
493+
objs: []client.Object{
494+
&schedulingv1.PriorityClass{
495+
ObjectMeta: metav1.ObjectMeta{Name: "existing-priority"},
496+
Value: 100,
600497
},
601498
},
602-
trainJob: nil,
603-
objs: []client.Object{},
604-
wantErrors: false,
605-
wantWarnings: false,
499+
newObj: utiltesting.MakeTrainJobWrapper(metav1.NamespaceDefault, "test").Obj(),
606500
},
607501
}
608502

609503
for name, tc := range cases {
610504
t.Run(name, func(t *testing.T) {
505+
_, ctx := ktesting.NewTestContext(t)
506+
ctx, cancel := context.WithCancel(ctx)
507+
t.Cleanup(cancel)
508+
611509
clientBuilder := utiltesting.NewClientBuilder().WithObjects(tc.objs...)
612510
cli := clientBuilder.Build()
613-
v := &Volcano{client: cli}
614511

615-
warnings, errs := v.Validate(ctx, tc.info, nil, tc.trainJob)
512+
v, err := New(ctx, cli, nil)
513+
if err != nil {
514+
t.Fatalf("failed to init Volcano plugin: %v", err)
515+
}
516+
517+
warnings, errs := v.(framework.CustomValidationPlugin).Validate(ctx, tc.info, tc.oldObj, tc.newObj)
518+
519+
fmt.Printf("warnings: %v, errs: %v\n", warnings, errs)
616520

617-
if (len(errs) > 0) != tc.wantErrors {
618-
t.Errorf("Unexpected errors from Validate (-wantErrors %v, +got %d errors): %v", tc.wantErrors, len(errs), errs)
521+
if diff := gocmp.Diff(tc.wantError, errs); diff != "" {
522+
t.Errorf("Unexpected Validate errors (-want,+got):\n%s", diff)
619523
}
620524

621-
if (len(warnings) > 0) != tc.wantWarnings {
622-
t.Errorf("Unexpected warnings from Validate (-wantWarnings %v, +got %d warnings): %v", tc.wantWarnings, len(warnings), warnings)
525+
if diff := gocmp.Diff(tc.wantWarnings, warnings); diff != "" {
526+
t.Errorf("Unexpected Validate warnings (-want,+got):\n%s", diff)
623527
}
624528
})
625529
}

0 commit comments

Comments
 (0)