play kube: set defaults to container resources

this fixes #13115 the change tries to immitate k8s behavior. when limits are not set the container's limits are all CPU and all RAM when requests are missing then they are equal to limits Signed-off-by: Yaron Dayagi <ydayagi@redhat.com>
containers · Feb 7, 2022 · 5b2de01 · 5b2de01
1 parent ab4af50
commit 5b2de01
Show file tree

Hide file tree

Showing 2 changed files with 174 additions and 0 deletions.
diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go
@@ -8,6 +8,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"runtime"
 	"strconv"
 	"strings"
 
@@ -25,12 +26,14 @@ import (
 	"github.com/containers/podman/v4/pkg/specgen/generate/kube"
 	"github.com/containers/podman/v4/pkg/specgenutil"
 	"github.com/containers/podman/v4/pkg/util"
+	"github.com/containers/storage/pkg/system"
 	"github.com/ghodss/yaml"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	yamlv3 "gopkg.in/yaml.v3"
 	v1apps "k8s.io/api/apps/v1"
 	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
 )
 
 func (ic *ContainerEngine) PlayKube(ctx context.Context, path string, options entities.PlayKubeOptions) (*entities.PlayKubeReport, error) {
@@ -183,6 +186,12 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY
 		return nil, errors.Errorf("pod does not have a name")
 	}
 
+	// Set defaults. K8S uses defaults for fields such as container resources
+	err = setPodDefaults(podYAML)
+	if err != nil {
+		return nil, err
+	}
+
 	podOpt := entities.PodCreateOptions{Infra: true, Net: &entities.NetOptions{NoHosts: options.NoHosts}}
 	podOpt, err = kube.ToPodOpt(ctx, podName, podOpt, podYAML)
 	if err != nil {
@@ -814,3 +823,59 @@ func (ic *ContainerEngine) PlayKubeDown(ctx context.Context, path string, _ enti
 	}
 	return reports, nil
 }
+
+func setPodDefaults(podYAML *v1.PodTemplateSpec) error {
+	// set container resources defaults
+	for i := range podYAML.Spec.InitContainers {
+		err := setContainerDefaults(&podYAML.Spec.InitContainers[i])
+		if err != nil {
+			return err
+		}
+	}
+	for i := range podYAML.Spec.Containers {
+		err := setContainerDefaults(&podYAML.Spec.Containers[i])
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func setContainerDefaults(container *v1.Container) error {
+	mi, err := system.ReadMemInfo()
+	if err != nil {
+		return err
+	}
+
+	limits := container.Resources.Limits
+	requests := container.Resources.Requests
+	if limits == nil {
+		limits = v1.ResourceList{}
+		container.Resources.Limits = limits
+	}
+	if requests == nil {
+		requests = v1.ResourceList{}
+		container.Resources.Requests = requests
+	}
+
+	limitCPU := limits.Cpu()
+	limitMemory := limits.Memory()
+	requestCPU := requests.Cpu()
+	requestMemory := requests.Memory()
+
+	if limitCPU == nil || limitCPU.IsZero() {
+		limits[v1.ResourceCPU] = *resource.NewQuantity(int64(runtime.NumCPU()), resource.DecimalSI)
+	}
+	if limitMemory == nil || limitMemory.IsZero() {
+		limits[v1.ResourceMemory] = *resource.NewQuantity(mi.MemTotal, resource.DecimalSI)
+	}
+	if requestCPU == nil || requestCPU.IsZero() {
+		requests[v1.ResourceCPU] = *limits.Cpu()
+	}
+	if requestMemory == nil || requestMemory.IsZero() {
+		requests[v1.ResourceMemory] = *limits.Memory()
+	}
+
+	return nil
+}
diff --git a/pkg/domain/infra/abi/play_test.go b/pkg/domain/infra/abi/play_test.go
@@ -2,10 +2,13 @@ package abi
 
 import (
 	"bytes"
+	"runtime"
 	"testing"
 
+	"github.com/docker/docker/pkg/system"
 	"github.com/stretchr/testify/assert"
 	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
 	v12 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -186,3 +189,109 @@ kind: Pod
 		})
 	}
 }
