diff --git a/pkg/api/resource/quantity.go b/pkg/api/resource/quantity.go index 2b42e040fcc..ef8eaef9e6b 100644 --- a/pkg/api/resource/quantity.go +++ b/pkg/api/resource/quantity.go @@ -309,6 +309,14 @@ func (q *Quantity) Add(y Quantity) error { return nil } +func (q *Quantity) Sub(y Quantity) error { + if q.Format != y.Format { + return fmt.Errorf("format mismatch: %v vs. %v", q.Format, y.Format) + } + q.Amount.Sub(q.Amount, y.Amount) + return nil +} + // MarshalJSON implements the json.Marshaller interface. func (q Quantity) MarshalJSON() ([]byte, error) { return []byte(`"` + q.String() + `"`), nil diff --git a/pkg/controller/resourcequota/resource_quota_controller.go b/pkg/controller/resourcequota/resource_quota_controller.go index 3b92d2cd607..9d319aacaf8 100644 --- a/pkg/controller/resourcequota/resource_quota_controller.go +++ b/pkg/controller/resourcequota/resource_quota_controller.go @@ -17,6 +17,7 @@ limitations under the License. package resourcequotacontroller import ( + "fmt" "time" "github.com/golang/glog" @@ -163,18 +164,6 @@ func (rm *ResourceQuotaController) syncResourceQuota(quota api.ResourceQuota) (e switch k { case api.ResourcePods: value = resource.NewQuantity(int64(len(filteredPods)), resource.DecimalSI) - case api.ResourceMemory: - val := int64(0) - for _, pod := range filteredPods { - val = val + PodMemory(pod).Value() - } - value = resource.NewQuantity(int64(val), resource.DecimalSI) - case api.ResourceCPU: - val := int64(0) - for _, pod := range filteredPods { - val = val + PodCPU(pod).MilliValue() - } - value = resource.NewMilliQuantity(int64(val), resource.DecimalSI) case api.ResourceServices: items, err := rm.kubeClient.Services(usage.Namespace).List(labels.Everything()) if err != nil { @@ -205,6 +194,10 @@ func (rm *ResourceQuotaController) syncResourceQuota(quota api.ResourceQuota) (e return err } value = resource.NewQuantity(int64(len(items.Items)), resource.DecimalSI) + case api.ResourceMemory: + value = PodsRequests(filteredPods, api.ResourceMemory) + case api.ResourceCPU: + value = PodsRequests(filteredPods, api.ResourceCPU) } // ignore fields we do not understand (assume another controller is tracking it) @@ -224,7 +217,73 @@ func (rm *ResourceQuotaController) syncResourceQuota(quota api.ResourceQuota) (e return nil } -// PodCPU computes total cpu usage of a pod +// PodsRequests returns sum of each resource request for each pod in list +// If a given pod in the list does not have a request for the named resource, we log the error +// but still attempt to get the most representative count +func PodsRequests(pods []*api.Pod, resourceName api.ResourceName) *resource.Quantity { + var sum *resource.Quantity + for i := range pods { + pod := pods[i] + podQuantity, err := PodRequests(pod, resourceName) + if err != nil { + // log the error, but try to keep the most accurate count possible in log + // rationale here is that you may have had pods in a namespace that did not have + // explicit requests prior to adding the quota + glog.Infof("No explicit request for resource, pod %s/%s, %s", pod.Namespace, pod.Name, resourceName) + } else { + if sum == nil { + sum = podQuantity + } else { + sum.Add(*podQuantity) + } + } + } + // if list is empty + if sum == nil { + q := resource.MustParse("0") + sum = &q + } + return sum +} + +// PodRequests returns sum of each resource request across all containers in pod +func PodRequests(pod *api.Pod, resourceName api.ResourceName) (*resource.Quantity, error) { + if !PodHasRequests(pod, resourceName) { + return nil, fmt.Errorf("Each container in pod %s/%s does not have an explicit request for resource %s.", pod.Namespace, pod.Name, resourceName) + } + var sum *resource.Quantity + for j := range pod.Spec.Containers { + value, _ := pod.Spec.Containers[j].Resources.Requests[resourceName] + if sum == nil { + sum = value.Copy() + } else { + err := sum.Add(value) + if err != nil { + return sum, err + } + } + } + // if list is empty + if sum == nil { + q := resource.MustParse("0") + sum = &q + } + return sum, nil +} + +// PodHasRequests verifies that each container in the pod has an explicit request that is non-zero for a named resource +func PodHasRequests(pod *api.Pod, resourceName api.ResourceName) bool { + for j := range pod.Spec.Containers { + value, valueSet := pod.Spec.Containers[j].Resources.Requests[resourceName] + if !valueSet || value.Value() == int64(0) { + return false + } + } + return true +} + +// PodCPU computes total cpu limit across all containers in pod +// TODO: Remove this once the mesos scheduler becomes request aware func PodCPU(pod *api.Pod) *resource.Quantity { val := int64(0) for j := range pod.Spec.Containers { @@ -233,29 +292,8 @@ func PodCPU(pod *api.Pod) *resource.Quantity { return resource.NewMilliQuantity(int64(val), resource.DecimalSI) } -// IsPodCPUUnbounded returns true if the cpu use is unbounded for any container in pod -func IsPodCPUUnbounded(pod *api.Pod) bool { - for j := range pod.Spec.Containers { - container := pod.Spec.Containers[j] - if container.Resources.Limits.Cpu().MilliValue() == int64(0) { - return true - } - } - return false -} - -// IsPodMemoryUnbounded returns true if the memory use is unbounded for any container in pod -func IsPodMemoryUnbounded(pod *api.Pod) bool { - for j := range pod.Spec.Containers { - container := pod.Spec.Containers[j] - if container.Resources.Limits.Memory().Value() == int64(0) { - return true - } - } - return false -} - -// PodMemory computes the memory usage of a pod +// PodMemory computes total memory limit across all containers in a pod +// TODO: Remove this once the mesos scheduler becomes request aware func PodMemory(pod *api.Pod) *resource.Quantity { val := int64(0) for j := range pod.Spec.Containers { diff --git a/pkg/controller/resourcequota/resource_quota_controller_test.go b/pkg/controller/resourcequota/resource_quota_controller_test.go index 20d0677a21c..346d6bdf438 100644 --- a/pkg/controller/resourcequota/resource_quota_controller_test.go +++ b/pkg/controller/resourcequota/resource_quota_controller_test.go @@ -17,6 +17,7 @@ limitations under the License. package resourcequotacontroller import ( + "strconv" "testing" "k8s.io/kubernetes/pkg/api" @@ -25,19 +26,39 @@ import ( "k8s.io/kubernetes/pkg/util" ) -func getResourceRequirements(cpu, memory string) api.ResourceRequirements { - res := api.ResourceRequirements{} - res.Limits = api.ResourceList{} +func getResourceList(cpu, memory string) api.ResourceList { + res := api.ResourceList{} if cpu != "" { - res.Limits[api.ResourceCPU] = resource.MustParse(cpu) + res[api.ResourceCPU] = resource.MustParse(cpu) } if memory != "" { - res.Limits[api.ResourceMemory] = resource.MustParse(memory) + res[api.ResourceMemory] = resource.MustParse(memory) } - return res } +func getResourceRequirements(requests, limits api.ResourceList) api.ResourceRequirements { + res := api.ResourceRequirements{} + res.Requests = requests + res.Limits = limits + return res +} + +func validPod(name string, numContainers int, resources api.ResourceRequirements) *api.Pod { + pod := &api.Pod{ + ObjectMeta: api.ObjectMeta{Name: name, Namespace: "test"}, + Spec: api.PodSpec{}, + } + pod.Spec.Containers = make([]api.Container, 0, numContainers) + for i := 0; i < numContainers; i++ { + pod.Spec.Containers = append(pod.Spec.Containers, api.Container{ + Image: "foo:V" + strconv.Itoa(i), + Resources: resources, + }) + } + return pod +} + func TestFilterQuotaPods(t *testing.T) { pods := []api.Pod{ { @@ -105,7 +126,7 @@ func TestSyncResourceQuota(t *testing.T) { Status: api.PodStatus{Phase: api.PodRunning}, Spec: api.PodSpec{ Volumes: []api.Volume{{Name: "vol"}}, - Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("100m", "1Gi")}}, + Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements(getResourceList("100m", "1Gi"), getResourceList("", ""))}}, }, }, { @@ -113,7 +134,7 @@ func TestSyncResourceQuota(t *testing.T) { Status: api.PodStatus{Phase: api.PodRunning}, Spec: api.PodSpec{ Volumes: []api.Volume{{Name: "vol"}}, - Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("100m", "1Gi")}}, + Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements(getResourceList("100m", "1Gi"), getResourceList("", ""))}}, }, }, { @@ -121,7 +142,7 @@ func TestSyncResourceQuota(t *testing.T) { Status: api.PodStatus{Phase: api.PodFailed}, Spec: api.PodSpec{ Volumes: []api.Volume{{Name: "vol"}}, - Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("100m", "1Gi")}}, + Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements(getResourceList("100m", "1Gi"), getResourceList("", ""))}}, }, }, }, @@ -144,7 +165,7 @@ func TestSyncResourceQuota(t *testing.T) { }, Used: api.ResourceList{ api.ResourceCPU: resource.MustParse("200m"), - api.ResourceMemory: resource.MustParse("2147483648"), + api.ResourceMemory: resource.MustParse("2Gi"), api.ResourcePods: resource.MustParse("2"), }, }, @@ -177,7 +198,6 @@ func TestSyncResourceQuota(t *testing.T) { t.Errorf("Usage Used: Key: %v, Expected: %v, Actual: %v", k, expectedValue, actualValue) } } - } func TestSyncResourceQuotaSpecChange(t *testing.T) { @@ -269,62 +289,151 @@ func TestSyncResourceQuotaNoChange(t *testing.T) { } } -func TestIsPodCPUUnbounded(t *testing.T) { - pod := api.Pod{ - ObjectMeta: api.ObjectMeta{Name: "pod-running"}, - Status: api.PodStatus{Phase: api.PodRunning}, - Spec: api.PodSpec{ - Volumes: []api.Volume{{Name: "vol"}}, - Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("100m", "0")}}, +func TestPodHasRequests(t *testing.T) { + type testCase struct { + pod *api.Pod + resourceName api.ResourceName + expectedResult bool + } + testCases := []testCase{ + { + pod: validPod("request-cpu", 2, getResourceRequirements(getResourceList("100m", ""), getResourceList("", ""))), + resourceName: api.ResourceCPU, + expectedResult: true, + }, + { + pod: validPod("no-request-cpu", 2, getResourceRequirements(getResourceList("", ""), getResourceList("", ""))), + resourceName: api.ResourceCPU, + expectedResult: false, + }, + { + pod: validPod("request-zero-cpu", 2, getResourceRequirements(getResourceList("0", ""), getResourceList("", ""))), + resourceName: api.ResourceCPU, + expectedResult: false, + }, + { + pod: validPod("request-memory", 2, getResourceRequirements(getResourceList("", "2Mi"), getResourceList("", ""))), + resourceName: api.ResourceMemory, + expectedResult: true, + }, + { + pod: validPod("no-request-memory", 2, getResourceRequirements(getResourceList("", ""), getResourceList("", ""))), + resourceName: api.ResourceMemory, + expectedResult: false, + }, + { + pod: validPod("request-zero-memory", 2, getResourceRequirements(getResourceList("", "0"), getResourceList("", ""))), + resourceName: api.ResourceMemory, + expectedResult: false, }, } - if IsPodCPUUnbounded(&pod) { - t.Errorf("Expected false") - } - pod = api.Pod{ - ObjectMeta: api.ObjectMeta{Name: "pod-running"}, - Status: api.PodStatus{Phase: api.PodRunning}, - Spec: api.PodSpec{ - Volumes: []api.Volume{{Name: "vol"}}, - Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("0", "0")}}, - }, - } - if !IsPodCPUUnbounded(&pod) { - t.Errorf("Expected true") - } - - pod.Spec.Containers[0].Resources = api.ResourceRequirements{} - if !IsPodCPUUnbounded(&pod) { - t.Errorf("Expected true") + for _, item := range testCases { + if actual := PodHasRequests(item.pod, item.resourceName); item.expectedResult != actual { + t.Errorf("Pod %s for resource %s expected %v actual %v", item.pod.Name, item.resourceName, item.expectedResult, actual) + } } } -func TestIsPodMemoryUnbounded(t *testing.T) { - pod := api.Pod{ - ObjectMeta: api.ObjectMeta{Name: "pod-running"}, - Status: api.PodStatus{Phase: api.PodRunning}, - Spec: api.PodSpec{ - Volumes: []api.Volume{{Name: "vol"}}, - Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("0", "1Gi")}}, +func TestPodRequests(t *testing.T) { + type testCase struct { + pod *api.Pod + resourceName api.ResourceName + expectedResult string + expectedError bool + } + testCases := []testCase{ + { + pod: validPod("request-cpu", 2, getResourceRequirements(getResourceList("100m", ""), getResourceList("", ""))), + resourceName: api.ResourceCPU, + expectedResult: "200m", + expectedError: false, + }, + { + pod: validPod("no-request-cpu", 2, getResourceRequirements(getResourceList("", ""), getResourceList("", ""))), + resourceName: api.ResourceCPU, + expectedResult: "", + expectedError: true, + }, + { + pod: validPod("request-zero-cpu", 2, getResourceRequirements(getResourceList("0", ""), getResourceList("", ""))), + resourceName: api.ResourceCPU, + expectedResult: "", + expectedError: true, + }, + { + pod: validPod("request-memory", 2, getResourceRequirements(getResourceList("", "500Mi"), getResourceList("", ""))), + resourceName: api.ResourceMemory, + expectedResult: "1000Mi", + expectedError: false, + }, + { + pod: validPod("no-request-memory", 2, getResourceRequirements(getResourceList("", ""), getResourceList("", ""))), + resourceName: api.ResourceMemory, + expectedResult: "", + expectedError: true, + }, + { + pod: validPod("request-zero-memory", 2, getResourceRequirements(getResourceList("", "0"), getResourceList("", ""))), + resourceName: api.ResourceMemory, + expectedResult: "", + expectedError: true, }, } - if IsPodMemoryUnbounded(&pod) { - t.Errorf("Expected false") - } - pod = api.Pod{ - ObjectMeta: api.ObjectMeta{Name: "pod-running"}, - Status: api.PodStatus{Phase: api.PodRunning}, - Spec: api.PodSpec{ - Volumes: []api.Volume{{Name: "vol"}}, - Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("0", "0")}}, - }, - } - if !IsPodMemoryUnbounded(&pod) { - t.Errorf("Expected true") - } - - pod.Spec.Containers[0].Resources = api.ResourceRequirements{} - if !IsPodMemoryUnbounded(&pod) { - t.Errorf("Expected true") + for _, item := range testCases { + actual, err := PodRequests(item.pod, item.resourceName) + if item.expectedError != (err != nil) { + t.Errorf("Unexpected error result for pod %s for resource %s expected error %v got %v", item.pod.Name, item.resourceName, item.expectedError, err) + } + if item.expectedResult != "" && (item.expectedResult != actual.String()) { + t.Errorf("Expected %s, Actual %s, pod %s for resource %s", item.expectedResult, actual.String(), item.pod.Name, item.resourceName) + } + } +} + +func TestPodsRequests(t *testing.T) { + type testCase struct { + pods []*api.Pod + resourceName api.ResourceName + expectedResult string + } + testCases := []testCase{ + { + pods: []*api.Pod{ + validPod("request-cpu-1", 1, getResourceRequirements(getResourceList("100m", ""), getResourceList("", ""))), + validPod("request-cpu-2", 1, getResourceRequirements(getResourceList("1", ""), getResourceList("", ""))), + }, + resourceName: api.ResourceCPU, + expectedResult: "1100m", + }, + { + pods: []*api.Pod{ + validPod("no-request-cpu-1", 1, getResourceRequirements(getResourceList("", ""), getResourceList("", ""))), + validPod("no-request-cpu-2", 1, getResourceRequirements(getResourceList("", ""), getResourceList("", ""))), + }, + resourceName: api.ResourceCPU, + expectedResult: "", + }, + { + pods: []*api.Pod{ + validPod("request-zero-cpu-1", 1, getResourceRequirements(getResourceList("0", ""), getResourceList("", ""))), + validPod("request-zero-cpu-1", 1, getResourceRequirements(getResourceList("0", ""), getResourceList("", ""))), + }, + resourceName: api.ResourceCPU, + expectedResult: "", + }, + { + pods: []*api.Pod{ + validPod("request-memory-1", 1, getResourceRequirements(getResourceList("", "500Mi"), getResourceList("", ""))), + validPod("request-memory-2", 1, getResourceRequirements(getResourceList("", "1Gi"), getResourceList("", ""))), + }, + resourceName: api.ResourceMemory, + expectedResult: "1524Mi", + }, + } + for _, item := range testCases { + actual := PodsRequests(item.pods, item.resourceName) + if item.expectedResult != "" && (item.expectedResult != actual.String()) { + t.Errorf("Expected %s, Actual %s, pod %s for resource %s", item.expectedResult, actual.String(), item.pods[0].Name, item.resourceName) + } } } diff --git a/plugin/pkg/admission/resourcequota/admission.go b/plugin/pkg/admission/resourcequota/admission.go index cfef57c4ab7..aba2d235f45 100644 --- a/plugin/pkg/admission/resourcequota/admission.go +++ b/plugin/pkg/admission/resourcequota/admission.go @@ -190,52 +190,69 @@ func IncrementUsage(a admission.Attributes, status *api.ResourceQuotaStatus, cli } } } - // handle memory/cpu constraints, and any diff of usage based on memory/cpu on updates - if a.GetResource() == "pods" && (set[api.ResourceMemory] || set[api.ResourceCPU]) { - pod := obj.(*api.Pod) - deltaCPU := resourcequotacontroller.PodCPU(pod) - deltaMemory := resourcequotacontroller.PodMemory(pod) - // if this is an update, we need to find the delta cpu/memory usage from previous state - if a.GetOperation() == admission.Update { - oldPod, err := client.Pods(a.GetNamespace()).Get(pod.Name) - if err != nil { - return false, err - } - oldCPU := resourcequotacontroller.PodCPU(oldPod) - oldMemory := resourcequotacontroller.PodMemory(oldPod) - deltaCPU = resource.NewMilliQuantity(deltaCPU.MilliValue()-oldCPU.MilliValue(), resource.DecimalSI) - deltaMemory = resource.NewQuantity(deltaMemory.Value()-oldMemory.Value(), resource.DecimalSI) - } - hardMem, hardMemFound := status.Hard[api.ResourceMemory] - if hardMemFound { - if set[api.ResourceMemory] && resourcequotacontroller.IsPodMemoryUnbounded(pod) { - return false, fmt.Errorf("Limited to %s memory, but pod has no specified memory limit", hardMem.String()) + if a.GetResource() == "pods" { + for _, resourceName := range []api.ResourceName{api.ResourceMemory, api.ResourceCPU} { + + // ignore tracking the resource if its not in the quota document + if !set[resourceName] { + continue } - used, usedFound := status.Used[api.ResourceMemory] + + hard, hardFound := status.Hard[resourceName] + if !hardFound { + continue + } + + // if we do not yet know how much of the current resource is used, we cannot accept any request + used, usedFound := status.Used[resourceName] if !usedFound { - return false, fmt.Errorf("Quota usage stats are not yet known, unable to admit resource until an accurate count is completed.") + return false, fmt.Errorf("Unable to admit pod until quota usage stats are calculated.") } - if used.Value()+deltaMemory.Value() > hardMem.Value() { - return false, fmt.Errorf("Limited to %s memory", hardMem.String()) + + // the amount of resource being requested, or an error if it does not make a request that is tracked + pod := obj.(*api.Pod) + delta, err := resourcequotacontroller.PodRequests(pod, resourceName) + + if err != nil { + return false, fmt.Errorf("Must make a non-zero request for %s since it is tracked by quota.", resourceName) + } + + // if this operation is an update, we need to find the delta usage from the previous state + if a.GetOperation() == admission.Update { + oldPod, err := client.Pods(a.GetNamespace()).Get(pod.Name) + if err != nil { + return false, err + } + + // if the previous version of the resource made a resource request, we need to subtract the old request + // from the current to get the actual resource request delta. if the previous version of the pod + // made no request on the resource, then we get an err value. we ignore the err value, and delta + // will just be equal to the total resource request on the pod since there is nothing to subtract. + oldRequest, err := resourcequotacontroller.PodRequests(oldPod, resourceName) + if err == nil { + err = delta.Sub(*oldRequest) + if err != nil { + return false, err + } + } + } + + newUsage := used.Copy() + newUsage.Add(*delta) + + // make the most precise comparison possible + newUsageValue := newUsage.Value() + hardUsageValue := hard.Value() + if newUsageValue <= resource.MaxMilliValue && hardUsageValue <= resource.MaxMilliValue { + newUsageValue = newUsage.MilliValue() + hardUsageValue = hard.MilliValue() + } + + if newUsageValue > hardUsageValue { + return false, fmt.Errorf("Unable to admit pod without exceeding quota for resource %s. Limited to %s but require %s to succeed.", resourceName, hard.String(), newUsage.String()) } else { - status.Used[api.ResourceMemory] = *resource.NewQuantity(used.Value()+deltaMemory.Value(), resource.DecimalSI) - dirty = true - } - } - hardCPU, hardCPUFound := status.Hard[api.ResourceCPU] - if hardCPUFound { - if set[api.ResourceCPU] && resourcequotacontroller.IsPodCPUUnbounded(pod) { - return false, fmt.Errorf("Limited to %s CPU, but pod has no specified cpu limit", hardCPU.String()) - } - used, usedFound := status.Used[api.ResourceCPU] - if !usedFound { - return false, fmt.Errorf("Quota usage stats are not yet known, unable to admit resource until an accurate count is completed.") - } - if used.MilliValue()+deltaCPU.MilliValue() > hardCPU.MilliValue() { - return false, fmt.Errorf("Limited to %s CPU", hardCPU.String()) - } else { - status.Used[api.ResourceCPU] = *resource.NewMilliQuantity(used.MilliValue()+deltaCPU.MilliValue(), resource.DecimalSI) + status.Used[resourceName] = *newUsage dirty = true } } diff --git a/plugin/pkg/admission/resourcequota/admission_test.go b/plugin/pkg/admission/resourcequota/admission_test.go index c066f31d81a..83c0efe05b1 100644 --- a/plugin/pkg/admission/resourcequota/admission_test.go +++ b/plugin/pkg/admission/resourcequota/admission_test.go @@ -17,6 +17,7 @@ limitations under the License. package resourcequota import ( + "strconv" "testing" "k8s.io/kubernetes/pkg/admission" @@ -24,21 +25,42 @@ import ( "k8s.io/kubernetes/pkg/api/resource" "k8s.io/kubernetes/pkg/client/unversioned/cache" "k8s.io/kubernetes/pkg/client/unversioned/testclient" + "k8s.io/kubernetes/pkg/controller/resourcequota" ) -func getResourceRequirements(cpu, memory string) api.ResourceRequirements { - res := api.ResourceRequirements{} - res.Limits = api.ResourceList{} +func getResourceList(cpu, memory string) api.ResourceList { + res := api.ResourceList{} if cpu != "" { - res.Limits[api.ResourceCPU] = resource.MustParse(cpu) + res[api.ResourceCPU] = resource.MustParse(cpu) } if memory != "" { - res.Limits[api.ResourceMemory] = resource.MustParse(memory) + res[api.ResourceMemory] = resource.MustParse(memory) } - return res } +func getResourceRequirements(requests, limits api.ResourceList) api.ResourceRequirements { + res := api.ResourceRequirements{} + res.Requests = requests + res.Limits = limits + return res +} + +func validPod(name string, numContainers int, resources api.ResourceRequirements) *api.Pod { + pod := &api.Pod{ + ObjectMeta: api.ObjectMeta{Name: name, Namespace: "test"}, + Spec: api.PodSpec{}, + } + pod.Spec.Containers = make([]api.Container, 0, numContainers) + for i := 0; i < numContainers; i++ { + pod.Spec.Containers = append(pod.Spec.Containers, api.Container{ + Image: "foo:V" + strconv.Itoa(i), + Resources: resources, + }) + } + return pod +} + func TestAdmissionIgnoresDelete(t *testing.T) { namespace := "default" handler := createResourceQuota(&testclient.Fake{}, nil) @@ -64,38 +86,118 @@ func TestAdmissionIgnoresSubresources(t *testing.T) { indexer.Add(quota) - newPod := &api.Pod{ - ObjectMeta: api.ObjectMeta{Name: "123", Namespace: quota.Namespace}, - Spec: api.PodSpec{ - Volumes: []api.Volume{{Name: "vol"}}, - Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("100m", "2Gi")}}, - }} - - err := handler.Admit(admission.NewAttributesRecord(newPod, "Pod", newPod.Namespace, "123", "pods", "", admission.Create, nil)) + newPod := validPod("123", 1, getResourceRequirements(getResourceList("100m", "2Gi"), getResourceList("", ""))) + err := handler.Admit(admission.NewAttributesRecord(newPod, "Pod", newPod.Namespace, newPod.Name, "pods", "", admission.Create, nil)) if err == nil { t.Errorf("Expected an error because the pod exceeded allowed quota") } - err = handler.Admit(admission.NewAttributesRecord(newPod, "Pod", newPod.Namespace, "123", "pods", "subresource", admission.Create, nil)) + err = handler.Admit(admission.NewAttributesRecord(newPod, "Pod", newPod.Namespace, newPod.Name, "pods", "subresource", admission.Create, nil)) if err != nil { t.Errorf("Did not expect an error because the action went to a subresource: %v", err) } } -func TestIncrementUsagePods(t *testing.T) { - namespace := "default" - client := testclient.NewSimpleFake(&api.PodList{ - Items: []api.Pod{ - { - ObjectMeta: api.ObjectMeta{Name: "123", Namespace: namespace}, - Spec: api.PodSpec{ - Volumes: []api.Volume{{Name: "vol"}}, - Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("100m", "1Gi")}}, - }, - }, +func TestIncrementUsagePodResources(t *testing.T) { + type testCase struct { + testName string + existing *api.Pod + input *api.Pod + resourceName api.ResourceName + hard resource.Quantity + expectedUsage resource.Quantity + expectedError bool + } + testCases := []testCase{ + { + testName: "memory-allowed", + existing: validPod("a", 1, getResourceRequirements(getResourceList("", "100Mi"), getResourceList("", ""))), + input: validPod("b", 1, getResourceRequirements(getResourceList("", "100Mi"), getResourceList("", ""))), + resourceName: api.ResourceMemory, + hard: resource.MustParse("500Mi"), + expectedUsage: resource.MustParse("200Mi"), + expectedError: false, }, - }) + { + testName: "memory-not-allowed", + existing: validPod("a", 1, getResourceRequirements(getResourceList("", "100Mi"), getResourceList("", ""))), + input: validPod("b", 1, getResourceRequirements(getResourceList("", "450Mi"), getResourceList("", ""))), + resourceName: api.ResourceMemory, + hard: resource.MustParse("500Mi"), + expectedError: true, + }, + { + testName: "memory-no-request", + existing: validPod("a", 1, getResourceRequirements(getResourceList("", "100Mi"), getResourceList("", ""))), + input: validPod("b", 1, getResourceRequirements(getResourceList("", ""), getResourceList("", ""))), + resourceName: api.ResourceMemory, + hard: resource.MustParse("500Mi"), + expectedError: true, + }, + { + testName: "cpu-allowed", + existing: validPod("a", 1, getResourceRequirements(getResourceList("1", ""), getResourceList("", ""))), + input: validPod("b", 1, getResourceRequirements(getResourceList("1", ""), getResourceList("", ""))), + resourceName: api.ResourceCPU, + hard: resource.MustParse("2"), + expectedUsage: resource.MustParse("2"), + expectedError: false, + }, + { + testName: "cpu-not-allowed", + existing: validPod("a", 1, getResourceRequirements(getResourceList("1", ""), getResourceList("", ""))), + input: validPod("b", 1, getResourceRequirements(getResourceList("600m", ""), getResourceList("", ""))), + resourceName: api.ResourceCPU, + hard: resource.MustParse("1500m"), + expectedError: true, + }, + { + testName: "cpu-no-request", + existing: validPod("a", 1, getResourceRequirements(getResourceList("1", ""), getResourceList("", ""))), + input: validPod("b", 1, getResourceRequirements(getResourceList("", ""), getResourceList("", ""))), + resourceName: api.ResourceCPU, + hard: resource.MustParse("1500m"), + expectedError: true, + }, + } + for _, item := range testCases { + podList := &api.PodList{Items: []api.Pod{*item.existing}} + client := testclient.NewSimpleFake(podList) + status := &api.ResourceQuotaStatus{ + Hard: api.ResourceList{}, + Used: api.ResourceList{}, + } + used, err := resourcequotacontroller.PodRequests(item.existing, item.resourceName) + if err != nil { + t.Errorf("Test %s, unexpected error %v", item.testName, err) + } + status.Hard[item.resourceName] = item.hard + status.Used[item.resourceName] = *used + + dirty, err := IncrementUsage(admission.NewAttributesRecord(item.input, "Pod", item.input.Namespace, item.input.Name, "pods", "", admission.Create, nil), status, client) + if err == nil && item.expectedError { + t.Errorf("Test %s, expected error", item.testName) + } + if err != nil && !item.expectedError { + t.Errorf("Test %s, unexpected error", err) + } + if !item.expectedError { + if !dirty { + t.Errorf("Test %s, expected the quota to be dirty", item.testName) + } + quantity := status.Used[item.resourceName] + if quantity.String() != item.expectedUsage.String() { + t.Errorf("Test %s, expected usage %s, actual usage %s", item.testName, item.expectedUsage.String(), quantity.String()) + } + } + } +} + +func TestIncrementUsagePods(t *testing.T) { + pod := validPod("123", 1, getResourceRequirements(getResourceList("100m", "1Gi"), getResourceList("", ""))) + podList := &api.PodList{Items: []api.Pod{*pod}} + client := testclient.NewSimpleFake(podList) status := &api.ResourceQuotaStatus{ Hard: api.ResourceList{}, Used: api.ResourceList{}, @@ -103,7 +205,7 @@ func TestIncrementUsagePods(t *testing.T) { r := api.ResourcePods status.Hard[r] = resource.MustParse("2") status.Used[r] = resource.MustParse("1") - dirty, err := IncrementUsage(admission.NewAttributesRecord(&api.Pod{}, "Pod", namespace, "name", "pods", "", admission.Create, nil), status, client) + dirty, err := IncrementUsage(admission.NewAttributesRecord(&api.Pod{}, "Pod", pod.Namespace, "new-pod", "pods", "", admission.Create, nil), status, client) if err != nil { t.Errorf("Unexpected error: %v", err) } @@ -116,233 +218,10 @@ func TestIncrementUsagePods(t *testing.T) { } } -func TestIncrementUsageMemory(t *testing.T) { - namespace := "default" - client := testclient.NewSimpleFake(&api.PodList{ - Items: []api.Pod{ - { - ObjectMeta: api.ObjectMeta{Name: "123", Namespace: namespace}, - Spec: api.PodSpec{ - Volumes: []api.Volume{{Name: "vol"}}, - Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("100m", "1Gi")}}, - }, - }, - }, - }) - status := &api.ResourceQuotaStatus{ - Hard: api.ResourceList{}, - Used: api.ResourceList{}, - } - r := api.ResourceMemory - status.Hard[r] = resource.MustParse("2Gi") - status.Used[r] = resource.MustParse("1Gi") - - newPod := &api.Pod{ - ObjectMeta: api.ObjectMeta{Name: "123", Namespace: namespace}, - Spec: api.PodSpec{ - Volumes: []api.Volume{{Name: "vol"}}, - Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("100m", "1Gi")}}, - }} - dirty, err := IncrementUsage(admission.NewAttributesRecord(newPod, "Pod", namespace, "name", "pods", "", admission.Create, nil), status, client) - if err != nil { - t.Errorf("Unexpected error: %v", err) - } - if !dirty { - t.Errorf("Expected the status to get incremented, therefore should have been dirty") - } - expectedVal := resource.MustParse("2Gi") - quantity := status.Used[r] - if quantity.Value() != expectedVal.Value() { - t.Errorf("Expected %v was %v", expectedVal.Value(), quantity.Value()) - } -} - -func TestExceedUsageMemory(t *testing.T) { - namespace := "default" - client := testclient.NewSimpleFake(&api.PodList{ - Items: []api.Pod{ - { - ObjectMeta: api.ObjectMeta{Name: "123", Namespace: namespace}, - Spec: api.PodSpec{ - Volumes: []api.Volume{{Name: "vol"}}, - Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("100m", "1Gi")}}, - }, - }, - }, - }) - status := &api.ResourceQuotaStatus{ - Hard: api.ResourceList{}, - Used: api.ResourceList{}, - } - r := api.ResourceMemory - status.Hard[r] = resource.MustParse("2Gi") - status.Used[r] = resource.MustParse("1Gi") - - newPod := &api.Pod{ - ObjectMeta: api.ObjectMeta{Name: "123", Namespace: namespace}, - Spec: api.PodSpec{ - Volumes: []api.Volume{{Name: "vol"}}, - Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("100m", "3Gi")}}, - }} - _, err := IncrementUsage(admission.NewAttributesRecord(newPod, "Pod", namespace, "name", "pods", "", admission.Create, nil), status, client) - if err == nil { - t.Errorf("Expected memory usage exceeded error") - } -} - -func TestIncrementUsageCPU(t *testing.T) { - namespace := "default" - client := testclient.NewSimpleFake(&api.PodList{ - Items: []api.Pod{ - { - ObjectMeta: api.ObjectMeta{Name: "123", Namespace: namespace}, - Spec: api.PodSpec{ - Volumes: []api.Volume{{Name: "vol"}}, - Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("100m", "1Gi")}}, - }, - }, - }, - }) - status := &api.ResourceQuotaStatus{ - Hard: api.ResourceList{}, - Used: api.ResourceList{}, - } - r := api.ResourceCPU - status.Hard[r] = resource.MustParse("200m") - status.Used[r] = resource.MustParse("100m") - - newPod := &api.Pod{ - ObjectMeta: api.ObjectMeta{Name: "123", Namespace: namespace}, - Spec: api.PodSpec{ - Volumes: []api.Volume{{Name: "vol"}}, - Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("100m", "1Gi")}}, - }} - dirty, err := IncrementUsage(admission.NewAttributesRecord(newPod, "Pod", namespace, "name", "pods", "", admission.Create, nil), status, client) - if err != nil { - t.Errorf("Unexpected error: %v", err) - } - if !dirty { - t.Errorf("Expected the status to get incremented, therefore should have been dirty") - } - expectedVal := resource.MustParse("200m") - quantity := status.Used[r] - if quantity.Value() != expectedVal.Value() { - t.Errorf("Expected %v was %v", expectedVal.Value(), quantity.Value()) - } -} - -func TestUnboundedCPU(t *testing.T) { - namespace := "default" - client := testclient.NewSimpleFake(&api.PodList{ - Items: []api.Pod{ - { - ObjectMeta: api.ObjectMeta{Name: "123", Namespace: namespace}, - Spec: api.PodSpec{ - Volumes: []api.Volume{{Name: "vol"}}, - Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("100m", "1Gi")}}, - }, - }, - }, - }) - status := &api.ResourceQuotaStatus{ - Hard: api.ResourceList{}, - Used: api.ResourceList{}, - } - r := api.ResourceCPU - status.Hard[r] = resource.MustParse("200m") - status.Used[r] = resource.MustParse("100m") - - newPod := &api.Pod{ - ObjectMeta: api.ObjectMeta{Name: "123", Namespace: namespace}, - Spec: api.PodSpec{ - Volumes: []api.Volume{{Name: "vol"}}, - Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("0m", "1Gi")}}, - }} - _, err := IncrementUsage(admission.NewAttributesRecord(newPod, "Pod", namespace, "name", "pods", "", admission.Create, nil), status, client) - if err == nil { - t.Errorf("Expected CPU unbounded usage error") - } -} - -func TestUnboundedMemory(t *testing.T) { - namespace := "default" - client := testclient.NewSimpleFake(&api.PodList{ - Items: []api.Pod{ - { - ObjectMeta: api.ObjectMeta{Name: "123", Namespace: namespace}, - Spec: api.PodSpec{ - Volumes: []api.Volume{{Name: "vol"}}, - Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("100m", "1Gi")}}, - }, - }, - }, - }) - status := &api.ResourceQuotaStatus{ - Hard: api.ResourceList{}, - Used: api.ResourceList{}, - } - r := api.ResourceMemory - status.Hard[r] = resource.MustParse("10Gi") - status.Used[r] = resource.MustParse("1Gi") - - newPod := &api.Pod{ - ObjectMeta: api.ObjectMeta{Name: "123", Namespace: namespace}, - Spec: api.PodSpec{ - Volumes: []api.Volume{{Name: "vol"}}, - Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("250m", "0")}}, - }} - _, err := IncrementUsage(admission.NewAttributesRecord(newPod, "Pod", namespace, "name", "pods", "", admission.Create, nil), status, client) - if err == nil { - t.Errorf("Expected memory unbounded usage error") - } -} - -func TestExceedUsageCPU(t *testing.T) { - namespace := "default" - client := testclient.NewSimpleFake(&api.PodList{ - Items: []api.Pod{ - { - ObjectMeta: api.ObjectMeta{Name: "123", Namespace: namespace}, - Spec: api.PodSpec{ - Volumes: []api.Volume{{Name: "vol"}}, - Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("100m", "1Gi")}}, - }, - }, - }, - }) - status := &api.ResourceQuotaStatus{ - Hard: api.ResourceList{}, - Used: api.ResourceList{}, - } - r := api.ResourceCPU - status.Hard[r] = resource.MustParse("200m") - status.Used[r] = resource.MustParse("100m") - - newPod := &api.Pod{ - ObjectMeta: api.ObjectMeta{Name: "123", Namespace: namespace}, - Spec: api.PodSpec{ - Volumes: []api.Volume{{Name: "vol"}}, - Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("500m", "1Gi")}}, - }} - _, err := IncrementUsage(admission.NewAttributesRecord(newPod, "Pod", namespace, newPod.Name, "pods", "", admission.Create, nil), status, client) - if err == nil { - t.Errorf("Expected CPU usage exceeded error") - } -} - func TestExceedUsagePods(t *testing.T) { - namespace := "default" - client := testclient.NewSimpleFake(&api.PodList{ - Items: []api.Pod{ - { - ObjectMeta: api.ObjectMeta{Name: "123", Namespace: namespace}, - Spec: api.PodSpec{ - Volumes: []api.Volume{{Name: "vol"}}, - Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("100m", "1Gi")}}, - }, - }, - }, - }) + pod := validPod("123", 1, getResourceRequirements(getResourceList("100m", "1Gi"), getResourceList("", ""))) + podList := &api.PodList{Items: []api.Pod{*pod}} + client := testclient.NewSimpleFake(podList) status := &api.ResourceQuotaStatus{ Hard: api.ResourceList{}, Used: api.ResourceList{}, @@ -350,7 +229,7 @@ func TestExceedUsagePods(t *testing.T) { r := api.ResourcePods status.Hard[r] = resource.MustParse("1") status.Used[r] = resource.MustParse("1") - _, err := IncrementUsage(admission.NewAttributesRecord(&api.Pod{}, "Pod", namespace, "name", "pods", "", admission.Create, nil), status, client) + _, err := IncrementUsage(admission.NewAttributesRecord(&api.Pod{}, "Pod", pod.Namespace, "name", "pods", "", admission.Create, nil), status, client) if err == nil { t.Errorf("Expected error because this would exceed your quota") }