Skip to content

Commit

Permalink
build nodeselector support with defaults and overrides
Browse files Browse the repository at this point in the history
  • Loading branch information
bparees committed Oct 10, 2016
1 parent 606716e commit 000a45e
Show file tree
Hide file tree
Showing 31 changed files with 577 additions and 269 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 10 additions & 2 deletions api/swagger-spec/oapi-v1.json
Original file line number Diff line number Diff line change
Expand Up @@ -21383,7 +21383,7 @@
},
"v1.BuildConfig": {
"id": "v1.BuildConfig",
"description": "Build configurations define a build process for new Docker images. There are three types of builds possible - a Docker build using a Dockerfile, a Source-to-Image build that uses a specially prepared base image that accepts source code that it can make runnable, and a custom build that can run // arbitrary Docker images as a base and accept the build parameters. Builds run on the cluster and on completion are pushed to the Docker registry specified in the \"output\" section. A build can be triggered via a webhook, when the base image changes, or when a user manually requests a new build be // created.\n\nEach build created by a build configuration is numbered and refers back to its parent configuration. Multiple builds can be triggered at once. Builds that do not have \"output\" set can be used to test code or run a verification build.",
"description": "BuildConfig is a template which can be used to create new builds.",
"required": [
"spec",
"status"
Expand Down Expand Up @@ -21462,6 +21462,10 @@
"type": "integer",
"format": "int64",
"description": "completionDeadlineSeconds is an optional duration in seconds, counted from the time when a build pod gets scheduled in the system, that the build may be active on a node before the system actively tries to terminate the build; value must be positive integer"
},
"nodeSelector": {
"type": "object",
"description": "nodeSelector is a selector which must be true for the build pod to fit on a node"
}
}
},
Expand Down Expand Up @@ -21640,7 +21644,7 @@
},
"v1.ImageSource": {
"id": "v1.ImageSource",
"description": "ImageSource is used to describe build source that will be extracted from an image. A reference of type ImageStreamTag, ImageStreamImage or DockerImage may be used. A pull secret can be specified to pull the image from an external registry or override the default service account secret if pulling from the internal registry. A list of paths to copy from the image and their respective destination within the build directory must be specified in the paths array.",
"description": "ImageSource describes an image that is used as source for the build",
"required": [
"from",
"paths"
Expand Down Expand Up @@ -22489,6 +22493,10 @@
"format": "int64",
"description": "completionDeadlineSeconds is an optional duration in seconds, counted from the time when a build pod gets scheduled in the system, that the build may be active on a node before the system actively tries to terminate the build; value must be positive integer"
},
"nodeSelector": {
"type": "object",
"description": "nodeSelector is a selector which must be true for the build pod to fit on a node"
},
"triggeredBy": {
"type": "array",
"items": {
Expand Down
12 changes: 10 additions & 2 deletions api/swagger-spec/openshift-openapi-spec.json
Original file line number Diff line number Diff line change
Expand Up @@ -44617,7 +44617,7 @@
}
},
"v1.BuildConfig": {
"description": "Build configurations define a build process for new Docker images. There are three types of builds possible - a Docker build using a Dockerfile, a Source-to-Image build that uses a specially prepared base image that accepts source code that it can make runnable, and a custom build that can run // arbitrary Docker images as a base and accept the build parameters. Builds run on the cluster and on completion are pushed to the Docker registry specified in the \"output\" section. A build can be triggered via a webhook, when the base image changes, or when a user manually requests a new build be // created.\n\nEach build created by a build configuration is numbered and refers back to its parent configuration. Multiple builds can be triggered at once. Builds that do not have \"output\" set can be used to test code or run a verification build.",
"description": "BuildConfig is a template which can be used to create new builds.",
"required": [
"spec",
"status"
Expand Down Expand Up @@ -44680,6 +44680,10 @@
"type": "integer",
"format": "int64"
},
"nodeSelector": {
"description": "nodeSelector is a selector which must be true for the build pod to fit on a node",
"type": "object"
},
"output": {
"$ref": "#/definitions/v1.BuildOutput"
},
Expand Down Expand Up @@ -44906,6 +44910,10 @@
"type": "integer",
"format": "int64"
},
"nodeSelector": {
"description": "nodeSelector is a selector which must be true for the build pod to fit on a node",
"type": "object"
},
"output": {
"$ref": "#/definitions/v1.BuildOutput"
},
Expand Down Expand Up @@ -47845,7 +47853,7 @@
}
},
"v1.ImageSource": {
"description": "ImageSource is used to describe build source that will be extracted from an image. A reference of type ImageStreamTag, ImageStreamImage or DockerImage may be used. A pull secret can be specified to pull the image from an external registry or override the default service account secret if pulling from the internal registry. A list of paths to copy from the image and their respective destination within the build directory must be specified in the paths array.",
"description": "ImageSource describes an image that is used as source for the build",
"required": [
"from",
"paths"
Expand Down
24 changes: 24 additions & 0 deletions pkg/build/admission/defaults/admission.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,22 @@ func (a *buildDefaults) Admit(attributes admission.Attributes) error {

a.applyBuildDefaults(build)

pod, err := buildadmission.GetPod(attributes)
if err != nil {
return err
}

if len(a.defaultsConfig.NodeSelector) != 0 && pod.Spec.NodeSelector == nil {
pod.Spec.NodeSelector = map[string]string{}
}
for k, v := range a.defaultsConfig.NodeSelector {
if addDefaultNodeSelector(k, v, pod.Spec.NodeSelector) {
glog.V(5).Infof("Added default nodeselector %s=%s to build pod %s/%s", k, v, pod.Namespace, pod.Name)
} else {
glog.V(5).Infof("Not defaulting pre-existing nodeselector %s on build pod %s/%s", k, pod.Namespace, pod.Name)
}
}

err = buildadmission.SetBuildLogLevel(attributes, build)
if err != nil {
return err
Expand Down Expand Up @@ -153,3 +169,11 @@ func addDefaultEnvVar(v kapi.EnvVar, envVars *[]kapi.EnvVar) {
*envVars = append(*envVars, v)
}
}

func addDefaultNodeSelector(k, v string, selectors map[string]string) bool {
if _, ok := selectors[k]; !ok {
selectors[k] = v
return true
}
return false
}
3 changes: 3 additions & 0 deletions pkg/build/admission/defaults/api/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ type BuildDefaultsConfig struct {
// SourceStrategyDefaults are default values that apply to builds using the
// source strategy.
SourceStrategyDefaults *SourceStrategyDefaultsConfig

// NodeSelector is a selector which must be true for the build pod to fit on a node
NodeSelector map[string]string
}

// SourceStrategyDefaultsConfig contains values that apply to builds using the
Expand Down
13 changes: 7 additions & 6 deletions pkg/build/admission/defaults/api/v1/swagger_doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@ package v1

var map_BuildDefaultsConfig = map[string]string{
"": "BuildDefaultsConfig controls the default information for Builds",
"gitHTTPProxy": "GitHTTPProxy is the location of the HTTPProxy for Git source",
"gitHTTPSProxy": "GitHTTPSProxy is the location of the HTTPSProxy for Git source",
"gitNoProxy": "GitNoProxy is the list of domains for which the proxy should not be used",
"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.",
"gitHTTPProxy": "gitHTTPProxy is the location of the HTTPProxy for Git source",
"gitHTTPSProxy": "gitHTTPSProxy is the location of the HTTPSProxy for Git source",
"gitNoProxy": "gitNoProxy is the list of domains for which the proxy should not be used",
"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.",
"nodeSelector": "nodeSelector is a selector which must be true for the build pod to fit on a node",
}

func (BuildDefaultsConfig) SwaggerDoc() map[string]string {
Expand All @@ -20,7 +21,7 @@ func (BuildDefaultsConfig) SwaggerDoc() map[string]string {

var map_SourceStrategyDefaultsConfig = map[string]string{
"": "SourceStrategyDefaultsConfig contains values that apply to builds using the source strategy.",
"incremental": "Incremental indicates if s2i build strategies should perform an incremental build or not",
"incremental": "incremental indicates if s2i build strategies should perform an incremental build or not",
}

func (SourceStrategyDefaultsConfig) SwaggerDoc() map[string]string {
Expand Down
15 changes: 9 additions & 6 deletions pkg/build/admission/defaults/api/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,32 @@ import (
type BuildDefaultsConfig struct {
unversioned.TypeMeta `json:",inline"`

// GitHTTPProxy is the location of the HTTPProxy for Git source
// gitHTTPProxy is the location of the HTTPProxy for Git source
GitHTTPProxy string `json:"gitHTTPProxy,omitempty"`

// GitHTTPSProxy is the location of the HTTPSProxy for Git source
// gitHTTPSProxy is the location of the HTTPSProxy for Git source
GitHTTPSProxy string `json:"gitHTTPSProxy,omitempty"`

// GitNoProxy is the list of domains for which the proxy should not be used
// gitNoProxy is the list of domains for which the proxy should not be used
GitNoProxy string `json:"gitNoProxy,omitempty"`

// Env is a set of default environment variables that will be applied to the
// 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
Env []kapi.EnvVar `json:"env,omitempty"`

// SourceStrategyDefaults are default values that apply to builds using the
// sourceStrategyDefaults are default values that apply to builds using the
// source strategy.
SourceStrategyDefaults *SourceStrategyDefaultsConfig `json:"sourceStrategyDefaults,omitempty"`

// nodeSelector is a selector which must be true for the build pod to fit on a node
NodeSelector map[string]string `json:"nodeSelector,omitempty"`
}

// SourceStrategyDefaultsConfig contains values that apply to builds using the
// source strategy.
type SourceStrategyDefaultsConfig struct {

// Incremental indicates if s2i build strategies should perform an incremental
// incremental indicates if s2i build strategies should perform an incremental
// build or not
Incremental *bool `json:"incremental,omitempty"`
}
18 changes: 16 additions & 2 deletions pkg/build/admission/overrides/admission.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,11 @@ type buildOverrides struct {
// NewBuildOverrides returns an admission control for builds that overrides
// settings on builds
func NewBuildOverrides(overridesConfig *overridesapi.BuildOverridesConfig) admission.Interface {
return &buildOverrides{
a := &buildOverrides{
Handler: admission.NewHandler(admission.Create, admission.Update),
overridesConfig: overridesConfig,
}
return a
}

// Admit appplies configured overrides to a build in a build pod
Expand All @@ -59,7 +60,7 @@ func (a *buildOverrides) Admit(attributes admission.Attributes) error {
}

func (a *buildOverrides) applyOverrides(attributes admission.Attributes) error {
if !a.overridesConfig.ForcePull {
if !a.overridesConfig.ForcePull && len(a.overridesConfig.NodeSelector) == 0 {
return nil
}
build, version, err := buildadmission.GetBuild(attributes)
Expand All @@ -83,6 +84,19 @@ func (a *buildOverrides) applyOverrides(attributes admission.Attributes) error {
glog.V(5).Infof("Setting custom strategy ForcePull to true in build %s/%s", build.Namespace, build.Name)
build.Spec.Strategy.CustomStrategy.ForcePull = true
}

pod, err := buildadmission.GetPod(attributes)
if err != nil {
return err
}
if len(a.overridesConfig.NodeSelector) != 0 && pod.Spec.NodeSelector == nil {
pod.Spec.NodeSelector = map[string]string{}
}
for k, v := range a.overridesConfig.NodeSelector {
glog.V(5).Infof("Adding override nodeselector %s=%s to build pod %s/%s", k, v, pod.Namespace, pod.Name)
pod.Spec.NodeSelector[k] = v
}

return buildadmission.SetBuild(attributes, build, version)
}

Expand Down
42 changes: 42 additions & 0 deletions pkg/build/admission/overrides/admission_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,45 @@ func TestBuildOverrideForcePull(t *testing.T) {
}
}
}

func TestBuildOverrideNodeSelector(t *testing.T) {
tests := []struct {
name string
build *buildapi.Build
overrides map[string]string
expected map[string]string
}{
{
name: "build - full override",
build: u.Build().WithNodeSelector(map[string]string{"key1": "value1"}).AsBuild(),
overrides: map[string]string{"key1": "override1", "key2": "override2"},
expected: map[string]string{"key1": "override1", "key2": "override2"},
},
{
name: "build - partial override",
build: u.Build().WithNodeSelector(map[string]string{"key1": "value1"}).AsBuild(),
overrides: map[string]string{"key2": "override2"},
expected: map[string]string{"key1": "value1", "key2": "override2"},
},
}

for _, test := range tests {
overrides := NewBuildOverrides(&overridesapi.BuildOverridesConfig{NodeSelector: test.overrides})
pod := u.Pod().WithBuild(t, test.build, "v1")
// normally the pod will have the nodeselectors from the build, due to the pod creation logic
// in the build controller flow. fake it out here.
pod.Spec.NodeSelector = test.build.Spec.NodeSelector
err := overrides.Admit(pod.ToAttributes())
if err != nil {
t.Errorf("%s: unexpected error: %v", test.name, err)
}
if len(pod.Spec.NodeSelector) != len(test.expected) {
t.Errorf("%s: incorrect number of selectors, expected %v, got %v", test.name, test.expected, pod.Spec.NodeSelector)
}
for k, v := range pod.Spec.NodeSelector {
if ev, ok := test.expected[k]; !ok || ev != v {
t.Errorf("%s: incorrect selector value for key %s, expected %s, got %s", test.name, k, ev, v)
}
}
}
}
3 changes: 3 additions & 0 deletions pkg/build/admission/overrides/api/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,7 @@ type BuildOverridesConfig struct {

// ForcePull indicates whether the build strategy should always be set to ForcePull=true
ForcePull bool

// nodeSelector is a selector which must be true for the build pod to fit on a node
NodeSelector map[string]string
}
5 changes: 3 additions & 2 deletions pkg/build/admission/overrides/api/v1/swagger_doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ package v1
// ==== DO NOT EDIT THIS FILE MANUALLY ====

var map_BuildOverridesConfig = map[string]string{
"": "BuildOverridesConfig controls override settings for builds",
"forcePull": "ForcePull indicates whether the build strategy should always be set to ForcePull=true",
"": "BuildOverridesConfig controls override settings for builds",
"forcePull": "ForcePull indicates whether the build strategy should always be set to ForcePull=true",
"nodeSelector": "nodeSelector is a selector which must be true for the build pod to fit on a node",
}

func (BuildOverridesConfig) SwaggerDoc() map[string]string {
Expand Down
3 changes: 3 additions & 0 deletions pkg/build/admission/overrides/api/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,7 @@ type BuildOverridesConfig struct {

// ForcePull indicates whether the build strategy should always be set to ForcePull=true
ForcePull bool `json:"forcePull"`

// nodeSelector is a selector which must be true for the build pod to fit on a node
NodeSelector map[string]string `json:"nodeSelector,omitempty"`
}
5 changes: 5 additions & 0 deletions pkg/build/admission/testutil/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ func (b *TestBuild) WithCustomStrategy() *TestBuild {
return b
}

func (b *TestBuild) WithNodeSelector(ns map[string]string) *TestBuild {
b.Spec.NodeSelector = ns
return b
}

func (b *TestBuild) AsBuild() *buildapi.Build {
return (*buildapi.Build)(b)
}
3 changes: 3 additions & 0 deletions pkg/build/api/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,9 @@ type CommonSpec struct {
// be active on a node before the system actively tries to terminate the
// build; value must be positive integer.
CompletionDeadlineSeconds *int64

// NodeSelector is a selector which must be true for the build pod to fit on a node
NodeSelector map[string]string
}

// BuildTriggerCause holds information about a triggered build. It is used for
Expand Down
Loading

0 comments on commit 000a45e

Please sign in to comment.