Merge pull request #7003 from derekwaynecarr/enforce_unbounded

Reject unbounded cpu and memory pods if quota is restricting it
kubernetes · Apr 22, 2015 · ee06097 · ee06097
2 parents d790cc1 + 75482ca
commit ee06097
Show file tree

Hide file tree

Showing 4 changed files with 154 additions and 0 deletions.
diff --git a/pkg/resourcequota/resource_quota_controller.go b/pkg/resourcequota/resource_quota_controller.go
@@ -240,6 +240,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
@@ -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
@@ -171,6 +171,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.")
@@ -184,6 +187,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
@@ -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{