Skip to content

Commit

Permalink
Always set default memory and cpu
Browse files Browse the repository at this point in the history
  • Loading branch information
Richard87 committed Apr 23, 2024
1 parent 2067523 commit b141359
Show file tree
Hide file tree
Showing 11 changed files with 196 additions and 167 deletions.
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ CONTROLLER_GEN=$(shell which controller-gen)
endif

.PHONY: test
test:
test:
go test -cover `go list ./... | grep -v 'pkg/client'`

.PHONY: mocks
Expand Down Expand Up @@ -129,7 +129,7 @@ CUSTOM_RESOURCE_NAME=radix
CUSTOM_RESOURCE_VERSION=v1

.PHONY: code-gen
code-gen:
code-gen:
$(GOPATH)/pkg/mod/k8s.io/[email protected]/generate-groups.sh all $(ROOT_PACKAGE)/pkg/client $(ROOT_PACKAGE)/pkg/apis $(CUSTOM_RESOURCE_NAME):$(CUSTOM_RESOURCE_VERSION) --go-header-file $(GOPATH)/pkg/mod/k8s.io/[email protected]/hack/boilerplate.go.txt

.PHONY: crds
Expand Down
5 changes: 3 additions & 2 deletions pkg/apis/batch/kubejob.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
operatorUtils "github.com/equinor/radix-operator/pkg/apis/utils"
"github.com/equinor/radix-operator/pkg/apis/utils/annotations"
radixlabels "github.com/equinor/radix-operator/pkg/apis/utils/labels"
"github.com/equinor/radix-operator/pkg/apis/utils/resources"
batchv1 "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -269,10 +270,10 @@ func (s *syncer) getContainerEnvironmentVariables(rd *radixv1.RadixDeployment, j

func (s *syncer) getContainerResources(batchJob *radixv1.RadixBatchJob, jobComponent *radixv1.RadixDeployJobComponent) corev1.ResourceRequirements {
if batchJob.Resources != nil {
return operatorUtils.BuildResourceRequirement(batchJob.Resources)
return resources.New(resources.WithDefaults(), resources.WithSource(batchJob.Resources))
}

return operatorUtils.GetResourceRequirements(jobComponent)
return resources.GetResourceRequirements(jobComponent)
}

