From 75482cabb465844366e5db8aea44d81d32a62181 Mon Sep 17 00:00:00 2001 From: derekwaynecarr Date: Fri, 17 Apr 2015 16:59:54 -0400 Subject: [PATCH] Reject unbounded cpu and memory pods if quota is restricting it --- .../resource_quota_controller.go | 22 +++++++ .../resource_quota_controller_test.go | 60 +++++++++++++++++ .../pkg/admission/resourcequota/admission.go | 6 ++ .../admission/resourcequota/admission_test.go | 66 +++++++++++++++++++ 4 files changed, 154 insertions(+) diff --git a/pkg/resourcequota/resource_quota_controller.go b/pkg/resourcequota/resource_quota_controller.go index 3d651c12fc8a..62ad59f01074 100644 --- a/pkg/resourcequota/resource_quota_controller.go +++ b/pkg/resourcequota/resource_quota_controller.go @@ -226,6 +226,28 @@ 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 func PodMemory(pod *api.Pod) *resource.Quantity { val := int64(0) diff --git a/pkg/resourcequota/resource_quota_controller_test.go b/pkg/resourcequota/resource_quota_controller_test.go index 8d03ab44c34f..2f112ccbf02c 100644 --- a/pkg/resourcequota/resource_quota_controller_test.go +++ b/pkg/resourcequota/resource_quota_controller_test.go @@ -267,3 +267,63 @@ func TestSyncResourceQuotaNoChange(t *testing.T) { t.Errorf("SyncResourceQuota made an unexpected client action when state was not dirty: %v", kubeClient.Actions) } } + +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")}}, + }, + } + 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") + } +} + +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")}}, + }, + } + 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") + } +} diff --git a/plugin/pkg/admission/resourcequota/admission.go b/plugin/pkg/admission/resourcequota/admission.go index 092832e15a77..3a9469c41068 100644 --- a/plugin/pkg/admission/resourcequota/admission.go +++ b/plugin/pkg/admission/resourcequota/admission.go @@ -169,6 +169,9 @@ func IncrementUsage(a admission.Attributes, status *api.ResourceQuotaStatus, cli hardMem, hardMemFound := status.Hard[api.ResourceMemory] if hardMemFound { + if set[api.ResourceMemory] && resourcequota.IsPodMemoryUnbounded(pod) { + return false, fmt.Errorf("Limited to %s memory, but pod has no specified memory limit", hardMem.String()) + } used, usedFound := status.Used[api.ResourceMemory] if !usedFound { return false, fmt.Errorf("Quota usage stats are not yet known, unable to admit resource until an accurate count is completed.") @@ -182,6 +185,9 @@ func IncrementUsage(a admission.Attributes, status *api.ResourceQuotaStatus, cli } hardCPU, hardCPUFound := status.Hard[api.ResourceCPU] if hardCPUFound { + if set[api.ResourceCPU] && resourcequota.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.") diff --git a/plugin/pkg/admission/resourcequota/admission_test.go b/plugin/pkg/admission/resourcequota/admission_test.go index 2b8e9e881c0f..31fc71619369 100644 --- a/plugin/pkg/admission/resourcequota/admission_test.go +++ b/plugin/pkg/admission/resourcequota/admission_test.go @@ -195,6 +195,72 @@ func TestIncrementUsageCPU(t *testing.T) { } } +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, "pods", "CREATE"), 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, "pods", "CREATE"), status, client) + if err == nil { + t.Errorf("Expected memory unbounded usage error") + } +} + func TestExceedUsageCPU(t *testing.T) { namespace := "default" client := testclient.NewSimpleFake(&api.PodList{