Remove conflict logic from PodTolerationRestriction

This commit is contained in:
Tim Allclair 2019-08-21 11:56:23 -07:00
parent 5a50b3f4a2
commit 2e08288144
4 changed files with 23 additions and 38 deletions

View File

@ -46,7 +46,7 @@ next:
// another, only the superset is kept.
func MergeTolerations(first, second []api.Toleration) []api.Toleration {
all := append(first, second...)
merged := []api.Toleration{}
var merged []api.Toleration
next:
for i, t := range all {
@ -58,7 +58,7 @@ next:
if i+1 < len(all) {
for _, t2 := range all[i+1:] {
// If the tolerations are equal, prefer the first.
if isSuperset(t2, t) && !apiequality.Semantic.DeepEqual(&t, &t2) {
if !apiequality.Semantic.DeepEqual(&t, &t2) && isSuperset(t2, t) {
continue next // t is redundant; ignore it
}
}

View File

@ -14,7 +14,6 @@ go_test(
"//pkg/apis/core:go_default_library",
"//pkg/features:go_default_library",
"//pkg/scheduler/api:go_default_library",
"//pkg/util/tolerations:go_default_library",
"//plugin/pkg/admission/podtolerationrestriction/apis/podtolerationrestriction:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library",
@ -27,6 +26,7 @@ go_test(
"//staging/src/k8s.io/client-go/kubernetes:go_default_library",
"//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library",
"//staging/src/k8s.io/component-base/featuregate/testing:go_default_library",
"//vendor/github.com/stretchr/testify/assert:go_default_library",
],
)

View File

@ -84,7 +84,7 @@ func (p *Plugin) Admit(ctx context.Context, a admission.Attributes, o admission.
}
pod := a.GetObject().(*api.Pod)
var finalTolerations []api.Toleration
var extraTolerations []api.Toleration
if a.GetOperation() == admission.Create {
ts, err := p.getNamespaceDefaultTolerations(a.GetNamespace())
if err != nil {
@ -97,37 +97,20 @@ func (p *Plugin) Admit(ctx context.Context, a admission.Attributes, o admission.
ts = p.pluginConfig.Default
}
if len(ts) > 0 {
if len(pod.Spec.Tolerations) > 0 {
if tolerations.IsConflict(ts, pod.Spec.Tolerations) {
return fmt.Errorf("namespace tolerations and pod tolerations conflict")
}
// modified pod tolerations = namespace tolerations + current pod tolerations
finalTolerations = tolerations.MergeTolerations(ts, pod.Spec.Tolerations)
} else {
finalTolerations = ts
}
} else {
finalTolerations = pod.Spec.Tolerations
}
} else {
finalTolerations = pod.Spec.Tolerations
extraTolerations = ts
}
if qoshelper.GetPodQOS(pod) != api.PodQOSBestEffort {
finalTolerations = tolerations.MergeTolerations(finalTolerations, []api.Toleration{
{
Key: schedulerapi.TaintNodeMemoryPressure,
Operator: api.TolerationOpExists,
Effect: api.TaintEffectNoSchedule,
},
extraTolerations = append(extraTolerations, api.Toleration{
Key: schedulerapi.TaintNodeMemoryPressure,
Operator: api.TolerationOpExists,
Effect: api.TaintEffectNoSchedule,
})
}
// Final merge of tolerations irrespective of pod type, if the user while creating pods gives
// conflicting tolerations(with same key+effect), the existing ones should be overwritten by latest one
pod.Spec.Tolerations = tolerations.MergeTolerations(finalTolerations, []api.Toleration{})
// Final merge of tolerations irrespective of pod type.
if len(extraTolerations) > 0 {
pod.Spec.Tolerations = tolerations.MergeTolerations(pod.Spec.Tolerations, extraTolerations)
}
return p.Validate(ctx, a, o)
}

View File

@ -22,6 +22,7 @@ import (
"testing"
"time"
"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@ -36,7 +37,6 @@ import (
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/features"
schedulerapi "k8s.io/kubernetes/pkg/scheduler/api"
"k8s.io/kubernetes/pkg/util/tolerations"
pluginapi "k8s.io/kubernetes/plugin/pkg/admission/podtolerationrestriction/apis/podtolerationrestriction"
)
@ -139,10 +139,11 @@ func TestPodAdmission(t *testing.T) {
{
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",
namespaceTolerations: []api.Toleration{{Key: "testKey", Operator: "Equal", Value: "testValue", Effect: "NoSchedule"}},
podTolerations: []api.Toleration{{Key: "testKey", Operator: "Equal", Value: "testValue1", Effect: "NoSchedule"}},
mergedTolerations: []api.Toleration{{Key: "testKey", Operator: "Equal", Value: "testValue", Effect: "NoSchedule"}, {Key: "testKey", Operator: "Equal", Value: "testValue1", Effect: "NoSchedule"}},
admit: true,
testName: "duplicate key pod and namespace tolerations",
},
{
pod: bestEffortPod,
@ -210,10 +211,11 @@ func TestPodAdmission(t *testing.T) {
whitelist: []api.Toleration{},
podTolerations: []api.Toleration{{Key: "testKey", Operator: "Equal", Value: "testValue", Effect: "NoSchedule", TolerationSeconds: nil}, {Key: "testKey", Operator: "Equal", Value: "testValue1", Effect: "NoSchedule", TolerationSeconds: nil}},
mergedTolerations: []api.Toleration{
{Key: "testKey", Operator: "Equal", Value: "testValue", Effect: "NoSchedule", TolerationSeconds: nil},
{Key: "testKey", Operator: "Equal", Value: "testValue1", Effect: "NoSchedule", TolerationSeconds: nil},
},
admit: true,
testName: "Besteffort pod should overwrite for conflicting tolerations",
testName: "Pod with duplicate key tolerations should not be modified",
},
{
pod: guaranteedPod,
@ -275,8 +277,8 @@ func TestPodAdmission(t *testing.T) {
}
updatedPodTolerations := pod.Spec.Tolerations
if test.admit && !tolerations.EqualTolerations(updatedPodTolerations, test.mergedTolerations) {
t.Errorf("Test: %s, expected: %#v but got: %#v", test.testName, test.mergedTolerations, updatedPodTolerations)
if test.admit {
assert.ElementsMatch(t, updatedPodTolerations, test.mergedTolerations)
}
})
}