From 727f2fd6bcd01c103723f3035e8b8c46b500f619 Mon Sep 17 00:00:00 2001 From: Dylan Orzel Date: Wed, 13 Nov 2024 11:48:17 -0700 Subject: [PATCH] Add Tolerations to Build and BuildRun objects Signed-off-by: Dylan Orzel --- deploy/crds/shipwright.io_buildruns.yaml | 117 +++++++++++++++++++ deploy/crds/shipwright.io_builds.yaml | 39 +++++++ pkg/apis/build/v1beta1/build_types.go | 8 ++ pkg/apis/build/v1beta1/buildrun_types.go | 6 + pkg/reconciler/build/build.go | 1 + pkg/reconciler/buildrun/buildrun.go | 1 + pkg/reconciler/buildrun/resources/taskrun.go | 33 +++++- pkg/validate/tolerations.go | 61 ++++++++++ pkg/validate/validate.go | 4 + 9 files changed, 268 insertions(+), 2 deletions(-) create mode 100644 pkg/validate/tolerations.go diff --git a/deploy/crds/shipwright.io_buildruns.yaml b/deploy/crds/shipwright.io_buildruns.yaml index d2f00f59c..9797b1387 100644 --- a/deploy/crds/shipwright.io_buildruns.yaml +++ b/deploy/crds/shipwright.io_buildruns.yaml @@ -7535,6 +7535,45 @@ spec: Build should take to execute. format: duration type: string + tolerations: + description: If specified, the pod's tolerations. + items: + description: |- + The pod this Toleration is attached to tolerates any taint that matches + the triple using the matching operator . + properties: + effect: + description: |- + Effect indicates the taint effect to match. Empty means match all taint effects. + When specified, allowed values are NoSchedule, PreferNoSchedule and NoExecute. + type: string + key: + description: |- + Key is the taint key that the toleration applies to. Empty means match all taint keys. + If the key is empty, operator must be Exists; this combination means to match all values and all keys. + type: string + operator: + description: |- + Operator represents a key's relationship to the value. + Valid operators are Exists and Equal. Defaults to Equal. + Exists is equivalent to wildcard for value, so that a pod can + tolerate all taints of a particular category. + type: string + tolerationSeconds: + description: |- + TolerationSeconds represents the period of time the toleration (which must be + of effect NoExecute, otherwise this field is ignored) tolerates the taint. By default, + it is not set, which means tolerate the taint forever (do not evict). Zero and + negative values will be treated as 0 (evict immediately) by the system. + format: int64 + type: integer + value: + description: |- + Value is the taint value the toleration matches to. + If the operator is Exists, the value should be empty, otherwise just a regular string. + type: string + type: object + type: array trigger: description: Trigger defines the scenarios where a new build should be triggered. @@ -9753,6 +9792,45 @@ spec: description: Timeout defines the maximum run time of this BuildRun. format: duration type: string + tolerations: + description: If specified, the pod's tolerations. + items: + description: |- + The pod this Toleration is attached to tolerates any taint that matches + the triple using the matching operator . + properties: + effect: + description: |- + Effect indicates the taint effect to match. Empty means match all taint effects. + When specified, allowed values are NoSchedule, PreferNoSchedule and NoExecute. + type: string + key: + description: |- + Key is the taint key that the toleration applies to. Empty means match all taint keys. + If the key is empty, operator must be Exists; this combination means to match all values and all keys. + type: string + operator: + description: |- + Operator represents a key's relationship to the value. + Valid operators are Exists and Equal. Defaults to Equal. + Exists is equivalent to wildcard for value, so that a pod can + tolerate all taints of a particular category. + type: string + tolerationSeconds: + description: |- + TolerationSeconds represents the period of time the toleration (which must be + of effect NoExecute, otherwise this field is ignored) tolerates the taint. By default, + it is not set, which means tolerate the taint forever (do not evict). Zero and + negative values will be treated as 0 (evict immediately) by the system. + format: int64 + type: integer + value: + description: |- + Value is the taint value the toleration matches to. + If the operator is Exists, the value should be empty, otherwise just a regular string. + type: string + type: object + type: array volumes: description: |- Volumes contains volume Overrides of the BuildStrategy volumes in case those are allowed @@ -11959,6 +12037,45 @@ spec: should take to execute. format: duration type: string + tolerations: + description: If specified, the pod's tolerations. + items: + description: |- + The pod this Toleration is attached to tolerates any taint that matches + the triple using the matching operator . + properties: + effect: + description: |- + Effect indicates the taint effect to match. Empty means match all taint effects. + When specified, allowed values are NoSchedule, PreferNoSchedule and NoExecute. + type: string + key: + description: |- + Key is the taint key that the toleration applies to. Empty means match all taint keys. + If the key is empty, operator must be Exists; this combination means to match all values and all keys. + type: string + operator: + description: |- + Operator represents a key's relationship to the value. + Valid operators are Exists and Equal. Defaults to Equal. + Exists is equivalent to wildcard for value, so that a pod can + tolerate all taints of a particular category. + type: string + tolerationSeconds: + description: |- + TolerationSeconds represents the period of time the toleration (which must be + of effect NoExecute, otherwise this field is ignored) tolerates the taint. By default, + it is not set, which means tolerate the taint forever (do not evict). Zero and + negative values will be treated as 0 (evict immediately) by the system. + format: int64 + type: integer + value: + description: |- + Value is the taint value the toleration matches to. + If the operator is Exists, the value should be empty, otherwise just a regular string. + type: string + type: object + type: array trigger: description: Trigger defines the scenarios where a new build should be triggered. diff --git a/deploy/crds/shipwright.io_builds.yaml b/deploy/crds/shipwright.io_builds.yaml index ca02694d9..9873c9594 100644 --- a/deploy/crds/shipwright.io_builds.yaml +++ b/deploy/crds/shipwright.io_builds.yaml @@ -2912,6 +2912,45 @@ spec: should take to execute. format: duration type: string + tolerations: + description: If specified, the pod's tolerations. + items: + description: |- + The pod this Toleration is attached to tolerates any taint that matches + the triple using the matching operator . + properties: + effect: + description: |- + Effect indicates the taint effect to match. Empty means match all taint effects. + When specified, allowed values are NoSchedule, PreferNoSchedule and NoExecute. + type: string + key: + description: |- + Key is the taint key that the toleration applies to. Empty means match all taint keys. + If the key is empty, operator must be Exists; this combination means to match all values and all keys. + type: string + operator: + description: |- + Operator represents a key's relationship to the value. + Valid operators are Exists and Equal. Defaults to Equal. + Exists is equivalent to wildcard for value, so that a pod can + tolerate all taints of a particular category. + type: string + tolerationSeconds: + description: |- + TolerationSeconds represents the period of time the toleration (which must be + of effect NoExecute, otherwise this field is ignored) tolerates the taint. By default, + it is not set, which means tolerate the taint forever (do not evict). Zero and + negative values will be treated as 0 (evict immediately) by the system. + format: int64 + type: integer + value: + description: |- + Value is the taint value the toleration matches to. + If the operator is Exists, the value should be empty, otherwise just a regular string. + type: string + type: object + type: array trigger: description: Trigger defines the scenarios where a new build should be triggered. diff --git a/pkg/apis/build/v1beta1/build_types.go b/pkg/apis/build/v1beta1/build_types.go index 22a6e0b7f..73822614b 100644 --- a/pkg/apis/build/v1beta1/build_types.go +++ b/pkg/apis/build/v1beta1/build_types.go @@ -78,6 +78,8 @@ const ( OutputTimestampNotValid BuildReason = "OutputTimestampNotValid" // NodeSelectorNotValid indicates that the nodeSelector value is not valid NodeSelectorNotValid BuildReason = "NodeSelectorNotValid" + // TolerationNotValid indicates that the Toleration value is not valid + TolerationNotValid BuildReason = "TolerationNotValid" // AllValidationsSucceeded indicates a Build was successfully validated AllValidationsSucceeded = "all validations succeeded" @@ -183,6 +185,12 @@ type BuildSpec struct { // // +optional NodeSelector map[string]string `json:"nodeSelector,omitempty"` + + // If specified, the pod's tolerations. + // +optional + // +patchMergeKey=Key + // +patchStrategy=merge + Tolerations []corev1.Toleration `json:"tolerations,omitempty" patchStrategy:"merge" patchMergeKey:"Key"` } // BuildVolume is a volume that will be mounted in build pod during build step diff --git a/pkg/apis/build/v1beta1/buildrun_types.go b/pkg/apis/build/v1beta1/buildrun_types.go index 05bc7723f..af2882fbb 100644 --- a/pkg/apis/build/v1beta1/buildrun_types.go +++ b/pkg/apis/build/v1beta1/buildrun_types.go @@ -115,6 +115,12 @@ type BuildRunSpec struct { // // +optional NodeSelector map[string]string `json:"nodeSelector,omitempty"` + + // If specified, the pod's tolerations. + // +optional + // +patchMergeKey=Key + // +patchStrategy=merge + Tolerations []corev1.Toleration `json:"tolerations,omitempty" patchStrategy:"merge" patchMergeKey:"Key"` } // BuildRunRequestedState defines the buildrun state the user can provide to override whatever is the current state. diff --git a/pkg/reconciler/build/build.go b/pkg/reconciler/build/build.go index cfdb15db5..805d13a63 100644 --- a/pkg/reconciler/build/build.go +++ b/pkg/reconciler/build/build.go @@ -34,6 +34,7 @@ var validationTypes = [...]string{ validate.Envs, validate.Triggers, validate.NodeSelector, + validate.Tolerations, } // ReconcileBuild reconciles a Build object diff --git a/pkg/reconciler/buildrun/buildrun.go b/pkg/reconciler/buildrun/buildrun.go index e17255bab..b5945cda9 100644 --- a/pkg/reconciler/buildrun/buildrun.go +++ b/pkg/reconciler/buildrun/buildrun.go @@ -161,6 +161,7 @@ func (r *ReconcileBuildRun) Reconcile(ctx context.Context, request reconcile.Req validate.NewBuildName(build), validate.NewEnv(build), validate.NewNodeSelector(build), + validate.NewTolerations(build), ) // an internal/technical error during validation happened diff --git a/pkg/reconciler/buildrun/resources/taskrun.go b/pkg/reconciler/buildrun/resources/taskrun.go index 17b790368..f4874f11c 100644 --- a/pkg/reconciler/buildrun/resources/taskrun.go +++ b/pkg/reconciler/buildrun/resources/taskrun.go @@ -7,6 +7,7 @@ package resources import ( "fmt" "path" + "slices" "strconv" "strings" @@ -235,12 +236,25 @@ func GenerateTaskRun( }, } + taskRunPodTemplate := &pod.PodTemplate{} // Merge Build and BuildRun NodeSelectors, giving preference to BuildRun NodeSelector taskRunNodeSelector := mergeMaps(build.Spec.NodeSelector, buildRun.Spec.NodeSelector) if len(taskRunNodeSelector) > 0 { - expectedTaskRun.Spec.PodTemplate = &pod.PodTemplate{ - NodeSelector: taskRunNodeSelector, + taskRunPodTemplate.NodeSelector = taskRunNodeSelector + } + + // Merge Build and BuildRun Tolerations, giving preference to BuildRun Tolerations values + taskRunTolerations := mergeTolerations(build.Spec.Tolerations, buildRun.Spec.Tolerations) + if len(taskRunTolerations) > 0 { + // only support "NoSchedule" effect, make sure it is set + for i := range taskRunTolerations { + taskRunTolerations[i].Effect = corev1.TaintEffectNoSchedule } + taskRunPodTemplate.Tolerations = taskRunTolerations + } + + if !(taskRunPodTemplate.Equals(&pod.PodTemplate{})) { + expectedTaskRun.Spec.PodTemplate = taskRunPodTemplate } // assign the annotations from the build strategy, filter out those that should not be propagated @@ -354,6 +368,21 @@ func effectiveTimeout(build *buildv1beta1.Build, buildRun *buildv1beta1.BuildRun return nil } +// mergeTolerations merges the values for Spec.Tolerations in the given Build and BuildRun objects, with values in the BuildRun object overriding values +// in the Build object (if present). +func mergeTolerations(buildTolerations []corev1.Toleration, buildRunTolerations []corev1.Toleration) []corev1.Toleration { + mergedTolerations := []corev1.Toleration{} + mergedTolerations = append(mergedTolerations, buildRunTolerations...) + for _, toleration := range buildTolerations { + if !slices.ContainsFunc(mergedTolerations, func(t corev1.Toleration) bool { + return t.Key == toleration.Key + }) { + mergedTolerations = append(mergedTolerations, toleration) + } + } + return mergedTolerations +} + // isPropagatableAnnotation filters the last-applied-configuration annotation from kubectl because this would break the meaning of this annotation on the target object; // also, annotations using our own custom resource domains are filtered out because we have no annotations with a semantic for both TaskRun and Pod func isPropagatableAnnotation(key string) bool { diff --git a/pkg/validate/tolerations.go b/pkg/validate/tolerations.go new file mode 100644 index 000000000..8a805def6 --- /dev/null +++ b/pkg/validate/tolerations.go @@ -0,0 +1,61 @@ +// Copyright The Shipwright Contributors +// +// SPDX-License-Identifier: Apache-2.0 + +package validate + +import ( + "context" + "fmt" + "strings" + + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/validation" + "k8s.io/utils/ptr" + + build "github.com/shipwright-io/build/pkg/apis/build/v1beta1" +) + +// TolerationsRef contains all required fields +// to validate tolerations +type TolerationsRef struct { + Build *build.Build // build instance for analysis +} + +func NewTolerations(build *build.Build) *TolerationsRef { + return &TolerationsRef{build} +} + +// ValidatePath implements BuildPath interface and validates +// that tolerations key/operator/value are valid +func (b *TolerationsRef) ValidatePath(_ context.Context) error { + for _, toleration := range b.Build.Spec.Tolerations { + // validate Key + if errs := validation.IsQualifiedName(toleration.Key); errs != nil { + b.Build.Status.Reason = ptr.To(build.TolerationNotValid) + b.Build.Status.Message = ptr.To(strings.Join(errs, ", ")) + } + // validate Operator + if !((toleration.Operator == v1.TolerationOpExists) || (toleration.Operator == v1.TolerationOpEqual)) { + b.Build.Status.Reason = ptr.To(build.TolerationNotValid) + b.Build.Status.Message = ptr.To(fmt.Sprintf("Toleration operator not valid. Must be one of: '%v', '%v'", v1.TolerationOpExists, v1.TolerationOpEqual)) + } + // validate Value + if errs := validation.IsValidLabelValue(toleration.Value); errs != nil { + b.Build.Status.Reason = ptr.To(build.TolerationNotValid) + b.Build.Status.Message = ptr.To(strings.Join(errs, ", ")) + } + // validate Effect, which should not be specified + if len(toleration.Effect) > 0 { + b.Build.Status.Reason = ptr.To(build.TolerationNotValid) + b.Build.Status.Message = ptr.To("Specifying Toleration Effect is not supported.") + } + // validate TolerationSeconds, which should not be specified + if toleration.TolerationSeconds != nil { + b.Build.Status.Reason = ptr.To(build.TolerationNotValid) + b.Build.Status.Message = ptr.To("Specifying TolerationSeconds is not supported.") + } + } + + return nil +} diff --git a/pkg/validate/validate.go b/pkg/validate/validate.go index d39396652..219d84c0b 100644 --- a/pkg/validate/validate.go +++ b/pkg/validate/validate.go @@ -37,6 +37,8 @@ const ( Triggers = "triggers" // NodeSelector for validating `spec.nodeSelector` entry NodeSelector = "nodeselector" + // Tolerations for validating `spec.tolerations` entry + Tolerations = "tolerations" ) const ( @@ -79,6 +81,8 @@ func NewValidation( return &Trigger{build: build}, nil case NodeSelector: return &NodeSelectorRef{Build: build}, nil + case Tolerations: + return &TolerationsRef{Build: build}, nil default: return nil, fmt.Errorf("unknown validation type") }