func getContainerPorts(radixJobComponent *radixv1.RadixDeployJobComponent) []corev1.ContainerPort {
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/batch/syncer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -491,8 +491,8 @@ func (s *syncerTestSuite) Test_BatchStaticConfiguration() {
s.Equal(securitycontext.Pod(securitycontext.WithPodSeccompProfile(corev1.SeccompProfileTypeRuntimeDefault)), kubejob.Spec.Template.Spec.SecurityContext)
s.Equal(imageName, kubejob.Spec.Template.Spec.Containers[0].Image)
s.Equal(securitycontext.Container(securitycontext.WithContainerSeccompProfileType(corev1.SeccompProfileTypeRuntimeDefault)), kubejob.Spec.Template.Spec.Containers[0].SecurityContext)
s.Len(kubejob.Spec.Template.Spec.Containers[0].Resources.Limits, 0)
s.Len(kubejob.Spec.Template.Spec.Containers[0].Resources.Requests, 0)
s.Len(kubejob.Spec.Template.Spec.Containers[0].Resources.Limits, 1)
s.Len(kubejob.Spec.Template.Spec.Containers[0].Resources.Requests, 1)
s.Len(kubejob.Spec.Template.Spec.Containers[0].Env, 5)
s.True(slice.Any(kubejob.Spec.Template.Spec.Containers[0].Env, func(env corev1.EnvVar) bool {
return env.Name == "VAR1" && env.ValueFrom.ConfigMapKeyRef.Key == "VAR1" && env.ValueFrom.ConfigMapKeyRef.LocalObjectReference.Name == kube.GetEnvVarsConfigMapName(componentName)
Expand Down
15 changes: 14 additions & 1 deletion pkg/apis/deployment/jobschedulercomponent.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"os"

"github.com/equinor/radix-common/utils/pointers"
"github.com/equinor/radix-operator/pkg/apis/defaults"
v1 "github.com/equinor/radix-operator/pkg/apis/radix/v1"
)
Expand Down Expand Up @@ -59,7 +60,19 @@ func (js *jobSchedulerComponent) GetMonitoring() bool {
}

func (js *jobSchedulerComponent) GetResources() *v1.ResourceRequirements {
return &v1.ResourceRequirements{}
return &v1.ResourceRequirements{
Limits: map[string]string{
"memory": "500M",
},
Requests: map[string]string{
"cpu": "20m",
"memory": "500M",
},
}
}

func (js *jobSchedulerComponent) GetReadOnlyFileSystem() *bool {
return pointers.Ptr(true)
}

func (js *jobSchedulerComponent) IsAlwaysPullImageOnDeploy() bool {
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/deployment/kubedeployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ import (
"github.com/equinor/radix-operator/pkg/apis/defaults"
"github.com/equinor/radix-operator/pkg/apis/kube"
v1 "github.com/equinor/radix-operator/pkg/apis/radix/v1"
"github.com/equinor/radix-operator/pkg/apis/resources"
"github.com/equinor/radix-operator/pkg/apis/securitycontext"
"github.com/equinor/radix-operator/pkg/apis/utils"
radixannotations "github.com/equinor/radix-operator/pkg/apis/utils/annotations"
radixlabels "github.com/equinor/radix-operator/pkg/apis/utils/labels"
"github.com/equinor/radix-operator/pkg/apis/utils/resources"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
k8sErrors "k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -270,7 +270,7 @@ func (deploy *Deployment) setDesiredDeploymentProperties(deployComponent v1.Radi
desiredDeployment.Spec.Template.Spec.Containers[0].Ports = getContainerPorts(deployComponent)
desiredDeployment.Spec.Template.Spec.Containers[0].ImagePullPolicy = corev1.PullAlways
desiredDeployment.Spec.Template.Spec.Containers[0].SecurityContext = containerSecurityCtx
desiredDeployment.Spec.Template.Spec.Containers[0].Resources = utils.GetResourceRequirements(deployComponent)
desiredDeployment.Spec.Template.Spec.Containers[0].Resources = resources.GetResourceRequirements(deployComponent)

volumeMounts, err := GetRadixDeployComponentVolumeMounts(deployComponent, deploy.radixDeployment.GetName())
if err != nil {
Expand Down
52 changes: 27 additions & 25 deletions pkg/apis/deployment/kubedeployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ import (
"os"
"testing"

"github.com/equinor/radix-common/utils/pointers"
"github.com/equinor/radix-operator/pkg/apis/defaults"
v1 "github.com/equinor/radix-operator/pkg/apis/radix/v1"
"github.com/equinor/radix-operator/pkg/apis/test"
"github.com/equinor/radix-operator/pkg/apis/utils"
"github.com/equinor/radix-operator/pkg/apis/utils/resources"
"github.com/stretchr/testify/assert"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -34,7 +36,7 @@ func TestGetResourceRequirements_BothProvided_BothReturned(t *testing.T) {
component := utils.NewDeployComponentBuilder().
WithResource(request, limit).
BuildComponent()
requirements := utils.GetResourceRequirements(&component)
requirements := resources.GetResourceRequirements(&component)

assert.Equal(t, 0, requirements.Requests.Cpu().Cmp(resource.MustParse("0.1")), "CPU request should be included")
assert.Equal(t, 0, requirements.Requests.Memory().Cmp(resource.MustParse("32Mi")), "Memory request should be included")
Expand All @@ -54,13 +56,13 @@ func TestGetResourceRequirements_ProvideRequests_OnlyRequestsReturned(t *testing
component := utils.NewDeployComponentBuilder().
WithResourceRequestsOnly(request).
BuildComponent()
requirements := utils.GetResourceRequirements(&component)
requirements := resources.GetResourceRequirements(&component)

assert.Equal(t, 0, requirements.Requests.Cpu().Cmp(resource.MustParse("0.2")), "CPU request should be included")
assert.Equal(t, 0, requirements.Requests.Memory().Cmp(resource.MustParse("128Mi")), "Memory request should be included")

assert.Equal(t, 0, requirements.Limits.Cpu().Cmp(resource.MustParse("0")), "Missing CPU limit should be 0")
assert.Equal(t, 0, requirements.Limits.Memory().Cmp(resource.MustParse("0")), "Missing memory limit should be 0")
assert.Equal(t, 0, requirements.Limits.Memory().Cmp(resource.MustParse("300M")), "Missing memory limit should default to 300M")
}

func TestGetResourceRequirements_ProvideRequestsCpu_OnlyRequestsCpuReturned(t *testing.T) {
Expand All @@ -73,13 +75,13 @@ func TestGetResourceRequirements_ProvideRequestsCpu_OnlyRequestsCpuReturned(t *t
component := utils.NewDeployComponentBuilder().
WithResourceRequestsOnly(request).
BuildComponent()
requirements := utils.GetResourceRequirements(&component)
requirements := resources.GetResourceRequirements(&component)

assert.Equal(t, 0, requirements.Requests.Cpu().Cmp(resource.MustParse("0.3")), "CPU request should be included")
assert.Equal(t, 0, requirements.Requests.Memory().Cmp(resource.MustParse("0")), "Missing memory request should be 0")
assert.Equal(t, "300m", requirements.Requests.Cpu().String(), "CPU request should be included")
assert.Equal(t, "300M", requirements.Requests.Memory().String(), "Missing memory request should be 0")

assert.Equal(t, 0, requirements.Limits.Cpu().Cmp(resource.MustParse("0")), "Missing CPU limit should be 0")
assert.Equal(t, 0, requirements.Limits.Memory().Cmp(resource.MustParse("0")), "Missing memory limit should be 0")
assert.Equal(t, "0", requirements.Limits.Cpu().String(), "Missing CPU limit should be 0")
assert.Equal(t, "300M", requirements.Limits.Memory().String(), "Missing memory limit should be default")
}

func TestGetResourceRequirements_BothProvided_OverDefaultLimits(t *testing.T) {
Expand All @@ -93,7 +95,7 @@ func TestGetResourceRequirements_BothProvided_OverDefaultLimits(t *testing.T) {
component := utils.NewDeployComponentBuilder().
WithResourceRequestsOnly(request).
BuildComponent()
requirements := utils.GetResourceRequirements(&component)
requirements := resources.GetResourceRequirements(&component)

assert.Equal(t, 0, requirements.Requests.Cpu().Cmp(resource.MustParse("5")), "CPU request should be included")
assert.Equal(t, 0, requirements.Requests.Memory().Cmp(resource.MustParse("5Gi")), "Memory request should be included")
Expand All @@ -111,13 +113,13 @@ func TestGetResourceRequirements_ProvideRequestsCpu_OverDefaultLimits(t *testing
component := utils.NewDeployComponentBuilder().
WithResourceRequestsOnly(request).
BuildComponent()
requirements := utils.GetResourceRequirements(&component)
requirements := resources.GetResourceRequirements(&component)

assert.Equal(t, 0, requirements.Requests.Cpu().Cmp(resource.MustParse("6")), "CPU request should be included")
assert.Equal(t, 0, requirements.Requests.Memory().Cmp(resource.MustParse("0")), "Missing memory request should be 0")
assert.Equal(t, "6", requirements.Requests.Cpu().String(), "CPU request should be included")
assert.Equal(t, "300M", requirements.Requests.Memory().String(), "Missing memory request should be default")

assert.True(t, requirements.Limits.Cpu().IsZero())
assert.Equal(t, 0, requirements.Limits.Memory().Cmp(resource.MustParse("0")), "Missing memory limit should be 0")
assert.True(t, requirements.Limits.Cpu().IsZero(), "Missing CPU limit should be Zero")
assert.Equal(t, "300M", requirements.Limits.Memory().String(), "Missing memory limit should be default")
}

func TestGetReadinessProbe_MissingDefaultEnvVars(t *testing.T) {
Expand Down Expand Up @@ -260,20 +262,20 @@ func Test_UpdateResourcesInDeployment(t *testing.T) {
desiredDeployment, _ := deployment.getDesiredCreatedDeploymentConfig(&component)

desiredRes := desiredDeployment.Spec.Template.Spec.Containers[0].Resources
assert.Equal(t, 0, len(desiredRes.Requests))
assert.Equal(t, 0, len(desiredRes.Limits))
assert.Equal(t, 1, len(desiredRes.Requests), "Should have default values")
assert.Equal(t, 1, len(desiredRes.Limits), "Should have default values")
})
t.Run("update requests and remove limit", func(t *testing.T) {
deployment := applyDeploymentWithSyncWithComponentResources(t, origRequests, origLimits)

expectedRequests := map[string]string{"cpu": "20mi", "memory": "200M"}
expectedRequests := map[string]string{"cpu": "20m", "memory": "200M"}
component := utils.NewDeployComponentBuilder().WithName("comp1").WithResource(expectedRequests, nil).BuildComponent()
desiredDeployment, _ := deployment.getDesiredCreatedDeploymentConfig(&component)

desiredRes := desiredDeployment.Spec.Template.Spec.Containers[0].Resources
assert.Equal(t, parseQuantity(expectedRequests["cpu"]), desiredRes.Requests["cpu"])
assert.Equal(t, parseQuantity(expectedRequests["memory"]), desiredRes.Requests["memory"])
assert.Equal(t, 0, len(desiredRes.Limits))
assert.Equal(t, expectedRequests["cpu"], pointers.Ptr(desiredRes.Requests["cpu"]).String())
assert.Equal(t, expectedRequests["memory"], pointers.Ptr(desiredRes.Requests["memory"]).String())
assert.Equal(t, 1, len(desiredRes.Limits))
})
t.Run("remove requests and update limit", func(t *testing.T) {
deployment := applyDeploymentWithSyncWithComponentResources(t, origRequests, origLimits)
Expand All @@ -284,7 +286,7 @@ func Test_UpdateResourcesInDeployment(t *testing.T) {
desiredDeployment, _ := deployment.getDesiredCreatedDeploymentConfig(&component)

desiredRes := desiredDeployment.Spec.Template.Spec.Containers[0].Resources
assert.Equal(t, 0, len(desiredRes.Requests))
assert.Equal(t, 1, len(desiredRes.Requests))
assert.Equal(t, parseQuantity(expectedLimits["cpu"]), desiredRes.Limits["cpu"])
assert.Equal(t, parseQuantity(expectedLimits["memory"]), desiredRes.Limits["memory"])
})
Expand All @@ -311,8 +313,8 @@ func TestDeployment_createJobAuxDeployment(t *testing.T) {
assert.Equal(t, "job1-aux", jobAuxDeployment.GetName())
resources := jobAuxDeployment.Spec.Template.Spec.Containers[0].Resources
s := resources.Requests.Cpu().String()
assert.Equal(t, "50m", s)
assert.Equal(t, "50M", resources.Requests.Memory().String())
assert.Equal(t, "50m", resources.Limits.Cpu().String())
assert.Equal(t, "50M", resources.Limits.Memory().String())
assert.Equal(t, "1m", s)
assert.Equal(t, "10M", resources.Requests.Memory().String())
assert.Equal(t, "0", resources.Limits.Cpu().String())
assert.Equal(t, "10M", resources.Limits.Memory().String())
}
2 changes: 1 addition & 1 deletion pkg/apis/deployment/oauthproxyresourcemanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ import (
"github.com/equinor/radix-operator/pkg/apis/ingress"
"github.com/equinor/radix-operator/pkg/apis/kube"
v1 "github.com/equinor/radix-operator/pkg/apis/radix/v1"
"github.com/equinor/radix-operator/pkg/apis/resources"
"github.com/equinor/radix-operator/pkg/apis/securitycontext"
"github.com/equinor/radix-operator/pkg/apis/utils"
radixlabels "github.com/equinor/radix-operator/pkg/apis/utils/labels"
oauthutil "github.com/equinor/radix-operator/pkg/apis/utils/oauth"
"github.com/equinor/radix-operator/pkg/apis/utils/resources"
"github.com/rs/zerolog"
"github.com/rs/zerolog/log"
appsv1 "k8s.io/api/apps/v1"
Expand Down
6 changes: 3 additions & 3 deletions pkg/apis/metrics/custom_metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ package metrics
import (
"time"

"github.com/equinor/radix-operator/pkg/apis/utils"
resourceutils "github.com/equinor/radix-operator/pkg/apis/utils/resources"
"k8s.io/apimachinery/pkg/api/resource"

"github.com/equinor/radix-operator/pkg/apis/defaults"
v1 "github.com/equinor/radix-operator/pkg/apis/radix/v1"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
)

var (
Expand Down Expand Up @@ -72,7 +72,7 @@ func RequestedResources(rr *v1.RadixRegistration, rd *v1.RadixDeployment) {
defaultMemory := defaults.GetDefaultMemoryRequest()

for _, comp := range rd.Spec.Components {
resources := utils.GetResourceRequirements(&comp)
resources := resourceutils.GetResourceRequirements(&comp)
nrReplicas := float64(comp.GetNrOfReplicas())
var cpu, memory resource.Quantity

Expand Down
65 changes: 0 additions & 65 deletions pkg/apis/resources/resources.go

This file was deleted.

Loading

0 comments on commit b141359

Please sign in to comment.