critical pod test should not rely on feature gate set in framework; non-critical pods are always preemptable

pull/8/head
David Ashpole 2018-09-07 17:43:42 -07:00
parent b90ca3e2f9
commit 90f58c1157
3 changed files with 22 additions and 10 deletions

View File

@ -130,7 +130,7 @@ func TestEvictPodsToFreeRequests(t *testing.T) {
}
for _, r := range runs {
podProvider.setPods(r.inputPods)
outErr := criticalPodAdmissionHandler.evictPodsToFreeRequests(nil, r.insufficientResources)
outErr := criticalPodAdmissionHandler.evictPodsToFreeRequests(allPods[critical], r.insufficientResources)
outputPods := podKiller.getKilledPods()
if !r.expectErr && outErr != nil {
t.Errorf("evictPodsToFreeRequests returned an unexpected error during the %s test. Err: %v", r.testName, outErr)
@ -171,6 +171,7 @@ func TestGetPodsToPreempt(t *testing.T) {
runs := []testRun{
{
testName: "no requirements",
preemptor: allPods[critical],
inputPods: []*v1.Pod{},
insufficientResources: getAdmissionRequirementList(0, 0, 0),
expectErr: false,
@ -178,6 +179,7 @@ func TestGetPodsToPreempt(t *testing.T) {
},
{
testName: "no pods",
preemptor: allPods[critical],
inputPods: []*v1.Pod{},
insufficientResources: getAdmissionRequirementList(0, 0, 1),
expectErr: true,
@ -185,6 +187,7 @@ func TestGetPodsToPreempt(t *testing.T) {
},
{
testName: "equal pods and resources requirements",
preemptor: allPods[critical],
inputPods: []*v1.Pod{allPods[burstable]},
insufficientResources: getAdmissionRequirementList(100, 100, 1),
expectErr: false,
@ -192,6 +195,7 @@ func TestGetPodsToPreempt(t *testing.T) {
},
{
testName: "higher requirements than pod requests",
preemptor: allPods[critical],
inputPods: []*v1.Pod{allPods[burstable]},
insufficientResources: getAdmissionRequirementList(200, 200, 2),
expectErr: true,
@ -199,6 +203,7 @@ func TestGetPodsToPreempt(t *testing.T) {
},
{
testName: "choose between bestEffort and burstable",
preemptor: allPods[critical],
inputPods: []*v1.Pod{allPods[burstable], allPods[bestEffort]},
insufficientResources: getAdmissionRequirementList(0, 0, 1),
expectErr: false,
@ -206,6 +211,7 @@ func TestGetPodsToPreempt(t *testing.T) {
},
{
testName: "choose between burstable and guaranteed",
preemptor: allPods[critical],
inputPods: []*v1.Pod{allPods[burstable], allPods[guaranteed]},
insufficientResources: getAdmissionRequirementList(0, 0, 1),
expectErr: false,
@ -213,6 +219,7 @@ func TestGetPodsToPreempt(t *testing.T) {
},
{
testName: "choose lower request burstable if it meets requirements",
preemptor: allPods[critical],
inputPods: []*v1.Pod{allPods[bestEffort], allPods[highRequestBurstable], allPods[burstable]},
insufficientResources: getAdmissionRequirementList(100, 100, 0),
expectErr: false,
@ -220,6 +227,7 @@ func TestGetPodsToPreempt(t *testing.T) {
},
{
testName: "choose higher request burstable if lower does not meet requirements",
preemptor: allPods[critical],
inputPods: []*v1.Pod{allPods[bestEffort], allPods[burstable], allPods[highRequestBurstable]},
insufficientResources: getAdmissionRequirementList(150, 150, 0),
expectErr: false,
@ -227,6 +235,7 @@ func TestGetPodsToPreempt(t *testing.T) {
},
{
testName: "multiple pods required",
preemptor: allPods[critical],
inputPods: []*v1.Pod{allPods[bestEffort], allPods[burstable], allPods[highRequestBurstable], allPods[guaranteed], allPods[highRequestGuaranteed]},
insufficientResources: getAdmissionRequirementList(350, 350, 0),
expectErr: false,
@ -234,6 +243,7 @@ func TestGetPodsToPreempt(t *testing.T) {
},
{
testName: "evict guaranteed when we have to, and dont evict the extra burstable",
preemptor: allPods[critical],
inputPods: []*v1.Pod{allPods[bestEffort], allPods[burstable], allPods[highRequestBurstable], allPods[guaranteed], allPods[highRequestGuaranteed]},
insufficientResources: getAdmissionRequirementList(0, 550, 0),
expectErr: false,

View File

@ -159,13 +159,12 @@ func IsCriticalPod(pod *v1.Pod) bool {
return false
}
// Preemptable returns true if preemptor pod can preempt preemptee pod:
// - If preemptor's is greater than preemptee's priority, it's preemptable (return true)
// - If preemptor (or its priority) is nil and preemptee bears the critical pod annotation key,
// preemptee can not be preempted (return false)
// - If preemptor (or its priority) is nil and preemptee's priority is greater than or equal to
// SystemCriticalPriority, preemptee can not be preempted (return false)
// Preemptable returns true if preemptor pod can preempt preemptee pod
// if preemptee is not critical or if preemptor's priority is greater than preemptee's priority
func Preemptable(preemptor, preemptee *v1.Pod) bool {
if IsCriticalPod(preemptor) && !IsCriticalPod(preemptee) {
return true
}
if utilfeature.DefaultFeatureGate.Enabled(features.PodPriority) {
if (preemptor != nil && preemptor.Spec.Priority != nil) &&
(preemptee != nil && preemptee.Spec.Priority != nil) {
@ -173,7 +172,7 @@ func Preemptable(preemptor, preemptee *v1.Pod) bool {
}
}
return !IsCriticalPod(preemptee)
return false
}
// IsCritical returns true if parameters bear the critical pod annotation

View File

@ -45,6 +45,9 @@ var _ = framework.KubeDescribe("CriticalPod [Serial] [Disruptive] [NodeFeature:C
Context("when we need to admit a critical pod", func() {
tempSetCurrentKubeletConfig(f, func(initialConfig *kubeletconfig.KubeletConfiguration) {
if initialConfig.FeatureGates == nil {
initialConfig.FeatureGates = make(map[string]bool)
}
initialConfig.FeatureGates[string(features.ExperimentalCriticalPodAnnotation)] = true
})
@ -142,9 +145,9 @@ func getTestPod(critical bool, name string, resources v1.ResourceRequirements) *
pod.ObjectMeta.Annotations = map[string]string{
kubelettypes.CriticalPodAnnotationKey: "",
}
Expect(kubelettypes.IsCriticalPod(pod)).To(BeTrue(), "pod should be a critical pod")
Expect(kubelettypes.IsCritical(pod.Namespace, pod.Annotations)).To(BeTrue(), "pod should be a critical pod")
} else {
Expect(kubelettypes.IsCriticalPod(pod)).To(BeFalse(), "pod should not be a critical pod")
Expect(kubelettypes.IsCritical(pod.Namespace, pod.Annotations)).To(BeFalse(), "pod should not be a critical pod")
}
return pod
}