Skip to content

Commit

Permalink
Merge pull request kubernetes#29356 from smarterclayton/init_containers
Browse files Browse the repository at this point in the history
Automatic merge from submit-queue

LimitRanger and PodSecurityPolicy need to check more on init containers

Container limits not applied to init containers. HostPorts not checked on podsecuritypolicy

@pweil- @derekwaynecarr
  • Loading branch information
k8s-merge-robot authored Jul 27, 2016
2 parents 78f7b32 + 522930b commit 3301f6d
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 52 deletions.
6 changes: 6 additions & 0 deletions pkg/security/podsecuritypolicy/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,12 @@ func (s *simpleProvider) ValidateContainerSecurityContext(pod *api.Pod, containe
allErrs = append(allErrs, s.hasInvalidHostPort(&c, idxPath)...)
}

containersPath = fldPath.Child("initContainers")
for idx, c := range pod.Spec.InitContainers {
idxPath := containersPath.Index(idx)
allErrs = append(allErrs, s.hasInvalidHostPort(&c, idxPath)...)
}

if !s.psp.Spec.HostPID && pod.Spec.SecurityContext.HostPID {
allErrs = append(allErrs, field.Invalid(fldPath.Child("hostPID"), pod.Spec.SecurityContext.HostPID, "Host PID is not allowed to be used"))
}
Expand Down
97 changes: 53 additions & 44 deletions plugin/pkg/admission/initialresources/admission.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package initialresources

