Simplify min/max evaluation, make limitType a type

pkg/api/types.go

@@ -1139,17 +1139,20 @@ type List struct {
 	Items []runtime.Object `json:"items"`
 }
 
+// A type of object that is limited
+type LimitType string
+
 const (
 	// Limit that applies to all pods in a namespace
-	LimitTypePod string = "Pod"
+	LimitTypePod LimitType = "Pod"
 	// Limit that applies to all containers in a namespace
-	LimitTypeContainer string = "Container"
+	LimitTypeContainer LimitType = "Container"
 )
 
 // LimitRangeItem defines a min/max usage limit for any resource that matches on kind
 type LimitRangeItem struct {
 	// Type of resource that this limit applies to
-	Type string `json:"type,omitempty"`
+	Type LimitType `json:"type,omitempty"`
 	// Max usage constraints on this kind by resource name
 	Max ResourceList `json:"max,omitempty"`
 	// Min usage constraints on this kind by resource name

pkg/api/v1beta1/conversion.go

@@ -614,7 +614,7 @@ func init() {
 		},
 		func(in *newer.LimitRangeItem, out *LimitRangeItem, s conversion.Scope) error {
 			*out = LimitRangeItem{}
-			out.Type = in.Type
+			out.Type = LimitType(in.Type)
 			if err := s.Convert(&in.Max, &out.Max, 0); err != nil {
 				return err
 			}
@@ -625,7 +625,7 @@ func init() {
 		},
 		func(in *LimitRangeItem, out *newer.LimitRangeItem, s conversion.Scope) error {
 			*out = newer.LimitRangeItem{}
-			out.Type = in.Type
+			out.Type = newer.LimitType(in.Type)
 			if err := s.Convert(&in.Max, &out.Max, 0); err != nil {
 				return err
 			}

pkg/api/v1beta1/types.go

@@ -905,17 +905,20 @@ type List struct {
 	Items    []runtime.RawExtension `json:"items" description:"list of objects"`
 }
 
+// A type of object that is limited
+type LimitType string
+
 const (
 	// Limit that applies to all pods in a namespace
-	LimitTypePod string = "Pod"
+	LimitTypePod LimitType = "Pod"
 	// Limit that applies to all containers in a namespace
-	LimitTypeContainer string = "Container"
+	LimitTypeContainer LimitType = "Container"
 )
 
 // LimitRangeItem defines a min/max usage limit for any resource that matches on kind
 type LimitRangeItem struct {
 	// Type of resource that this limit applies to
-	Type string `json:"type,omitempty"`
+	Type LimitType `json:"type,omitempty"`
 	// Max usage constraints on this kind by resource name
 	Max ResourceList `json:"max,omitempty"`
 	// Min usage constraints on this kind by resource name

pkg/api/v1beta2/conversion.go

@@ -531,7 +531,7 @@ func init() {
 		},
 		func(in *newer.LimitRangeItem, out *LimitRangeItem, s conversion.Scope) error {
 			*out = LimitRangeItem{}
-			out.Type = in.Type
+			out.Type = LimitType(in.Type)
 			if err := s.Convert(&in.Max, &out.Max, 0); err != nil {
 				return err
 			}
@@ -542,7 +542,7 @@ func init() {
 		},
 		func(in *LimitRangeItem, out *newer.LimitRangeItem, s conversion.Scope) error {
 			*out = newer.LimitRangeItem{}
-			out.Type = in.Type
+			out.Type = newer.LimitType(in.Type)
 			if err := s.Convert(&in.Max, &out.Max, 0); err != nil {
 				return err
 			}

pkg/api/v1beta2/types.go

@@ -907,17 +907,20 @@ type List struct {
 	Items    []runtime.RawExtension `json:"items" description:"list of objects"`
 }
 
+// A type of object that is limited
+type LimitType string
+
 const (
 	// Limit that applies to all pods in a namespace
-	LimitTypePod string = "Pod"
+	LimitTypePod LimitType = "Pod"
 	// Limit that applies to all containers in a namespace
-	LimitTypeContainer string = "Container"
+	LimitTypeContainer LimitType = "Container"
 )
 
 // LimitRangeItem defines a min/max usage limit for any resource that matches on kind
 type LimitRangeItem struct {
 	// Type of resource that this limit applies to
-	Type string `json:"type,omitempty"`
+	Type LimitType `json:"type,omitempty"`
 	// Max usage constraints on this kind by resource name
 	Max ResourceList `json:"max,omitempty"`
 	// Min usage constraints on this kind by resource name

pkg/api/v1beta3/types.go

@@ -1067,17 +1067,20 @@ type List struct {
 	Items []runtime.RawExtension `json:"items" description:"list of objects"`
 }
 
+// A type of object that is limited
+type LimitType string
+
 const (
 	// Limit that applies to all pods in a namespace
-	LimitTypePod string = "Pod"
+	LimitTypePod LimitType = "Pod"
 	// Limit that applies to all containers in a namespace
-	LimitTypeContainer string = "Container"
+	LimitTypeContainer LimitType = "Container"
 )
 
 // LimitRangeItem defines a min/max usage limit for any resource that matches on kind
 type LimitRangeItem struct {
 	// Type of resource that this limit applies to
-	Type string `json:"type,omitempty"`
+	Type LimitType `json:"type,omitempty"`
 	// Max usage constraints on this kind by resource name
 	Max ResourceList `json:"max,omitempty"`
 	// Min usage constraints on this kind by resource name

plugin/pkg/admission/limitranger/admission.go

@@ -49,6 +49,7 @@ func (l *limitRanger) Admit(a admission.Attributes) (err error) {
 	}
 
 	// look for a limit range in current namespace that requires enforcement
+	// TODO: Move to cache when issue is resolved: https://github.com/GoogleCloudPlatform/kubernetes/issues/2294
 	items, err := l.client.LimitRanges(a.GetNamespace()).List(labels.Everything())
 	if err != nil {
 		return err
@@ -123,8 +124,15 @@ func PodLimitFunc(limitRange *api.LimitRange, kind string, obj runtime.Object) e
 
 	for i := range limitRange.Spec.Limits {
 		limit := limitRange.Spec.Limits[i]
-		// enforce max
+		for _, minOrMax := range []string{"Min", "Max"} {
-		for k, v := range limit.Max {
+			var rl api.ResourceList
+			switch minOrMax {
+			case "Min":
+				rl = limit.Min
+			case "Max":
+				rl = limit.Max
+			}
+			for k, v := range rl {
 				observed := int64(0)
 				enforced := int64(0)
 				var err error
@@ -134,57 +142,34 @@ func PodLimitFunc(limitRange *api.LimitRange, kind string, obj runtime.Object) e
 					switch limit.Type {
 					case api.LimitTypePod:
 						observed = podMem
-					err = fmt.Errorf("Maximum memory usage per pod is %s", v.String())
+						err = fmt.Errorf("%simum memory usage per pod is %s", minOrMax, v.String())
 					case api.LimitTypeContainer:
 						observed = maxContainerMem
-					err = fmt.Errorf("Maximum memory usage per container is %s", v.String())
+						err = fmt.Errorf("%simum memory usage per container is %s", minOrMax, v.String())
 					}
 				case api.ResourceCPU:
 					enforced = v.MilliValue()
 					switch limit.Type {
 					case api.LimitTypePod:
 						observed = podCPU
-					err = fmt.Errorf("Maximum CPU usage per pod is %s, but requested %s", v.String(), resource.NewMilliQuantity(observed, resource.DecimalSI))
+						err = fmt.Errorf("%simum CPU usage per pod is %s, but requested %s", minOrMax, v.String(), resource.NewMilliQuantity(observed, resource.DecimalSI))
 					case api.LimitTypeContainer:
 						observed = maxContainerCPU
-					err = fmt.Errorf("Maximum CPU usage per container is %s", v.String())
+						err = fmt.Errorf("%simum CPU usage per container is %s", minOrMax, v.String())
 					}
 				}
+				switch minOrMax {
+				case "Min":
+					if observed < enforced {
+						return apierrors.NewForbidden(kind, pod.Name, err)
+					}
+				case "Max":
 					if observed > enforced {
 						return apierrors.NewForbidden(kind, pod.Name, err)
 					}
 				}
-		for k, v := range limit.Min {
-			observed := int64(0)
-			enforced := int64(0)
-			var err error
-			switch k {
-			case api.ResourceMemory:
-				enforced = v.Value()
-				switch limit.Type {
-				case api.LimitTypePod:
-					observed = podMem
-					err = fmt.Errorf("Minimum memory usage per pod is %s", v.String())
-				case api.LimitTypeContainer:
-					observed = maxContainerMem
-					err = fmt.Errorf("Minimum memory usage per container is %s", v.String())
-				}
-			case api.ResourceCPU:
-				enforced = v.MilliValue()
-				switch limit.Type {
-				case api.LimitTypePod:
-					observed = podCPU
-					err = fmt.Errorf("Minimum CPU usage per pod is %s", v.String())
-				case api.LimitTypeContainer:
-					observed = maxContainerCPU
-					err = fmt.Errorf("Minimum CPU usage per container is %s", v.String())
-				}
-			}
-			if observed < enforced {
-				return apierrors.NewForbidden(kind, pod.Name, err)
 			}
 		}
 	}
-
 	return nil
 }