diff --git a/pkg/quota/evaluator/core/persistent_volume_claims.go b/pkg/quota/evaluator/core/persistent_volume_claims.go index 230f196d3d7..d6a58a756c0 100644 --- a/pkg/quota/evaluator/core/persistent_volume_claims.go +++ b/pkg/quota/evaluator/core/persistent_volume_claims.go @@ -17,6 +17,9 @@ limitations under the License. package core import ( + "fmt" + "strings" + "k8s.io/kubernetes/pkg/admission" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/resource" @@ -24,6 +27,7 @@ import ( "k8s.io/kubernetes/pkg/quota" "k8s.io/kubernetes/pkg/quota/generic" "k8s.io/kubernetes/pkg/runtime" + "k8s.io/kubernetes/pkg/util/sets" ) // NewPersistentVolumeClaimEvaluator returns an evaluator that can evaluate persistent volume claims @@ -37,7 +41,7 @@ func NewPersistentVolumeClaimEvaluator(kubeClient clientset.Interface) quota.Eva }, MatchedResourceNames: allResources, MatchesScopeFunc: generic.MatchesNoScopeFunc, - ConstraintsFunc: generic.ObjectCountConstraintsFunc(api.ResourcePersistentVolumeClaims), + ConstraintsFunc: PersistentVolumeClaimConstraintsFunc, UsageFunc: PersistentVolumeClaimUsageFunc, ListFuncByNamespace: func(namespace string, options api.ListOptions) (runtime.Object, error) { return kubeClient.Core().PersistentVolumeClaims(namespace).List(options) @@ -58,3 +62,24 @@ func PersistentVolumeClaimUsageFunc(object runtime.Object) api.ResourceList { } return result } + +// PersistentVolumeClaimConstraintsFunc verifies that all required resources are present on the claim +// In addition, it validates that the resources are valid (i.e. requests < limits) +func PersistentVolumeClaimConstraintsFunc(required []api.ResourceName, object runtime.Object) error { + pvc, ok := object.(*api.PersistentVolumeClaim) + if !ok { + return fmt.Errorf("unexpected input object %v", object) + } + + requiredSet := quota.ToSet(required) + missingSet := sets.NewString() + pvcUsage := PersistentVolumeClaimUsageFunc(pvc) + pvcSet := quota.ToSet(quota.ResourceNames(pvcUsage)) + if diff := requiredSet.Difference(pvcSet); len(diff) > 0 { + missingSet.Insert(diff.List()...) + } + if len(missingSet) == 0 { + return nil + } + return fmt.Errorf("must specify %s", strings.Join(missingSet.List(), ",")) +} diff --git a/pkg/quota/evaluator/core/persistent_volume_claims_test.go b/pkg/quota/evaluator/core/persistent_volume_claims_test.go new file mode 100644 index 00000000000..4babc6630ad --- /dev/null +++ b/pkg/quota/evaluator/core/persistent_volume_claims_test.go @@ -0,0 +1,149 @@ +/* +Copyright 2016 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 core + +import ( + "testing" + + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/api/resource" + "k8s.io/kubernetes/pkg/api/unversioned" + "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" + "k8s.io/kubernetes/pkg/quota" +) + +func testVolumeClaim(name string, namespace string, spec api.PersistentVolumeClaimSpec) *api.PersistentVolumeClaim { + return &api.PersistentVolumeClaim{ + ObjectMeta: api.ObjectMeta{Name: name, Namespace: namespace}, + Spec: spec, + } +} + +func TestPersistentVolumeClaimsConstraintsFunc(t *testing.T) { + validClaim := testVolumeClaim("foo", "ns", api.PersistentVolumeClaimSpec{ + Selector: &unversioned.LabelSelector{ + MatchExpressions: []unversioned.LabelSelectorRequirement{ + { + Key: "key2", + Operator: "Exists", + }, + }, + }, + AccessModes: []api.PersistentVolumeAccessMode{ + api.ReadWriteOnce, + api.ReadOnlyMany, + }, + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{ + api.ResourceName(api.ResourceStorage): resource.MustParse("10G"), + }, + }, + }) + missingStorage := testVolumeClaim("foo", "ns", api.PersistentVolumeClaimSpec{ + Selector: &unversioned.LabelSelector{ + MatchExpressions: []unversioned.LabelSelectorRequirement{ + { + Key: "key2", + Operator: "Exists", + }, + }, + }, + AccessModes: []api.PersistentVolumeAccessMode{ + api.ReadWriteOnce, + api.ReadOnlyMany, + }, + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{}, + }, + }) + + testCases := map[string]struct { + pvc *api.PersistentVolumeClaim + required []api.ResourceName + err string + }{ + "missing storage": { + pvc: missingStorage, + required: []api.ResourceName{api.ResourceRequestsStorage}, + err: `must specify requests.storage`, + }, + "valid-claim-quota-storage": { + pvc: validClaim, + required: []api.ResourceName{api.ResourceRequestsStorage}, + }, + "valid-claim-quota-pvc": { + pvc: validClaim, + required: []api.ResourceName{api.ResourcePersistentVolumeClaims}, + }, + "valid-claim-quota-storage-and-pvc": { + pvc: validClaim, + required: []api.ResourceName{api.ResourceRequestsStorage, api.ResourcePersistentVolumeClaims}, + }, + } + for testName, test := range testCases { + err := PersistentVolumeClaimConstraintsFunc(test.required, test.pvc) + switch { + case err != nil && len(test.err) == 0, + err == nil && len(test.err) != 0, + err != nil && test.err != err.Error(): + t.Errorf("%s unexpected error: %v", testName, err) + } + } +} + +func TestPersistentVolumeClaimEvaluatorUsage(t *testing.T) { + validClaim := testVolumeClaim("foo", "ns", api.PersistentVolumeClaimSpec{ + Selector: &unversioned.LabelSelector{ + MatchExpressions: []unversioned.LabelSelectorRequirement{ + { + Key: "key2", + Operator: "Exists", + }, + }, + }, + AccessModes: []api.PersistentVolumeAccessMode{ + api.ReadWriteOnce, + api.ReadOnlyMany, + }, + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{ + api.ResourceName(api.ResourceStorage): resource.MustParse("10Gi"), + }, + }, + }) + + kubeClient := fake.NewSimpleClientset() + evaluator := NewPersistentVolumeClaimEvaluator(kubeClient) + testCases := map[string]struct { + pvc *api.PersistentVolumeClaim + usage api.ResourceList + }{ + "pvc-usage": { + pvc: validClaim, + usage: api.ResourceList{ + api.ResourceRequestsStorage: resource.MustParse("10Gi"), + api.ResourcePersistentVolumeClaims: resource.MustParse("1"), + }, + }, + } + for testName, testCase := range testCases { + actual := evaluator.Usage(testCase.pvc) + if !quota.Equals(testCase.usage, actual) { + t.Errorf("%s expected: %v, actual: %v", testName, testCase.usage, actual) + } + } +} diff --git a/pkg/quota/evaluator/core/services.go b/pkg/quota/evaluator/core/services.go index 225573f0253..d5d83e551f9 100644 --- a/pkg/quota/evaluator/core/services.go +++ b/pkg/quota/evaluator/core/services.go @@ -17,6 +17,9 @@ limitations under the License. package core import ( + "fmt" + "strings" + "k8s.io/kubernetes/pkg/admission" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/resource" @@ -24,6 +27,7 @@ import ( "k8s.io/kubernetes/pkg/quota" "k8s.io/kubernetes/pkg/quota/generic" "k8s.io/kubernetes/pkg/runtime" + "k8s.io/kubernetes/pkg/util/sets" ) // NewServiceEvaluator returns an evaluator that can evaluate service quotas @@ -42,7 +46,7 @@ func NewServiceEvaluator(kubeClient clientset.Interface) quota.Evaluator { }, MatchedResourceNames: allResources, MatchesScopeFunc: generic.MatchesNoScopeFunc, - ConstraintsFunc: generic.ObjectCountConstraintsFunc(api.ResourceServices), + ConstraintsFunc: ServiceConstraintsFunc, UsageFunc: ServiceUsageFunc, ListFuncByNamespace: func(namespace string, options api.ListOptions) (runtime.Object, error) { return kubeClient.Core().Services(namespace).List(options) @@ -54,12 +58,17 @@ func NewServiceEvaluator(kubeClient clientset.Interface) quota.Evaluator { func ServiceUsageFunc(object runtime.Object) api.ResourceList { result := api.ResourceList{} if service, ok := object.(*api.Service); ok { + // default service usage result[api.ResourceServices] = resource.MustParse("1") + result[api.ResourceServicesLoadBalancers] = resource.MustParse("0") + result[api.ResourceServicesNodePorts] = resource.MustParse("0") switch service.Spec.Type { case api.ServiceTypeNodePort: + // node port services need to count node ports value := resource.NewQuantity(int64(len(service.Spec.Ports)), resource.DecimalSI) result[api.ResourceServicesNodePorts] = *value case api.ServiceTypeLoadBalancer: + // load balancer services need to count load balancers result[api.ResourceServicesLoadBalancers] = resource.MustParse("1") } } @@ -85,3 +94,24 @@ func GetQuotaServiceType(service *api.Service) api.ServiceType { } return api.ServiceType("") } + +// ServiceConstraintsFunc verifies that all required resources are captured in service usage. +func ServiceConstraintsFunc(required []api.ResourceName, object runtime.Object) error { + service, ok := object.(*api.Service) + if !ok { + return fmt.Errorf("unexpected input object %v", object) + } + + requiredSet := quota.ToSet(required) + missingSet := sets.NewString() + serviceUsage := ServiceUsageFunc(service) + serviceSet := quota.ToSet(quota.ResourceNames(serviceUsage)) + if diff := requiredSet.Difference(serviceSet); len(diff) > 0 { + missingSet.Insert(diff.List()...) + } + + if len(missingSet) == 0 { + return nil + } + return fmt.Errorf("must specify %s", strings.Join(missingSet.List(), ",")) +} diff --git a/pkg/quota/evaluator/core/services_test.go b/pkg/quota/evaluator/core/services_test.go index 100f6605b42..fc0c5a5bec8 100644 --- a/pkg/quota/evaluator/core/services_test.go +++ b/pkg/quota/evaluator/core/services_test.go @@ -53,6 +53,7 @@ func TestServiceEvaluatorUsage(t *testing.T) { }, }, usage: api.ResourceList{ + api.ResourceServicesNodePorts: resource.MustParse("0"), api.ResourceServicesLoadBalancers: resource.MustParse("1"), api.ResourceServices: resource.MustParse("1"), }, @@ -64,7 +65,9 @@ func TestServiceEvaluatorUsage(t *testing.T) { }, }, usage: api.ResourceList{ - api.ResourceServices: resource.MustParse("1"), + api.ResourceServices: resource.MustParse("1"), + api.ResourceServicesNodePorts: resource.MustParse("0"), + api.ResourceServicesLoadBalancers: resource.MustParse("0"), }, }, "nodeports": { @@ -79,8 +82,9 @@ func TestServiceEvaluatorUsage(t *testing.T) { }, }, usage: api.ResourceList{ - api.ResourceServices: resource.MustParse("1"), - api.ResourceServicesNodePorts: resource.MustParse("1"), + api.ResourceServices: resource.MustParse("1"), + api.ResourceServicesNodePorts: resource.MustParse("1"), + api.ResourceServicesLoadBalancers: resource.MustParse("0"), }, }, "multi-nodeports": { @@ -98,8 +102,9 @@ func TestServiceEvaluatorUsage(t *testing.T) { }, }, usage: api.ResourceList{ - api.ResourceServices: resource.MustParse("1"), - api.ResourceServicesNodePorts: resource.MustParse("2"), + api.ResourceServices: resource.MustParse("1"), + api.ResourceServicesNodePorts: resource.MustParse("2"), + api.ResourceServicesLoadBalancers: resource.MustParse("0"), }, }, } @@ -110,3 +115,66 @@ func TestServiceEvaluatorUsage(t *testing.T) { } } } + +func TestServiceConstraintsFunc(t *testing.T) { + testCases := map[string]struct { + service *api.Service + required []api.ResourceName + err string + }{ + "loadbalancer": { + service: &api.Service{ + Spec: api.ServiceSpec{ + Type: api.ServiceTypeLoadBalancer, + }, + }, + required: []api.ResourceName{api.ResourceServicesLoadBalancers}, + }, + "clusterip": { + service: &api.Service{ + Spec: api.ServiceSpec{ + Type: api.ServiceTypeClusterIP, + }, + }, + required: []api.ResourceName{api.ResourceServicesLoadBalancers, api.ResourceServices}, + }, + "nodeports": { + service: &api.Service{ + Spec: api.ServiceSpec{ + Type: api.ServiceTypeNodePort, + Ports: []api.ServicePort{ + { + Port: 27443, + }, + }, + }, + }, + required: []api.ResourceName{api.ResourceServicesNodePorts}, + }, + "multi-nodeports": { + service: &api.Service{ + Spec: api.ServiceSpec{ + Type: api.ServiceTypeNodePort, + Ports: []api.ServicePort{ + { + Port: 27443, + }, + { + Port: 27444, + }, + }, + }, + }, + required: []api.ResourceName{api.ResourceServicesNodePorts}, + }, + } + for testName, test := range testCases { + err := ServiceConstraintsFunc(test.required, test.service) + switch { + case err != nil && len(test.err) == 0, + err == nil && len(test.err) != 0, + err != nil && test.err != err.Error(): + t.Errorf("%s unexpected error: %v", testName, err) + } + } +}