From 083b92acf3e79c29b91afdc2ded57448fbc04bbb Mon Sep 17 00:00:00 2001 From: "Da K. Ma" Date: Wed, 1 Aug 2018 16:44:42 +0800 Subject: [PATCH] Set PriorityClassName when there's a default PirorityClass. Signed-off-by: Da K. Ma --- plugin/pkg/admission/priority/admission.go | 13 ++- .../pkg/admission/priority/admission_test.go | 100 ++++++++++-------- 2 files changed, 66 insertions(+), 47 deletions(-) diff --git a/plugin/pkg/admission/priority/admission.go b/plugin/pkg/admission/priority/admission.go index 8ae7afbe048..11ade693533 100644 --- a/plugin/pkg/admission/priority/admission.go +++ b/plugin/pkg/admission/priority/admission.go @@ -188,10 +188,12 @@ func (p *priorityPlugin) admitPod(a admission.Attributes) error { } if len(pod.Spec.PriorityClassName) == 0 { var err error - priority, err = p.getDefaultPriority() + var pcName string + pcName, priority, err = p.getDefaultPriority() if err != nil { return fmt.Errorf("failed to get default priority class: %v", err) } + pod.Spec.PriorityClassName = pcName } else { pcName := pod.Spec.PriorityClassName if !priorityClassPermittedInNamespace(pcName, a.GetNamespace()) { @@ -260,13 +262,14 @@ func (p *priorityPlugin) getDefaultPriorityClass() (*schedulingv1beta1.PriorityC return defaultPC, nil } -func (p *priorityPlugin) getDefaultPriority() (int32, error) { +func (p *priorityPlugin) getDefaultPriority() (string, int32, error) { dpc, err := p.getDefaultPriorityClass() if err != nil { - return 0, err + return "", 0, err } if dpc != nil { - return dpc.Value, nil + return dpc.Name, dpc.Value, nil } - return int32(scheduling.DefaultPriorityWhenNoDefaultClassExists), nil + + return "", int32(scheduling.DefaultPriorityWhenNoDefaultClassExists), nil } diff --git a/plugin/pkg/admission/priority/admission_test.go b/plugin/pkg/admission/priority/admission_test.go index 19e599b778f..ef1c43c2a11 100644 --- a/plugin/pkg/admission/priority/admission_test.go +++ b/plugin/pkg/admission/priority/admission_test.go @@ -177,52 +177,64 @@ func TestDefaultPriority(t *testing.T) { updatedDefaultClass1.GlobalDefault = false tests := []struct { - name string - classesBefore []*scheduling.PriorityClass - classesAfter []*scheduling.PriorityClass - attributes admission.Attributes - expectedDefaultBefore int32 - expectedDefaultAfter int32 + name string + classesBefore []*scheduling.PriorityClass + classesAfter []*scheduling.PriorityClass + attributes admission.Attributes + expectedDefaultBefore int32 + expectedDefaultNameBefore string + expectedDefaultAfter int32 + expectedDefaultNameAfter string }{ { - name: "simple resolution with a default class", - classesBefore: []*scheduling.PriorityClass{defaultClass1}, - classesAfter: []*scheduling.PriorityClass{defaultClass1}, - attributes: nil, - expectedDefaultBefore: defaultClass1.Value, - expectedDefaultAfter: defaultClass1.Value, + name: "simple resolution with a default class", + classesBefore: []*scheduling.PriorityClass{defaultClass1}, + classesAfter: []*scheduling.PriorityClass{defaultClass1}, + attributes: nil, + expectedDefaultBefore: defaultClass1.Value, + expectedDefaultNameBefore: defaultClass1.Name, + expectedDefaultAfter: defaultClass1.Value, + expectedDefaultNameAfter: defaultClass1.Name, }, { - name: "add a default class", - classesBefore: []*scheduling.PriorityClass{nondefaultClass1}, - classesAfter: []*scheduling.PriorityClass{nondefaultClass1, defaultClass1}, - attributes: admission.NewAttributesRecord(defaultClass1, nil, pcKind, "", defaultClass1.Name, pcResource, "", admission.Create, false, nil), - expectedDefaultBefore: scheduling.DefaultPriorityWhenNoDefaultClassExists, - expectedDefaultAfter: defaultClass1.Value, + name: "add a default class", + classesBefore: []*scheduling.PriorityClass{nondefaultClass1}, + classesAfter: []*scheduling.PriorityClass{nondefaultClass1, defaultClass1}, + attributes: admission.NewAttributesRecord(defaultClass1, nil, pcKind, "", defaultClass1.Name, pcResource, "", admission.Create, false, nil), + expectedDefaultBefore: scheduling.DefaultPriorityWhenNoDefaultClassExists, + expectedDefaultNameBefore: "", + expectedDefaultAfter: defaultClass1.Value, + expectedDefaultNameAfter: defaultClass1.Name, }, { - name: "multiple default classes resolves to the minimum value among them", - classesBefore: []*scheduling.PriorityClass{defaultClass1, defaultClass2}, - classesAfter: []*scheduling.PriorityClass{defaultClass2}, - attributes: admission.NewAttributesRecord(nil, nil, pcKind, "", defaultClass1.Name, pcResource, "", admission.Delete, false, nil), - expectedDefaultBefore: defaultClass1.Value, - expectedDefaultAfter: defaultClass2.Value, + name: "multiple default classes resolves to the minimum value among them", + classesBefore: []*scheduling.PriorityClass{defaultClass1, defaultClass2}, + classesAfter: []*scheduling.PriorityClass{defaultClass2}, + attributes: admission.NewAttributesRecord(nil, nil, pcKind, "", defaultClass1.Name, pcResource, "", admission.Delete, false, nil), + expectedDefaultBefore: defaultClass1.Value, + expectedDefaultNameBefore: defaultClass1.Name, + expectedDefaultAfter: defaultClass2.Value, + expectedDefaultNameAfter: defaultClass2.Name, }, { - name: "delete default priority class", - classesBefore: []*scheduling.PriorityClass{defaultClass1}, - classesAfter: []*scheduling.PriorityClass{}, - attributes: admission.NewAttributesRecord(nil, nil, pcKind, "", defaultClass1.Name, pcResource, "", admission.Delete, false, nil), - expectedDefaultBefore: defaultClass1.Value, - expectedDefaultAfter: scheduling.DefaultPriorityWhenNoDefaultClassExists, + name: "delete default priority class", + classesBefore: []*scheduling.PriorityClass{defaultClass1}, + classesAfter: []*scheduling.PriorityClass{}, + attributes: admission.NewAttributesRecord(nil, nil, pcKind, "", defaultClass1.Name, pcResource, "", admission.Delete, false, nil), + expectedDefaultBefore: defaultClass1.Value, + expectedDefaultNameBefore: defaultClass1.Name, + expectedDefaultAfter: scheduling.DefaultPriorityWhenNoDefaultClassExists, + expectedDefaultNameAfter: "", }, { - name: "update default class and remove its global default", - classesBefore: []*scheduling.PriorityClass{defaultClass1}, - classesAfter: []*scheduling.PriorityClass{&updatedDefaultClass1}, - attributes: admission.NewAttributesRecord(&updatedDefaultClass1, defaultClass1, pcKind, "", defaultClass1.Name, pcResource, "", admission.Update, false, nil), - expectedDefaultBefore: defaultClass1.Value, - expectedDefaultAfter: scheduling.DefaultPriorityWhenNoDefaultClassExists, + name: "update default class and remove its global default", + classesBefore: []*scheduling.PriorityClass{defaultClass1}, + classesAfter: []*scheduling.PriorityClass{&updatedDefaultClass1}, + attributes: admission.NewAttributesRecord(&updatedDefaultClass1, defaultClass1, pcKind, "", defaultClass1.Name, pcResource, "", admission.Update, false, nil), + expectedDefaultBefore: defaultClass1.Value, + expectedDefaultNameBefore: defaultClass1.Name, + expectedDefaultAfter: scheduling.DefaultPriorityWhenNoDefaultClassExists, + expectedDefaultNameAfter: "", }, } @@ -232,12 +244,14 @@ func TestDefaultPriority(t *testing.T) { if err := addPriorityClasses(ctrl, test.classesBefore); err != nil { t.Errorf("Test %q: unable to add object to informer: %v", test.name, err) } - defaultPriority, err := ctrl.getDefaultPriority() + pcName, defaultPriority, err := ctrl.getDefaultPriority() if err != nil { t.Errorf("Test %q: unexpected error while getting default priority: %v", test.name, err) } - if err == nil && defaultPriority != test.expectedDefaultBefore { - t.Errorf("Test %q: expected default priority %d, but got %d", test.name, test.expectedDefaultBefore, defaultPriority) + if err == nil && + (defaultPriority != test.expectedDefaultBefore || pcName != test.expectedDefaultNameBefore) { + t.Errorf("Test %q: expected default priority %s(%d), but got %s(%d)", + test.name, test.expectedDefaultNameBefore, test.expectedDefaultBefore, pcName, defaultPriority) } if test.attributes != nil { err := ctrl.Validate(test.attributes) @@ -248,12 +262,14 @@ func TestDefaultPriority(t *testing.T) { if err := addPriorityClasses(ctrl, test.classesAfter); err != nil { t.Errorf("Test %q: unable to add object to informer: %v", test.name, err) } - defaultPriority, err = ctrl.getDefaultPriority() + pcName, defaultPriority, err = ctrl.getDefaultPriority() if err != nil { t.Errorf("Test %q: unexpected error while getting default priority: %v", test.name, err) } - if err == nil && defaultPriority != test.expectedDefaultAfter { - t.Errorf("Test %q: expected default priority %d, but got %d", test.name, test.expectedDefaultAfter, defaultPriority) + if err == nil && + (defaultPriority != test.expectedDefaultAfter || pcName != test.expectedDefaultNameAfter) { + t.Errorf("Test %q: expected default priority %s(%d), but got %s(%d)", + test.name, test.expectedDefaultNameAfter, test.expectedDefaultAfter, pcName, defaultPriority) } } }