Resources prefixed with *kubernetes.io/ should remain unscheduled if they are not exposed on the node.

Currently, resources prefixed with *kubernetes.io/ get scheduled to any
node whether it's exposing that resource or not.

On the other hand, resources prefixed with someother.domain/ don't get
scheduled to a node until that node is exposing that resource (or if the
resource is ignored because of scheduler extender).

This commit brings the behavior of *kubernetes.io/ prefixed resources in
line with other extended resources and they will remain unscheduled
until some node exposes these resources.

This also includes renaming IsDefaultNamespaceResource() to
IsNativeResource().
pull/8/head
Rohit Agarwal 2018-03-28 13:09:23 -07:00
parent 5d1a3287b6
commit e6db88b12d
7 changed files with 45 additions and 19 deletions

View File

@ -153,7 +153,7 @@ func IsStandardContainerResourceName(str string) bool {
// to avoid confusion with the convention in quota // to avoid confusion with the convention in quota
// 3. it satisfies the rules in IsQualifiedName() after converted into quota resource name // 3. it satisfies the rules in IsQualifiedName() after converted into quota resource name
func IsExtendedResourceName(name core.ResourceName) bool { func IsExtendedResourceName(name core.ResourceName) bool {
if IsDefaultNamespaceResource(name) || strings.HasPrefix(string(name), core.DefaultResourceRequestsPrefix) { if IsNativeResource(name) || strings.HasPrefix(string(name), core.DefaultResourceRequestsPrefix) {
return false return false
} }
// Ensure it satisfies the rules in IsQualifiedName() after converted into quota resource name // Ensure it satisfies the rules in IsQualifiedName() after converted into quota resource name
@ -164,10 +164,10 @@ func IsExtendedResourceName(name core.ResourceName) bool {
return true return true
} }
// IsDefaultNamespaceResource returns true if the resource name is in the // IsNativeResource returns true if the resource name is in the
// *kubernetes.io/ namespace. Partially-qualified (unprefixed) names are // *kubernetes.io/ namespace. Partially-qualified (unprefixed) names are
// implicitly in the kubernetes.io/ namespace. // implicitly in the kubernetes.io/ namespace.
func IsDefaultNamespaceResource(name core.ResourceName) bool { func IsNativeResource(name core.ResourceName) bool {
return !strings.Contains(string(name), "/") || return !strings.Contains(string(name), "/") ||
strings.Contains(string(name), core.ResourceDefaultNamespacePrefix) strings.Contains(string(name), core.ResourceDefaultNamespacePrefix)
} }
@ -177,7 +177,7 @@ var overcommitBlacklist = sets.NewString(string(core.ResourceNvidiaGPU))
// IsOvercommitAllowed returns true if the resource is in the default // IsOvercommitAllowed returns true if the resource is in the default
// namespace and not blacklisted. // namespace and not blacklisted.
func IsOvercommitAllowed(name core.ResourceName) bool { func IsOvercommitAllowed(name core.ResourceName) bool {
return IsDefaultNamespaceResource(name) && return IsNativeResource(name) &&
!IsHugePageResourceName(name) && !IsHugePageResourceName(name) &&
!overcommitBlacklist.Has(string(name)) !overcommitBlacklist.Has(string(name))
} }

View File

@ -36,7 +36,7 @@ import (
// to avoid confusion with the convention in quota // to avoid confusion with the convention in quota
// 3. it satisfies the rules in IsQualifiedName() after converted into quota resource name // 3. it satisfies the rules in IsQualifiedName() after converted into quota resource name
func IsExtendedResourceName(name v1.ResourceName) bool { func IsExtendedResourceName(name v1.ResourceName) bool {
if IsDefaultNamespaceResource(name) || strings.HasPrefix(string(name), v1.DefaultResourceRequestsPrefix) { if IsNativeResource(name) || strings.HasPrefix(string(name), v1.DefaultResourceRequestsPrefix) {
return false return false
} }
// Ensure it satisfies the rules in IsQualifiedName() after converted into quota resource name // Ensure it satisfies the rules in IsQualifiedName() after converted into quota resource name
@ -47,13 +47,18 @@ func IsExtendedResourceName(name v1.ResourceName) bool {
return true return true
} }
// IsDefaultNamespaceResource returns true if the resource name is in the // IsPrefixedNativeResource returns true if the resource name is in the
// *kubernetes.io/ namespace.
func IsPrefixedNativeResource(name v1.ResourceName) bool {
return strings.Contains(string(name), v1.ResourceDefaultNamespacePrefix)
}
// IsNativeResource returns true if the resource name is in the
// *kubernetes.io/ namespace. Partially-qualified (unprefixed) names are // *kubernetes.io/ namespace. Partially-qualified (unprefixed) names are
// implicitly in the kubernetes.io/ namespace. // implicitly in the kubernetes.io/ namespace.
func IsDefaultNamespaceResource(name v1.ResourceName) bool { func IsNativeResource(name v1.ResourceName) bool {
return !strings.Contains(string(name), "/") || return !strings.Contains(string(name), "/") ||
strings.Contains(string(name), v1.ResourceDefaultNamespacePrefix) IsPrefixedNativeResource(name)
} }
// IsHugePageResourceName returns true if the resource name has the huge page // IsHugePageResourceName returns true if the resource name has the huge page
@ -85,14 +90,15 @@ var overcommitBlacklist = sets.NewString(string(v1.ResourceNvidiaGPU))
// IsOvercommitAllowed returns true if the resource is in the default // IsOvercommitAllowed returns true if the resource is in the default
// namespace and not blacklisted and is not hugepages. // namespace and not blacklisted and is not hugepages.
func IsOvercommitAllowed(name v1.ResourceName) bool { func IsOvercommitAllowed(name v1.ResourceName) bool {
return IsDefaultNamespaceResource(name) && return IsNativeResource(name) &&
!IsHugePageResourceName(name) && !IsHugePageResourceName(name) &&
!overcommitBlacklist.Has(string(name)) !overcommitBlacklist.Has(string(name))
} }
// Extended and Hugepages resources // Extended and Hugepages resources
func IsScalarResourceName(name v1.ResourceName) bool { func IsScalarResourceName(name v1.ResourceName) bool {
return IsExtendedResourceName(name) || IsHugePageResourceName(name) return IsExtendedResourceName(name) || IsHugePageResourceName(name) ||
IsPrefixedNativeResource(name)
} }
// this function aims to check if the service's ClusterIP is set or not // this function aims to check if the service's ClusterIP is set or not

View File

@ -28,7 +28,7 @@ import (
"k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/labels"
) )
func TestIsDefaultNamespaceResource(t *testing.T) { func TestIsNativeResource(t *testing.T) {
testCases := []struct { testCases := []struct {
resourceName v1.ResourceName resourceName v1.ResourceName
expectVal bool expectVal bool
@ -58,7 +58,7 @@ func TestIsDefaultNamespaceResource(t *testing.T) {
for _, tc := range testCases { for _, tc := range testCases {
t.Run(fmt.Sprintf("resourceName input=%s, expected value=%v", tc.resourceName, tc.expectVal), func(t *testing.T) { t.Run(fmt.Sprintf("resourceName input=%s, expected value=%v", tc.resourceName, tc.expectVal), func(t *testing.T) {
t.Parallel() t.Parallel()
v := IsDefaultNamespaceResource(tc.resourceName) v := IsNativeResource(tc.resourceName)
if v != tc.expectVal { if v != tc.expectVal {
t.Errorf("Got %v but expected %v", v, tc.expectVal) t.Errorf("Got %v but expected %v", v, tc.expectVal)
} }

View File

@ -75,7 +75,7 @@ func validateContainerResourceName(value string, fldPath *field.Path) field.Erro
if !helper.IsStandardContainerResourceName(value) { if !helper.IsStandardContainerResourceName(value) {
return append(allErrs, field.Invalid(fldPath, value, "must be a standard resource for containers")) return append(allErrs, field.Invalid(fldPath, value, "must be a standard resource for containers"))
} }
} else if !v1helper.IsDefaultNamespaceResource(v1.ResourceName(value)) { } else if !v1helper.IsNativeResource(v1.ResourceName(value)) {
if !v1helper.IsExtendedResourceName(v1.ResourceName(value)) { if !v1helper.IsExtendedResourceName(v1.ResourceName(value)) {
return append(allErrs, field.Invalid(fldPath, value, "doesn't follow extended resource name standard")) return append(allErrs, field.Invalid(fldPath, value, "doesn't follow extended resource name standard"))
} }

View File

@ -4140,7 +4140,7 @@ func validateContainerResourceName(value string, fldPath *field.Path) field.Erro
if !helper.IsStandardContainerResourceName(value) { if !helper.IsStandardContainerResourceName(value) {
return append(allErrs, field.Invalid(fldPath, value, "must be a standard resource for containers")) return append(allErrs, field.Invalid(fldPath, value, "must be a standard resource for containers"))
} }
} else if !helper.IsDefaultNamespaceResource(core.ResourceName(value)) { } else if !helper.IsNativeResource(core.ResourceName(value)) {
if !helper.IsExtendedResourceName(core.ResourceName(value)) { if !helper.IsExtendedResourceName(core.ResourceName(value)) {
return append(allErrs, field.Invalid(fldPath, value, "doesn't follow extended resource name standard")) return append(allErrs, field.Invalid(fldPath, value, "doesn't follow extended resource name standard"))
} }

View File

@ -81,7 +81,7 @@ func maskResourceWithPrefix(resource api.ResourceName, prefix string) api.Resour
func isExtendedResourceNameForQuota(name api.ResourceName) bool { func isExtendedResourceNameForQuota(name api.ResourceName) bool {
// As overcommit is not supported by extended resources for now, // As overcommit is not supported by extended resources for now,
// only quota objects in format of "requests.resourceName" is allowed. // only quota objects in format of "requests.resourceName" is allowed.
return !helper.IsDefaultNamespaceResource(name) && strings.HasPrefix(string(name), api.DefaultResourceRequestsPrefix) return !helper.IsNativeResource(name) && strings.HasPrefix(string(name), api.DefaultResourceRequestsPrefix)
} }
// NOTE: it was a mistake, but if a quota tracks cpu or memory related resources, // NOTE: it was a mistake, but if a quota tracks cpu or memory related resources,

View File

@ -37,9 +37,11 @@ import (
) )
var ( var (
extendedResourceA = v1.ResourceName("example.com/aaa") extendedResourceA = v1.ResourceName("example.com/aaa")
extendedResourceB = v1.ResourceName("example.com/bbb") extendedResourceB = v1.ResourceName("example.com/bbb")
hugePageResourceA = v1helper.HugePageResourceName(resource.MustParse("2Mi")) kubernetesIOResourceA = v1.ResourceName("kubernetes.io/something")
kubernetesIOResourceB = v1.ResourceName("subdomain.kubernetes.io/something")
hugePageResourceA = v1helper.HugePageResourceName(resource.MustParse("2Mi"))
) )
func makeResources(milliCPU, memory, nvidiaGPUs, pods, extendedA, storage, hugePageA int64) v1.NodeResources { func makeResources(milliCPU, memory, nvidiaGPUs, pods, extendedA, storage, hugePageA int64) v1.NodeResources {
@ -297,6 +299,24 @@ func TestPodFitsResources(t *testing.T) {
test: "extended resource allocatable enforced for unknown resource for init container", test: "extended resource allocatable enforced for unknown resource for init container",
reasons: []algorithm.PredicateFailureReason{NewInsufficientResourceError(extendedResourceB, 1, 0, 0)}, reasons: []algorithm.PredicateFailureReason{NewInsufficientResourceError(extendedResourceB, 1, 0, 0)},
}, },
{
pod: newResourcePod(
schedulercache.Resource{MilliCPU: 1, Memory: 1, ScalarResources: map[v1.ResourceName]int64{kubernetesIOResourceA: 10}}),
nodeInfo: schedulercache.NewNodeInfo(
newResourcePod(schedulercache.Resource{MilliCPU: 0, Memory: 0})),
fits: false,
test: "kubernetes.io resource capacity enforced",
reasons: []algorithm.PredicateFailureReason{NewInsufficientResourceError(kubernetesIOResourceA, 10, 0, 0)},
},
{
pod: newResourceInitPod(newResourcePod(schedulercache.Resource{}),
schedulercache.Resource{MilliCPU: 1, Memory: 1, ScalarResources: map[v1.ResourceName]int64{kubernetesIOResourceB: 10}}),
nodeInfo: schedulercache.NewNodeInfo(
newResourcePod(schedulercache.Resource{MilliCPU: 0, Memory: 0})),
fits: false,
test: "kubernetes.io resource capacity enforced for init container",
reasons: []algorithm.PredicateFailureReason{NewInsufficientResourceError(kubernetesIOResourceB, 10, 0, 0)},
},
{ {
pod: newResourcePod( pod: newResourcePod(
schedulercache.Resource{MilliCPU: 1, Memory: 1, ScalarResources: map[v1.ResourceName]int64{hugePageResourceA: 10}}), schedulercache.Resource{MilliCPU: 1, Memory: 1, ScalarResources: map[v1.ResourceName]int64{hugePageResourceA: 10}}),