import (
"flag"
"fmt"
"io"
"sort"
"strings"
Expand Down Expand Up @@ -89,51 +90,12 @@ func (ir initialResources) Admit(a admission.Attributes) (err error) {
// The method veryfies whether resources should be set for the given pod and
// if there is estimation available the method fills Request field.
func (ir initialResources) estimateAndFillResourcesIfNotSet(pod *api.Pod) {
annotations := []string{}
var annotations []string
for i := range pod.Spec.InitContainers {
annotations = append(annotations, ir.estimateContainer(pod, &pod.Spec.InitContainers[i], "init container")...)
}
for i := range pod.Spec.Containers {
c := &pod.Spec.Containers[i]
req := c.Resources.Requests
lim := c.Resources.Limits
var cpu, mem *resource.Quantity
var err error
if _, ok := req[api.ResourceCPU]; !ok {
if _, ok2 := lim[api.ResourceCPU]; !ok2 {
cpu, err = ir.getEstimation(api.ResourceCPU, c, pod.ObjectMeta.Namespace)
if err != nil {
glog.Errorf("Error while trying to estimate resources: %v", err)
}
}
}
if _, ok := req[api.ResourceMemory]; !ok {
if _, ok2 := lim[api.ResourceMemory]; !ok2 {
mem, err = ir.getEstimation(api.ResourceMemory, c, pod.ObjectMeta.Namespace)
if err != nil {
glog.Errorf("Error while trying to estimate resources: %v", err)
}
}
}

// If Requests doesn't exits and an estimation was made, create Requests.
if req == nil && (cpu != nil || mem != nil) {
c.Resources.Requests = api.ResourceList{}
req = c.Resources.Requests
}
setRes := []string{}
if cpu != nil {
glog.Infof("CPU estimation for container %v in pod %v/%v is %v", c.Name, pod.ObjectMeta.Namespace, pod.ObjectMeta.Name, cpu.String())
setRes = append(setRes, string(api.ResourceCPU))
req[api.ResourceCPU] = *cpu
}
if mem != nil {
glog.Infof("Memory estimation for container %v in pod %v/%v is %v", c.Name, pod.ObjectMeta.Namespace, pod.ObjectMeta.Name, mem.String())
setRes = append(setRes, string(api.ResourceMemory))
req[api.ResourceMemory] = *mem
}
if len(setRes) > 0 {
sort.Strings(setRes)
a := strings.Join(setRes, ", ") + " request for container " + c.Name
annotations = append(annotations, a)
}
annotations = append(annotations, ir.estimateContainer(pod, &pod.Spec.Containers[i], "container")...)
}
if len(annotations) > 0 {
if pod.ObjectMeta.Annotations == nil {
Expand All @@ -144,6 +106,53 @@ func (ir initialResources) estimateAndFillResourcesIfNotSet(pod *api.Pod) {
}
}

func (ir initialResources) estimateContainer(pod *api.Pod, c *api.Container, message string) []string {
var annotations []string
req := c.Resources.Requests
lim := c.Resources.Limits
var cpu, mem *resource.Quantity
var err error
if _, ok := req[api.ResourceCPU]; !ok {
if _, ok2 := lim[api.ResourceCPU]; !ok2 {
cpu, err = ir.getEstimation(api.ResourceCPU, c, pod.ObjectMeta.Namespace)
if err != nil {
glog.Errorf("Error while trying to estimate resources: %v", err)
}
}
}
if _, ok := req[api.ResourceMemory]; !ok {
if _, ok2 := lim[api.ResourceMemory]; !ok2 {
mem, err = ir.getEstimation(api.ResourceMemory, c, pod.ObjectMeta.Namespace)
if err != nil {
glog.Errorf("Error while trying to estimate resources: %v", err)
}
}
}

// If Requests doesn't exits and an estimation was made, create Requests.
if req == nil && (cpu != nil || mem != nil) {
c.Resources.Requests = api.ResourceList{}
req = c.Resources.Requests
}
setRes := []string{}
if cpu != nil {
glog.Infof("CPU estimation for %s %v in pod %v/%v is %v", message, c.Name, pod.ObjectMeta.Namespace, pod.ObjectMeta.Name, cpu.String())
setRes = append(setRes, string(api.ResourceCPU))
req[api.ResourceCPU] = *cpu
}
if mem != nil {
glog.Infof("Memory estimation for %s %v in pod %v/%v is %v", message, c.Name, pod.ObjectMeta.Namespace, pod.ObjectMeta.Name, mem.String())
setRes = append(setRes, string(api.ResourceMemory))
req[api.ResourceMemory] = *mem
}
if len(setRes) > 0 {
sort.Strings(setRes)
a := strings.Join(setRes, ", ") + fmt.Sprintf(" request for %s %s", message, c.Name)
annotations = append(annotations, a)
}
return annotations
}

func (ir initialResources) getEstimation(kind api.ResourceName, c *api.Container, ns string) (*resource.Quantity, error) {
end := time.Now()
start := end.Add(-week)
Expand Down
16 changes: 10 additions & 6 deletions plugin/pkg/admission/initialresources/admission_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ func createPod(name string, image string, request api.ResourceList) *api.Pod {
Spec: api.PodSpec{},
}
pod.Spec.Containers = []api.Container{}
addContainer(pod, "i0", image, request)
pod.Spec.InitContainers = pod.Spec.Containers
pod.Spec.Containers = []api.Container{}
addContainer(pod, "c0", image, request)
return pod
}
Expand All @@ -86,6 +89,7 @@ func verifyContainer(t *testing.T, c *api.Container, cpu, mem int64) {

func verifyPod(t *testing.T, pod *api.Pod, cpu, mem int64) {
verifyContainer(t, &pod.Spec.Containers[0], cpu, mem)
verifyContainer(t, &pod.Spec.InitContainers[0], cpu, mem)
}

func verifyAnnotation(t *testing.T, pod *api.Pod, expected string) {
Expand All @@ -94,13 +98,13 @@ func verifyAnnotation(t *testing.T, pod *api.Pod, expected string) {
t.Errorf("No annotation but expected %v", expected)
}
if a != expected {
t.Errorf("Wrong annatation set by Initial Resources: got %v, expected %v", a, expected)
t.Errorf("Wrong annotation set by Initial Resources: got %v, expected %v", a, expected)
}
}

func expectNoAnnotation(t *testing.T, pod *api.Pod) {
if a, ok := pod.ObjectMeta.Annotations[initialResourcesAnnotation]; ok {
t.Errorf("Expected no annatation but got %v", a)
t.Errorf("Expected no annotation but got %v", a)
}
}

Expand All @@ -122,9 +126,9 @@ func performTest(t *testing.T, ir admission.Interface) {
verifyPod(t, pods[2], 300, 100)
verifyPod(t, pods[3], 300, 300)

verifyAnnotation(t, pods[0], "Initial Resources plugin set: cpu, memory request for container c0")
verifyAnnotation(t, pods[1], "Initial Resources plugin set: cpu request for container c0")
verifyAnnotation(t, pods[2], "Initial Resources plugin set: memory request for container c0")
verifyAnnotation(t, pods[0], "Initial Resources plugin set: cpu, memory request for init container i0; cpu, memory request for container c0")
verifyAnnotation(t, pods[1], "Initial Resources plugin set: cpu request for init container i0")
verifyAnnotation(t, pods[2], "Initial Resources plugin set: memory request for init container i0")
expectNoAnnotation(t, pods[3])
}

Expand Down Expand Up @@ -229,7 +233,7 @@ func TestManyContainers(t *testing.T) {
verifyContainer(t, &pod.Spec.Containers[2], 300, 100)
verifyContainer(t, &pod.Spec.Containers[3], 300, 300)

verifyAnnotation(t, pod, "Initial Resources plugin set: cpu, memory request for container c0; cpu request for container c1; memory request for container c2")
verifyAnnotation(t, pod, "Initial Resources plugin set: cpu, memory request for init container i0; cpu, memory request for container c0; cpu request for container c1; memory request for container c2")
}

func TestNamespaceAware(t *testing.T) {
Expand Down
18 changes: 18 additions & 0 deletions plugin/pkg/admission/limitranger/admission.go
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,24 @@ func PodLimitFunc(limitRange *api.LimitRange, pod *api.Pod) error {
}
}
}
for j := range pod.Spec.InitContainers {
container := &pod.Spec.InitContainers[j]
for k, v := range limit.Min {
if err := minConstraint(limitType, k, v, container.Resources.Requests, container.Resources.Limits); err != nil {
errs = append(errs, err)
}
}
for k, v := range limit.Max {
if err := maxConstraint(limitType, k, v, container.Resources.Requests, container.Resources.Limits); err != nil {
errs = append(errs, err)
}
}
for k, v := range limit.MaxLimitRequestRatio {
if err := limitRequestRatioConstraint(limitType, k, v, container.Resources.Requests, container.Resources.Limits); err != nil {
errs = append(errs, err)
}
}
}
}

// enforce pod limits on init containers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -394,8 +394,11 @@ func TestAdmitHostPorts(t *testing.T) {
},
}

for k, v := range tests {
testPSPAdmit(k, v.psps, v.pod, v.shouldPass, v.expectedPSP, t)
for i := 0; i < 2; i++ {
for k, v := range tests {
v.pod.Spec.Containers, v.pod.Spec.InitContainers = v.pod.Spec.InitContainers, v.pod.Spec.Containers
testPSPAdmit(k, v.psps, v.pod, v.shouldPass, v.expectedPSP, t)
}
}
}

Expand Down

0 comments on commit 3301f6d

Please sign in to comment.