+
+func TestSetPodDefaults(t *testing.T) {
+	// Create POD object
+	podTemplateSpec := v1.PodTemplateSpec{
+		Spec: v1.PodSpec{
+			InitContainers: []v1.Container{
+				{
+					Name:  "c1-init",
+					Image: "fedora",
+				},
+			},
+			Containers: []v1.Container{
+				{
+					Name:  "c1",
+					Image: "fedora",
+				},
+			},
+		},
+	}
+
+	// Verify that pod indeed is missing what you expect
+	containers := append(podTemplateSpec.Spec.InitContainers, podTemplateSpec.Spec.Containers...)
+	for _, container := range containers {
+		assert.Nil(t, container.Resources.Limits)
+		assert.Nil(t, container.Resources.Requests)
+	}
+
+	// Set pod defaults
+	err := setPodDefaults(&podTemplateSpec)
+	assert.Nil(t, err)
+
+	// Assert results
+	containers = append(podTemplateSpec.Spec.InitContainers, podTemplateSpec.Spec.Containers...)
+	for _, container := range containers {
+		assert.NotNil(t, container.Resources.Limits)
+		assert.NotNil(t, container.Resources.Requests)
+	}
+}
+func TestSetContainerDefault(t *testing.T) {
+	mi, err := system.ReadMemInfo()
+	assert.Nil(t, err)
+
+	memTotal := *resource.NewQuantity(mi.MemTotal, resource.DecimalSI)
+	numCPUs := *resource.NewQuantity(int64(runtime.NumCPU()), resource.DecimalSI)
+
+	tests := []struct {
+		name      string
+		container v1.Container
+		// put the expected defaults here
+		requestCPU    resource.Quantity
+		requestMemory resource.Quantity
+		limitCPU      resource.Quantity
+		limitMemory   resource.Quantity
+	}{
+		{
+			name:          "NoResources",
+			container:     v1.Container{},
+			requestCPU:    numCPUs,
+			requestMemory: memTotal,
+			limitCPU:      numCPUs,
+			limitMemory:   memTotal,
+		},
+		{
+			name: "NoRequests",
+			container: v1.Container{
+				Resources: v1.ResourceRequirements{
+					Limits: v1.ResourceList{
+						v1.ResourceCPU:    resource.MustParse("4000m"),
+						v1.ResourceMemory: resource.MustParse("30Mi"),
+					},
+				},
+			},
+			requestCPU:    resource.MustParse("4000m"),
+			requestMemory: resource.MustParse("30Mi"),
+			limitCPU:      resource.MustParse("4000m"),
+			limitMemory:   resource.MustParse("30Mi"),
+		},
+		{
+			name: "NoLimits",
+			container: v1.Container{
+				Resources: v1.ResourceRequirements{
+					Requests: v1.ResourceList{
+						v1.ResourceCPU:    resource.MustParse("4000m"),
+						v1.ResourceMemory: resource.MustParse("30Mi"),
+					},
+				},
+			},
+			requestCPU:    resource.MustParse("4000m"),
+			requestMemory: resource.MustParse("30Mi"),
+			limitCPU:      numCPUs,
+			limitMemory:   memTotal,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			container := &test.container
+			err := setContainerDefaults(container)
+			assert.Nil(t, err)
+			assert.Equal(t, test.requestCPU, *container.Resources.Requests.Cpu())
+			assert.Equal(t, test.requestMemory, *container.Resources.Requests.Memory())
+			assert.Equal(t, test.limitCPU, *container.Resources.Limits.Cpu())
+			assert.Equal(t, test.limitMemory, *container.Resources.Limits.Memory())
+		})
+	}
+}