Skip to content

Commit

Permalink
Make build pod nodeSelector, Tolerations, and Affinity configurable
Browse files Browse the repository at this point in the history
- Build pods will no longer determine tolerations

fix #621
  • Loading branch information
Tyler Phelan committed Sep 9, 2021
1 parent c29d0d6 commit 501f8b9
Show file tree
Hide file tree
Showing 24 changed files with 295 additions and 146 deletions.
19 changes: 19 additions & 0 deletions docs/build.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,22 @@ spec:
limits:
cpu: "0.5"
memory: "256M"
tolerations:
- key: "key1"
operator: "Exists"
effect: "NoSchedule"
nodeSelector:
disktype: ssd
affinity:
nodeAffinity:
requiredDuringSchedulingIgnoredDuringExecution:
nodeSelectorTerms:
- matchExpressions:
- key: kubernetes.io/e2e-az-name
operator: In
values:
- e2e-az1
- e2e-az2
```
- `tags`: A list of docker tags to build. At least one tag is required.
Expand All @@ -50,6 +66,9 @@ spec:
- `env`: Optional list of build time environment variables.
- `projectDescriptorPath`: Path to the [project descriptor file](https://buildpacks.io/docs/reference/config/project-descriptor/) relative to source root dir or `subPath` if set. If unset, kpack will look for `project.toml` at the root dir or `subPath` if set.
- `resources`: Optional configurable resource limits on `CPU` and `memory`.
- `tolerations`: Optional configurable pod spec tolerations
- `nodeSelector`: Optional configurable pod spec nodeSelector
- `affinity`: Optional configurabl pod spec affinity

> Note: All fields on a build are immutable. Instead of updating a build, create a new one.

Expand Down
18 changes: 17 additions & 1 deletion docs/image.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ The `source` field is a composition of a source code location and a `subpath`. I

### <a id='build-config'></a>Build Configuration

The `build` field on the `image` resource can be used to configure env variables required during the build process and to configure resource limits on `CPU` and `memory`.
The `build` field on the `image` resource can be used to configure env variables required during the build process, to configure resource limits on `CPU` and `memory`, and to configure pod tolerations, node selector, and affinity.

```yaml
build:
Expand All @@ -106,6 +106,22 @@ build:
limits:
cpu: "0.5"
memory: "256M"
tolerations:
- key: "key1"
operator: "Exists"
effect: "NoSchedule"
nodeSelector:
disktype: ssd
affinity:
nodeAffinity:
requiredDuringSchedulingIgnoredDuringExecution:
nodeSelectorTerms:
- matchExpressions:
- key: kubernetes.io/e2e-az-name
operator: In
values:
- e2e-az1
- e2e-az2
```

See the kubernetes documentation on [setting environment variables](https://kubernetes.io/docs/tasks/inject-data-application/define-environment-variable-container/) and [resource limits and requests](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#resource-requests-and-limits-of-pod-and-container) for more information.
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/build/v1alpha1/image_builds_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ func testImageBuilds(t *testing.T, when spec.G, it spec.S) {
})

it("adds the env vars to the build spec", func() {
image.Spec.Build = &corev1alpha1.ImageBuild{
image.Spec.Build = &ImageBuild{
Env: []corev1.EnvVar{
{Name: "keyA", Value: "new"},
},
Expand All @@ -228,7 +228,7 @@ func testImageBuilds(t *testing.T, when spec.G, it spec.S) {
})

it("adds build resources", func() {
image.Spec.Build = &corev1alpha1.ImageBuild{
image.Spec.Build = &ImageBuild{
Resources: corev1.ResourceRequirements{
Limits: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("2"),
Expand Down
10 changes: 9 additions & 1 deletion pkg/apis/build/v1alpha1/image_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,18 @@ type ImageSpec struct {
FailedBuildHistoryLimit *int64 `json:"failedBuildHistoryLimit,omitempty"`
SuccessBuildHistoryLimit *int64 `json:"successBuildHistoryLimit,omitempty"`
ImageTaggingStrategy corev1alpha1.ImageTaggingStrategy `json:"imageTaggingStrategy,omitempty"`
Build *corev1alpha1.ImageBuild `json:"build,omitempty"`
Build *ImageBuild `json:"build,omitempty"`
Notary *corev1alpha1.NotaryConfig `json:"notary,omitempty"`
}

type ImageBuild struct {
// +listType
Bindings corev1alpha1.Bindings `json:"bindings,omitempty"`
// +listType
Env []corev1.EnvVar `json:"env,omitempty"`
Resources corev1.ResourceRequirements `json:"resources,omitempty"`
}

// +k8s:openapi-gen=true
type ImageStatus struct {
corev1alpha1.Status `json:",inline"`
Expand Down
8 changes: 8 additions & 0 deletions pkg/apis/build/v1alpha1/image_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,14 @@ func (is *ImageSpec) validateCacheSize(ctx context.Context) *apis.FieldError {
return nil
}

func (ib *ImageBuild) Validate(ctx context.Context) *apis.FieldError {
if ib == nil {
return nil
}

return ib.Bindings.Validate(ctx).ViaField("bindings")
}

func validateBuilder(builder v1.ObjectReference) *apis.FieldError {
if builder.Name == "" {
return apis.ErrMissingField("name")
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/build/v1alpha1/image_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func testImageValidation(t *testing.T, when spec.G, it spec.S) {
FailedBuildHistoryLimit: &limit,
SuccessBuildHistoryLimit: &limit,
ImageTaggingStrategy: corev1alpha1.None,
Build: &corev1alpha1.ImageBuild{
Build: &ImageBuild{
Env: []corev1.EnvVar{
{
Name: "keyA",
Expand Down
33 changes: 32 additions & 1 deletion pkg/apis/build/v1alpha1/zz_generated.deepcopy.go

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

30 changes: 10 additions & 20 deletions pkg/apis/build/v1alpha2/build_pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const (
BuildLabel = "kpack.io/build"
DOCKERSecretAnnotationPrefix = "kpack.io/docker"
GITSecretAnnotationPrefix = "kpack.io/git"
k8sOSLabel = "kubernetes.io/os"

cacheDirName = "cache-dir"
layersDirName = "layers-dir"
Expand Down Expand Up @@ -443,10 +444,9 @@ func (b *Build) BuildPod(images BuildPodImages, secrets []corev1.Secret, taints
)
}),
ServiceAccountName: b.Spec.ServiceAccount,
NodeSelector: map[string]string{
"kubernetes.io/os": config.OS,
},
Tolerations: tolerations(taints),
NodeSelector: b.nodeSelector(config.OS),
Tolerations: b.Spec.Tolerations,
Affinity: b.Spec.Affinity,
Volumes: append(append(
append(secretVolumes, b.cacheVolume(config.OS)...),
corev1.Volume{
Expand Down Expand Up @@ -586,9 +586,9 @@ func (b *Build) rebasePod(secrets []corev1.Secret, images BuildPodImages, config
},
Spec: corev1.PodSpec{
ServiceAccountName: b.Spec.ServiceAccount,
NodeSelector: map[string]string{
"kubernetes.io/os": "linux",
},
NodeSelector: b.nodeSelector("linux"),
Tolerations: b.Spec.Tolerations,
Affinity: b.Spec.Affinity,
Volumes: append(
secretVolumes,
corev1.Volume{
Expand Down Expand Up @@ -809,19 +809,9 @@ func (bc *BuildPodBuilderConfig) highestSupportedPlatformAPI(b *Build) (*semver.
return nil, errors.Errorf("unsupported builder platform API versions: %s", strings.Join(bc.PlatformAPIs, ","))
}

func tolerations(taints []corev1.Taint) []corev1.Toleration {
t := make([]corev1.Toleration, 0, len(taints))

for _, taint := range taints {
t = append(t, corev1.Toleration{
Key: taint.Key,
Operator: corev1.TolerationOpEqual,
Value: taint.Value,
Effect: taint.Effect,
})
}

return t
func (b Build) nodeSelector(os string) map[string]string {
b.Spec.NodeSelector[k8sOSLabel] = os
return b.Spec.NodeSelector
}

func builderSecretVolume(bbs corev1alpha1.BuildBuilderSpec) corev1.Volume {
Expand Down
38 changes: 11 additions & 27 deletions pkg/apis/build/v1alpha2/build_pod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) {
Image: previousAppImage,
StackId: "com.builder.stack.io",
},
Tolerations: []corev1.Toleration{{Key: "some-key"}},
NodeSelector: map[string]string{"foo": "bar"},
Affinity: &corev1.Affinity{},
},
}

Expand Down Expand Up @@ -211,11 +214,13 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) {
assert.Equal(t, serviceAccount, pod.Spec.ServiceAccountName)
})

it("creates a pod with the correct node selector", func() {
it("sets the pod tolerations and affinity from the build and merges the os node selector", func() {
pod, err := build.BuildPod(config, secrets, nil, buildPodBuilderConfig)
require.NoError(t, err)

assert.Equal(t, map[string]string{"kubernetes.io/os": "linux"}, pod.Spec.NodeSelector)
assert.Equal(t, map[string]string{"kubernetes.io/os": "linux", "foo": "bar"}, pod.Spec.NodeSelector)
assert.Equal(t, build.Spec.Tolerations, pod.Spec.Tolerations)
assert.Equal(t, build.Spec.Affinity, pod.Spec.Affinity)
})

it("configures the pod security context to match the builder config user and group", func() {
Expand Down Expand Up @@ -918,7 +923,10 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) {
ServiceAccountName: build.Spec.ServiceAccount,
NodeSelector: map[string]string{
"kubernetes.io/os": "linux",
"foo": "bar",
},
Tolerations: build.Spec.Tolerations,
Affinity: build.Spec.Affinity,
Volumes: []corev1.Volume{
{
Name: "secret-volume-docker-secret-1",
Expand Down Expand Up @@ -1136,7 +1144,7 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) {
pod, err := build.BuildPod(config, secrets, nil, buildPodBuilderConfig)
require.NoError(t, err)

assert.Equal(t, map[string]string{"kubernetes.io/os": "windows"}, pod.Spec.NodeSelector)
assert.Equal(t, map[string]string{"kubernetes.io/os": "windows", "foo": "bar"}, pod.Spec.NodeSelector)
})

it("removes the spec securityContext", func() {
Expand Down Expand Up @@ -1411,30 +1419,6 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) {

assert.Len(t, pod.Spec.Volumes, len(podWithCache.Spec.Volumes)-1)
})

it("adds pod tolerations based on node taints", func() {
pod, err := build.BuildPod(config, nil, []corev1.Taint{
{
Key: "test-key",
Value: "test-value",
Effect: corev1.TaintEffectNoSchedule,
},
}, buildPodBuilderConfig)
require.NoError(t, err)

assert.Equal(t,
[]corev1.Toleration{
{
Key: "test-key",
Operator: corev1.TolerationOpEqual,
Value: "test-value",
Effect: corev1.TaintEffectNoSchedule,
},
},
pod.Spec.Tolerations,
)
})

})
})
}
Expand Down
7 changes: 4 additions & 3 deletions pkg/apis/build/v1alpha2/build_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ type BuildSpec struct {
Resources corev1.ResourceRequirements `json:"resources,omitempty"`
LastBuild *LastBuild `json:"lastBuild,omitempty"`
Notary *corev1alpha1.NotaryConfig `json:"notary,omitempty"`
// +listType
Tolerations []corev1.Toleration `json:"tolerations,omitempty"`
NodeSelector map[string]string `json:"nodeSelector,omitempty"`
Affinity *corev1.Affinity `json:"affinity,omitempty"`
}

func (bs *BuildSpec) NeedVolumeCache() bool {
Expand All @@ -80,9 +84,6 @@ type BuildPersistentVolumeCache struct {
ClaimName string `json:"persistentVolumeClaimName,omitempty"`
}

// +k8s:openapi-gen=true
type Bindings []Binding

// +k8s:openapi-gen=true
type Binding struct {
Name string `json:"name,omitempty"`
Expand Down
15 changes: 14 additions & 1 deletion pkg/apis/build/v1alpha2/build_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ func (bs *BuildSpec) Validate(ctx context.Context) *apis.FieldError {
Also(bs.Source.Validate(ctx).ViaField("source")).
Also(bs.Bindings.Validate(ctx).ViaField("bindings")).
Also(bs.LastBuild.Validate(ctx).ViaField("lastBuild")).
Also(bs.validateImmutableFields(ctx))
Also(bs.validateImmutableFields(ctx)).
Also(bs.validateNodeSelector(ctx))
}

func (bs *BuildSpec) validateImmutableFields(ctx context.Context) *apis.FieldError {
Expand All @@ -52,6 +53,18 @@ func (bs *BuildSpec) validateImmutableFields(ctx context.Context) *apis.FieldErr
return nil
}

func (bs *BuildSpec) validateNodeSelector(_ context.Context) *apis.FieldError {
if len(bs.NodeSelector) == 0 {
return nil
}

if _, ok := bs.NodeSelector[k8sOSLabel]; ok {
return apis.ErrInvalidKeyName(k8sOSLabel, "nodeSelector", "os is determined automatically")
}
return nil

}

func (lb *LastBuild) Validate(context context.Context) *apis.FieldError {
if lb == nil || lb.Image == "" {
return nil
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/build/v1alpha2/build_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,5 +238,10 @@ func testBuildValidation(t *testing.T, when spec.G, it spec.S) {
assert.Contains(t, err.Error(), "http://something/different")

})

it("validates kubernetes.io/os node selector is unset", func() {
build.Spec.NodeSelector = map[string]string{k8sOSLabel: "some-os"}
assertValidationError(build, apis.ErrInvalidKeyName(k8sOSLabel, "spec.nodeSelector", "os is determined automatically"))
})
})
}
Loading

0 comments on commit 501f8b9

Please sign in to comment.