Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Task 3: Add MemoryPressure toleration for no BestEffort pod. #50180

Merged
merged 1 commit into from
Aug 14, 2017
Merged

Task 3: Add MemoryPressure toleration for no BestEffort pod. #50180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions plugin/pkg/admission/podtolerationrestriction/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,11 @@ go_test(
"//pkg/kubeapiserver/admission:go_default_library",
"//pkg/util/tolerations:go_default_library",
"//plugin/pkg/admission/podtolerationrestriction/apis/podtolerationrestriction:go_default_library",
"//plugin/pkg/scheduler/algorithm:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apiserver/pkg/admission:go_default_library",
"//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library",
],
)

Expand All @@ -35,6 +38,7 @@ go_library(
tags = ["automanaged"],
deps = [
"//pkg/api:go_default_library",
"//pkg/api/helper/qos:go_default_library",
"//pkg/api/v1:go_default_library",
"//pkg/client/clientset_generated/internalclientset:go_default_library",
"//pkg/client/informers/informers_generated/internalversion:go_default_library",
Expand All @@ -45,6 +49,7 @@ go_library(
"//plugin/pkg/admission/podtolerationrestriction/apis/podtolerationrestriction/install:go_default_library",
"//plugin/pkg/admission/podtolerationrestriction/apis/podtolerationrestriction/v1alpha1:go_default_library",
"//plugin/pkg/admission/podtolerationrestriction/apis/podtolerationrestriction/validation:go_default_library",
"//plugin/pkg/scheduler/algorithm:go_default_library",
"//vendor/github.com/golang/glog:go_default_library",
"//vendor/k8s.io/api/core/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
Expand Down
12 changes: 12 additions & 0 deletions plugin/pkg/admission/podtolerationrestriction/admission.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,15 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apiserver/pkg/admission"
"k8s.io/kubernetes/pkg/api"
qoshelper "k8s.io/kubernetes/pkg/api/helper/qos"
k8s_api_v1 "k8s.io/kubernetes/pkg/api/v1"
clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
informers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion"
corelisters "k8s.io/kubernetes/pkg/client/listers/core/internalversion"
kubeapiserveradmission "k8s.io/kubernetes/pkg/kubeapiserver/admission"
"k8s.io/kubernetes/pkg/util/tolerations"
pluginapi "k8s.io/kubernetes/plugin/pkg/admission/podtolerationrestriction/apis/podtolerationrestriction"
"k8s.io/kubernetes/plugin/pkg/scheduler/algorithm"
)

// Register registers a plugin
Expand Down Expand Up @@ -162,6 +164,16 @@ func (p *podTolerationsPlugin) Admit(a admission.Attributes) error {
}
}

if qoshelper.GetPodQOS(pod) != api.PodQOSBestEffort {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to ensure that this admission controller is run after all resource defaulting controllers.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm... , do you means this admission controller need to be the latest one in apiserver's args?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its a doc issue. you basically will want this controller to be the one that appears right before ResourceQuota

finalTolerations = tolerations.MergeTolerations(finalTolerations, []api.Toleration{
{
Key: algorithm.TaintNodeMemoryPressure,
Operator: api.TolerationOpExists,
Effect: api.TaintEffectNoSchedule,
},
})
}

pod.Spec.Tolerations = finalTolerations
return nil

Expand Down
85 changes: 84 additions & 1 deletion plugin/pkg/admission/podtolerationrestriction/admission_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,18 @@ import (
"testing"
"time"

"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apiserver/pkg/admission"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/kubernetes/pkg/api"
clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake"
informers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion"
kubeadmission "k8s.io/kubernetes/pkg/kubeapiserver/admission"
"k8s.io/kubernetes/pkg/util/tolerations"
pluginapi "k8s.io/kubernetes/plugin/pkg/admission/podtolerationrestriction/apis/podtolerationrestriction"
"k8s.io/kubernetes/plugin/pkg/scheduler/algorithm"
)

// TestPodAdmission verifies various scenarios involving pod/namespace tolerations
Expand All @@ -50,11 +53,56 @@ func TestPodAdmission(t *testing.T) {
defer close(stopCh)
informerFactory.Start(stopCh)

pod := &api.Pod{
CPU1000m := resource.MustParse("1000m")
CPU500m := resource.MustParse("500m")

burstablePod := &api.Pod{
ObjectMeta: metav1.ObjectMeta{Name: "testPod", Namespace: "testNamespace"},
Spec: api.PodSpec{
Containers: []api.Container{
{
Name: "test",
Resources: api.ResourceRequirements{
Limits: api.ResourceList{api.ResourceCPU: CPU1000m},
Requests: api.ResourceList{api.ResourceCPU: CPU500m},
},
},
},
},
}

guaranteedPod := &api.Pod{
ObjectMeta: metav1.ObjectMeta{Name: "testPod", Namespace: "testNamespace"},
Spec: api.PodSpec{
Containers: []api.Container{
{
Name: "test",
Resources: api.ResourceRequirements{
Limits: api.ResourceList{api.ResourceCPU: CPU1000m},
Requests: api.ResourceList{api.ResourceCPU: CPU1000m},
},
},
},
},
}

bestEffortPod := &api.Pod{
ObjectMeta: metav1.ObjectMeta{Name: "testPod", Namespace: "testNamespace"},
Spec: api.PodSpec{
Containers: []api.Container{
{
Name: "test",
},
},
},
}

if err := utilfeature.DefaultFeatureGate.Set("TaintNodesByCondition=true"); err != nil {
t.Errorf("Failed to enable TaintByCondition feature: %v.", err)
}

tests := []struct {
pod *api.Pod
defaultClusterTolerations []api.Toleration
namespaceTolerations []api.Toleration
whitelist []api.Toleration
Expand All @@ -65,6 +113,7 @@ func TestPodAdmission(t *testing.T) {
testName string
}{
{
pod: bestEffortPod,
defaultClusterTolerations: []api.Toleration{{Key: "testKey", Operator: "Equal", Value: "testValue", Effect: "NoSchedule", TolerationSeconds: nil}},
namespaceTolerations: []api.Toleration{},
podTolerations: []api.Toleration{},
Expand All @@ -73,6 +122,7 @@ func TestPodAdmission(t *testing.T) {
testName: "default cluster tolerations with empty pod tolerations",
},
{
pod: bestEffortPod,
defaultClusterTolerations: []api.Toleration{{Key: "testKey", Operator: "Equal", Value: "testValue", Effect: "NoSchedule", TolerationSeconds: nil}},
namespaceTolerations: []api.Toleration{},
podTolerations: []api.Toleration{{Key: "testKey", Operator: "Equal", Value: "testValue", Effect: "NoSchedule", TolerationSeconds: nil}},
Expand All @@ -81,6 +131,7 @@ func TestPodAdmission(t *testing.T) {
testName: "default cluster tolerations with pod tolerations specified",
},
{
pod: bestEffortPod,
defaultClusterTolerations: []api.Toleration{},
namespaceTolerations: []api.Toleration{{Key: "testKey", Operator: "Equal", Value: "testValue", Effect: "NoSchedule", TolerationSeconds: nil}},
podTolerations: []api.Toleration{{Key: "testKey", Operator: "Equal", Value: "testValue", Effect: "NoSchedule", TolerationSeconds: nil}},
Expand All @@ -89,6 +140,7 @@ func TestPodAdmission(t *testing.T) {
testName: "namespace tolerations",
},
{
pod: bestEffortPod,
defaultClusterTolerations: []api.Toleration{},
namespaceTolerations: []api.Toleration{{Key: "testKey", Operator: "Equal", Value: "testValue", Effect: "NoSchedule", TolerationSeconds: nil}},
podTolerations: []api.Toleration{},
Expand All @@ -97,20 +149,23 @@ func TestPodAdmission(t *testing.T) {
testName: "no pod tolerations",
},
{
pod: bestEffortPod,
defaultClusterTolerations: []api.Toleration{},
namespaceTolerations: []api.Toleration{{Key: "testKey", Operator: "Equal", Value: "testValue", Effect: "NoSchedule", TolerationSeconds: nil}},
podTolerations: []api.Toleration{{Key: "testKey", Operator: "Equal", Value: "testValue1", Effect: "NoSchedule", TolerationSeconds: nil}},
admit: false,
testName: "conflicting pod and namespace tolerations",
},
{
pod: bestEffortPod,
defaultClusterTolerations: []api.Toleration{{Key: "testKey", Operator: "Equal", Value: "testValue2", Effect: "NoSchedule", TolerationSeconds: nil}},
namespaceTolerations: []api.Toleration{},
podTolerations: []api.Toleration{{Key: "testKey", Operator: "Equal", Value: "testValue1", Effect: "NoSchedule", TolerationSeconds: nil}},
admit: false,
testName: "conflicting pod and default cluster tolerations",
},
{
pod: bestEffortPod,
defaultClusterTolerations: []api.Toleration{},
namespaceTolerations: []api.Toleration{{Key: "testKey", Operator: "Equal", Value: "testValue", Effect: "NoSchedule", TolerationSeconds: nil}},
whitelist: []api.Toleration{{Key: "testKey", Operator: "Equal", Value: "testValue", Effect: "NoSchedule", TolerationSeconds: nil}},
Expand All @@ -120,13 +175,40 @@ func TestPodAdmission(t *testing.T) {
testName: "merged pod tolerations satisfy whitelist",
},
{
pod: bestEffortPod,
defaultClusterTolerations: []api.Toleration{},
namespaceTolerations: []api.Toleration{{Key: "testKey", Operator: "Equal", Value: "testValue", Effect: "NoSchedule", TolerationSeconds: nil}},
whitelist: []api.Toleration{{Key: "testKey", Operator: "Equal", Value: "testValue1", Effect: "NoSchedule", TolerationSeconds: nil}},
podTolerations: []api.Toleration{},
admit: false,
testName: "merged pod tolerations conflict with the whitelist",
},
{
pod: burstablePod,
defaultClusterTolerations: []api.Toleration{},
namespaceTolerations: []api.Toleration{{Key: "testKey", Operator: "Equal", Value: "testValue", Effect: "NoSchedule", TolerationSeconds: nil}},
whitelist: []api.Toleration{},
podTolerations: []api.Toleration{},
mergedTolerations: []api.Toleration{
{Key: algorithm.TaintNodeMemoryPressure, Operator: api.TolerationOpExists, Effect: api.TaintEffectNoSchedule, TolerationSeconds: nil},
{Key: "testKey", Operator: "Equal", Value: "testValue", Effect: "NoSchedule", TolerationSeconds: nil},
},
admit: true,
testName: "added memoryPressure/DiskPressure for Burstable pod",
},
{
pod: guaranteedPod,
defaultClusterTolerations: []api.Toleration{},
namespaceTolerations: []api.Toleration{{Key: "testKey", Operator: "Equal", Value: "testValue", Effect: "NoSchedule", TolerationSeconds: nil}},
whitelist: []api.Toleration{},
podTolerations: []api.Toleration{},
mergedTolerations: []api.Toleration{
{Key: algorithm.TaintNodeMemoryPressure, Operator: api.TolerationOpExists, Effect: api.TaintEffectNoSchedule, TolerationSeconds: nil},
{Key: "testKey", Operator: "Equal", Value: "testValue", Effect: "NoSchedule", TolerationSeconds: nil},
},
admit: true,
testName: "added memoryPressure/DiskPressure for Guaranteed pod",
},
}
for _, test := range tests {
if len(test.namespaceTolerations) > 0 {
Expand All @@ -148,6 +230,7 @@ func TestPodAdmission(t *testing.T) {
informerFactory.Core().InternalVersion().Namespaces().Informer().GetStore().Update(namespace)

handler.pluginConfig = &pluginapi.Configuration{Default: test.defaultClusterTolerations, Whitelist: test.clusterWhitelist}
pod := test.pod
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add this explicitly to all test cases.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

pod.Spec.Tolerations = test.podTolerations

err := handler.Admit(admission.NewAttributesRecord(pod, nil, api.Kind("Pod").WithVersion("version"), "testNamespace", namespace.ObjectMeta.Name, api.Resource("pods").WithVersion("version"), "", admission.Create, nil))
Expand Down