Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default resource limits for build pods #11281

Closed
wants to merge 1 commit into from
Closed

Default resource limits for build pods #11281

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions pkg/build/admission/defaults/admission.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,29 @@ func (a *buildDefaults) applyBuildDefaults(build *buildapi.Build) {
build.Spec.Source.Git.NoProxy = &t
}
}

//Apply default resources
defaultResources := a.defaultsConfig.Resources
if defaultResources != nil {
if len(build.Spec.Resources.Limits) == 0 {
build.Spec.Resources.Limits = kapi.ResourceList{}
}
for name, value := range defaultResources.Limits {
if _, ok := build.Spec.Resources.Limits[name]; !ok {
glog.V(5).Infof("Setting default resource limit %s for build %s/%s to %s", name, build.Namespace, build.Name, value)
build.Spec.Resources.Limits[name] = value
}
}
if len(build.Spec.Resources.Requests) == 0 {
build.Spec.Resources.Requests = kapi.ResourceList{}
}
for name, value := range defaultResources.Requests {
if _, ok := build.Spec.Resources.Requests[name]; !ok {
glog.V(5).Infof("Setting default resource request %s for build %s/%s to %s", name, build.Namespace, build.Name, value)
build.Spec.Resources.Requests[name] = value
}
}
}
}

func getBuildEnv(build *buildapi.Build) *[]kapi.EnvVar {
Expand Down
197 changes: 195 additions & 2 deletions pkg/build/admission/defaults/admission_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ import (
"reflect"
"testing"

kapi "k8s.io/kubernetes/pkg/api"

buildadmission "github.com/openshift/origin/pkg/build/admission"
defaultsapi "github.com/openshift/origin/pkg/build/admission/defaults/api"
u "github.com/openshift/origin/pkg/build/admission/testutil"
buildapi "github.com/openshift/origin/pkg/build/api"
kapi "k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/resource"

_ "github.com/openshift/origin/pkg/api/install"
)
Expand Down Expand Up @@ -266,3 +266,196 @@ func TestLabelDefaults(t *testing.T) {
}
}
}

