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.

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

Admission plugin to merge pod and namespace tolerations for restricting pod placement on nodes #30302

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/kube-apiserver/app/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ go_library(
"//plugin/pkg/admission/persistentvolume/label:go_default_library",
"//plugin/pkg/admission/podnodeselector:go_default_library",
"//plugin/pkg/admission/podpreset:go_default_library",
"//plugin/pkg/admission/podtolerationrestriction:go_default_library",
"//plugin/pkg/admission/resourcequota:go_default_library",
"//plugin/pkg/admission/security/podsecuritypolicy:go_default_library",
"//plugin/pkg/admission/securitycontext/scdeny:go_default_library",
Expand Down
1 change: 1 addition & 0 deletions cmd/kube-apiserver/app/plugins.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (
_ "k8s.io/kubernetes/plugin/pkg/admission/persistentvolume/label"
_ "k8s.io/kubernetes/plugin/pkg/admission/podnodeselector"
_ "k8s.io/kubernetes/plugin/pkg/admission/podpreset"
_ "k8s.io/kubernetes/plugin/pkg/admission/podtolerationrestriction"
_ "k8s.io/kubernetes/plugin/pkg/admission/resourcequota"
_ "k8s.io/kubernetes/plugin/pkg/admission/security/podsecuritypolicy"
_ "k8s.io/kubernetes/plugin/pkg/admission/securitycontext/scdeny"
Expand Down
2 changes: 2 additions & 0 deletions hack/.linted_packages
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,8 @@ plugin/pkg/admission/gc
plugin/pkg/admission/imagepolicy
plugin/pkg/admission/namespace/autoprovision
plugin/pkg/admission/namespace/exists
plugin/pkg/admission/podtolerationrestriction/apis/podtolerationrestriction/install
plugin/pkg/admission/podtolerationrestriction/apis/podtolerationrestriction/validation
plugin/pkg/admission/resourcequota/apis/resourcequota/install
plugin/pkg/admission/resourcequota/apis/resourcequota/validation
plugin/pkg/admission/securitycontext/scdeny
Expand Down
10 changes: 5 additions & 5 deletions pkg/api/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ func ValidateTolerationsInPodAnnotations(annotations map[string]string, fldPath
}

if len(tolerations) > 0 {
allErrs = append(allErrs, validateTolerations(tolerations, fldPath.Child(api.TolerationsAnnotationKey))...)
allErrs = append(allErrs, ValidateTolerations(tolerations, fldPath.Child(api.TolerationsAnnotationKey))...)
}

return allErrs
Expand Down Expand Up @@ -2000,12 +2000,12 @@ func validateOnlyAddedTolerations(newTolerations []api.Toleration, oldToleration
}
}

allErrs = append(allErrs, validateTolerations(newTolerations, fldPath)...)
allErrs = append(allErrs, ValidateTolerations(newTolerations, fldPath)...)
return allErrs
}

