github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-23 11:50:44 +00:00
Code Issues Projects Releases Wiki Activity

bugfix: check Spec.AllocateLoadBalancerNodePorts for nodeport and skip zero usage in delta evaluator

Browse Source 
Signed-off-by: pacoxu <paco.xu@daocloud.io>

When Spec.AllocateLoadBalancerNodePorts is "false" NodePort shall
not be included when computing quota for type:LoadBalancer.

Co-authored-by: uablrek
This commit is contained in:
pacoxu 2020-12-22 19:19:15 +08:00
parent 0f7c2dc314
commit 15867d9e8a
7 changed files with 240 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
pkg/quota/v1/evaluator/core/BUILD
View File

@ -46,6 +46,7 @@ go_test(
embed = [":go_default_library"],
deps = [
"//pkg/apis/core:go_default_library",
"//pkg/features:go_default_library",
"//pkg/util/node:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library",
@ -54,6 +55,9 @@ go_test(
"//staging/src/k8s.io/apimachinery/pkg/util/clock:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/quota/v1:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/quota/v1/generic:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
"//staging/src/k8s.io/component-base/featuregate/testing:go_default_library",
"//vendor/k8s.io/utils/pointer:go_default_library",
],
)

27
pkg/quota/v1/evaluator/core/services.go
View File

@ -26,8 +26,10 @@ import (
"k8s.io/apiserver/pkg/admission"
quota "k8s.io/apiserver/pkg/quota/v1"
"k8s.io/apiserver/pkg/quota/v1/generic"
"k8s.io/apiserver/pkg/util/feature"
api "k8s.io/kubernetes/pkg/apis/core"
k8s_api_v1 "k8s.io/kubernetes/pkg/apis/core/v1"
"k8s.io/kubernetes/pkg/features"
)
// the name used for object count quota
@ -128,14 +130,33 @@ func (p *serviceEvaluator) Usage(item runtime.Object) (corev1.ResourceList, erro
value := resource.NewQuantity(int64(ports), resource.DecimalSI)
result[corev1.ResourceServicesNodePorts] = *value
case corev1.ServiceTypeLoadBalancer:
// load balancer services need to count node ports and load balancers
value := resource.NewQuantity(int64(ports), resource.DecimalSI)
result[corev1.ResourceServicesNodePorts] = *value
// load balancer services need to count node ports. If creation of node ports
// is suppressed only ports with explicit NodePort values are counted.
// nodeports won't be allocated yet, so we can't simply count the actual values.
// We need to look at the intent.
if feature.DefaultFeatureGate.Enabled(features.ServiceLBNodePortControl) &&
svc.Spec.AllocateLoadBalancerNodePorts != nil &&
*svc.Spec.AllocateLoadBalancerNodePorts == false {
result[corev1.ResourceServicesNodePorts] = *portsWithNodePorts(svc)
} else {
value := resource.NewQuantity(int64(ports), resource.DecimalSI)
result[corev1.ResourceServicesNodePorts] = *value
}
result[corev1.ResourceServicesLoadBalancers] = *(resource.NewQuantity(1, resource.DecimalSI))
}
return result, nil
}
func portsWithNodePorts(svc *corev1.Service) *resource.Quantity {
count := 0
for _, p := range svc.Spec.Ports {
if p.NodePort != 0 {
count++
}
}
return resource.NewQuantity(int64(count), resource.DecimalSI)
}
// UsageStats calculates aggregate usage for the object.
func (p *serviceEvaluator) UsageStats(options quota.UsageStatsOptions) (quota.UsageStats, error) {
return generic.CalculateUsageStats(options, p.listFuncByNamespace, generic.MatchesNoScopeFunc, p.Usage)

142
pkg/quota/v1/evaluator/core/services_test.go
View File

@ -24,7 +24,11 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"
quota "k8s.io/apiserver/pkg/quota/v1"
"k8s.io/apiserver/pkg/quota/v1/generic"
"k8s.io/apiserver/pkg/util/feature"
featuregatetesting "k8s.io/component-base/featuregate/testing"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/features"
utilpointer "k8s.io/utils/pointer"
)
func TestServiceEvaluatorMatchesResources(t *testing.T) {
@ -52,8 +56,9 @@ func TestServiceEvaluatorMatchesResources(t *testing.T) {
func TestServiceEvaluatorUsage(t *testing.T) {
evaluator := NewServiceEvaluator(nil)
testCases := map[string]struct {
service *api.Service
usage corev1.ResourceList
service *api.Service
usage corev1.ResourceList
serviceLBNodePortControlEnabled bool
}{
"loadbalancer": {
service: &api.Service{
@ -86,6 +91,27 @@ func TestServiceEvaluatorUsage(t *testing.T) {
generic.ObjectCountQuotaResourceNameFor(schema.GroupResource{Resource: "services"}): resource.MustParse("1"),
},
},
"loadbalancer_2_ports": {
service: &api.Service{
Spec: api.ServiceSpec{
Type: api.ServiceTypeLoadBalancer,
Ports: []api.ServicePort{
{
Port: 27443,
},
{
Port: 27444,
},
},
},
},
usage: corev1.ResourceList{
corev1.ResourceServicesNodePorts: resource.MustParse("2"),
corev1.ResourceServicesLoadBalancers: resource.MustParse("1"),
corev1.ResourceServices: resource.MustParse("1"),
generic.ObjectCountQuotaResourceNameFor(schema.GroupResource{Resource: "services"}): resource.MustParse("1"),
},
},
"clusterip": {
service: &api.Service{
Spec: api.ServiceSpec{
@ -138,15 +164,113 @@ func TestServiceEvaluatorUsage(t *testing.T) {
generic.ObjectCountQuotaResourceNameFor(schema.GroupResource{Resource: "services"}): resource.MustParse("1"),
},
},
"nodeports-disabled": {
service: &api.Service{
Spec: api.ServiceSpec{
Type: api.ServiceTypeLoadBalancer,
Ports: []api.ServicePort{
{
Port: 27443,
},
{
Port: 27444,
},
},
AllocateLoadBalancerNodePorts: utilpointer.BoolPtr(false),
},
},
usage: corev1.ResourceList{
corev1.ResourceServices: resource.MustParse("1"),
corev1.ResourceServicesNodePorts: resource.MustParse("0"),
corev1.ResourceServicesLoadBalancers: resource.MustParse("1"),
generic.ObjectCountQuotaResourceNameFor(schema.GroupResource{Resource: "services"}): resource.MustParse("1"),
},
serviceLBNodePortControlEnabled: true,
},
"nodeports-default-enabled": {
service: &api.Service{
Spec: api.ServiceSpec{
Type: api.ServiceTypeLoadBalancer,
Ports: []api.ServicePort{
{
Port: 27443,
NodePort: 32001,
},
{
Port: 27444,
NodePort: 32002,
},
},
AllocateLoadBalancerNodePorts: nil,
},
},
usage: corev1.ResourceList{
corev1.ResourceServices: resource.MustParse("1"),
corev1.ResourceServicesNodePorts: resource.MustParse("2"),
corev1.ResourceServicesLoadBalancers: resource.MustParse("1"),
generic.ObjectCountQuotaResourceNameFor(schema.GroupResource{Resource: "services"}): resource.MustParse("1"),
},
},
"nodeports-explicitly-enabled": {
service: &api.Service{
Spec: api.ServiceSpec{
Type: api.ServiceTypeLoadBalancer,
Ports: []api.ServicePort{
{
Port: 27443,
},
{
Port: 27444,
},
},
AllocateLoadBalancerNodePorts: utilpointer.BoolPtr(true),
},
},
usage: corev1.ResourceList{
corev1.ResourceServices: resource.MustParse("1"),
corev1.ResourceServicesNodePorts: resource.MustParse("2"),
corev1.ResourceServicesLoadBalancers: resource.MustParse("1"),
generic.ObjectCountQuotaResourceNameFor(schema.GroupResource{Resource: "services"}): resource.MustParse("1"),
},
serviceLBNodePortControlEnabled: true,
},
"nodeports-disabled-but-specified": {
service: &api.Service{
Spec: api.ServiceSpec{
Type: api.ServiceTypeLoadBalancer,
Ports: []api.ServicePort{
{
Port: 27443,
NodePort: 32001,
},
{
Port: 27444,
NodePort: 32002,
},
},
AllocateLoadBalancerNodePorts: utilpointer.BoolPtr(false),
},
},
usage: corev1.ResourceList{
corev1.ResourceServices: resource.MustParse("1"),
corev1.ResourceServicesNodePorts: resource.MustParse("2"),
corev1.ResourceServicesLoadBalancers: resource.MustParse("1"),
generic.ObjectCountQuotaResourceNameFor(schema.GroupResource{Resource: "services"}): resource.MustParse("1"),
},
serviceLBNodePortControlEnabled: true,
},
}
for testName, testCase := range testCases {
actual, err := evaluator.Usage(testCase.service)
if err != nil {
t.Errorf("%s unexpected error: %v", testName, err)
}
if !quota.Equals(testCase.usage, actual) {
t.Errorf("%s expected: %v, actual: %v", testName, testCase.usage, actual)
}
t.Run(testName, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.ServiceLBNodePortControl, testCase.serviceLBNodePortControlEnabled)()
actual, err := evaluator.Usage(testCase.service)
if err != nil {
t.Errorf("%s unexpected error: %v", testName, err)
}
if !quota.Equals(testCase.usage, actual) {
t.Errorf("%s expected: %v, actual: %v", testName, testCase.usage, actual)
}
})
}
}

5
staging/src/k8s.io/apiserver/pkg/admission/plugin/resourcequota/controller.go
View File

@ -510,7 +510,10 @@ func CheckRequest(quotas []corev1.ResourceQuota, a admission.Attributes, evaluat
}
}
if quota.IsZero(deltaUsage) {
// ignore items in deltaUsage with zero usage
deltaUsage = quota.RemoveZeros(deltaUsage)
// if there is no remaining non-zero usage, short-circuit and return
if len(deltaUsage) == 0 {
return quotas, nil
}

11
staging/src/k8s.io/apiserver/pkg/quota/v1/resources.go
View File

@ -226,6 +226,17 @@ func IsZero(a corev1.ResourceList) bool {
return true
}
// RemoveZeros returns a new resource list that only has no zero values
func RemoveZeros(a corev1.ResourceList) corev1.ResourceList {
result := corev1.ResourceList{}
for key, value := range a {
if !value.IsZero() {
result[key] = value
}
}
return result
}
// IsNegative returns the set of resource names that have a negative value.
func IsNegative(a corev1.ResourceList) []corev1.ResourceName {
results := []corev1.ResourceName{}

44
staging/src/k8s.io/apiserver/pkg/quota/v1/resources_test.go
View File

@ -287,6 +287,50 @@ func TestIsZero(t *testing.T) {
}
}
func TestRemoveZeros(t *testing.T) {
testCases := map[string]struct {
a corev1.ResourceList
expected corev1.ResourceList
}{
"empty": {
a: corev1.ResourceList{},
expected: corev1.ResourceList{},
},
"all-zeros": {
a: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("0"),
corev1.ResourceMemory: resource.MustParse("0"),
},
expected: corev1.ResourceList{},
},
"some-zeros": {
a: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("0"),
corev1.ResourceMemory: resource.MustParse("0"),
corev1.ResourceStorage: resource.MustParse("100Gi"),
},
expected: corev1.ResourceList{
corev1.ResourceStorage: resource.MustParse("100Gi"),
},
},
"non-zero": {
a: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("200m"),
corev1.ResourceMemory: resource.MustParse("1Gi"),
},
expected: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("200m"),
corev1.ResourceMemory: resource.MustParse("1Gi"),
},
},
}
for testName, testCase := range testCases {
if result := RemoveZeros(testCase.a); !Equals(result, testCase.expected) {
t.Errorf("%s expected: %v, actual: %v", testName, testCase.expected, result)
}
}
}
func TestIsNegative(t *testing.T) {
testCases := map[string]struct {
a corev1.ResourceList

24
test/e2e/apimachinery/resource_quota.go
View File

@ -36,6 +36,7 @@ import (
"k8s.io/kubernetes/test/e2e/framework"
"k8s.io/kubernetes/test/utils/crd"
imageutils "k8s.io/kubernetes/test/utils/image"
"k8s.io/utils/pointer"
"github.com/onsi/ginkgo"
)
@ -100,23 +101,37 @@ var _ = SIGDescribe("ResourceQuota", func() {
framework.ExpectNoError(err)
ginkgo.By("Creating a Service")
service := newTestServiceForQuota("test-service", v1.ServiceTypeClusterIP)
service := newTestServiceForQuota("test-service", v1.ServiceTypeClusterIP, false)
service, err = f.ClientSet.CoreV1().Services(f.Namespace.Name).Create(context.TODO(), service, metav1.CreateOptions{})
framework.ExpectNoError(err)
ginkgo.By("Creating a NodePort Service")
nodeport := newTestServiceForQuota("test-service-np", v1.ServiceTypeNodePort, false)
nodeport, err = f.ClientSet.CoreV1().Services(f.Namespace.Name).Create(context.TODO(), nodeport, metav1.CreateOptions{})
framework.ExpectNoError(err)
ginkgo.By("Not allowing a LoadBalancer Service with NodePort to be created that exceeds remaining quota")
loadbalancer := newTestServiceForQuota("test-service-lb", v1.ServiceTypeLoadBalancer, false)
_, err = f.ClientSet.CoreV1().Services(f.Namespace.Name).Create(context.TODO(), loadbalancer, metav1.CreateOptions{})
framework.ExpectError(err)
ginkgo.By("Ensuring resource quota status captures service creation")
usedResources = v1.ResourceList{}
usedResources[v1.ResourceQuotas] = resource.MustParse(strconv.Itoa(c + 1))
usedResources[v1.ResourceServices] = resource.MustParse("1")
usedResources[v1.ResourceServices] = resource.MustParse("2")
usedResources[v1.ResourceServicesNodePorts] = resource.MustParse("1")
err = waitForResourceQuota(f.ClientSet, f.Namespace.Name, quotaName, usedResources)
framework.ExpectNoError(err)
ginkgo.By("Deleting a Service")
ginkgo.By("Deleting Services")
err = f.ClientSet.CoreV1().Services(f.Namespace.Name).Delete(context.TODO(), service.Name, metav1.DeleteOptions{})
framework.ExpectNoError(err)
err = f.ClientSet.CoreV1().Services(f.Namespace.Name).Delete(context.TODO(), nodeport.Name, metav1.DeleteOptions{})
framework.ExpectNoError(err)
ginkgo.By("Ensuring resource quota status released usage")
usedResources[v1.ResourceServices] = resource.MustParse("0")
usedResources[v1.ResourceServicesNodePorts] = resource.MustParse("0")
err = waitForResourceQuota(f.ClientSet, f.Namespace.Name, quotaName, usedResources)
framework.ExpectNoError(err)
})
@ -1625,7 +1640,7 @@ func newTestReplicaSetForQuota(name, image string, replicas int32) *appsv1.Repli
}
// newTestServiceForQuota returns a simple service
func newTestServiceForQuota(name string, serviceType v1.ServiceType) *v1.Service {
func newTestServiceForQuota(name string, serviceType v1.ServiceType, allocateLoadBalancerNodePorts bool) *v1.Service {
return &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: name,
@ -1636,6 +1651,7 @@ func newTestServiceForQuota(name string, serviceType v1.ServiceType) *v1.Service
Port: 80,
TargetPort: intstr.FromInt(80),
}},
AllocateLoadBalancerNodePorts: pointer.BoolPtr(allocateLoadBalancerNodePorts),
},
}
}