func TestResourceDefaults(t *testing.T) {
tests := map[string]struct {
DefaultResource *kapi.ResourceRequirements
BuildResource kapi.ResourceRequirements
ExpectedResource kapi.ResourceRequirements
}{
"BuildDefaults plugin and Build object both defined resource limits and requests": {
DefaultResource: &kapi.ResourceRequirements{
Limits: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("10"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("1G"),
},
Requests: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("20"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("2G"),
},
},
BuildResource: kapi.ResourceRequirements{
Limits: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("30"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("3G"),
},
Requests: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("40"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("4G"),
},
},
ExpectedResource: kapi.ResourceRequirements{
Limits: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("30"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("3G"),
},
Requests: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("40"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("4G"),
},
},
},
"BuildDefaults plugin defined limits and requests, Build object defined resource requests": {
DefaultResource: &kapi.ResourceRequirements{
Limits: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("10"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("1G"),
},
Requests: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("20"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("2G"),
},
},
BuildResource: kapi.ResourceRequirements{
Requests: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("30"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("3G"),
},
},
ExpectedResource: kapi.ResourceRequirements{
Limits: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("10"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("1G"),
},
Requests: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("30"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("3G"),
},
},
},
"BuildDefaults plugin defined limits and requests, Build object defined resource limits": {
DefaultResource: &kapi.ResourceRequirements{
Limits: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("10"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("1G"),
},
Requests: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("20"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("2G"),
},
},
BuildResource: kapi.ResourceRequirements{
Limits: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("30"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("3G"),
},
},
ExpectedResource: kapi.ResourceRequirements{
Limits: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("30"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("3G"),
},
Requests: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("2G"),
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("20"),
},
},
},
"BuildDefaults plugin defined nothing, Build object defined resource limits": {
DefaultResource: &kapi.ResourceRequirements{},
BuildResource: kapi.ResourceRequirements{
Limits: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("10"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("1G"),
},
Requests: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("20"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("2G"),
},
},
ExpectedResource: kapi.ResourceRequirements{
Limits: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("10"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("1G"),
},
Requests: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("20"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("2G"),
},
},
},
"BuildDefaults plugin defined nil, Build object defined resource limits": {
DefaultResource: nil,
BuildResource: kapi.ResourceRequirements{
Limits: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("10"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("1G"),
},
Requests: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("20"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("2G"),
},
},
ExpectedResource: kapi.ResourceRequirements{
Limits: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("10"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("1G"),
},
Requests: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("20"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("2G"),
},
},
},
"BuildDefaults plugin and Build object defined nothing": {
DefaultResource: &kapi.ResourceRequirements{},
BuildResource: kapi.ResourceRequirements{},
ExpectedResource: kapi.ResourceRequirements{},
},
"BuildDefaults plugin defined limits and requests, Build object defined nothing": {
DefaultResource: &kapi.ResourceRequirements{
Limits: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("10"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("1G"),
},
Requests: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("20"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("2G"),
},
},
BuildResource: kapi.ResourceRequirements{},
ExpectedResource: kapi.ResourceRequirements{
Limits: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("10"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("1G"),
},
Requests: kapi.ResourceList{
kapi.ResourceName(kapi.ResourceCPU): resource.MustParse("20"),
kapi.ResourceName(kapi.ResourceMemory): resource.MustParse("2G"),
},
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a test where the build Resources object is empty to test your length checking/list initializing logic in the defaulter.

},
}

for name, test := range tests {
defaultsConfig := &defaultsapi.BuildDefaultsConfig{
Resources: test.DefaultResource,
}
admitter := NewBuildDefaults(defaultsConfig)

build := u.Build().WithSourceStrategy().AsBuild()
build.Spec.Resources = test.BuildResource
pod := u.Pod().WithBuild(t, build, "v1")
err := admitter.Admit(pod.ToAttributes())
if err != nil {
t.Fatalf("%v :unexpected error: %v", name, err)
}
build, _, err = buildadmission.GetBuild(pod.ToAttributes())
if err != nil {
t.Fatalf("%v :unexpected error: %v", name, err)
}
if !kapi.Semantic.DeepEqual(test.ExpectedResource, build.Spec.Resources) {
t.Fatalf("%v:Expected expected=actual, %v != %v", name, test.ExpectedResource, build.Spec.Resources)
}
}
}
3 changes: 3 additions & 0 deletions pkg/build/admission/defaults/api/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ type BuildDefaultsConfig struct {
// User can override a default label by providing a label with the same name in their
// Build/BuildConfig.
ImageLabels []buildapi.ImageLabel

// Resources computes resource requirements to execute the build.
Resources *kapi.ResourceRequirements
}

// SourceStrategyDefaultsConfig contains values that apply to builds using the
Expand Down
1 change: 1 addition & 0 deletions pkg/build/admission/defaults/api/v1/swagger_doc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ var map_BuildDefaultsConfig = map[string]string{
"env": "Env is a set of default environment variables that will be applied to the build if the specified variables do not exist on the build",
"sourceStrategyDefaults": "SourceStrategyDefaults are default values that apply to builds using the source strategy.",
"imageLabels": "ImageLabels is a list of docker labels that are applied to the resulting image. User can override a default label by providing a label with the same name in their Build/BuildConfig.",
"resources": "Resources computes resource requirements to execute the build.",
}

func (BuildDefaultsConfig) SwaggerDoc() map[string]string {
Expand Down
3 changes: 3 additions & 0 deletions pkg/build/admission/defaults/api/v1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ type BuildDefaultsConfig struct {
// User can override a default label by providing a label with the same name in their
// Build/BuildConfig.
ImageLabels []buildapi.ImageLabel `json:"imageLabels,omitempty"`

// Resources computes resource requirements to execute the build.
Resources *kapi.ResourceRequirements `json:"resources,omitempty"`
}

// SourceStrategyDefaultsConfig contains values that apply to builds using the
Expand Down