// validateTolerations tests if given tolerations have valid data.
func validateTolerations(tolerations []api.Toleration, fldPath *field.Path) field.ErrorList {
// ValidateTolerations tests if given tolerations have valid data.
func ValidateTolerations(tolerations []api.Toleration, fldPath *field.Path) field.ErrorList {
allErrors := field.ErrorList{}
for i, toleration := range tolerations {
idxPath := fldPath.Index(i)
Expand Down Expand Up @@ -2102,7 +2102,7 @@ func ValidatePodSpec(spec *api.PodSpec, fldPath *field.Path) field.ErrorList {
}

if len(spec.Tolerations) > 0 {
allErrs = append(allErrs, validateTolerations(spec.Tolerations, fldPath.Child("tolerations"))...)
allErrs = append(allErrs, ValidateTolerations(spec.Tolerations, fldPath.Child("tolerations"))...)
}

return allErrs
Expand Down
1 change: 1 addition & 0 deletions pkg/util/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ filegroup(
"//pkg/util/taints:all-srcs",
"//pkg/util/term:all-srcs",
"//pkg/util/threading:all-srcs",
"//pkg/util/tolerations:all-srcs",
"//pkg/util/uuid:all-srcs",
"//pkg/util/validation:all-srcs",
"//pkg/util/version:all-srcs",
Expand Down
40 changes: 40 additions & 0 deletions pkg/util/tolerations/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package(default_visibility = ["//visibility:public"])

licenses(["notice"])

load(
"@io_bazel_rules_go//go:def.bzl",
"go_library",
"go_test",
)

go_library(
name = "go_default_library",
srcs = [
"doc.go",
"tolerations.go",
],
tags = ["automanaged"],
deps = ["//pkg/api:go_default_library"],
)

filegroup(
name = "package-srcs",
srcs = glob(["**"]),
tags = ["automanaged"],
visibility = ["//visibility:private"],
)

filegroup(
name = "all-srcs",
srcs = [":package-srcs"],
tags = ["automanaged"],
)

go_test(
name = "go_default_test",
srcs = ["tolerations_test.go"],
library = ":go_default_library",
tags = ["automanaged"],
deps = ["//pkg/api:go_default_library"],
)
18 changes: 18 additions & 0 deletions pkg/util/tolerations/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
Copyright 2017 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

// Package tolerations provides utilities to work with pod spec tolerations.
package tolerations // import "k8s.io/kubernetes/pkg/util/tolerations"
125 changes: 125 additions & 0 deletions pkg/util/tolerations/tolerations.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
/*
Copyright 2017 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package tolerations
Copy link
Member

Choose a reason for hiding this comment

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

@davidopp is utils the proper location for this or should we keep scheduling specific utils near the scheduler? I'm just worried about sprawl. Anyone wanting to understand the implications of (X) now need to crawl through (Y) locations to understand how it behaves.

Copy link
Member Author

Choose a reason for hiding this comment

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

I noticed that the PR #31647 is putting taints related helpers in pkg/utils/taints, so this seems in line.

Copy link
Member

Choose a reason for hiding this comment

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

please file an issue to consolidate the sprawl. between the controllers, admission controllers and node side we are entering the state where minor changes ripple bugs throughout the codebase.


import (
"k8s.io/kubernetes/pkg/api"
)

type key struct {
tolerationKey string
effect api.TaintEffect
}

// VerifyAgainstWhitelist checks if the provided tolerations
// satisfy the provided whitelist and returns true, otherwise returns false
func VerifyAgainstWhitelist(tolerations []api.Toleration, whitelist []api.Toleration) bool {
if len(whitelist) == 0 {
return true
}

t := ConvertTolerationToAMap(tolerations)
w := ConvertTolerationToAMap(whitelist)

for k1, v1 := range t {
if v2, ok := w[k1]; !ok || !AreEqual(v1, v2) {
return false
}
}
return true
}

// IsConflict returns true if the key of two tolerations match
// but one or more other fields differ, otherwise returns false
func IsConflict(first []api.Toleration, second []api.Toleration) bool {
firstMap := ConvertTolerationToAMap(first)
secondMap := ConvertTolerationToAMap(second)

for k1, v1 := range firstMap {
if v2, ok := secondMap[k1]; ok && !AreEqual(v1, v2) {
return true
}
}
return false
}

// MergeTolerations merges two sets of tolerations into one
// it does not check for conflicts
Copy link
Member

Choose a reason for hiding this comment

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

unless i am mistaken, in conflict, it prioritizes the value in the second list over the first list.

Copy link
Member Author

Choose a reason for hiding this comment

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

yes. But in this plugin, its not a concern as we check for conflicts before calling merge.

func MergeTolerations(first []api.Toleration, second []api.Toleration) []api.Toleration {
var mergedTolerations []api.Toleration
mergedTolerations = append(mergedTolerations, second...)

firstMap := ConvertTolerationToAMap(first)
secondMap := ConvertTolerationToAMap(second)

for k1, v1 := range firstMap {
if _, ok := secondMap[k1]; !ok {
mergedTolerations = append(mergedTolerations, v1)
}
}
return mergedTolerations
}

// EqualTolerations returns true if two sets of tolerations are equal, otherwise false
// it assumes no duplicates in individual set of tolerations
func EqualTolerations(first []api.Toleration, second []api.Toleration) bool {
if len(first) != len(second) {
return false
}

firstMap := ConvertTolerationToAMap(first)
secondMap := ConvertTolerationToAMap(second)

for k1, v1 := range firstMap {
if v2, ok := secondMap[k1]; !ok || !AreEqual(v1, v2) {
return false
}
}
return true
}

// ConvertTolerationToAMap converts toleration list into a map[string]api.Toleration
func ConvertTolerationToAMap(in []api.Toleration) map[key]api.Toleration {
out := map[key]api.Toleration{}
for i := range in {
out[key{in[i].Key, in[i].Effect}] = in[i]
}
return out
}

// AreEqual checks if two provided tolerations are equal or not.
func AreEqual(first, second api.Toleration) bool {
if first.Key == second.Key &&
first.Operator == second.Operator &&
first.Value == second.Value &&
first.Effect == second.Effect &&
AreTolerationSecondsEqual(first.TolerationSeconds, second.TolerationSeconds) {
return true
}
return false
}

// AreTolerationSecondsEqual checks if two provided TolerationSeconds are equal or not.
func AreTolerationSecondsEqual(ts1, ts2 *int64) bool {
if ts1 == ts2 {
return true
}
if ts1 != nil && ts2 != nil && *ts1 == *ts2 {
return true
}
return false
}
82 changes: 82 additions & 0 deletions pkg/util/tolerations/tolerations_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*
Copyright 2017 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package tolerations

import (
"testing"

"k8s.io/kubernetes/pkg/api"
)

func TestVerifyAgainstWhitelist(t *testing.T) {
tests := []struct {
input []api.Toleration
whitelist []api.Toleration
testName string
testStatus bool
}{
{
input: []api.Toleration{{Key: "foo", Operator: "Equal", Value: "bar", Effect: "NoSchedule"}},
whitelist: []api.Toleration{{Key: "foo", Operator: "Equal", Value: "bar", Effect: "NoSchedule"}},
testName: "equal input and whitelist",
testStatus: true,
},
{
input: []api.Toleration{{Key: "foo", Operator: "Equal", Value: "bar", Effect: "NoSchedule"}},
whitelist: []api.Toleration{{Key: "foo", Operator: "Equal", Value: "bar", Effect: "NoExecute"}},
testName: "input does not exist in whitelist",
testStatus: false,
},
}

for _, c := range tests {
status := VerifyAgainstWhitelist(c.input, c.whitelist)
if status != c.testStatus {
t.Errorf("Test: %s, expected %v", c.testName, status)
}
}

}

func TestIsConflict(t *testing.T) {
tests := []struct {
input1 []api.Toleration
input2 []api.Toleration
testName string
testStatus bool
}{
{
input1: []api.Toleration{{Key: "foo", Operator: "Equal", Value: "bar", Effect: "NoSchedule"}},
input2: []api.Toleration{{Key: "foo", Operator: "Equal", Value: "bar", Effect: "NoSchedule"}},
testName: "equal inputs",
testStatus: true,
},
{
input1: []api.Toleration{{Key: "foo", Operator: "Equal", Value: "foo", Effect: "NoExecute"}},
input2: []api.Toleration{{Key: "foo", Operator: "Equal", Value: "bar", Effect: "NoExecute"}},
testName: "mismatch values in inputs",
testStatus: false,
},
}

for _, c := range tests {
status := IsConflict(c.input1, c.input2)
if status == c.testStatus {
t.Errorf("Test: %s, expected %v", c.testName, status)
}
}
}
1 change: 1 addition & 0 deletions plugin/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ filegroup(
"//plugin/pkg/admission/persistentvolume/label:all-srcs",
"//plugin/pkg/admission/podnodeselector:all-srcs",
"//plugin/pkg/admission/podpreset:all-srcs",
"//plugin/pkg/admission/podtolerationrestriction:all-srcs",
"//plugin/pkg/admission/resourcequota:all-srcs",
"//plugin/pkg/admission/security:all-srcs",
"//plugin/pkg/admission/securitycontext/scdeny:all-srcs",
Expand Down