Fix quota constraints func for services

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(), ","))
+}

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.ResourceServices:              resource.MustParse("1"),
-				api.ResourceServicesNodePorts: 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.ResourceServices:              resource.MustParse("1"),
-				api.ResourceServicesNodePorts: resource.MustParse("2"),
+				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)
+		}
+	}
+}